Browse Source

SDL_RenderReadPixels() now returns a surface

Fixes https://github.com/libsdl-org/SDL/issues/8977
Sam Lantinga 1 year ago
parent
commit
89b9d6cbdc

+ 2 - 0
docs/README-migration.md

@@ -1053,6 +1053,8 @@ The viewport, clipping state, and scale for render targets are now persistent an
 
 SDL_RenderGeometryRaw() and SDL_Vertex have been changed to use floating point colors, in the range of [0..1] for SDR content.
 
+SDL_RenderReadPixels() returns a surface instead of filling in preallocated memory.
+
 The following functions have been renamed:
 * SDL_GetRendererOutputSize() => SDL_GetCurrentRenderOutputSize()
 * SDL_RenderCopy() => SDL_RenderTexture()

+ 5 - 17
include/SDL3/SDL_render.h

@@ -1722,35 +1722,23 @@ extern DECLSPEC int SDLCALL SDL_RenderGeometryRaw(SDL_Renderer *renderer,
                                                const void *indices, int num_indices, int size_indices);
 
 /**
- * Read pixels from the current rendering target to an array of pixels.
+ * Read pixels from the current rendering target.
+ *
+ * The returned surface should be freed with SDL_DestroySurface()
  *
  * **WARNING**: This is a very slow operation, and should not be used
  * frequently. If you're using this on the main rendering target, it should be
  * called after rendering and before SDL_RenderPresent().
  *
- * `pitch` specifies the number of bytes between rows in the destination
- * `pixels` data. This allows you to write to a subrectangle or have padded
- * rows in the destination. Generally, `pitch` should equal the number of
- * pixels per row in the `pixels` data times the number of bytes per pixel,
- * but it might contain additional padding (for example, 24bit RGB Windows
- * Bitmap data pads all rows to multiples of 4 bytes).
- *
  * \param renderer the rendering context
  * \param rect an SDL_Rect structure representing the area in pixels relative
  *             to the to current viewport, or NULL for the entire viewport
- * \param format an SDL_PixelFormatEnum value of the desired format of the
- *               pixel data, or 0 to use the format of the rendering target
- * \param pixels a pointer to the pixel data to copy into
- * \param pitch the pitch of the `pixels` parameter
- * \returns 0 on success or a negative error code on failure; call
+ * \returns a new SDL_Surface on success or NULL on failure; call
  *          SDL_GetError() for more information.
  *
  * \since This function is available since SDL 3.0.0.
  */
-extern DECLSPEC int SDLCALL SDL_RenderReadPixels(SDL_Renderer *renderer,
-                                                 const SDL_Rect *rect,
-                                                 Uint32 format,
-                                                 void *pixels, int pitch);
+extern DECLSPEC SDL_Surface * SDLCALL SDL_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect);
 
 /**
  * Update the screen with any rendering performed since the previous call.

+ 1 - 1
src/dynapi/SDL_dynapi_procs.h

@@ -568,7 +568,7 @@ SDL_DYNAPI_PROC(int,SDL_RenderLines,(SDL_Renderer *a, const SDL_FPoint *b, int c
 SDL_DYNAPI_PROC(int,SDL_RenderPoint,(SDL_Renderer *a, float b, float c),(a,b,c),return)
 SDL_DYNAPI_PROC(int,SDL_RenderPoints,(SDL_Renderer *a, const SDL_FPoint *b, int c),(a,b,c),return)
 SDL_DYNAPI_PROC(int,SDL_RenderPresent,(SDL_Renderer *a),(a),return)
-SDL_DYNAPI_PROC(int,SDL_RenderReadPixels,(SDL_Renderer *a, const SDL_Rect *b, Uint32 c, void *d, int e),(a,b,c,d,e),return)
+SDL_DYNAPI_PROC(SDL_Surface *,SDL_RenderReadPixels,(SDL_Renderer *a, const SDL_Rect *b),(a,b),return)
 SDL_DYNAPI_PROC(int,SDL_RenderRect,(SDL_Renderer *a, const SDL_FRect *b),(a,b),return)
 SDL_DYNAPI_PROC(int,SDL_RenderRects,(SDL_Renderer *a, const SDL_FRect *b, int c),(a,b,c),return)
 SDL_DYNAPI_PROC(int,SDL_RenderTexture,(SDL_Renderer *a, SDL_Texture *b, const SDL_FRect *c, const SDL_FRect *d),(a,b,c,d),return)

+ 6 - 21
src/render/SDL_render.c

@@ -4134,43 +4134,28 @@ int SDL_RenderGeometryRaw(SDL_Renderer *renderer,
     return retval;
 }
 
-int SDL_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect, Uint32 format, void *pixels, int pitch)
+SDL_Surface *SDL_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
     SDL_Rect real_rect;
 
-    CHECK_RENDERER_MAGIC(renderer, -1);
+    CHECK_RENDERER_MAGIC(renderer, NULL);
 
     if (!renderer->RenderReadPixels) {
-        return SDL_Unsupported();
+        SDL_Unsupported();
+        return NULL;
     }
 
     FlushRenderCommands(renderer); /* we need to render before we read the results. */
 
-    if (!format) {
-        if (!renderer->target) {
-            format = SDL_GetWindowPixelFormat(renderer->window);
-        } else {
-            format = renderer->target->format;
-        }
-    }
-
     GetRenderViewportInPixels(renderer, &real_rect);
 
     if (rect) {
         if (!SDL_GetRectIntersection(rect, &real_rect, &real_rect)) {
-            return 0;
-        }
-        if (real_rect.y > rect->y) {
-            pixels = (Uint8 *)pixels + pitch * (real_rect.y - rect->y);
-        }
-        if (real_rect.x > rect->x) {
-            int bpp = SDL_BYTESPERPIXEL(format);
-            pixels = (Uint8 *)pixels + bpp * (real_rect.x - rect->x);
+            return NULL;
         }
     }
 
-    return renderer->RenderReadPixels(renderer, &real_rect,
-                                      format, pixels, pitch);
+    return renderer->RenderReadPixels(renderer, &real_rect);
 }
 
 static void SDL_SimulateRenderVSync(SDL_Renderer *renderer)

+ 1 - 2
src/render/SDL_sysrender.h

