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>
main
Simon McVittie 2023-09-29 14:23:36 +01:00 committed by Sam Lantinga
parent d65861f049
commit d95d2d7051
1 changed files with 24 additions and 2 deletions

View File

@ -33,6 +33,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)
{
@ -71,11 +90,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);