Commit fd495bb57c62e5b55381b98c0a732790f95d3531
2023-09-29T14:23:36
SDLTest_CompareSurfaces: Decode pixels correctly on big-endian platforms Previously, if acting on a surface with less than 32 bits per pixel, this code was placing the pixel value from the surface in the first few bytes of the Uint32 to be decoded, and unrelated data from a subsequent pixel in the remaining bytes. Because SDL_GetRGBA takes the bits to be decoded from the least-significant bits of the given value, ignoring the higher-order bits if any, this happened to be correct on little-endian platforms, where the first few bytes store the least-significant bits of an integer. However, it was incorrect on big-endian, where the first few bytes are the most-significant bits of an integer. The previous implementation also assumed that unaligned access to a 32-bit quantity is possible, which is not the case on all CPUs (but happens to be true on x86). These issues were not discovered until now because SDLTest_CompareSurfaces() is only used in testautomation, which until recently was not being run routinely at build-time, because it contained other assumptions that can fail in an autobuilder or CI environment. Resolves: https://github.com/libsdl-org/SDL/issues/8315 Signed-off-by: Simon McVittie <smcv@collabora.com> (cherry picked from commit d95d2d7051e23271db7cec43134582834fb3ac8b) (cherry picked from commit 6b5eadb10f02b51b400e710e2c95e9f0c2cb579c)
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
diff --git a/src/test/SDL_test_compare.c b/src/test/SDL_test_compare.c
index 9c4adf2..b840880 100644
--- a/src/test/SDL_test_compare.c
+++ b/src/test/SDL_test_compare.c
@@ -36,6 +36,25 @@
/* Counter for _CompareSurface calls; used for filename creation when comparisons fail */
static int _CompareSurfaceCount = 0;
+static Uint32
+GetPixel(Uint8 *p, size_t bytes_per_pixel)
+{
+ Uint32 ret = 0;
+
+ 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;
+}
+
/* Compare surfaces */
int SDLTest_CompareSurfaces(SDL_Surface *surface, SDL_Surface *referenceSurface, int allowable_error)
{
@@ -74,11 +93,14 @@ int SDLTest_CompareSurfaces(SDL_Surface *surface, SDL_Surface *referenceSurface,
/* Compare image - should be same format. */
for (j = 0; j < surface->h; j++) {
for (i = 0; i < surface->w; i++) {
+ Uint32 pixel;
p = (Uint8 *)surface->pixels + j * surface->pitch + i * bpp;
p_reference = (Uint8 *)referenceSurface->pixels + j * referenceSurface->pitch + i * bpp_reference;
- SDL_GetRGBA(*(Uint32 *)p, surface->format, &R, &G, &B, &A);
- SDL_GetRGBA(*(Uint32 *)p_reference, referenceSurface->format, &Rd, &Gd, &Bd, &Ad);
+ pixel = GetPixel(p, bpp);
+ SDL_GetRGBA(pixel, surface->format, &R, &G, &B, &A);
+ pixel = GetPixel(p_reference, bpp_reference);
+ SDL_GetRGBA(pixel, referenceSurface->format, &Rd, &Gd, &Bd, &Ad);
dist = 0;
dist += (R - Rd) * (R - Rd);