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.

https://github.com/python-pillow/Pillow/blob/5a087ccbb9b48cba04276fd1b7d0cec93488b7c3/src/libImaging/Imaging.h#L93-L95

Your past self also agrees with me.

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

[Yay295]

It's here:

https://github.com/python-pillow/Pillow/blob/5a087ccbb9b48cba04276fd1b7d0cec93488b7c3/src/libImaging/Storage.c#L211-L230

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): https://github.com/python-pillow/Pillow/blob/5a087ccbb9b48cba04276fd1b7d0cec93488b7c3/src/libImaging/Storage.c#L220-L230

[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: https://github.com/Yay295/Pillow/commit/2643a3d97133ff4d0968461f8b373d7a13834ad7

[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.