@@ -201,8 +201,7 @@ struct SDL_Renderer
     void (*UnlockTexture)(SDL_Renderer *renderer, SDL_Texture *texture);
     void (*SetTextureScaleMode)(SDL_Renderer *renderer, SDL_Texture *texture, SDL_ScaleMode scaleMode);
     int (*SetRenderTarget)(SDL_Renderer *renderer, SDL_Texture *texture);
-    int (*RenderReadPixels)(SDL_Renderer *renderer, const SDL_Rect *rect,
-                            Uint32 format, void *pixels, int pitch);
+    SDL_Surface *(*RenderReadPixels)(SDL_Renderer *renderer, const SDL_Rect *rect);
     int (*RenderPresent)(SDL_Renderer *renderer);
     void (*DestroyTexture)(SDL_Renderer *renderer, SDL_Texture *texture);
 

+ 13 - 11
src/render/direct3d/SDL_render_d3d.c

@@ -27,6 +27,7 @@
 #include "../SDL_sysrender.h"
 #include "../SDL_d3dmath.h"
 #include "../../video/windows/SDL_windowsvideo.h"
+#include "../../video/SDL_pixels_c.h"
 
 #ifdef SDL_VIDEO_RENDER_D3D
 #define D3D_DEBUG_INFO
@@ -1297,8 +1298,7 @@ static int D3D_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd, v
     return 0;
 }
 
-static int D3D_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                                Uint32 format, void *pixels, int pitch)
+static SDL_Surface *D3D_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
     D3D_RenderData *data = (D3D_RenderData *)renderer->driverdata;
     D3DSURFACE_DESC desc;
@@ -1307,7 +1307,7 @@ static int D3D_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
     RECT d3drect;
     D3DLOCKED_RECT locked;
     HRESULT result;
-    int status;
+    SDL_Surface *output;
 
     if (data->currentRenderTarget) {
         backBuffer = data->currentRenderTarget;
@@ -1317,18 +1317,21 @@ static int D3D_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
 
     result = IDirect3DSurface9_GetDesc(backBuffer, &desc);
     if (FAILED(result)) {
-        return D3D_SetError("GetDesc()", result);
+        D3D_SetError("GetDesc()", result);
+        return NULL;
     }
 
     result = IDirect3DDevice9_CreateOffscreenPlainSurface(data->device, desc.Width, desc.Height, desc.Format, D3DPOOL_SYSTEMMEM, &surface, NULL);
     if (FAILED(result)) {
-        return D3D_SetError("CreateOffscreenPlainSurface()", result);
+        D3D_SetError("CreateOffscreenPlainSurface()", result);
+        return NULL;
     }
 
     result = IDirect3DDevice9_GetRenderTargetData(data->device, backBuffer, surface);
     if (FAILED(result)) {
         IDirect3DSurface9_Release(surface);
-        return D3D_SetError("GetRenderTargetData()", result);
+        D3D_SetError("GetRenderTargetData()", result);
+        return NULL;
     }
 
     d3drect.left = rect->x;
@@ -1339,18 +1342,17 @@ static int D3D_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
     result = IDirect3DSurface9_LockRect(surface, &locked, &d3drect, D3DLOCK_READONLY);
     if (FAILED(result)) {
         IDirect3DSurface9_Release(surface);
-        return D3D_SetError("LockRect()", result);
+        D3D_SetError("LockRect()", result);
+        return NULL;
     }
 
-    status = SDL_ConvertPixels(rect->w, rect->h,
-                               D3DFMTToPixelFormat(desc.Format), locked.pBits, locked.Pitch,
-                               format, pixels, pitch);
+    output = SDL_DuplicatePixels(rect->w, rect->h, D3DFMTToPixelFormat(desc.Format), SDL_COLORSPACE_SRGB, locked.pBits, locked.Pitch);
 
     IDirect3DSurface9_UnlockRect(surface);
 
     IDirect3DSurface9_Release(surface);
 
-    return status;
+    return output;
 }
 
 static int D3D_RenderPresent(SDL_Renderer *renderer)

+ 6 - 13
src/render/direct3d11/SDL_render_d3d11.c

@@ -29,6 +29,7 @@
 #endif
 #include "../SDL_sysrender.h"
 #include "../SDL_d3dmath.h"
+#include "../../video/SDL_pixels_c.h"
 
 #include <d3d11_1.h>
 #include <dxgi1_4.h>
@@ -2342,19 +2343,18 @@ static int D3D11_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd,
     return 0;
 }
 
-static int D3D11_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                                  Uint32 format, void *pixels, int pitch)
+static SDL_Surface *D3D11_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
     D3D11_RenderData *data = (D3D11_RenderData *)renderer->driverdata;
     ID3D11RenderTargetView *renderTargetView = NULL;
     ID3D11Texture2D *backBuffer = NULL;
     ID3D11Texture2D *stagingTexture = NULL;
     HRESULT result;
-    int status = -1;
     D3D11_TEXTURE2D_DESC stagingTextureDesc;
     D3D11_RECT srcRect = { 0, 0, 0, 0 };
     D3D11_BOX srcBox;
     D3D11_MAPPED_SUBRESOURCE textureMemory;
+    SDL_Surface *output = NULL;
 
     renderTargetView = D3D11_GetCurrentRenderTargetView(renderer);
     if (!renderTargetView) {
@@ -2417,19 +2417,12 @@ static int D3D11_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
         goto done;
     }
 
-    /* Copy the data into the desired buffer, converting pixels to the
-     * desired format at the same time:
-     */
-    status = SDL_ConvertPixelsAndColorspace(
+    output = SDL_DuplicatePixels(
         rect->w, rect->h,
         D3D11_DXGIFormatToSDLPixelFormat(stagingTextureDesc.Format),
         renderer->target ? renderer->target->colorspace : renderer->output_colorspace,
         textureMemory.pData,
-        textureMemory.RowPitch,
-        format,
-        SDL_COLORSPACE_SRGB,
-        pixels,
-        pitch);
+        textureMemory.RowPitch);
 
     /* Unmap the texture: */
     ID3D11DeviceContext_Unmap(data->d3dContext,
@@ -2439,7 +2432,7 @@ static int D3D11_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
 done:
     SAFE_RELEASE(backBuffer);
     SAFE_RELEASE(stagingTexture);
-    return status;
+    return output;
 }
 
 static int D3D11_RenderPresent(SDL_Renderer *renderer)

+ 6 - 13
src/render/direct3d12/SDL_render_d3d12.c

