Support multi-byte values with Bitmap

It also corrects the behavior of single byte values.

Fixes #1744

1 files changed, 33 insertions, 39 deletions

diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c
index 2b2c70ab6..f8dc24c15 100644
--- a/shared-module/displayio/Bitmap.c
+++ b/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,41 +77,27 @@ 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) {
@@ -118,15 +105,22 @@ void common_hal_displayio_bitmap_set_pixel(displayio_bitmap_t *self, int16_t x,
         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;
+        }
     }
 }