Tweaks based on feedback

4 files changed, 43 insertions, 49 deletions

diff --git a/shared-bindings/displayio/Bitmap.c b/shared-bindings/displayio/Bitmap.c
index b01fe0e0c..d415127d2 100644
--- a/shared-bindings/displayio/Bitmap.c
+++ b/shared-bindings/displayio/Bitmap.c
@@ -45,30 +45,29 @@
 //|
 //| .. warning:: This will likely be changed before 4.0.0. Consider it very experimental.
 //|
-//| .. class:: Bitmap(width, height, value_size)
+//| .. class:: Bitmap(width, height, value_count)
 //|
-//|   Create a Bitmap object with the given fixed size.
+//|   Create a Bitmap object with the given fixed size. Each pixel stores a value that is used to
+//|   index into a corresponding palette. This enables differently colored sprites to share the
+//|   underlying Bitmap. value_count is used to minimize the memory used to store the Bitmap.
 //|
 //|   :param int width: The number of values wide
 //|   :param int height: The number of values high
-//|   :param int value_size: The value size in bits. Must be power of 2.
+//|   :param int value_count: The number of possible pixel values.
 //|
 STATIC mp_obj_t displayio_bitmap_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *pos_args) {
     mp_arg_check_num(n_args, n_kw, 3, 3, false);
     uint32_t width = mp_obj_get_int(pos_args[0]);
     uint32_t height = mp_obj_get_int(pos_args[1]);
-    uint32_t value_size = mp_obj_get_int(pos_args[2]);
+    uint32_t value_count = mp_obj_get_int(pos_args[2]);
     uint32_t power_of_two = 1;
-    while (value_size > power_of_two) {
+    while (value_count > (1U << power_of_two)) {
         power_of_two <<= 1;
     }
-    if (value_size != power_of_two) {
-        mp_raise_ValueError(translate("value_size must be power of two"));
-    }
 
     displayio_bitmap_t *self = m_new_obj(displayio_bitmap_t);
     self->base.type = &displayio_bitmap_type;
-    common_hal_displayio_bitmap_construct(self, width, height, value_size);
+    common_hal_displayio_bitmap_construct(self, width, height, power_of_two);
 
     return MP_OBJ_FROM_PTR(self);
 }
diff --git a/shared-bindings/displayio/Group.c b/shared-bindings/displayio/Group.c
index a2a879987..e6f1fbbf5 100644
--- a/shared-bindings/displayio/Group.c
+++ b/shared-bindings/displayio/Group.c
@@ -74,7 +74,7 @@ STATIC mp_obj_t displayio_group_make_new(const mp_obj_type_t *type, size_t n_arg
 
 //|   .. method:: append(layer)
 //|
-//|     Switches do displaying the given group of elements.
+//|     Append a layer to the group. It will be drawn above other layers.
 //|
 STATIC mp_obj_t displayio_group_obj_append(mp_obj_t self_in, mp_obj_t layer) {
     displayio_group_t *self = MP_OBJ_TO_PTR(self_in);
diff --git a/shared-bindings/displayio/Palette.c b/shared-bindings/displayio/Palette.c
index bb2af9410..99963f131 100644
--- a/shared-bindings/displayio/Palette.c
+++ b/shared-bindings/displayio/Palette.c
@@ -41,8 +41,8 @@
 //| :class:`Palette` -- Stores a mapping from bitmap pixel values to display colors
 //| ===============================================================================
 //|
-//| Manage updating a display over SPI four wire protocol in the background while Python code runs.
-//| It doesn't handle display initialization.
+//| Map a pixel value to a full color. Colors are transformed to the display's format internally to
+//| save memory.
 //|
 //| .. warning:: This will be changed before 4.0.0. Consider it very experimental.
 //|
@@ -76,42 +76,41 @@ STATIC mp_obj_t palette_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t val
     if (value == MP_OBJ_NULL) {
         // delete item
         return MP_OBJ_NULL; // op not supported
-    } else {
-        displayio_palette_t *self = MP_OBJ_TO_PTR(self_in);
-        if (MP_OBJ_IS_TYPE(index_in, &mp_type_slice)) {
-            return MP_OBJ_NULL; // Slicing not supported. Use a duplicate Palette to swap multiple colors atomically.
-        } else {
-            if (value == MP_OBJ_SENTINEL) {
-                return MP_OBJ_NULL; // index read is not supported
-            } else {
-                size_t index = mp_get_index(&displayio_palette_type, self->max_value, index_in, false);
+    }
+    // Slicing not supported. Use a duplicate Palette to swap multiple colors atomically.
+    if (MP_OBJ_IS_TYPE(index_in, &mp_type_slice)) {
+        return MP_OBJ_NULL;
+    }
+    // index read is not supported
+    if (value == MP_OBJ_SENTINEL) {
+        return MP_OBJ_NULL;
+    }
+    displayio_palette_t *self = MP_OBJ_TO_PTR(self_in);
+    size_t index = mp_get_index(&displayio_palette_type, self->max_value, index_in, false);
 
-                uint32_t color;
-                mp_int_t int_value;
-                mp_buffer_info_t bufinfo;
-                if (mp_get_buffer(value, &bufinfo, MP_BUFFER_READ)) {
-                    if (bufinfo.typecode != 'b' && bufinfo.typecode != 'B' && bufinfo.typecode != BYTEARRAY_TYPECODE) {
-                        mp_raise_ValueError(translate("color buffer must be a bytearray or array of type 'b' or 'B'"));
-                    }
-                    uint8_t* buf = bufinfo.buf;
-                    if (bufinfo.len == 3 || bufinfo.len == 4) {
-                        color = buf[0] << 16 | buf[1] << 8 | buf[2];
-                    } else {
-                        mp_raise_ValueError(translate("color buffer must be 3 bytes (RGB) or 4 bytes (RGBA)"));
-                    }
-                } else if (mp_obj_get_int_maybe(value, &int_value)) {
-                    if (int_value < 0 || int_value > 0xffffff) {
-                        mp_raise_TypeError(translate("color must be between 0x000000 and 0xffffff"));
-                    }
-                    color = int_value;
-                } else {
-                    mp_raise_TypeError(translate("color buffer must be a buffer or int"));
-                }
-                common_hal_displayio_palette_set_color(self, index, color);
-                return mp_const_none;
-            }
+    uint32_t color;
+    mp_int_t int_value;
+    mp_buffer_info_t bufinfo;
+    if (mp_get_buffer(value, &bufinfo, MP_BUFFER_READ)) {
+        if (bufinfo.typecode != 'b' && bufinfo.typecode != 'B' && bufinfo.typecode != BYTEARRAY_TYPECODE) {
+            mp_raise_ValueError(translate("color buffer must be a bytearray or array of type 'b' or 'B'"));
         }
+        uint8_t* buf = bufinfo.buf;
+        if (bufinfo.len == 3 || bufinfo.len == 4) {
+            color = buf[0] << 16 | buf[1] << 8 | buf[2];
+        } else {
+            mp_raise_ValueError(translate("color buffer must be 3 bytes (RGB) or 4 bytes (RGBA)"));
+        }
+    } else if (mp_obj_get_int_maybe(value, &int_value)) {
+        if (int_value < 0 || int_value > 0xffffff) {
+            mp_raise_TypeError(translate("color must be between 0x000000 and 0xffffff"));
+        }
+        color = int_value;
+    } else {
+        mp_raise_TypeError(translate("color buffer must be a buffer or int"));
     }
+    common_hal_displayio_palette_set_color(self, index, color);
+    return mp_const_none;
 }
 
 //|   .. method:: make_transparent(value)
diff --git a/shared-bindings/displayio/__init__.h b/shared-bindings/displayio/__init__.h
index 5c3fc2b55..a6663bf57 100644
--- a/shared-bindings/displayio/__init__.h
+++ b/shared-bindings/displayio/__init__.h
@@ -31,8 +31,4 @@
 
 // Nothing now.
 
-// typedef enum {
-//     PIXEL_
-// } displayio_pixel_format;
-
 #endif  // MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO___INIT___H