@@ -31,6 +31,7 @@
 #include "../../video/windows/SDL_windowswindow.h"
 #include "../SDL_sysrender.h"
 #include "../SDL_d3dmath.h"
+#include "../../video/SDL_pixels_c.h"
 
 #if defined(SDL_PLATFORM_XBOXONE) || defined(SDL_PLATFORM_XBOXSERIES)
 #include "SDL_render_d3d12_xbox.h"
@@ -2801,14 +2802,12 @@ static int D3D12_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd,
     return 0;
 }
 
-static int D3D12_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                                  Uint32 format, void *pixels, int pitch)
+static SDL_Surface *D3D12_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
     D3D12_RenderData *data = (D3D12_RenderData *)renderer->driverdata;
     ID3D12Resource *backBuffer = NULL;
     ID3D12Resource *readbackBuffer = NULL;
     HRESULT result;
-    int status = -1;
     D3D12_RESOURCE_DESC textureDesc;
     D3D12_RESOURCE_DESC readbackDesc;
     D3D12_HEAP_PROPERTIES heapProps;
@@ -2820,6 +2819,7 @@ static int D3D12_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
     D3D12_SUBRESOURCE_FOOTPRINT pitchedDesc;
     BYTE *textureMemory;
     int bpp;
+    SDL_Surface *output = NULL;
 
     if (data->textureRenderTarget) {
         backBuffer = data->textureRenderTarget->mainTexture;
@@ -2938,26 +2938,19 @@ static int D3D12_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
         goto done;
     }
 
-    /* Copy the data into the desired buffer, converting pixels to the
-     * desired format at the same time:
-     */
-    status = SDL_ConvertPixelsAndColorspace(
+    output = SDL_DuplicatePixels(
         rect->w, rect->h,
         D3D12_DXGIFormatToSDLPixelFormat(textureDesc.Format),
         renderer->target ? renderer->target->colorspace : renderer->output_colorspace,
         textureMemory,
-        pitchedDesc.RowPitch,
-        format,
-        SDL_COLORSPACE_SRGB,
-        pixels,
-        pitch);
+        pitchedDesc.RowPitch);
 
     /* Unmap the texture: */
     D3D_CALL(readbackBuffer, Unmap, 0, NULL);
 
 done:
     SAFE_RELEASE(readbackBuffer);
-    return status;
+    return output;
 }
 
 static int D3D12_RenderPresent(SDL_Renderer *renderer)

+ 11 - 19
src/render/metal/SDL_render_metal.m

@@ -1451,19 +1451,18 @@ static int METAL_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd,
     }
 }
 
-static int METAL_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                       Uint32 pixel_format, void *pixels, int pitch)
+static SDL_Surface *METAL_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
     @autoreleasepool {
         METAL_RenderData *data = (__bridge METAL_RenderData *)renderer->driverdata;
         id<MTLTexture> mtltexture;
         MTLRegion mtlregion;
-        size_t temp_pitch;
-        int status;
-        Uint32 temp_format;
-        void *temp_pixels;
+        Uint32 format;
+        SDL_Surface *surface;
+
         if (!METAL_ActivateRenderCommandEncoder(renderer, MTLLoadActionLoad, NULL, nil)) {
-            return SDL_SetError("Failed to activate render command encoder (is your window in the background?");
+            SDL_SetError("Failed to activate render command encoder (is your window in the background?");
+            return NULL;
         }
 
         [data.mtlcmdencoder endEncoding];
@@ -1490,19 +1489,12 @@ static int METAL_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
 
         mtlregion = MTLRegionMake2D(rect->x, rect->y, rect->w, rect->h);
 
-        // we only do BGRA8 or RGBA8 at the moment, so 4 will do.
-        temp_pitch = rect->w * 4UL;
-        temp_pixels = SDL_malloc(temp_pitch * rect->h);
-        if (!temp_pixels) {
-            return -1;
+        format = (mtltexture.pixelFormat == MTLPixelFormatBGRA8Unorm) ? SDL_PIXELFORMAT_ARGB8888 : SDL_PIXELFORMAT_ABGR8888;
+        surface = SDL_CreateSurface(rect->w, rect->h, format);
+        if (surface) {
+            [mtltexture getBytes:surface->pixels bytesPerRow:surface->pitch fromRegion:mtlregion mipmapLevel:0];
         }
-
-        [mtltexture getBytes:temp_pixels bytesPerRow:temp_pitch fromRegion:mtlregion mipmapLevel:0];
-
-        temp_format = (mtltexture.pixelFormat == MTLPixelFormatBGRA8Unorm) ? SDL_PIXELFORMAT_ARGB8888 : SDL_PIXELFORMAT_ABGR8888;
-        status = SDL_ConvertPixels(rect->w, rect->h, temp_format, temp_pixels, (int)temp_pitch, pixel_format, pixels, pitch);
-        SDL_free(temp_pixels);
-        return status;
+        return surface;
     }
 }
 

+ 23 - 39
src/render/opengl/SDL_render_gl.c

@@ -1457,74 +1457,58 @@ static int GL_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd, vo
     return GL_CheckError("", renderer);
 }
 
-static int GL_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                               Uint32 pixel_format, void *pixels, int pitch)
+static SDL_Surface *GL_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
     GL_RenderData *data = (GL_RenderData *)renderer->driverdata;
-    Uint32 temp_format = renderer->target ? renderer->target->format : SDL_PIXELFORMAT_ARGB8888;
-    void *temp_pixels;
-    int temp_pitch;
+    Uint32 format = renderer->target ? renderer->target->format : SDL_PIXELFORMAT_ARGB8888;
     GLint internalFormat;
-    GLenum format, type;
-    Uint8 *src, *dst, *tmp;
-    int w, h, length, rows;
-    int status;
+    GLenum targetFormat, type;
+    int w, h;
+    SDL_Surface *surface;
 
     GL_ActivateRenderer(renderer);
 
