feat: add SDL2::Renderer#read_pixels#29
Closed
takaokouji wants to merge 2 commits intoohai:masterfrom
Closed
Conversation
ea83589 to
984ba4f
Compare
Replace all deprecated Data_Wrap_Struct/Data_Get_Struct with TypedData_Wrap_Struct/TypedData_Get_Struct across all source files. Add DEFINE_DATA_TYPE helper macro to rubysdl2_internal.h for easy rb_data_type_t definition. Eliminates all deprecation warnings on Ruby 3.4.x while maintaining backward compatibility with Ruby 3.3.x. Files changed: - rubysdl2_internal.h: DEFINE_DATA_TYPE macro, DEFINE_GETTER uses TypedData - video.c.m4: Window, Renderer, Texture, Surface, DisplayMode, Rect, Point - event.c: SDL_Event (EVENT_READER/EVENT_WRITER macros + all event types) - mixer.c.m4: Chunk, Music - ttf.c.m4: TTF - joystick.c.m4: Joystick - gamecontroller.c.m4: GameController - gl.c.m4: GLContext Closes #1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wraps SDL_RenderReadPixels to read pixel data from the current rendering target. Returns raw pixel data as a binary string. Default format is ARGB8888. Pass nil for rect to read the entire target. Refs: ohai#28 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
984ba4f to
e3b7a77
Compare
Owner
|
Thank you for your PR. |
Contributor
Author
|
Thank you for reviewing! Here is a real-world usage from Smalruby. We use Here is a standalone sample program that saves the rendered screen as a BMP file (no extra image library needed): require "sdl2"
SDL2.init(SDL2::INIT_VIDEO)
window = SDL2::Window.create("read_pixels demo", 100, 100, 320, 240, 0)
renderer = window.create_renderer(-1, 0)
# Draw something
renderer.draw_color = [0, 128, 255]
renderer.fill_rect(SDL2::Rect.new(60, 40, 200, 160))
renderer.draw_color = [255, 255, 0]
renderer.fill_rect(SDL2::Rect.new(110, 80, 100, 80))
renderer.present
# Capture the rendered pixels (ARGB8888, little-endian → BGRA bytes)
pixels = renderer.read_pixels(nil, 0)
# Save as BMP
width, height = 320, 240
row_size = width * 4
file_size = 54 + row_size * height
bmp = String.new(encoding: Encoding::BINARY)
bmp << ["BM", file_size, 0, 0, 54].pack("A2Vv2V") # file header
bmp << [40, width, height, 1, 32, 0, 0, 0, 0, 0, 0].pack("VV2v2V6") # DIB header
# BMP stores rows bottom-to-top; pixel data is already BGRA which BMP expects
(height - 1).downto(0) do |y|
bmp << pixels.byteslice(y * row_size, row_size)
end
File.binwrite("screenshot.bmp", bmp)
puts "Saved screenshot.bmp (#{File.size("screenshot.bmp")} bytes)"PNG conversion requires an image utility library, so I used BMP here to keep the example dependency-free. |
Owner
|
After the assessment of the patch, I have concluded that the quality is not satisfactory:
|
takaokouji
added a commit
to smalruby/ruby-sdl2
that referenced
this pull request
Apr 14, 2026
- 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>
takaokouji
added a commit
to smalruby/smalruby3-editor
that referenced
this pull request
Apr 14, 2026
…xels - Update screenshot.rb to explicitly specify SDL2::PixelFormat::ARGB8888 instead of relying on format=0 default, ensuring consistent byte order across all platforms (including Wayland/Linux) - Update ruby-sdl2 submodule to include read_pixels fixes: - Accept SDL2::PixelFormat objects (not just Integer) - Remove incorrect ARGB8888 default override - Query renderer info for correct pitch when format=0 Addresses feedback from ohai/ruby-sdl2#29. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
takaokouji
added a commit
to smalruby/ruby-sdl2
that referenced
this pull request
Apr 14, 2026
- 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>
Contributor
Author
|
Thank you for the detailed review. I have addressed all three points and resubmitted as #31:
Would you please review #31 when you have a chance? |
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
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
Add
SDL2::Renderer#read_pixelsmethod that wrapsSDL_RenderReadPixels.This enables reading pixel data from the current rendering target, which is essential for taking screenshots of the rendered screen.
Usage
Parameters
rect—SDL2::Rectornil(nil reads the entire target)format— pixel format as Integer (0 defaults toSDL_PIXELFORMAT_ARGB8888)Returns
Raw pixel data as a binary
String.Motivation
As noted in #28, there is currently no way to capture screen contents for screenshots. This method provides the missing functionality by wrapping
SDL_RenderReadPixels.Closes #28