Some unpackers are misnamed

[Yay295]

RGB15 actually reads data as XBGR (XBBBBBGGGGGRRRRR).

Pillow/src/libImaging/Unpack.c

Lines 661 to 674 in a8f434f

	void
	ImagingUnpackRGB15(UINT8 *out, const UINT8 *in, int pixels) {
	int i, pixel;
	/* RGB, 5 bits per pixel */
	for (i = 0; i < pixels; i++) {
	pixel = in[0] + (in[1] << 8);
	out[R] = (pixel & 31) * 255 / 31;
	out[G] = ((pixel >> 5) & 31) * 255 / 31;
	out[B] = ((pixel >> 10) & 31) * 255 / 31;
	out[A] = 255;
	out += 4;
	in += 2;
	}
	}

RGBA15 actually reads data as ABGR (ABBBBBGGGGGRRRRR).

Pillow/src/libImaging/Unpack.c

Lines 676 to 689 in a8f434f

	void
	ImagingUnpackRGBA15(UINT8 *out, const UINT8 *in, int pixels) {
	int i, pixel;
	/* RGB, 5/5/5/1 bits per pixel */
	for (i = 0; i < pixels; i++) {
	pixel = in[0] + (in[1] << 8);
	out[R] = (pixel & 31) * 255 / 31;
	out[G] = ((pixel >> 5) & 31) * 255 / 31;
	out[B] = ((pixel >> 10) & 31) * 255 / 31;
	out[A] = (pixel >> 15) * 255;
	out += 4;
	in += 2;
	}
	}

RGBA4B actually reads data as ABGR (AAAABBBBGGGGRRRR).

Pillow/src/libImaging/Unpack.c

Lines 766 to 779 in a8f434f

	void
	ImagingUnpackRGBA4B(UINT8 *out, const UINT8 *in, int pixels) {
	int i, pixel;
	/* RGBA, 4 bits per pixel */
	for (i = 0; i < pixels; i++) {
	pixel = in[0] + (in[1] << 8);
	out[R] = (pixel & 15) * 17;
	out[G] = ((pixel >> 4) & 15) * 17;
	out[B] = ((pixel >> 8) & 15) * 17;
	out[A] = ((pixel >> 12) & 15) * 17;
	out += 4;
	in += 2;
	}
	}

etc.

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

[Yay295]

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

[Yay295]

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

Pillow/src/libImaging/Unpack.c

Lines 1534 to 1543 in 824db71

	/* raw mode syntax is "<mode>;<bits><flags>" where "bits" defaults
	depending on mode (1 for "1", 8 for "P" and "L", etc), and
	"flags" should be given in alphabetical order. if both bits
	and flags have their default values, the ; should be left out */
	/* flags: "I" inverted data; "R" reversed bit order; "B" big
	endian byte order (default is little endian); "L" line
	interleave, "S" signed, "F" floating point */
	/* exception: rawmodes "I" and "F" are always native endian byte order */

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]

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

[Yay295]

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

[Yay295]

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

[radarhere]

That branch has now become #8158