-    if (!convert_format(temp_format, &internalFormat, &format, &type)) {
-        return SDL_SetError("Texture format %s not supported by OpenGL",
-                            SDL_GetPixelFormatName(temp_format));
-    }
-
-    if (rect->w == 0 || rect->h == 0) {
-        return 0; /* nothing to do. */
+    if (!convert_format(format, &internalFormat, &targetFormat, &type)) {
+        SDL_SetError("Texture format %s not supported by OpenGL", SDL_GetPixelFormatName(format));
+        return NULL;
     }
 
-    temp_pitch = rect->w * SDL_BYTESPERPIXEL(temp_format);
-    temp_pixels = SDL_malloc((size_t)rect->h * temp_pitch);
-    if (!temp_pixels) {
-        return -1;
+    surface = SDL_CreateSurface(rect->w, rect->h, format);
+    if (!surface) {
+        return NULL;
     }
 
     SDL_GetCurrentRenderOutputSize(renderer, &w, &h);
 
     data->glPixelStorei(GL_PACK_ALIGNMENT, 1);
-    data->glPixelStorei(GL_PACK_ROW_LENGTH,
-                        (temp_pitch / SDL_BYTESPERPIXEL(temp_format)));
+    data->glPixelStorei(GL_PACK_ROW_LENGTH, (surface->pitch / SDL_BYTESPERPIXEL(format)));
 
     data->glReadPixels(rect->x, renderer->target ? rect->y : (h - rect->y) - rect->h,
-                       rect->w, rect->h, format, type, temp_pixels);
+                       rect->w, rect->h, targetFormat, type, surface->pixels);
 
     if (GL_CheckError("glReadPixels()", renderer) < 0) {
-        SDL_free(temp_pixels);
-        return -1;
+        SDL_DestroySurface(surface);
+        return NULL;
     }
 
     /* Flip the rows to be top-down if necessary */
     if (!renderer->target) {
         SDL_bool isstack;
-        length = rect->w * SDL_BYTESPERPIXEL(temp_format);
-        src = (Uint8 *)temp_pixels + (rect->h - 1) * temp_pitch;
-        dst = (Uint8 *)temp_pixels;
-        tmp = SDL_small_alloc(Uint8, length, &isstack);
-        rows = rect->h / 2;
+        int length = rect->w * SDL_BYTESPERPIXEL(format);
+        Uint8 *src = (Uint8 *)surface->pixels + (rect->h - 1) * surface->pitch;
+        Uint8 *dst = (Uint8 *)surface->pixels;
+        Uint8 *tmp = SDL_small_alloc(Uint8, length, &isstack);
+        int rows = rect->h / 2;
         while (rows--) {
             SDL_memcpy(tmp, dst, length);
             SDL_memcpy(dst, src, length);
             SDL_memcpy(src, tmp, length);
-            dst += temp_pitch;
-            src -= temp_pitch;
+            dst += surface->pitch;
+            src -= surface->pitch;
         }
         SDL_small_free(tmp, isstack);
     }
-
-    status = SDL_ConvertPixels(rect->w, rect->h,
-                               temp_format, temp_pixels, temp_pitch,
-                               pixel_format, pixels, pitch);
-    SDL_free(temp_pixels);
-
-    return status;
+    return surface;
 }
 
 static int GL_RenderPresent(SDL_Renderer *renderer)

+ 19 - 35
src/render/opengles2/SDL_render_gles2.c

@@ -1923,61 +1923,45 @@ static void GLES2_DestroyTexture(SDL_Renderer *renderer, SDL_Texture *texture)
     }
 }
 
-static int GLES2_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                                  Uint32 pixel_format, void *pixels, int pitch)
+static SDL_Surface *GLES2_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
     GLES2_RenderData *data = (GLES2_RenderData *)renderer->driverdata;
-    Uint32 temp_format = renderer->target ? renderer->target->format : SDL_PIXELFORMAT_RGBA32;
-    size_t buflen;
-    void *temp_pixels;
-    int temp_pitch;
-    Uint8 *src, *dst, *tmp;
-    int w, h, length, rows;
-    int status;
-
-    temp_pitch = rect->w * SDL_BYTESPERPIXEL(temp_format);
-    buflen = (size_t)rect->h * temp_pitch;
-    if (buflen == 0) {
-        return 0; /* nothing to do. */
-    }
-
-    temp_pixels = SDL_malloc(buflen);
-    if (!temp_pixels) {
-        return -1;
+    Uint32 format = renderer->target ? renderer->target->format : SDL_PIXELFORMAT_RGBA32;
+    int w, h;
+    SDL_Surface *surface;
+
+    surface = SDL_CreateSurface(rect->w, rect->h, format);
+    if (!surface) {
+        return NULL;
     }
 
     SDL_GetCurrentRenderOutputSize(renderer, &w, &h);
 
     data->glReadPixels(rect->x, renderer->target ? rect->y : (h - rect->y) - rect->h,
-                       rect->w, rect->h, GL_RGBA, GL_UNSIGNED_BYTE, temp_pixels);
+                       rect->w, rect->h, GL_RGBA, GL_UNSIGNED_BYTE, surface->pixels);
     if (GL_CheckError("glReadPixels()", renderer) < 0) {
-        return -1;
+        SDL_DestroySurface(surface);
+        return NULL;
     }
 
     /* Flip the rows to be top-down if necessary */
     if (!renderer->target) {
         SDL_bool isstack;
-        length = rect->w * SDL_BYTESPERPIXEL(temp_format);
-        src = (Uint8 *)temp_pixels + (rect->h - 1) * temp_pitch;
-        dst = (Uint8 *)temp_pixels;
-        tmp = SDL_small_alloc(Uint8, length, &isstack);
-        rows = rect->h / 2;
+        int length = rect->w * SDL_BYTESPERPIXEL(format);
+        Uint8 *src = (Uint8 *)surface->pixels + (rect->h - 1) * surface->pitch;
+        Uint8 *dst = (Uint8 *)surface->pixels;
+        Uint8 *tmp = SDL_small_alloc(Uint8, length, &isstack);
+        int rows = rect->h / 2;
         while (rows--) {
             SDL_memcpy(tmp, dst, length);
             SDL_memcpy(dst, src, length);
             SDL_memcpy(src, tmp, length);
-            dst += temp_pitch;
-            src -= temp_pitch;
+            dst += surface->pitch;
+            src -= surface->pitch;
         }
         SDL_small_free(tmp, isstack);
     }
-
-    status = SDL_ConvertPixels(rect->w, rect->h,
-                               temp_format, temp_pixels, temp_pitch,
-                               pixel_format, pixels, pitch);
-    SDL_free(temp_pixels);
-
-    return status;
+    return surface;
 }
 
 static int GLES2_RenderPresent(SDL_Renderer *renderer)

