python-pillow / Pillow

Python Imaging Library (Fork)
https://python-pillow.org
Other
11.8k stars 2.13k forks source link

Some unpackers are misnamed #8021

Open Yay295 opened 1 month ago

Yay295 commented 1 month ago

RGB15 actually reads data as XBGR (XBBBBBGGGGGRRRRR).

https://github.com/python-pillow/Pillow/blob/a8f434f67657d9286bafb0f132ac608276fa96c0/src/libImaging/Unpack.c#L661-L674

RGBA15 actually reads data as ABGR (ABBBBBGGGGGRRRRR).

https://github.com/python-pillow/Pillow/blob/a8f434f67657d9286bafb0f132ac608276fa96c0/src/libImaging/Unpack.c#L676-L689

RGBA4B actually reads data as ABGR (AAAABBBBGGGGRRRR).

https://github.com/python-pillow/Pillow/blob/a8f434f67657d9286bafb0f132ac608276fa96c0/src/libImaging/Unpack.c#L766-L779

etc.

Basically, all of the unpackers that read less than 8 bits per band appear to be backwards.

Yay295 commented 1 month ago

This is related to #8019, because I wrote those packers in the correct order, which is actually the opposite of the unpackers.

Yay295 commented 1 month ago

Here's the names they should be based on what they actually do and the format specified in Unpack.c.

https://github.com/python-pillow/Pillow/blob/824db7152d1159bf3a895e7699b16ebc77298e77/src/libImaging/Unpack.c#L1534-L1543

RGB;15 → XBGR;1555 RGB;16 → BGR;565 BGR;5 → XRGB;1555 BGR;15 → XRGB;1555 BGR;16 → RGB;565 RGB;4B → XBGR;4 RGBA;4B → ABGR;4 RGBA;15 → ARGB;1555 BGRA;15 → ABGR;1555

Unfortunately the specified format doesn't quite work because the bands in these modes aren't all the same size, so I just listed all of the sizes for those modes.

The good thing about listing out the band sizes though is that those names aren't already being used, so I could add all of them as "new" rawmodes and the old ones could be deprecated without interfering with one another.

radarhere commented 1 month ago

There's a slightly awkward overlap in code that would change with this and with #7965.

Yay295 commented 1 month ago

Yes. This change would supersede both #7965 and #8019.

Yay295 commented 3 weeks ago

I have a branch for these changes, but it's based on #8026 because it affects the same test file.