fix: address review feedback for Renderer#read_pixels#10
Merged
takaokouji merged 1 commit intomasterfrom Apr 14, 2026
Merged
Conversation
- Accept SDL2::PixelFormat in addition to Integer by using uint32_for_format() instead of NUM2UINT() - Remove incorrect ARGB8888 default override; pass format=0 through to SDL_RenderReadPixels as-is (SDL2 uses the rendering target's format when 0 is specified) - Query renderer info to determine correct pitch when format is 0 - Improve RDoc with @overload, usage examples, and caveats Addresses feedback from ohai#29. Fixes #9 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SDL2::Renderer#read_pixelsの修正。ohai#29 でのレビューフィードバックに対応。Changes Made
SDL2::PixelFormatオブジェクトを受け付けるように修正（uint32_for_format()を使用）@overload、使用例、注意事項を追加）Test Coverage
screenshot.rbでSDL2::PixelFormat::ARGB8888を明示指定して動作確認済みRelated Issues
Fixes #9
Upstream PR: ohai#29