+ 0 - 7
src/render/ps2/SDL_render_ps2.c

@@ -537,12 +537,6 @@ static int PS2_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd, v
     return 0;
 }
 
-static int PS2_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                                Uint32 format, void *pixels, int pitch)
-{
-    return SDL_Unsupported();
-}
-
 static int PS2_RenderPresent(SDL_Renderer *renderer)
 {
     PS2_RenderData *data = (PS2_RenderData *)renderer->driverdata;
@@ -703,7 +697,6 @@ static SDL_Renderer *PS2_CreateRenderer(SDL_Window *window, SDL_PropertiesID cre
     renderer->QueueGeometry = PS2_QueueGeometry;
     renderer->InvalidateCachedState = PS2_InvalidateCachedState;
     renderer->RunCommandQueue = PS2_RunCommandQueue;
-    renderer->RenderReadPixels = PS2_RenderReadPixels;
     renderer->RenderPresent = PS2_RenderPresent;
     renderer->DestroyTexture = PS2_DestroyTexture;
     renderer->DestroyRenderer = PS2_DestroyRenderer;

+ 0 - 7
src/render/psp/SDL_render_psp.c

@@ -1212,12 +1212,6 @@ static int PSP_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd, v
     return 0;
 }
 
-static int PSP_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                                Uint32 pixel_format, void *pixels, int pitch)
-{
-    return SDL_Unsupported();
-}
-
 static int PSP_RenderPresent(SDL_Renderer *renderer)
 {
     PSP_RenderData *data = (PSP_RenderData *)renderer->driverdata;
@@ -1335,7 +1329,6 @@ SDL_Renderer *PSP_CreateRenderer(SDL_Window *window, SDL_PropertiesID create_pro
     renderer->QueueCopyEx = PSP_QueueCopyEx;
     renderer->InvalidateCachedState = PSP_InvalidateCachedState;
     renderer->RunCommandQueue = PSP_RunCommandQueue;
-    renderer->RenderReadPixels = PSP_RenderReadPixels;
     renderer->RenderPresent = PSP_RenderPresent;
     renderer->DestroyTexture = PSP_DestroyTexture;
     renderer->DestroyRenderer = PSP_DestroyRenderer;

+ 10 - 13
src/render/software/SDL_render_sw.c

@@ -33,6 +33,7 @@
 #include "SDL_drawpoint.h"
 #include "SDL_rotate.h"
 #include "SDL_triangle.h"
+#include "../../video/SDL_pixels_c.h"
 
 /* SDL surface based renderer implementation */
 
@@ -967,15 +968,13 @@ static int SW_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd, vo
     return 0;
 }
 
-static int SW_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                               Uint32 format, void *pixels, int pitch)
+static SDL_Surface *SW_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
     SDL_Surface *surface = SW_ActivateRenderer(renderer);
-    Uint32 src_format;
-    void *src_pixels;
+    void *pixels;
 
     if (!surface) {
-        return -1;
+        return NULL;
     }
 
     /* NOTE: The rect is already adjusted according to the viewport by
@@ -984,17 +983,15 @@ static int SW_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
 
     if (rect->x < 0 || rect->x + rect->w > surface->w ||
         rect->y < 0 || rect->y + rect->h > surface->h) {
-        return SDL_SetError("Tried to read outside of surface bounds");
+        SDL_SetError("Tried to read outside of surface bounds");
+        return NULL;
     }
 
-    src_format = surface->format->format;
-    src_pixels = (void *)((Uint8 *)surface->pixels +
-                          rect->y * surface->pitch +
-                          rect->x * surface->format->BytesPerPixel);
+    pixels = (void *)((Uint8 *)surface->pixels +
+                      rect->y * surface->pitch +
+                      rect->x * surface->format->BytesPerPixel);
 
-    return SDL_ConvertPixels(rect->w, rect->h,
-                             src_format, src_pixels, surface->pitch,
-                             format, pixels, pitch);
+    return SDL_DuplicatePixels(rect->w, rect->h, surface->format->format, SDL_COLORSPACE_SRGB, pixels, surface->pitch);
 }
 
 static int SW_RenderPresent(SDL_Renderer *renderer)

+ 21 - 38
src/render/vitagxm/SDL_render_vita_gxm.c

@@ -93,8 +93,7 @@ static void VITA_GXM_InvalidateCachedState(SDL_Renderer *renderer);
 
 static int VITA_GXM_RunCommandQueue(SDL_Renderer *renderer, SDL_RenderCommand *cmd, void *vertices, size_t vertsize);
 
-static int VITA_GXM_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                                     Uint32 pixel_format, void *pixels, int pitch);
+static SDL_Surface *VITA_GXM_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect);
 
 static int VITA_GXM_RenderPresent(SDL_Renderer *renderer);
 static void VITA_GXM_DestroyTexture(SDL_Renderer *renderer, SDL_Texture *texture);
@@ -1089,63 +1088,47 @@ void read_pixels(int x, int y, size_t width, size_t height, void *data)
     }
 }
 
-static int VITA_GXM_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect,
-                                     Uint32 pixel_format, void *pixels, int pitch)
+static SDL_Surface *VITA_GXM_RenderReadPixels(SDL_Renderer *renderer, const SDL_Rect *rect)
 {
-    Uint32 temp_format = renderer->target ? renderer->target->format : SDL_PIXELFORMAT_ABGR8888;
-    size_t buflen;
-    void *temp_pixels;
-    int temp_pitch;
-    Uint8 *src, *dst, *tmp;
-    int w, h, length, rows;
-    int status;
-
-    // TODO: read from texture rendertarget. Although no-one sane should do it.
-    if (renderer->target) {
-        return SDL_Unsupported();
-    }
+    Uint32 format = renderer->target ? renderer->target->format : SDL_PIXELFORMAT_ABGR8888;
+    int w, h;
+    SDL_Surface *surface;
 
-    temp_pitch = rect->w * SDL_BYTESPERPIXEL(temp_format);
-    buflen = rect->h * temp_pitch;
-    if (buflen == 0) {
-        return 0; /* nothing to do. */
+    // TODO: read from texture rendertarget.
+    if (renderer->target) {
+        SDL_Unsupported();
+        return NULL;
     }
 
