Support multi-byte values with Bitmap

It also corrects the behavior of single byte values.

Fixes #1744

Author: tannewt

shared-module/displayio/Bitmap.c

@@ -33,20 +33,21 @@
 void common_hal_displayio_bitmap_construct(displayio_bitmap_t *self, uint32_t width,
     uint32_t height, uint32_t bits_per_value) {
     uint32_t row_width = width * bits_per_value;
-    // word align
-    if (row_width % 32 != 0) {
-        self->stride = (row_width / 32 + 1);
+    // align to size_t
+    uint8_t align_bits = 8 * sizeof(size_t);
+    if (row_width % align_bits != 0) {
+        self->stride = (row_width / align_bits + 1);
     } else {
-        self->stride = row_width / 32;
+        self->stride = row_width / align_bits;
     }
     self->width = width;
     self->height = height;
-    self->data = m_malloc(self->stride * height * sizeof(uint32_t), false);
+    self->data = m_malloc(self->stride * height * sizeof(size_t), false);
     self->read_only = false;
     self->bits_per_value = bits_per_value;
 
-    if (bits_per_value > 8) {
-        mp_raise_NotImplementedError(translate("Only bit maps of 8 bit color or less are supported"));
+    if (bits_per_value > 8 && bits_per_value != 16 && bits_per_value != 32) {
+        mp_raise_NotImplementedError(translate("Invalid bits per value"));
     }
 
     // Division and modulus can be slow because it has to handle any integer. We know bits_per_value
@@ -56,7 +57,7 @@ void common_hal_displayio_bitmap_construct(displayio_bitmap_t *self, uint32_t wi
     self->x_shift = 0; // Used to divide the index by the number of pixels per word. Its used in a
                        // shift which effectively divides by 2 ** x_shift.
     uint32_t power_of_two = 1;
-    while (power_of_two < 32 / bits_per_value ) {
+    while (power_of_two < align_bits / bits_per_value ) {
         self->x_shift++;
         power_of_two <<= 1;
     }
@@ -76,57 +77,50 @@ uint32_t common_hal_displayio_bitmap_get_bits_per_value(displayio_bitmap_t *self
     return self->bits_per_value;
 }
 
-void common_hal_displayio_bitmap_load_row(displayio_bitmap_t *self, uint16_t y, uint8_t* data, uint16_t len) {
-    if (len != self->stride * sizeof(uint32_t)) {
-        mp_raise_ValueError(translate("row must be packed and word aligned"));
-    }
-    uint32_t* row_value = self->data + (y * self->stride);
-    // Do the memcpy ourselves since we may want to flip endianness.
-    for (uint32_t i = 0; i < self->stride; i++) {
-        #pragma GCC diagnostic push
-        #pragma GCC diagnostic ignored "-Wcast-align"
-        uint32_t value = ((uint32_t *)data)[i];
-        #pragma GCC diagnostic pop
-        if (self->bits_per_value < 16) {
-            value = ((value >> 24) & 0xff) |
-                    ((value << 8) & 0xff0000) |
-                    ((value >> 8) & 0xff00) |
-                    ((value << 24) & 0xff000000);
-        }
-        *row_value = value;
-        row_value++;
-    }
-}
-
 uint32_t common_hal_displayio_bitmap_get_pixel(displayio_bitmap_t *self, int16_t x, int16_t y) {
     if (x >= self->width || x < 0 || y >= self->height || y < 0) {
         return 0;
     }
     int32_t row_start = y * self->stride;
-    if (self->bits_per_value < 8) {
-        uint32_t word = self->data[row_start + (x >> self->x_shift)];
+    uint32_t bytes_per_value = self->bits_per_value / 8;
+    if (bytes_per_value < 1) {
+        size_t word = self->data[row_start + (x >> self->x_shift)];
 
-        return (word >> (32 - ((x & self->x_mask) + 1) * self->bits_per_value)) & self->bitmask;
+        return (word >> (sizeof(size_t) * 8 - ((x & self->x_mask) + 1) * self->bits_per_value)) & self->bitmask;
     } else {
-        uint32_t bytes_per_value = self->bits_per_value / 8;
-        return self->data[row_start + x * bytes_per_value];
+        size_t* row = self->data + row_start;
+        if (bytes_per_value == 1) {
+            return ((uint8_t*) row)[x];
+        } else if (bytes_per_value == 2) {
+            return ((uint16_t*) row)[x];
+        } else if (bytes_per_value == 4) {
+            return ((uint32_t*) row)[x];
+        }
     }
+    return 0;
 }
 
 void common_hal_displayio_bitmap_set_pixel(displayio_bitmap_t *self, int16_t x, int16_t y, uint32_t value) {
     if (self->read_only) {
         mp_raise_RuntimeError(translate("Read-only object"));
     }
     int32_t row_start = y * self->stride;
-    if (self->bits_per_value < 8) {
-        uint32_t bit_position = (32 - ((x & self->x_mask) + 1) * self->bits_per_value);
+    uint32_t bytes_per_value = self->bits_per_value / 8;
+    if (bytes_per_value < 1) {
+        uint32_t bit_position = (sizeof(size_t) * 8 - ((x & self->x_mask) + 1) * self->bits_per_value);
         uint32_t index = row_start + (x >> self->x_shift);
         uint32_t word = self->data[index];
         word &= ~(self->bitmask << bit_position);
         word |= (value & self->bitmask) << bit_position;
         self->data[index] = word;
     } else {
-        uint32_t bytes_per_value = self->bits_per_value / 8;
-        self->data[row_start + x * bytes_per_value] = value;
+        size_t* row = self->data + row_start;
+        if (bytes_per_value == 1) {
+            ((uint8_t*) row)[x] = value;
+        } else if (bytes_per_value == 2) {
+            ((uint16_t*) row)[x] = value;
+        } else if (bytes_per_value == 4) {
+            ((uint32_t*) row)[x] = value;
+        }
     }
 }

shared-module/displayio/Bitmap.h

@@ -36,11 +36,11 @@ typedef struct {
     mp_obj_base_t base;
     uint16_t width;
     uint16_t height;
-    uint32_t* data;
-    uint16_t stride; // words
+    size_t* data;
+    uint16_t stride; // size_t's
     uint8_t bits_per_value;
     uint8_t x_shift;
-    uint8_t x_mask;
+    size_t x_mask;
     uint16_t bitmask;
     bool read_only;
 } displayio_bitmap_t;

supervisor/shared/display.c

@@ -34,7 +34,7 @@
 #include "shared-bindings/displayio/TileGrid.h"
 #include "supervisor/memory.h"
 
-extern uint32_t blinka_bitmap_data[];
+extern size_t blinka_bitmap_data[];
 extern displayio_bitmap_t blinka_bitmap;
 extern displayio_group_t circuitpython_splash;
 
@@ -107,7 +107,7 @@ void supervisor_display_move_memory(void) {
     #endif
 }
 
-uint32_t blinka_bitmap_data[32] = {
+size_t blinka_bitmap_data[32] = {
     0x00000011, 0x11000000,
     0x00000111, 0x53100000,
     0x00000111, 0x56110000,

tools/gen_display_resources.py

@@ -153,7 +153,7 @@ def _load_row(self, y, row):
     .base = {{.type = &displayio_bitmap_type }},
     .width = {},
     .height = {},
-    .data = (uint32_t*) font_bitmap_data,
+    .data = (size_t*) font_bitmap_data,
     .stride = {},
     .bits_per_value = 1,
     .x_shift = 5,