py-sdl / py-sdl2

Python ctypes wrapper around SDL2
Other
296 stars 49 forks source link

Fix unit tests on big-endian systems #232

Closed a-hurst closed 2 years ago

a-hurst commented 2 years ago

PR Description

Closes #227. Tested and working on a Power Mac G5 with SDL2 2.0.22 and Void Linux PowerPC 64-bit, except for the renderer tests which segfault because of a GL/Mesa/nouveau configuration issue (glxgears segfaults immediately too, so not a SDL2/PySDL2 problem).

So far all the fixes have been to the unit tests themselves, which were typically failing because they weren't written properly with big-endian in mind. The only area I have hesitations about are the pixels3d and related Numpy access functions, since the orders of RGBA tuples are inverted from little-endian systems, meaning that any code making assumptions about the byte order in those arrays wouldn't be readily portable. Not sure if that's something that should be left to developers themselves or taken care of internally in PySDL2.

Merge Checklist

smcv commented 2 years ago

The only area I have hesitations about are the pixels3d and related Numpy access functions, since the orders of RGBA tuples are inverted from little-endian systems, meaning that any code making assumptions about the byte order in those arrays wouldn't be readily portable

This is a matter of documentation more than anything else. You can have portable code by normalizing to a known byte order (like SDL_PIXELFORMAT_RGBA32) and having API users iterate a byte at a time; or you can have portable code by normalizing to a known encoding in native endianness (like SDL_PIXELFORMAT_RGBA8888) and having API users iterate a 32-bit word at a time and then pick out the colour channels from that word; but if you go one way and API users go the other, then they will have a problem. As long as the documentation is clear about what is meant to happen, when API users get it wrong you can point to the documentation and say "you're holding it wrong".

One major advantage of changing the tests to match the implementation as you have done here, rather than changing the implementation to match the tests, is that if a developer has already written some code that works correctly on both LE and BE (either intentionally or by mistake), it will continue to work.