-    temp_pixels = SDL_malloc(buflen);
-    if (!temp_pixels) {
-        return -1;
+    surface = SDL_CreateSurface(rect->w, rect->h, format);
+    if (!surface) {
+        return NULL;
     }
 
     SDL_GetCurrentRenderOutputSize(renderer, &w, &h);
 
     read_pixels(rect->x, renderer->target ? rect->y : (h - rect->y) - rect->h,
-                rect->w, rect->h, temp_pixels);
+                rect->w, rect->h, surface->pixels);
 
     /* Flip the rows to be top-down if necessary */
 
     if (!renderer->target) {
         SDL_bool isstack;
-        length = rect->w * SDL_BYTESPERPIXEL(temp_format);
-        src = (Uint8 *)temp_pixels + (rect->h - 1) * temp_pitch;
-        dst = (Uint8 *)temp_pixels;
-        tmp = SDL_small_alloc(Uint8, length, &isstack);
-        rows = rect->h / 2;
+        int length = rect->w * SDL_BYTESPERPIXEL(format);
+        Uint8 *src = (Uint8 *)surface->pixels + (rect->h - 1) * surface->pitch;
+        Uint8 *dst = (Uint8 *)surface->pixels;
+        Uint8 *tmp = SDL_small_alloc(Uint8, length, &isstack);
+        int rows = rect->h / 2;
         while (rows--) {
             SDL_memcpy(tmp, dst, length);
             SDL_memcpy(dst, src, length);
             SDL_memcpy(src, tmp, length);
-            dst += temp_pitch;
-            src -= temp_pitch;
+            dst += surface->pitch;
+            src -= surface->pitch;
         }
         SDL_small_free(tmp, isstack);
     }
-
-    status = SDL_ConvertPixels(rect->w, rect->h,
-                               temp_format, temp_pixels, temp_pitch,
-                               pixel_format, pixels, pitch);
-    SDL_free(temp_pixels);
-
-    return status;
+    return surface;
 }
 
 static int VITA_GXM_RenderPresent(SDL_Renderer *renderer)

+ 3 - 14
src/test/SDL_test_common.c

