SDL: Made SDL_ReadSurfacePixel a public function

From f8dfee01bb4fdca3d7f77d4d2a30ed58c63e869c Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Thu, 18 Jan 2024 04:36:37 -0800
Subject: [PATCH] Made SDL_ReadSurfacePixel a public function

Fixes https://github.com/libsdl-org/SDL/issues/8320
---
 include/SDL3/SDL_surface.h         | 25 ++++++++++
 include/SDL3/SDL_test_compare.h    | 21 --------
 src/dynapi/SDL_dynapi.sym          |  1 +
 src/dynapi/SDL_dynapi_overrides.h  |  1 +
 src/dynapi/SDL_dynapi_procs.h      |  1 +
 src/test/SDL_test_compare.c        | 12 +----
 src/video/SDL_surface.c            | 68 ++++++++++++++++++++++---
 src/video/SDL_surface_pixel_impl.h | 80 ------------------------------
 test/testshape.c                   |  2 +-
 9 files changed, 93 insertions(+), 118 deletions(-)
 delete mode 100644 src/video/SDL_surface_pixel_impl.h

diff --git a/include/SDL3/SDL_surface.h b/include/SDL3/SDL_surface.h
index 7ccbda915557..b85f0324d340 100644
--- a/include/SDL3/SDL_surface.h
+++ b/include/SDL3/SDL_surface.h
@@ -932,6 +932,31 @@ extern DECLSPEC int SDLCALL SDL_BlitSurfaceUncheckedScaled(SDL_Surface *src,
                                                            const SDL_Rect *dstrect,
                                                            SDL_ScaleMode scaleMode);
 
+/**
+ * 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 to read
+ * \param x the horizontal coordinate, 0 <= x < width
+ * \param y the vertical coordinate, 0 <= y < height
+ * \param r a pointer filled in with the red channel, 0-255, or NULL to ignore this channel
+ * \param g a pointer filled in with the green channel, 0-255, or NULL to ignore this channel
+ * \param b a pointer filled in with the blue channel, 0-255, or NULL to ignore this channel
+ * \param a a pointer filled in with the alpha channel, 0-255, or NULL to ignore this channel
+ * \returns 0 on success or a negative error code on failure; call
+ *          SDL_GetError() for more information.
+ *
+ * \since This function is available since SDL 3.0.0.
+ */
+extern DECLSPEC int SDLCALL SDL_ReadSurfacePixel(SDL_Surface *surface, int x, int y, Uint8 *r, Uint8 *g, Uint8 *b, Uint8 *a);
+
+
 /**
  * Set the YUV conversion mode
  *
diff --git a/include/SDL3/SDL_test_compare.h b/include/SDL3/SDL_test_compare.h
index 4bd0db315701..82de5d2494c8 100644
--- a/include/SDL3/SDL_test_compare.h
+++ b/include/SDL3/SDL_test_compare.h
@@ -44,27 +44,6 @@
 extern "C" {
 #endif
 
-/**
- * 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);
-
 /**
  * Compares a surface and with reference image data for equality
  *
diff --git a/src/dynapi/SDL_dynapi.sym b/src/dynapi/SDL_dynapi.sym
index 037207aa2e81..be517c11b4c4 100644
--- a/src/dynapi/SDL_dynapi.sym
+++ b/src/dynapi/SDL_dynapi.sym
@@ -963,6 +963,7 @@ SDL3_0.0.0 {
     SDL_GetHapticFromInstanceID;
     SDL_GetHapticInstanceID;
     SDL_GetHapticName;
+    SDL_ReadSurfacePixel;
     # extra symbols go here (don't modify this line)
   local: *;
 };
diff --git a/src/dynapi/SDL_dynapi_overrides.h b/src/dynapi/SDL_dynapi_overrides.h
index 3e939ae2be33..ca74790f485c 100644
--- a/src/dynapi/SDL_dynapi_overrides.h
+++ b/src/dynapi/SDL_dynapi_overrides.h
@@ -988,3 +988,4 @@
 #define SDL_GetHapticFromInstanceID SDL_GetHapticFromInstanceID_REAL
 #define SDL_GetHapticInstanceID SDL_GetHapticInstanceID_REAL
 #define SDL_GetHapticName SDL_GetHapticName_REAL
+#define SDL_ReadSurfacePixel SDL_ReadSurfacePixel_REAL
diff --git a/src/dynapi/SDL_dynapi_procs.h b/src/dynapi/SDL_dynapi_procs.h
index 54f05b9f5e64..55421f113310 100644
--- a/src/dynapi/SDL_dynapi_procs.h
+++ b/src/dynapi/SDL_dynapi_procs.h
@@ -1013,3 +1013,4 @@ SDL_DYNAPI_PROC(const char*,SDL_GetHapticInstanceName,(SDL_HapticID a),(a),retur
 SDL_DYNAPI_PROC(SDL_Haptic*,SDL_GetHapticFromInstanceID,(SDL_HapticID a),(a),return)
 SDL_DYNAPI_PROC(SDL_HapticID,SDL_GetHapticInstanceID,(SDL_Haptic *a),(a),return)
 SDL_DYNAPI_PROC(const char*,SDL_GetHapticName,(SDL_Haptic *a),(a),return)
+SDL_DYNAPI_PROC(int,SDL_ReadSurfacePixel,(SDL_Surface *a, int b, int c, Uint8 *d, Uint8 *e, Uint8 *f, Uint8 *g),(a,b,c,d,e,f,g),return)
diff --git a/src/test/SDL_test_compare.c b/src/test/SDL_test_compare.c
index d7adb22daf6b..db49719a23f0 100644
--- a/src/test/SDL_test_compare.c
+++ b/src/test/SDL_test_compare.c
@@ -28,19 +28,11 @@
 */
 #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;
 
