On Wed, 14 Feb 2024 at 16:47:23 +0000, Gayathri Berli wrote: > In librsvg also we have seen some test failures and regressions. when we > debugged, we came to know that pixman code changes are triggering the > regression in librsvg.
That's good to know. I see in https://gitlab.freedesktop.org/pixman/pixman/-/issues/78 that there is some confusion over which layer of the stack is the right one to be doing this byteswapping: the original change was made to fix a bug seen in gnome-characters, but then that change caused the librsvg bug. In my experience the only way to solve endianness issues without causing regressions is to have clear documentation of what endianness each layer of the stack is aiming to use, so that you can check the behaviour of each function against that documentation/specification and say "this is correct" or "this is wrong" with confidence. The majority of computers this decade are little-endian, so it's mainly the people who are interested in supporting big-endian computers who need to get this right. For instance, if a function is returning 32-bit RGBA data, you need to be able to know whether the correct format is "red first" or "alpha first" or "red in the least significant byte of the first 32-bit word" or "alpha in the least significant byte". Otherwise, you can't know whether a change is actually correct, or whether it's a workaround for the layer above or below also being wrong. I think this is what Simon Ser means in their comments on https://gitlab.freedesktop.org/pixman/pixman/-/merge_requests/92. In your commit message you said "This will write out the pixels as BGRA on big endian systems but obviously that's wrong", but that isn't obvious (at least not to me, and probably not immediately obvious to the other Simon either). In https://gitlab.freedesktop.org/pixman/pixman/-/merge_requests/92 you mentioned that before reverting that change, pixman was failing its test suite on big-endian, and reverting the change makes its test suite pass. That's probably relatively good evidence that the revert is the correct thing to do, but that means it deserves to be mentioned in the commit message. This SDL change might be a good example of making sure that an intended endianness is documented clearly: https://github.com/libsdl-org/SDL/pull/8318/files and this one might be a good example of justifying why a change is the correct change: https://github.com/libsdl-org/SDL/pull/8819 smcv
