displayio: Fix several bugs (transparency and palettes of OnDiskBitmaps) #3834
Conversation
The transparent_color field was never initialized. I _think_ this means its value was always set to 0, or the blackest of blacks. Instead, initialize it to the sentinel value, newly given the name NO_TRANSPARENT_COLOR. This exposed a second problem: The test for whether there was an existing transparent color was wrong (backwards). I am guessing that this was not found due to the first bug; since the converter had a transparent color, the correct test would have led to always getting the error "Only one color can be transparent at a time". Closes adafruit#3723
|
@PaintYourDragon Test the artifacts from this build for the transparency/full black problem. The "testpattern4.bmp" file DOES still seem to crash, and I'll address it in a separate commit (probably a separate PR)
|
Microsoft documentation says: > If biCompression equals BI_RGB and the bitmap uses 8 bpp or less, the bitmap has a color table immediatelly following the BITMAPINFOHEADER structure. The color table consists of an array of RGBQUAD values. The size of the array is given by the biClrUsed member. If biClrUsed is zero, the array contains the maximum number of colors for the given bitdepth; that is, 2^biBitCount colors. Formerly, we treated 0 colors as "no image palette" during construction, but then during common_hal_displayio_ondiskbitmap_get_pixel indexed into the palette anyway. This could have unpredictable results. On a pygamer, it gave an image that was blue and black. On magtag, it gave a crash.
|
This PR has been updated with a fix for the 4-bit bitmap. Microsoft documentation says:
We were treating 0 colors as "no palette" during construction of the OnDiskBitmap, and then indexing into the palette anyway during common_hal_displayio_ondiskbitmap_get_pixel. This caused unpredictable results—my pygamer showed the image in black and blue, and my magtag crashed. |
jepler
changed the title
|
[CI failure is a problem uploading artifacts, which is reported at https://github.com/actions/upload-artifact/issues/116 but not metoo'd by us yet. should not affect accepting this PR] |
|
With latest from S3… displayio still exhibiting transparency issue at various depths and 4bpp image crash. Is there some still-pending build in the queue I should be waiting for? |
|
@PaintYourDragon Until this PR is merged, it won't appear on S3. You need to get it from the Actions build instead. How to navigate there: Click the "Checks" tab below the PR title, then click "artifacts", and find the "adafruit-magtag-..." zip near the top of the list. Direct-er link: https://github.com/adafruit/circuitpython/suites/1687216126/artifacts/31633147 |
|
I'm happy to approve this after @PaintYourDragon verifies the fix. |
|
The “artifact” version works and appears to fix both the transparency and crash-on-odd-size-BMP issues. Thanks. |
The transparent_color field was never initialized. I think this means its value was always set to 0, or the blackest of blacks. Instead, initialize it to the sentinel value, newly given the name NO_TRANSPARENT_COLOR.
This exposed a second problem: The test for whether there was an existing transparent color was wrong (backwards). I am guessing that this was not found due to the first bug; since the converter had a transparent color, the correct test would have led to always getting the error "Only one color can be transparent at a time".
Closes #3723