test: Extract SDLTest_ReadSurfacePixel

This is essentially the same as was added in d95d2d70, but with clearer
error handling. It's implemented in a private header file so that it
can be shared with SDL_shape, which also wants this functionality.

Resolves: https://github.com/libsdl-org/SDL/issues/8319
Signed-off-by: Simon McVittie <smcv@collabora.com>

include/SDL3/SDL_test_compare.h

@@ -44,6 +44,27 @@
 extern "C" {
 #endif
 
+/**
+ * \brief Retrieves a single pixel from a surface.
+ *
+ * This function prioritizes correctness over speed: it is suitable for
+ * unit tests, but is not intended for use in a game engine.
+ *
+ * Like SDL_GetRGBA, this uses the entire 0..255 range when converting
+ * color components from pixel formats with less than 8 bits per RGB
+ * component.
+ *
+ * \param surface The surface
+ * \param x Horizontal coordinate, 0 <= x < width
+ * \param y Vertical coordinate, 0 <= y < height
+ * \param r Pointer to location to store red channel, 0-255
+ * \param g Pointer to location to store green channel, 0-255
+ * \param b Pointer to location to store blue channel, 0-255
+ * \param a Pointer to location to store alpha channel, 0-255
+ * \returns 0 if the surface is valid and the coordinates are in-bounds
+ */
+int SDLTest_ReadSurfacePixel(SDL_Surface *surface, int x, int y, Uint8 *r, Uint8 *g, Uint8 *b, Uint8 *a);
+
 /**
  * \brief Compares a surface and with reference image data for equality
  *

src/test/SDL_test_compare.c

@@ -28,28 +28,17 @@
 */
 #include <SDL3/SDL_test.h>
 
+#include "../video/SDL_surface_pixel_impl.h"
+
 #define FILENAME_SIZE 128
 
 /* Counter for _CompareSurface calls; used for filename creation when comparisons fail */
 static int _CompareSurfaceCount = 0;
 
-static Uint32
+int
-GetPixel(Uint8 *p, size_t bytes_per_pixel)
+SDLTest_ReadSurfacePixel(SDL_Surface *surface, int x, int y, Uint8 *r, Uint8 *g, Uint8 *b, Uint8 *a)
 {
-    Uint32 ret = 0;
+    return SDL_ReadSurfacePixel_impl(surface, x, y, r, g, b, a);
-
-    SDL_assert(bytes_per_pixel <= sizeof(ret));
-
-    /* Fill the appropriate number of least-significant bytes of ret,
-     * leaving the most-significant bytes set to zero, so that ret can
-     * be decoded with SDL_GetRGBA afterwards. */
-#if SDL_BYTEORDER == SDL_BIG_ENDIAN
-    SDL_memcpy(((Uint8 *) &ret) + (sizeof(ret) - bytes_per_pixel), p, bytes_per_pixel);
-#else
-    SDL_memcpy(&ret, p, bytes_per_pixel);
-#endif
-
-    return ret;
 }
 
 static void
@@ -67,8 +56,6 @@ int SDLTest_CompareSurfaces(SDL_Surface *surface, SDL_Surface *referenceSurface,
 {
     int ret;
     int i, j;
-    int bpp, bpp_reference;
-    Uint8 *p, *p_reference;
     int dist;
     int sampleErrorX = 0, sampleErrorY = 0, sampleDist = 0;
     SDL_Color sampleReference = { 0, 0, 0, 0 };
@@ -104,19 +91,24 @@ int SDLTest_CompareSurfaces(SDL_Surface *surface, SDL_Surface *referenceSurface,
     SDL_LockSurface(referenceSurface);
 
     ret = 0;
-    bpp = surface->format->BytesPerPixel;
-    bpp_reference = referenceSurface->format->BytesPerPixel;
     /* Compare image - should be same format. */
     for (j = 0; j < surface->h; j++) {
         for (i = 0; i < surface->w; i++) {
-            Uint32 pixel;
+            int temp;
-            p = (Uint8 *)surface->pixels + j * surface->pitch + i * bpp;
-            p_reference = (Uint8 *)referenceSurface->pixels + j * referenceSurface->pitch + i * bpp_reference;
 
-            pixel = GetPixel(p, bpp);
+            temp = SDLTest_ReadSurfacePixel(surface, i, j, &R, &G, &B, &A);
-            SDL_GetRGBA(pixel, surface->format, &R, &G, &B, &A);
+            if (temp != 0) {
-            pixel = GetPixel(p_reference, bpp_reference);
+                SDLTest_LogError("Failed to retrieve pixel (%d,%d): %s", i, j, SDL_GetError());
-            SDL_GetRGBA(pixel, referenceSurface->format, &Rd, &Gd, &Bd, &Ad);
+                ret++;
+                continue;
+            }
+
+            temp = SDLTest_ReadSurfacePixel(referenceSurface, i, j, &Rd, &Gd, &Bd, &Ad);
+            if (temp != 0) {
+                SDLTest_LogError("Failed to retrieve reference pixel (%d,%d): %s", i, j, SDL_GetError());
+                ret++;
+                continue;
+            }
 
             dist = 0;
             dist += (R - Rd) * (R - Rd);

src/video/SDL_surface_pixel_impl.h

@@ -0,0 +1,81 @@
+/*
+  Copyright 1997-2023 Sam Lantinga <slouken@libsdl.org>
+  Copyright 2023 Collabora Ltd.
+
+  This software is provided 'as-is', without any express or implied
+  warranty.  In no event will the authors be held liable for any damages
+  arising from the use of this software.
+
+  Permission is granted to anyone to use this software for any purpose,
+  including commercial applications, and to alter it and redistribute it
+  freely, subject to the following restrictions:
+
+  1. The origin of this software must not be misrepresented; you must not
+     claim that you wrote the original software. If you use this software
+     in a product, an acknowledgment in the product documentation would be
+     appreciated but is not required.
+  2. Altered source versions must be plainly marked as such, and must not be
+     misrepresented as being the original software.
+  3. This notice may not be removed or altered from any source distribution.
+*/
+
+#include "SDL3/SDL.h"
+
+/* Internal implementation of SDL_ReadSurfacePixel, shared between SDL_shape
+ * and SDLTest */
+static inline int
+SDL_ReadSurfacePixel_impl(SDL_Surface *surface, int x, int y, Uint8 *r, Uint8 *g, Uint8 *b, Uint8 *a)
+{
+    Uint32 pixel = 0;
+    size_t bytes_per_pixel;
+    void *p;
+
+    if (surface == NULL || surface->format == NULL || surface->pixels == NULL) {
+        return SDL_InvalidParamError("surface");
+    }
+
+    if (x < 0 || x >= surface->w) {
+        return SDL_InvalidParamError("x");
+    }
+
+    if (y < 0 || y >= surface->h) {
+        return SDL_InvalidParamError("y");
+    }
+
+    if (r == NULL) {
+        return SDL_InvalidParamError("r");
+    }
+
+    if (g == NULL) {
+        return SDL_InvalidParamError("g");
+    }
+
+    if (b == NULL) {
+        return SDL_InvalidParamError("b");
+    }
+
+    if (a == NULL) {
+        return SDL_InvalidParamError("a");
+    }
+
+    bytes_per_pixel = surface->format->BytesPerPixel;
+
+    if (bytes_per_pixel > sizeof(pixel)) {
+        return SDL_InvalidParamError("surface->format->BytesPerPixel");
+    }
+
+    SDL_LockSurface(surface);
+
+    p = (Uint8 *)surface->pixels + y * surface->pitch + x * bytes_per_pixel;
+    /* Fill the appropriate number of least-significant bytes of pixel,
+     * leaving the most-significant bytes set to zero */
+#if SDL_BYTEORDER == SDL_BIG_ENDIAN
+    SDL_memcpy(((Uint8 *) &pixel) + (sizeof(pixel) - bytes_per_pixel), p, bytes_per_pixel);
+#else
+    SDL_memcpy(&pixel, p, bytes_per_pixel);
+#endif
+    SDL_GetRGBA(pixel, surface->format, r, g, b, a);
+
+    SDL_UnlockSurface(surface);
+    return 0;
+}