displayio color converter incorrect

[qutefox]

CircuitPython version and board name

I have rewritten the displayio, vectorio and adafruit_imageload code as a seperate c++ codebase with tests. So if you think this "version" should be ignored you could just close this bug report.

Code/REPL

# Well my test could be reproduced with something similar.
# Images are from: https://entropymine.com/jason/bmpsuite/bmpsuite/html/bmpsuite.html

import displayio
import adafruit_imageload

# bmp image load test description:
# A 16-bit image with the default color format: 5 bits each for red, green, and blue, and 1 unused bit.

imageBmp, pixelShaderBmp = adafruit_imageload.load(
    "images/g/rgb16.bmp", bitmap=displayio.Bitmap, palette=displayio.ColorConverter
)

imagePng, pixelShaderPng = adafruit_imageload.load(
    "images/g/rgb16.png", bitmap=displayio.Bitmap, palette=displayio.ColorConverter
)

for y in range(imageBmp.height):
    for x in range(imageBmp.width):
        bmpValue = pixelShaderBmp.convert(imageBmp[x, y])
        pngValue = pixelShaderPng.convert(imagePng[x, y])
        if bmpValue != pngValue:
            print("got pixel (in the bmp image) does not match the expected pixel (in the png image)")

Behavior

I get non-matching pixel messages.
(See the description for the explanation, and additional information for the "fix".)

Description

The bmp image that is being loaded has RGB555 color space. Inside the imageload module the color is converted like this:
bitmap_obj[offset + x] = converter_obj.convert(pixel)
https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad/blob/main/adafruit_imageload/bmp/truecolor.py#L129
where the converter_obj is created like this:
input_colorspace = Colorspace.RGB555
https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad/blob/main/adafruit_imageload/bmp/truecolor.py#L90

This conversion will happen here:
https://github.com/adafruit/circuitpython/blob/main/shared-module/displayio/ColorConverter.c#L318
Where the rgb555 color is first converted into rgb888, then the rgb888 is converted into rgb565.
The problem happens during the first conversion here:

case DISPLAYIO_COLORSPACE_RGB555: {
            uint32_t r8 = (pixel >> 10) << 3;
            uint32_t g8 = ((pixel >> 5) << 3) & 0xff;
            uint32_t b8 = (pixel << 3) & 0xff;
            pixel = (r8 << 16) | (g8 << 8) | b8;
        }

https://github.com/adafruit/circuitpython/blob/main/shared-module/displayio/ColorConverter.c#L199

If the rgb555 input color is: 0x7e10 then the calculated rgb888 is 0xf88080 instead of the correct 0xff8484. And from here (rolling the error into the 2nd conversion) the rgb565 will be 0xfc10 instead of the correct 0xfc30.

Additional information

Based on this article: https://stackoverflow.com/questions/71106056/what-is-the-correct-way-to-convert-rgb555-to-rgb888
I changed the above code to:

case ColorSpace::COLORSPACE_RGB555:
    {
        uint8_t r8 = (pixel >> 10) & 0x1f;
        uint8_t g8 = (pixel >> 5) & 0x1f;
        uint8_t b8 = pixel & 0x1f;
        r8 = (r8 << 3) | ((r8 >> 2) & 0b111);
        g8 = (g8 << 3) | ((g8 >> 2) & 0b111);
        b8 = (b8 << 3) | ((b8 >> 2) & 0b111);
        pixel = (r8 << 16) | (g8 << 8) | b8;
    }

In file: https://github.com/adafruit/circuitpython/blob/main/shared-module/displayio/ColorConverter.c#L199

and it solved the problem for me. After the change my test passed.
I hope my finding is good and valid and a little bit appreciated.

[tannewt]

Thank you for the thorough issue. Would you mind filing a pull request with this change? I bet the other conversions to RGB888 have the same issue. (RP2350 HSTX has this issue too.)