From 28bd29eb42d61d0a7116b6c4295e166fcfbe7145 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 16 Dec 2020 13:48:07 -0600 Subject: [PATCH 1/2] displayio: ColorConverter: fix logic errors about transparent pixels 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 --- shared-module/displayio/ColorConverter.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/shared-module/displayio/ColorConverter.c b/shared-module/displayio/ColorConverter.c index 80558d037a689..40dcd9d167818 100644 --- a/shared-module/displayio/ColorConverter.c +++ b/shared-module/displayio/ColorConverter.c @@ -29,6 +29,8 @@ #include "py/misc.h" #include "py/runtime.h" +#define NO_TRANSPARENT_COLOR (0x1000000) + uint32_t displayio_colorconverter_dither_noise_1 (uint32_t n) { n = (n >> 13) ^ n; @@ -42,6 +44,7 @@ uint32_t displayio_colorconverter_dither_noise_2(uint32_t x, uint32_t y) { void common_hal_displayio_colorconverter_construct(displayio_colorconverter_t* self, bool dither) { self->dither = dither; + self->transparent_color = NO_TRANSPARENT_COLOR; } uint16_t displayio_colorconverter_compute_rgb565(uint32_t color_rgb888) { @@ -130,7 +133,7 @@ bool common_hal_displayio_colorconverter_get_dither(displayio_colorconverter_t* } void common_hal_displayio_colorconverter_make_transparent(displayio_colorconverter_t* self, uint32_t transparent_color) { - if (self->transparent_color >= 0x1000000) { + if (self->transparent_color != NO_TRANSPARENT_COLOR) { mp_raise_RuntimeError(translate("Only one color can be transparent at a time")); } self->transparent_color = transparent_color; @@ -138,8 +141,8 @@ void common_hal_displayio_colorconverter_make_transparent(displayio_colorconvert void common_hal_displayio_colorconverter_make_opaque(displayio_colorconverter_t* self, uint32_t transparent_color) { (void) transparent_color; - // 0x1000000 will never equal a valid color - self->transparent_color = 0x1000000; + // NO_TRANSPARENT_COLOR will never equal a valid color + self->transparent_color = NO_TRANSPARENT_COLOR; } void displayio_colorconverter_convert(displayio_colorconverter_t *self, const _displayio_colorspace_t* colorspace, const displayio_input_pixel_t *input_pixel, displayio_output_pixel_t *output_color) { From f224ed18488b1e92b6d13b09b696769a2ac380ce Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Thu, 17 Dec 2020 10:54:37 -0600 Subject: [PATCH 2/2] OnDiskBitmap: Correct handling of "0 color palette" images 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. --- shared-module/displayio/OnDiskBitmap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shared-module/displayio/OnDiskBitmap.c b/shared-module/displayio/OnDiskBitmap.c index 993fc03de62ec..c1a0ddeb7e6b5 100644 --- a/shared-module/displayio/OnDiskBitmap.c +++ b/shared-module/displayio/OnDiskBitmap.c @@ -57,7 +57,7 @@ void common_hal_displayio_ondiskbitmap_construct(displayio_ondiskbitmap_t *self, uint32_t compression = read_word(bmp_header, 15); uint32_t number_of_colors = read_word(bmp_header, 23); - bool indexed = ((bits_per_pixel <= 8) && (number_of_colors != 0)); + bool indexed = bits_per_pixel <= 8; self->bitfield_compressed = (compression == 3); self->bits_per_pixel = bits_per_pixel; self->width = read_word(bmp_header, 9); @@ -75,6 +75,9 @@ void common_hal_displayio_ondiskbitmap_construct(displayio_ondiskbitmap_t *self, self->b_bitmask = 0x1f; } } else if (indexed && self->bits_per_pixel != 1) { + if (number_of_colors == 0) { + number_of_colors = 1 << bits_per_pixel; + } uint16_t palette_size = number_of_colors * sizeof(uint32_t); uint16_t palette_offset = 0xe + header_size;