@@ -2016,7 +2016,6 @@ static const void *SDLTest_ScreenShotClipboardProvider(void *context, const char
 
 static void SDLTest_CopyScreenShot(SDL_Renderer *renderer)
 {
-    SDL_Rect viewport;
     SDL_Surface *surface;
     const char *image_formats[] = {
         "text/plain;charset=utf-8",
@@ -2028,28 +2027,18 @@ static void SDLTest_CopyScreenShot(SDL_Renderer *renderer)
         return;
     }
 
-    SDL_GetRenderViewport(renderer, &viewport);
-
-    surface = SDL_CreateSurface(viewport.w, viewport.h, SDL_PIXELFORMAT_BGR24);
-
+    surface = SDL_RenderReadPixels(renderer, NULL);
     if (!surface) {
-        SDL_Log("Couldn't create surface: %s\n", SDL_GetError());
-        return;
-    }
-
-    if (SDL_RenderReadPixels(renderer, NULL, surface->format->format,
-                             surface->pixels, surface->pitch) < 0) {
         SDL_Log("Couldn't read screen: %s\n", SDL_GetError());
-        SDL_free(surface);
         return;
     }
 
     if (SDL_SaveBMP(surface, SCREENSHOT_FILE) < 0) {
         SDL_Log("Couldn't save %s: %s\n", SCREENSHOT_FILE, SDL_GetError());
-        SDL_free(surface);
+        SDL_DestroySurface(surface);
         return;
     }
-    SDL_free(surface);
+    SDL_DestroySurface(surface);
 
     clipboard_data = (SDLTest_ClipboardData *)SDL_calloc(1, sizeof(*clipboard_data));
     if (!clipboard_data) {

+ 1 - 2
src/video/SDL_pixels_c.h

@@ -43,8 +43,6 @@ extern float SDL_PQfromNits(float v);
 extern const float *SDL_GetColorPrimariesConversionMatrix(SDL_ColorPrimaries src, SDL_ColorPrimaries dst);
 extern void SDL_ConvertColorPrimaries(float *fR, float *fG, float *fB, const float *matrix);
 
-
-
 /* Blit mapping functions */
 extern SDL_BlitMap *SDL_AllocBlitMap(void);
 extern void SDL_InvalidateMap(SDL_BlitMap *map);
@@ -57,5 +55,6 @@ extern void SDL_InvalidateAllBlitMap(SDL_Surface *surface);
 extern void SDL_DitherColors(SDL_Color *colors, int bpp);
 extern Uint8 SDL_FindColor(SDL_Palette *pal, Uint8 r, Uint8 g, Uint8 b, Uint8 a);
 extern void SDL_DetectPalette(SDL_Palette *pal, SDL_bool *is_opaque, SDL_bool *has_alpha_channel);
+extern SDL_Surface *SDL_DuplicatePixels(int width, int height, Uint32 format, SDL_Colorspace colorspace, void *pixels, int pitch);
 
 #endif /* SDL_pixels_c_h_ */

+ 31 - 18
src/video/SDL_surface.c

@@ -27,6 +27,7 @@
 #include "SDL_pixels_c.h"
 #include "SDL_yuv_c.h"
 #include "../render/SDL_sysrender.h"
+#include "../video/SDL_pixels_c.h"
 #include "../video/SDL_yuv_c.h"
 
 /* Check to make sure we can safely check multiplication of surface w and pitch and it won't overflow size_t */
@@ -1508,9 +1509,7 @@ SDL_Surface *SDL_ConvertSurfaceFormatAndColorspace(SDL_Surface *surface, Uint32
 /*
  * Create a surface on the stack for quick blit operations
  */
-static SDL_INLINE SDL_bool SDL_CreateSurfaceOnStack(int width, int height, Uint32 pixel_format,
-                                                    void *pixels, int pitch, SDL_Surface *surface,
-                                                    SDL_PixelFormat *format, SDL_BlitMap *blitmap)
+static SDL_bool SDL_CreateSurfaceOnStack(int width, int height, Uint32 pixel_format, SDL_Colorspace colorspace, void *pixels, int pitch, SDL_Surface *surface, SDL_PixelFormat *format, SDL_BlitMap *blitmap)
 {
     if (SDL_ISPIXELFORMAT_INDEXED(pixel_format)) {
         SDL_SetError("Indexed pixel formats not supported");
@@ -1538,11 +1537,36 @@ static SDL_INLINE SDL_bool SDL_CreateSurfaceOnStack(int width, int height, Uint3
     blitmap->info.a = 0xFF;
     surface->map = blitmap;
 
+    SDL_SetNumberProperty(SDL_GetSurfaceProperties(surface), SDL_PROP_SURFACE_COLORSPACE_NUMBER, colorspace);
+
     /* The surface is ready to go */
     surface->refcount = 1;
     return SDL_TRUE;
 }
 
+static void SDL_DestroySurfaceOnStack(SDL_Surface *surface)
+{
+    /* Free blitmap reference, after blitting between stack'ed surfaces */
+    SDL_InvalidateMap(surface->map);
+
+    SDL_DestroyProperties(SDL_GetSurfaceProperties(surface));
+}
+
+SDL_Surface *SDL_DuplicatePixels(int width, int height, Uint32 format, SDL_Colorspace colorspace, void *pixels, int pitch)
+{
+    SDL_Surface surface;
+    SDL_PixelFormat fmt;
+    SDL_BlitMap blitmap;
+    SDL_Surface *result = NULL;
+
+    if (SDL_CreateSurfaceOnStack(width, height, format, colorspace, pixels, pitch, &surface, &fmt, &blitmap)) {
+        result = SDL_DuplicateSurface(&surface);
+
+        SDL_DestroySurfaceOnStack(&surface);
+    }
+    return result;
+}
+
 int SDL_ConvertPixelsAndColorspace(int width, int height,
                       Uint32 src_format, SDL_Colorspace src_colorspace, const void *src, int src_pitch,
                       Uint32 dst_format, SDL_Colorspace dst_colorspace, void *dst, int dst_pitch)
@@ -1601,18 +1625,13 @@ int SDL_ConvertPixelsAndColorspace(int width, int height,
         return 0;
     }
 
-    if (!SDL_CreateSurfaceOnStack(width, height, src_format, nonconst_src,
-                                  src_pitch,
-                                  &src_surface, &src_fmt, &src_blitmap)) {
+    if (!SDL_CreateSurfaceOnStack(width, height, src_format, src_colorspace, nonconst_src, src_pitch, &src_surface, &src_fmt, &src_blitmap)) {
         return -1;
     }
-    SDL_SetNumberProperty(SDL_GetSurfaceProperties(&src_surface), SDL_PROP_SURFACE_COLORSPACE_NUMBER, src_colorspace);
 
-    if (!SDL_CreateSurfaceOnStack(width, height, dst_format, dst, dst_pitch,
-                                  &dst_surface, &dst_fmt, &dst_blitmap)) {
+    if (!SDL_CreateSurfaceOnStack(width, height, dst_format, dst_colorspace, dst, dst_pitch, &dst_surface, &dst_fmt, &dst_blitmap)) {
         return -1;
     }
-    SDL_SetNumberProperty(SDL_GetSurfaceProperties(&dst_surface), SDL_PROP_SURFACE_COLORSPACE_NUMBER, dst_colorspace);
 
     /* Set up the rect and go! */
     rect.x = 0;
@@ -1621,15 +1640,9 @@ int SDL_ConvertPixelsAndColorspace(int width, int height,
     rect.h = height;
     ret = SDL_BlitSurfaceUnchecked(&src_surface, &rect, &dst_surface, &rect);
 
-    /* Free blitmap reference, after blitting between stack'ed surfaces */
-    SDL_InvalidateMap(src_surface.map);
+    SDL_DestroySurfaceOnStack(&src_surface);
+    SDL_DestroySurfaceOnStack(&dst_surface);
 
-    if (src_colorspace != SDL_COLORSPACE_UNKNOWN) {
-        SDL_DestroyProperties(SDL_GetSurfaceProperties(&src_surface));
-    }
-    if (dst_colorspace != SDL_COLORSPACE_UNKNOWN) {
-        SDL_DestroyProperties(SDL_GetSurfaceProperties(&dst_surface));
-    }
     return ret;
 }
 

+ 39 - 34
test/testautomation_render.c

@@ -343,7 +343,7 @@ static int render_testPrimitivesBlend(void *arg)
 static int render_testPrimitivesWithViewport(void *arg)
 {
     SDL_Rect viewport;
-    Uint32 pixel;
+    SDL_Surface *surface;
 
     /* Clear surface. */
     clearScreen();
@@ -363,9 +363,15 @@ static int render_testPrimitivesWithViewport(void *arg)
     viewport.h = 1;
     CHECK_FUNC(SDL_SetRenderViewport, (renderer, &viewport));
 
-    pixel = 0;
-    CHECK_FUNC(SDL_RenderReadPixels, (renderer, NULL, SDL_PIXELFORMAT_RGBA8888, &pixel, sizeof(pixel)));
-    SDLTest_AssertCheck(pixel == 0xFFFFFFFF, "Validate diagonal line drawing with viewport, expected 0xFFFFFFFF, got 0x%.8x", (unsigned int)pixel);
+    surface = SDL_RenderReadPixels(renderer, NULL);
+    if (surface) {
+        Uint8 r, g, b, a;
+        CHECK_FUNC(SDL_ReadSurfacePixel, (surface, 0, 0, &r, &g, &b, &a));
+        SDLTest_AssertCheck(r == 0xFF && g == 0xFF && b == 0xFF && a == 0xFF, "Validate diagonal line drawing with viewport, expected 0xFFFFFFFF, got 0x%.2x%.2x%.2x%.2x", r, g, b, a);
+        SDL_DestroySurface(surface);
+    } else {
+        SDLTest_AssertCheck(surface != NULL, "Validate result from SDL_RenderReadPixels, got NULL, %s", SDL_GetError());
+    }
 
     return TEST_COMPLETED;
 }
@@ -1205,36 +1211,35 @@ hasTexAlpha(void)
 static void
 compare(SDL_Surface *referenceSurface, int allowable_error)
 {
-   int ret;
-   SDL_Rect rect;
-   Uint8 *pixels;
-   SDL_Surface *testSurface;
-
-   /* Read pixels. */
-   pixels = (Uint8 *)SDL_malloc(4*TESTRENDER_SCREEN_W*TESTRENDER_SCREEN_H);
-   SDLTest_AssertCheck(pixels != NULL, "Validate allocated temp pixel buffer");
-   if (pixels == NULL) {
-      return;
-   }
-
-   /* Explicitly specify the rect in case the window isn't the expected size... */
-   rect.x = 0;
-   rect.y = 0;
-   rect.w = TESTRENDER_SCREEN_W;
-   rect.h = TESTRENDER_SCREEN_H;
-   CHECK_FUNC(SDL_RenderReadPixels, (renderer, &rect, RENDER_COMPARE_FORMAT, pixels, 80*4 ))
-
-   /* Create surface. */
-   testSurface = SDL_CreateSurfaceFrom(pixels, TESTRENDER_SCREEN_W, TESTRENDER_SCREEN_H, TESTRENDER_SCREEN_W*4, RENDER_COMPARE_FORMAT);
-   SDLTest_AssertCheck(testSurface != NULL, "Verify result from SDL_CreateSurfaceFrom is not NULL");
-
-   /* Compare surface. */
-   ret = SDLTest_CompareSurfaces( testSurface, referenceSurface, allowable_error );
-   SDLTest_AssertCheck(ret == 0, "Validate result from SDLTest_CompareSurfaces, expected: 0, got: %i", ret);
-
-   /* Clean up. */
-   SDL_free(pixels);
-   SDL_DestroySurface(testSurface);
+    int ret;
+    SDL_Rect rect;
+    SDL_Surface *surface, *testSurface;
+
+    /* Explicitly specify the rect in case the window isn't the expected size... */
+    rect.x = 0;
+    rect.y = 0;
+    rect.w = TESTRENDER_SCREEN_W;
+    rect.h = TESTRENDER_SCREEN_H;
+
+    surface = SDL_RenderReadPixels(renderer, &rect);
+    if (!surface) {
+        SDLTest_AssertCheck(surface != NULL, "Validate result from SDL_RenderReadPixels, got NULL, %s", SDL_GetError());
+        return;
+    }
+
+    testSurface = SDL_ConvertSurfaceFormat(surface, RENDER_COMPARE_FORMAT);
+    SDL_DestroySurface(surface);
+    if (!testSurface) {
+        SDLTest_AssertCheck(testSurface != NULL, "Validate result from SDL_ConvertSurfaceFormat, got NULL, %s", SDL_GetError());
+        return;
+    }
+
+    /* Compare surface. */
+    ret = SDLTest_CompareSurfaces(testSurface, referenceSurface, allowable_error);
+    SDLTest_AssertCheck(ret == 0, "Validate result from SDLTest_CompareSurfaces, expected: 0, got: %i", ret);
+
+    /* Clean up. */
+    SDL_DestroySurface(testSurface);
 }
 
 /**

+ 14 - 3
test/testcolorspace.c

@@ -114,16 +114,27 @@ static void PrevStage(void)
 
 static SDL_bool ReadPixel(int x, int y, SDL_Color *c)
 {
+    SDL_Surface *surface;
     SDL_Rect r;
+    SDL_bool result = SDL_FALSE;
+
     r.x = x;
     r.y = y;
     r.w = 1;
     r.h = 1;
-    if (SDL_RenderReadPixels(renderer, &r, SDL_PIXELFORMAT_RGBA32, c, sizeof(*c)) < 0) {
+
+    surface = SDL_RenderReadPixels(renderer, &r);
+    if (surface) {
+        if (SDL_ReadSurfacePixel(surface, 0, 0, &c->r, &c->g, &c->b, &c->a) == 0) {
+            result = SDL_TRUE;
+        } else {
+            SDL_Log("Couldn't read pixel: %s\n", SDL_GetError());
+        }
+        SDL_DestroySurface(surface);
+    } else {
         SDL_Log("Couldn't read back pixels: %s\n", SDL_GetError());
-        return SDL_FALSE;
     }
-    return SDL_TRUE;
+    return result;
 }
 
 static void DrawText(float x, float y, const char *fmt, ...)

+ 2 - 7
test/testoffscreen.c

@@ -52,15 +52,10 @@ static void draw(void)
 
 static void save_surface_to_bmp(void)
 {
-    SDL_Surface* surface;
-    Uint32 pixel_format;
+    SDL_Surface *surface;
     char file[128];
 
-    pixel_format = SDL_GetWindowPixelFormat(window);
-
-    surface = SDL_CreateSurface(width, height, pixel_format);
-
-    SDL_RenderReadPixels(renderer, NULL, pixel_format, surface->pixels, surface->pitch);
+    surface = SDL_RenderReadPixels(renderer, NULL);
 
     (void)SDL_snprintf(file, sizeof(file), "SDL_window%" SDL_PRIs32 "-%8.8d.bmp",
                        SDL_GetWindowID(window), ++frame_number);

+ 9 - 3
test/testrendertarget.c

@@ -54,11 +54,11 @@ DrawComposite(DrawState *s)
     SDL_Rect viewport;
     SDL_FRect R;
     SDL_Texture *target;
+    SDL_Surface *surface;
 
     static SDL_bool blend_tested = SDL_FALSE;
     if (!blend_tested) {
         SDL_Texture *A, *B;
-        Uint32 P;
 
         A = SDL_CreateTexture(s->renderer, SDL_PIXELFORMAT_ARGB8888, SDL_TEXTUREACCESS_TARGET, 1, 1);
         SDL_SetTextureBlendMode(A, SDL_BLENDMODE_BLEND);
@@ -74,9 +74,15 @@ DrawComposite(DrawState *s)
         SDL_SetRenderDrawColor(s->renderer, 0x00, 0x00, 0x00, 0x00);
         SDL_RenderFillRect(s->renderer, NULL);
         SDL_RenderTexture(s->renderer, A, NULL, NULL);
-        SDL_RenderReadPixels(s->renderer, NULL, SDL_PIXELFORMAT_ARGB8888, &P, sizeof(P));
 
-        SDL_Log("Blended pixel: 0x%8.8" SDL_PRIX32 "\n", P);
+        surface = SDL_RenderReadPixels(s->renderer, NULL);
+        if (surface) {
+            Uint8 r, g, b, a;
+            if (SDL_ReadSurfacePixel(surface, 0, 0, &r, &g, &b, &a) == 0) {
+                SDL_Log("Blended pixel: 0x%.2x%.2x%.2x%.2x\n", r, g, b, a);
+            }
+            SDL_DestroySurface(surface);
+        }
 
         SDL_DestroyTexture(A);
         SDL_DestroyTexture(B);