Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SDL3] Several rendering tests failing on big-endian architectures #8315

Closed
smcv opened this issue Sep 29, 2023 · 7 comments · Fixed by #8317
Closed

[SDL3] Several rendering tests failing on big-endian architectures #8315

smcv opened this issue Sep 29, 2023 · 7 comments · Fixed by #8317

Comments

@smcv
Copy link
Contributor

smcv commented Sep 29, 2023

A snapshot of 441a5b7 (plus #8312 applied as a patch) is failing build-time tests on big-endian architectures like s390x and powerpc in Debian experimental, with errors like this:

INFO:  09/29/23 04:28:51: �[0;93m----- Test Case 15.2: 'render_testPrimitives' started�[0m
INFO:  09/29/23 04:28:51: Test Description: 'Tests rendering primitives'
INFO:  09/29/23 04:28:51: Test Iteration 1: execKey 14735344628637601718
INFO:  09/29/23 04:28:51: Assert 'SDL_CreateWindow()': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Check SDL_CreateWindow result': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'SDL_CreateRenderer()': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Check SDL_CreateRenderer result': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Validate result from SDL_SetRenderDrawColor, expected: 0, got: 0': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Validate result from SDL_RenderClear, expected: 0, got: 0': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Validate result from SDL_SetRenderDrawBlendMode, expected: 0, got: 0': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Validate result from SDL_SetRenderDrawColor, expected: 0, got: 0': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert '_hasDrawColor': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Validate results from calls to SDL_SetRenderDrawColor, expected: 0, got: 0': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Validate results from calls to SDL_RenderPoint, expected: 0, got: 0': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Validate allocated temp pixel buffer': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'Verify result from SDL_CreateSurfaceFrom is not NULL': �[0;32mPassed�[0m
ERROR: 09/29/23 04:28:51: Comparison of pixels with allowable error of 0 failed 3779 times.
ERROR: 09/29/23 04:28:51: First detected occurrence at position 0,0 with a squared RGB-difference of 21025.
ERROR: 09/29/23 04:28:51: Surfaces from failed comparison saved as 'CompareSurfaces0001_TestOutput.bmp' and 'CompareSurfaces0001_Reference.bmp'
ERROR: 09/29/23 04:28:51: Assert 'Validate result from SDLTest_CompareSurfaces, expected: 0, got: 3779': �[0;31mFailed�[0m
INFO:  09/29/23 04:28:51: Assert 'SDL_DestroyRenderer()': �[0;32mPassed�[0m
INFO:  09/29/23 04:28:51: Assert 'SDL_DestroyWindow': �[0;32mPassed�[0m
ERROR: 09/29/23 04:28:51: Assert Summary: Total=16 �[0;32mPassed=15�[0m �[0;31mFailed=1�[0m
INFO:  09/29/23 04:28:51: Total Test runtime: 0.0 sec
ERROR: 09/29/23 04:28:51: �[0;93m>>> Test 'render_testPrimitives':�[0m �[0;31mFailed�[0m

render_testBlit, render_testBlitColor, surface_testBlit, surface_testBlitBlendNone, surface_testBlitColorMod, surface_testBlitAlphaMod fail similarly. Unfortunately I don't have access to the output bitmap files until/unless I can reproduce this via remote access to a big-endian machine (all I get from the official autobuilders is a text log).

This is almost certainly confusion between byte orders (red in least significant byte vs. red in first byte, or similar).
@smcv smcv mentioned this issue Sep 29, 2023
Merged
@smcv
Copy link
Contributor Author

smcv commented Sep 29, 2023

This is a bit weird. The output .bmp files all look right, except that one has a non-trivial alpha channel - but the comparison says they are different. So I think we have some compensating errors somewhere, with the result of SDL_GetRGBA() differing from the result of conversion in SDL_SaveBMP_RW().

render_testPrimitives

ERROR: 09/29/23 12:00:59: Comparison of pixels with allowable error of 0 failed 3779 times.
ERROR: 09/29/23 12:00:59: Reference surface format: 17101803 SDL_PIXELFORMAT_RGB24, 24 bits/3 bytes per pixel
ERROR: 09/29/23 12:00:59: Reference surface format: R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:00:59: Reference surface format: G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:00:59: Reference surface format: B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:00:59: Reference surface format: A mask 00000000, loss 8, shift 0
ERROR: 09/29/23 12:00:59: Actual surface format   : 16362004 SDL_PIXELFORMAT_ARGB8888, 32 bits/4 bytes per pixel
ERROR: 09/29/23 12:00:59: Actual surface format   : R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:00:59: Actual surface format   : G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:00:59: Actual surface format   : B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:00:59: Actual surface format   : A mask ff000000, loss 0, shift 24
ERROR: 09/29/23 12:00:59: First detected occurrence at position 0,0 with a squared RGB-difference of 21025.
ERROR: 09/29/23 12:00:59: Reference pixel: R=105 G=105 B=0 A=255
ERROR: 09/29/23 12:00:59: Actual pixel   : R=5 G=105 B=105 A=255
ERROR: 09/29/23 12:00:59: Surfaces from failed comparison saved as 'CompareSurfaces0001_TestOutput.bmp' and 'CompareSurfaces0001_Reference.bmp'

CompareSurfaces0001_Reference
CompareSurfaces0001_TestOutput

render_testBlit

ERROR: 09/29/23 12:05:19: Comparison of pixels with allowable error of 0 failed 2552 times.
ERROR: 09/29/23 12:05:19: Reference surface format: 17101803 SDL_PIXELFORMAT_RGB24, 24 bits/3 bytes per pixel
ERROR: 09/29/23 12:05:19: Reference surface format: R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Reference surface format: G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Reference surface format: B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: Reference surface format: A mask 00000000, loss 8, shift 0
ERROR: 09/29/23 12:05:19: Actual surface format   : 16362004 SDL_PIXELFORMAT_ARGB8888, 32 bits/4 bytes per pixel
ERROR: 09/29/23 12:05:19: Actual surface format   : R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Actual surface format   : G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Actual surface format   : B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: Actual surface format   : A mask ff000000, loss 0, shift 24
ERROR: 09/29/23 12:05:19: First detected occurrence at position 11,8 with a squared RGB-difference of 65025.
ERROR: 09/29/23 12:05:19: Reference pixel: R=0 G=0 B=255 A=255
ERROR: 09/29/23 12:05:19: Actual pixel   : R=0 G=0 B=0 A=255

CompareSurfaces0002_Reference
CompareSurfaces0002_TestOutput

render_testBlitColor

ERROR: 09/29/23 12:05:19: Comparison of pixels with allowable error of 0 failed 2484 times.
ERROR: 09/29/23 12:05:19: Reference surface format: 17101803 SDL_PIXELFORMAT_RGB24, 24 bits/3 bytes per pixel
ERROR: 09/29/23 12:05:19: Reference surface format: R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Reference surface format: G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Reference surface format: B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: Reference surface format: A mask 00000000, loss 8, shift 0
ERROR: 09/29/23 12:05:19: Actual surface format   : 16362004 SDL_PIXELFORMAT_ARGB8888, 32 bits/4 bytes per pixel
ERROR: 09/29/23 12:05:19: Actual surface format   : R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Actual surface format   : G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Actual surface format   : B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: Actual surface format   : A mask ff000000, loss 0, shift 24
ERROR: 09/29/23 12:05:19: First detected occurrence at position 16,8 with a squared RGB-difference of 800.
ERROR: 09/29/23 12:05:19: Reference pixel: R=20 G=0 B=0 A=255
ERROR: 09/29/23 12:05:19: Actual pixel   : R=0 G=20 B=0 A=255

CompareSurfaces0003_Reference
CompareSurfaces0003_TestOutput

surface_testBlit

ERROR: 09/29/23 12:05:19: Comparison of pixels with allowable error of 0 failed 2552 times.
ERROR: 09/29/23 12:05:19: Reference surface format: 17101803 SDL_PIXELFORMAT_RGB24, 24 bits/3 bytes per pixel
ERROR: 09/29/23 12:05:19: Reference surface format: R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Reference surface format: G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Reference surface format: B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: Reference surface format: A mask 00000000, loss 8, shift 0
ERROR: 09/29/23 12:05:19: Actual surface format   : 16462004 SDL_PIXELFORMAT_RGBA8888, 32 bits/4 bytes per pixel
ERROR: 09/29/23 12:05:19: Actual surface format   : R mask ff000000, loss 0, shift 24
ERROR: 09/29/23 12:05:19: Actual surface format   : G mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Actual surface format   : B mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Actual surface format   : A mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: First detected occurrence at position 11,8 with a squared RGB-difference of 65025.
ERROR: 09/29/23 12:05:19: Reference pixel: R=0 G=0 B=255 A=255
ERROR: 09/29/23 12:05:19: Actual pixel   : R=0 G=0 B=0 A=255

CompareSurfaces0009_Reference
CompareSurfaces0009_TestOutput

surface_testBlitBlendNone

ERROR: 09/29/23 12:05:19: Comparison of pixels with allowable error of 0 failed 329 times.
ERROR: 09/29/23 12:05:19: Reference surface format: 17101803 SDL_PIXELFORMAT_RGB24, 24 bits/3 bytes per pixel
ERROR: 09/29/23 12:05:19: Reference surface format: R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Reference surface format: G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Reference surface format: B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: Reference surface format: A mask 00000000, loss 8, shift 0
ERROR: 09/29/23 12:05:19: Actual surface format   : 16462004 SDL_PIXELFORMAT_RGBA8888, 32 bits/4 bytes per pixel
ERROR: 09/29/23 12:05:19: Actual surface format   : R mask ff000000, loss 0, shift 24
ERROR: 09/29/23 12:05:19: Actual surface format   : G mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Actual surface format   : B mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Actual surface format   : A mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: First detected occurrence at position 59,35 with a squared RGB-difference of 65025.
ERROR: 09/29/23 12:05:19: Reference pixel: R=255 G=255 B=0 A=255
ERROR: 09/29/23 12:05:19: Actual pixel   : R=255 G=255 B=255 A=0

CompareSurfaces0010_Reference
CompareSurfaces0010_TestOutput

surface_testBlitColorMod

ERROR: 09/29/23 12:05:19: Comparison of pixels with allowable error of 0 failed 2484 times.
ERROR: 09/29/23 12:05:19: Reference surface format: 17101803 SDL_PIXELFORMAT_RGB24, 24 bits/3 bytes per pixel
ERROR: 09/29/23 12:05:19: Reference surface format: R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Reference surface format: G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Reference surface format: B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: Reference surface format: A mask 00000000, loss 8, shift 0
ERROR: 09/29/23 12:05:19: Actual surface format   : 16462004 SDL_PIXELFORMAT_RGBA8888, 32 bits/4 bytes per pixel
ERROR: 09/29/23 12:05:19: Actual surface format   : R mask ff000000, loss 0, shift 24
ERROR: 09/29/23 12:05:19: Actual surface format   : G mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Actual surface format   : B mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Actual surface format   : A mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: First detected occurrence at position 16,8 with a squared RGB-difference of 800.
ERROR: 09/29/23 12:05:19: Reference pixel: R=20 G=0 B=0 A=255
ERROR: 09/29/23 12:05:19: Actual pixel   : R=0 G=20 B=0 A=255

CompareSurfaces0037_Reference
CompareSurfaces0037_TestOutput

surface_testBlitColorMod

ERROR: 09/29/23 12:05:19: Comparison of pixels with allowable error of 0 failed 2963 times.
ERROR: 09/29/23 12:05:19: Reference surface format: 17101803 SDL_PIXELFORMAT_RGB24, 24 bits/3 bytes per pixel
ERROR: 09/29/23 12:05:19: Reference surface format: R mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Reference surface format: G mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Reference surface format: B mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: Reference surface format: A mask 00000000, loss 8, shift 0
ERROR: 09/29/23 12:05:19: Actual surface format   : 16462004 SDL_PIXELFORMAT_RGBA8888, 32 bits/4 bytes per pixel
ERROR: 09/29/23 12:05:19: Actual surface format   : R mask ff000000, loss 0, shift 24
ERROR: 09/29/23 12:05:19: Actual surface format   : G mask 00ff0000, loss 0, shift 16
ERROR: 09/29/23 12:05:19: Actual surface format   : B mask 0000ff00, loss 0, shift 8
ERROR: 09/29/23 12:05:19: Actual surface format   : A mask 000000ff, loss 0, shift 0
ERROR: 09/29/23 12:05:19: First detected occurrence at position 15,8 with a squared RGB-difference of 400.
ERROR: 09/29/23 12:05:19: Reference pixel: R=0 G=0 B=20 A=255
ERROR: 09/29/23 12:05:19: Actual pixel   : R=0 G=0 B=0 A=20

CompareSurfaces0038_Reference
CompareSurfaces0038_TestOutput

@smcv
Copy link
Contributor Author

smcv commented Sep 29, 2023

On a big-endian system like this, we expect SDL_PIXELFORMAT_RGBA8888 to have 4 bytes per pixel, encoding R, G, B, A in that order, equivalent to SDL_PIXELFORMAT_RGBA32 (red in the most significant byte of each 4-byte word, and alpha in the least). The surface tests seem like they match that.

Meanwhile, the render tests are in SDL_PIXELFORMAT_ARGB8888, which is 4 bytes per pixel, encoding A, R, G, B in that order, equivalent to SDL_PIXELFORMAT_ARGB32 (alpha in the most significant byte and blue in the least).

Both of those seem like they match up with the masks shown.

@smcv
Copy link
Contributor Author

smcv commented Sep 29, 2023

I think the problem might be that test/testautomation_images.c contains test images that are opaque blobs of bytes, but declares them to be in format SDL_PIXELFORMAT_RGB24, which if I understand correctly is defined to be in CPU byte-order (encoding B, G, R in that order on LE, but R, G, B in that order on BE).

@smcv
Copy link
Contributor Author

smcv commented Sep 29, 2023

No, that's wrong, I think I'm being confused by SDL's terminology for image formats again. SDL_PIXELFORMAT_RGB24 and SDL_PIXELFORMAT_RGBA24 always have red in the first byte, and it's the 8888 formats where the in-memory byte order is dependent on CPU endianness.

smcv added a commit to smcv/SDL that referenced this issue Sep 29, 2023
@smcv
SDLTest_CompareSurfaces: Decode pixels correctly on big-endian platforms
328bbc0 
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: libsdl-org#8315
Signed-off-by: Simon McVittie <[email protected]>
@smcv smcv mentioned this issue Sep 29, 2023
Merged
@smcv
Copy link
Contributor Author

smcv commented Sep 29, 2023

It looks like the real problem is that SDLTest_CompareSurfaces() has never worked correctly on big-endian platforms (s390x, powerpc, etc.) or on platforms that require correct alignment (mostly RISC architectures), but because testautomation was never really hooked up to the build system until recently, nobody had run it on non-x86. Fixed in #8317, I think.

@slouken
Copy link
Collaborator

slouken commented Sep 29, 2023

No, that's wrong, I think I'm being confused by SDL's terminology for image formats again. SDL_PIXELFORMAT_RGB24 and SDL_PIXELFORMAT_RGBA24 always have red in the first byte, and it's the 8888 formats where the in-memory byte order is dependent on CPU endianness.

That's correct.

slouken pushed a commit that referenced this issue Sep 29, 2023
@smcv @slouken
SDLTest_CompareSurfaces: Decode pixels correctly on big-endian platforms
d95d2d7 
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: #8315
Signed-off-by: Simon McVittie <[email protected]>
slouken pushed a commit that referenced this issue Sep 29, 2023
@smcv @slouken
SDLTest_CompareSurfaces: Decode pixels correctly on big-endian platforms
6b5eadb 
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: #8315
Signed-off-by: Simon McVittie <[email protected]>
(cherry picked from commit d95d2d7)
slouken pushed a commit that referenced this issue Sep 29, 2023
@smcv @slouken
SDLTest_CompareSurfaces: Decode pixels correctly on big-endian platforms
fd495bb 
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: #8315
Signed-off-by: Simon McVittie <[email protected]>
(cherry picked from commit d95d2d7)
(cherry picked from commit 6b5eadb)
@smcv smcv mentioned this issue Sep 29, 2023
Merged
@smcv
Copy link
Contributor Author

smcv commented Sep 29, 2023

I think I'm being confused by SDL's terminology for image formats again. SDL_PIXELFORMAT_RGB24 and SDL_PIXELFORMAT_RGBA24 always have red in the first byte, and it's the 8888 formats where the in-memory byte order is dependent on CPU endianness.

That's correct.

I opened #8318 to try to clarify this for future contributors.

Kontrabant pushed a commit to Kontrabant/SDL that referenced this issue Oct 22, 2023
@smcv @Kontrabant
SDLTest_CompareSurfaces: Decode pixels correctly on big-endian platforms
c869c09 
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: libsdl-org#8315
Signed-off-by: Simon McVittie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

SDLTest_CompareSurfaces: Decode pixels correctly on big-endian platforms smcv/SDL
2 participants
@smcv @slouken