Change pixel size of "LA", "La", and "PA" from 4 to 2

[Yay295]

These image modes currently use 4 bytes for each pixel, but they only need two. I've started looking into this, but there are a lot more changes than I expected, so I'm creating this issue for documentation. The files I've found that need to be changed are:

• /src/libImaging/Access.c
  • ImagingAccessInit() contains a mapping to three functions (also defined in this file) for each of these modes.
• /src/libImaging/Bands.c
  • Lots of special casing for "LXXA", and some functions assume an image with 2 bands uses 4 bytes per pixel.
• /src/libImaging/Convert.c
  • All of the conversion functions for these modes.
• /src/libImaging/Geometry.c
  • bicubic_filter32LA() and bilinear_filter32LA().
• /src/libImaging/GetBBox.c
  • ImagingGetBBox() creates a bit mask for the alpha channel.
• /src/libImaging/Imaging.h
  • IMAGING_PIXEL_LA and IMAGING_PIXEL_PA are defined in this file, though they aren't used anywhere.
• /src/libImaging/Jpeg2KDecode.c
  • j2ku_graya_la() is currently used for both "LA" and "RGBA", so it will first have to be duplicated and renamed for "RGBA", and then the original can be modified.
• /src/libImaging/Jpeg2KEncode.c
  • j2k_pack_la().
• /src/libImaging/Pack.c
  • The references to packLA() can be replaced with copy2(), and packLAL() needs to be modified. packLA() is no longer used after this change, so it can be removed.
• /src/libImaging/Paste.c
  • fill_mask_L() uses a mask for the alpha channel, and ImagingPaste() calls the same function for "LA" and "RGBA".
• /src/libImaging/Storage.c
  • The image pixel size and line size are set in ImagingNewPrologueSubtype().
• /src/libImaging/Unpack.c
  • unpackLA() and unpackLAL().
• /src/PIL/PyAccess.py
  • The mode_map uses _PyAccess32_2() for these modes.

Changing im->pixelsize from 4 to 2 also means the image data will be stored in im->image8 instead of im->image32. im->image also exists and always holds the image data. The only difference between these pointers is their type: image is char, image8 is UINT8, and image32 is INT32. Some functions check for im->image8 being set and do different things based on that.

[radarhere]

Changing im->pixelsize from 4 to 2 also means the image data will be stored in im->image8 instead of im->image32.

This doesn't seem right to me. A byte is 8 bits, allowing for each channel to have a range of 0 to 255 - the maximum value of UINT8. 2 bytes would be 16 bits, not fitting into image8.

Pillow/src/libImaging/Imaging.h

Lines 93 to 95 in 5a087cc

	/* Data pointers */
	UINT8 **image8; /* Set for 8-bit images (pixelsize=1). */
	INT32 **image32; /* Set for 32-bit images (pixelsize=4). */

Your past self also agrees with me.

Shouldn't PA only need two bytes for each pixel though?

[Yay295]

It's here:

Pillow/src/libImaging/Storage.c

Lines 211 to 230 in 5a087cc

	/* Pointer array (allocate at least one line, to avoid MemoryError
	exceptions on platforms where calloc(0, x) returns NULL) */
	im->image = (char **)calloc((ysize > 0) ? ysize : 1, sizeof(void *));
	if (!im->image) {
	free(im);
	return (Imaging)ImagingError_MemoryError();
	}
	/* Initialize alias pointers to pixel data. */
	switch (im->pixelsize) {
	case 1:
	case 2:
	case 3:
	im->image8 = (UINT8 **)im->image;
	break;
	case 4:
	im->image32 = (INT32 **)im->image;
	break;
	}

First image is set to the image data, and then there's a switch on the pixel size that determines if image8 or image32 is set.

[radarhere]

Oh, ok. I guess I was wrong. Thanks for that.

[nulano]

EDIT: You beat me by a few minutes :)

If I understand this snippet initializing new images correctly, there are already existing modes with im->pixelsize==2 that do in fact use im->image8 (e.g. I;16 is listed earlier in the function):

Pillow/src/libImaging/Storage.c

Lines 220 to 230 in 5a087cc

	/* Initialize alias pointers to pixel data. */
	switch (im->pixelsize) {
	case 1:
	case 2:
	case 3:
	im->image8 = (UINT8 **)im->image;
	break;
	case 4:
	im->image32 = (INT32 **)im->image;
	break;
	}

[nulano]

FWIW this list may be helpful for implementing support for >4 byte images, tracked in #1888.

[Yay295]

I'm still debugging it, but I wanted to commit something, so here's what I have so far: Yay295@2643a3d

[radarhere]

#6540 was created as an implementation of this, but was closed, in favour of #6547.

Does that mean this issue can also be closed?

[Yay295]

I would say no, because "LA", "La", and "PA" are still 4 bytes.