-int
-SDLTest_ReadSurfacePixel(SDL_Surface *surface, int x, int y, Uint8 *r, Uint8 *g, Uint8 *b, Uint8 *a)
-{
-    return SDL_ReadSurfacePixel_impl(surface, x, y, r, g, b, a);
-}
-
 static void
 LogErrorFormat(const char *name, const SDL_PixelFormat *format)
 {
@@ -96,14 +88,14 @@ int SDLTest_CompareSurfaces(SDL_Surface *surface, SDL_Surface *referenceSurface,
         for (i = 0; i < surface->w; i++) {
             int temp;
 
-            temp = SDLTest_ReadSurfacePixel(surface, i, j, &R, &G, &B, &A);
+            temp = SDL_ReadSurfacePixel(surface, i, j, &R, &G, &B, &A);
             if (temp != 0) {
                 SDLTest_LogError("Failed to retrieve pixel (%d,%d): %s", i, j, SDL_GetError());
                 ret++;
                 continue;
             }
 
-            temp = SDLTest_ReadSurfacePixel(referenceSurface, i, j, &Rd, &Gd, &Bd, &Ad);
+            temp = SDL_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++;
diff --git a/src/video/SDL_surface.c b/src/video/SDL_surface.c
index c051d83687b7..3ca8519c837a 100644
--- a/src/video/SDL_surface.c
+++ b/src/video/SDL_surface.c
@@ -24,7 +24,6 @@
 #include "SDL_video_c.h"
 #include "SDL_blit.h"
 #include "SDL_RLEaccel_c.h"
-#include "SDL_surface_pixel_impl.h"
 #include "SDL_pixels_c.h"
 #include "SDL_yuv_c.h"
 #include "../render/SDL_sysrender.h"
@@ -36,11 +35,6 @@ SDL_COMPILE_TIME_ASSERT(surface_size_assumptions,
 
 SDL_COMPILE_TIME_ASSERT(can_indicate_overflow, SDL_SIZE_MAX > SDL_MAX_SINT32);
 
-int SDL_ReadSurfacePixel(SDL_Surface *surface, int x, int y, Uint8 *r, Uint8 *g, Uint8 *b, Uint8 *a)
-{
-    return SDL_ReadSurfacePixel_impl(surface, x, y, r, g, b, a);
-}
-
 /* Public routines */
 
 /*
@@ -1560,6 +1554,68 @@ int SDL_PremultiplyAlpha(int width, int height,
     return 0;
 }
 
+/* This function Copyright 2023 Collabora Ltd., contributed to SDL under the ZLib license */
+int SDL_ReadSurfacePixel(SDL_Surface *surface, int x, int y, Uint8 *r, Uint8 *g, Uint8 *b, Uint8 *a)
+{
+    Uint32 pixel = 0;
+    size_t bytes_per_pixel;
+    Uint8 unused;
+    Uint8 *p;
+
+    if (!surface || !surface->format || !surface->pixels) {
+        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) {
+        r = &unused;
+    }
+
+    if (!g) {
+        g = &unused;
+    }
+
+    if (!b) {
+        b = &unused;
+    }
+
+    if (!a) {
+        a = &unused;
+    }
+
+    bytes_per_pixel = surface->format->BytesPerPixel;
+
+    if (bytes_per_pixel > sizeof(pixel)) {
+        return SDL_InvalidParamError("surface->format->BytesPerPixel");
+    }
+
+    if (SDL_MUSTLOCK(surface)) {
+        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);
+
+    if (SDL_MUSTLOCK(surface)) {
+        SDL_UnlockSurface(surface);
+    }
+    return 0;
+}
+
 /*
  * Free a surface created by the above function.
  */
diff --git a/src/video/SDL_surface_pixel_impl.h b/src/video/SDL_surface_pixel_impl.h
deleted file mode 100644
index 67f772fe1533..000000000000
--- a/src/video/SDL_surface_pixel_impl.h
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
-  Copyright 1997-2024 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 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 || !surface->format || !surface->pixels) {
-        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) {
-        return SDL_InvalidParamError("r");
-    }
-
-    if (!g) {
-        return SDL_InvalidParamError("g");
-    }
-
-    if (!b) {
-        return SDL_InvalidParamError("b");
-    }
-
-    if (!a) {
-        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;
-}
diff --git a/test/testshape.c b/test/testshape.c
index 302be4ea0afd..a3136d3a3c6d 100644
--- a/test/testshape.c
+++ b/test/testshape.c
@@ -88,7 +88,7 @@ static void SDL_CalculateShapeBitmap(SDL_WindowShapeMode mode, SDL_Surface *shap
         bitmap_scanline = bitmap + y * bytes_per_scanline;
         for (x = 0; x < shape->w; x++) {
             alpha = 0;
-            if (SDLTest_ReadSurfacePixel(shape, x, y, &r, &g, &b, &alpha) != 0) {
+            if (SDL_ReadSurfacePixel(shape, x, y, &r, &g, &b, &alpha) != 0) {
                 continue;
             }