Encapsulate buffers inside PixelBuf and refactor it.

5 files changed, 148 insertions, 297 deletions

diff --git a/shared-bindings/_pixelbuf/PixelBuf.c b/shared-bindings/_pixelbuf/PixelBuf.c
index 3e9319047..61b4c9ae0 100644
--- a/shared-bindings/_pixelbuf/PixelBuf.c
+++ b/shared-bindings/_pixelbuf/PixelBuf.c
@@ -36,13 +36,14 @@
 
 #include <string.h>
 
-#include "PixelBuf.h"
-#include "shared-bindings/_pixelbuf/types.h"
-#include "../../shared-module/_pixelbuf/PixelBuf.h"
+#include "shared-bindings/_pixelbuf/PixelBuf.h"
+#include "shared-module/_pixelbuf/PixelBuf.h"
 #include "shared-bindings/digitalio/DigitalInOut.h"
 
 extern const int32_t colorwheel(float pos);
 
+static void parse_byteorder(mp_obj_t byteorder_obj, pixelbuf_byteorder_details_t* parsed);
+
 //| .. currentmodule:: pixelbuf
 //|
 //| :class:`PixelBuf` -- A fast RGB[W] pixel buffer for LED and similar devices
@@ -50,140 +51,109 @@ extern const int32_t colorwheel(float pos);
 //|
 //| :class:`~_pixelbuf.PixelBuf` implements an RGB[W] bytearray abstraction.
 //|
-//| .. class:: PixelBuf(size, buf, byteorder="BGR", brightness=0, rawbuf=None, offset=0, auto_write=False)
+//| .. class:: PixelBuf(size, *, byteorder="BGR", brightness=0, auto_write=False, header=b"", trailer=b"")
 //|
 //|   Create a PixelBuf object of the specified size, byteorder, and bits per pixel.
 //|
-//|   When given a second bytearray (``rawbuf``), changing brightness adjusts the
-//|   brightness of all members of ``buf``.
-//|
-//|   When only given ``buf``, ``brightness`` applies to the next pixel assignment.
+//|   When brightness is less than 1.0, a second buffer will be used to store the color values
+//|   before they are adjusted for brightness.
 //|
-//|   When ``P`` (pwm duration) is present as the 4th character of the byteorder 
-//|   string, the 4th value in the tuple/list for a pixel is the individual pixel 
+//|   When ``P`` (pwm duration) is present as the 4th character of the byteorder
+//|   string, the 4th value in the tuple/list for a pixel is the individual pixel
 //|   brightness (0.0-1.0) and will enable a Dotstar compatible 1st byte in the
 //|   output buffer (``buf``).
 //|
 //|   :param ~int size: Number of pixelsx
-//|   :param ~bytearray buf: Bytearray in which to store pixel data
 //|   :param ~str byteorder: Byte order string (such as "BGR" or "PBGR")
 //|   :param ~float brightness: Brightness (0 to 1.0, default 1.0)
-//|   :param ~bytearray rawbuf: Bytearray in which to store raw pixel data (before brightness adjustment)
-//|   :param ~int offset: Offset from start of buffer (default 0)
 //|   :param ~bool auto_write: Whether to automatically write pixels (Default False)
+//|   :param bytes header: Sequence of bytes to always send before pixel values.
+//|   :param bytes trailer: Sequence of bytes to always send after pixel values.
 //|
 STATIC mp_obj_t pixelbuf_pixelbuf_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
-    mp_arg_check_num(n_args, kw_args, 2, MP_OBJ_FUN_ARGS_MAX, true);
-    enum { ARG_size, ARG_buf, ARG_byteorder, ARG_brightness, ARG_rawbuf, ARG_offset,
-           ARG_auto_write };
+    mp_arg_check_num(n_args, kw_args, 1, MP_OBJ_FUN_ARGS_MAX, true);
+    enum { ARG_size, ARG_byteorder, ARG_brightness, ARG_auto_write, ARG_header, ARG_trailer };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_size, MP_ARG_REQUIRED | MP_ARG_INT },
-        { MP_QSTR_buf, MP_ARG_REQUIRED | MP_ARG_OBJ },
-        { MP_QSTR_byteorder, MP_ARG_OBJ, { .u_obj = MP_OBJ_NEW_QSTR(MP_QSTR_BGR) } },
-        { MP_QSTR_brightness, MP_ARG_OBJ, { .u_obj = mp_const_none } },
-        { MP_QSTR_rawbuf, MP_ARG_OBJ, { .u_obj = mp_const_none } },
-        { MP_QSTR_offset, MP_ARG_INT, { .u_int = 0 } },
-        { MP_QSTR_auto_write, MP_ARG_BOOL, {.u_bool = false} },
+        { MP_QSTR_byteorder, MP_ARG_KW_ONLY | MP_ARG_OBJ, { .u_obj = MP_OBJ_NEW_QSTR(MP_QSTR_BGR) } },
+        { MP_QSTR_brightness, MP_ARG_KW_ONLY | MP_ARG_OBJ, { .u_obj = mp_const_none } },
+        { MP_QSTR_auto_write, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} },
+        { MP_QSTR_header, MP_ARG_KW_ONLY | MP_ARG_OBJ, { .u_obj = mp_const_none } },
+        { MP_QSTR_trailer, MP_ARG_KW_ONLY | MP_ARG_OBJ, { .u_obj = mp_const_none } },
     };
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
-    const char *byteorder = NULL;
     pixelbuf_byteorder_details_t byteorder_details;
-    size_t bo_len;
 
-    if (!MP_OBJ_IS_STR(args[ARG_byteorder].u_obj))
+    parse_byteorder(args[ARG_byteorder].u_obj, &byteorder_details);
+
+    mp_buffer_info_t header_bufinfo;
+    mp_buffer_info_t trailer_bufinfo;
+
+    if (!mp_get_buffer(args[ARG_header].u_obj, &header_bufinfo, MP_BUFFER_READ)) {
+        header_bufinfo.buf = NULL;
+        header_bufinfo.len = 0;
+    }
+    if (!mp_get_buffer(args[ARG_trailer].u_obj, &trailer_bufinfo, MP_BUFFER_READ)) {
+        trailer_bufinfo.buf = NULL;
+        trailer_bufinfo.len = 0;
+    }
+
+    float brightness = 1.0;
+    if (args[ARG_brightness].u_obj != mp_const_none) {
+        brightness = mp_obj_get_float(args[ARG_brightness].u_obj);
+        if (brightness < 0) {
+            brightness = 0;
+        } else if (brightness > 1) {
+            brightness = 1;
+        }
+    }
+
+    // Validation complete, allocate and populate object.
+    pixelbuf_pixelbuf_obj_t *self = m_new_obj(pixelbuf_pixelbuf_obj_t);
+    self->base.type = &pixelbuf_pixelbuf_type;
+    common_hal__pixelbuf_pixelbuf_construct(self, args[ARG_size].u_int,
+    &byteorder_details, brightness, args[ARG_auto_write].u_bool, header_bufinfo.buf,
+    header_bufinfo.len, trailer_bufinfo.buf, trailer_bufinfo.len);
+
+    return MP_OBJ_FROM_PTR(self);
+}
+
+static void parse_byteorder(mp_obj_t byteorder_obj, pixelbuf_byteorder_details_t* parsed) {
+    if (!MP_OBJ_IS_STR(byteorder_obj)) {
         mp_raise_TypeError(translate("byteorder is not a string"));
+    }
 
-    byteorder = mp_obj_str_get_data(args[ARG_byteorder].u_obj, &bo_len);
-    if (bo_len < 3 || bo_len > 4)
+    size_t bo_len;
+    const char *byteorder = mp_obj_str_get_data(byteorder_obj, &bo_len);
+    if (bo_len < 3 || bo_len > 4) {
         mp_raise_ValueError(translate("Invalid byteorder string"));
-    byteorder_details.order = args[ARG_byteorder].u_obj;
+    }
+    parsed->order_string = byteorder_obj;
 
-    byteorder_details.bpp = bo_len;
+    parsed->bpp = bo_len;
     char *dotstar = strchr(byteorder, 'P');
     char *r = strchr(byteorder, 'R');
     char *g = strchr(byteorder, 'G');
     char *b = strchr(byteorder, 'B');
     char *w = strchr(byteorder, 'W');
     int num_chars = (dotstar ? 1 : 0) + (w ? 1 : 0) + (r ? 1 : 0) + (g ? 1 : 0) + (b ? 1 : 0);
-    if ((num_chars < byteorder_details.bpp) || !(r && b && g))
+    if ((num_chars < parsed->bpp) || !(r && b && g)) {
         mp_raise_ValueError(translate("Invalid byteorder string"));
-    byteorder_details.is_dotstar = dotstar ? true : false;
-    byteorder_details.has_white = w ? true : false;
-    byteorder_details.byteorder.r = r - byteorder;
-    byteorder_details.byteorder.g = g - byteorder;
-    byteorder_details.byteorder.b = b - byteorder;
-    byteorder_details.byteorder.w = w ? w - byteorder : 0;
+    }
+    parsed->is_dotstar = dotstar ? true : false;
+    parsed->has_white = w ? true : false;
+    parsed->byteorder.r = r - byteorder;
+    parsed->byteorder.g = g - byteorder;
+    parsed->byteorder.b = b - byteorder;
+    parsed->byteorder.w = w ? w - byteorder : 0;
     // The dotstar brightness byte is always first (as it goes with the pixel start bits)
     if (dotstar && byteorder[0] != 'P') {
         mp_raise_ValueError(translate("Invalid byteorder string"));
     }
-    if (byteorder_details.has_white && byteorder_details.is_dotstar)
+    if (parsed->has_white && parsed->is_dotstar) {
         mp_raise_ValueError(translate("Invalid byteorder string"));
-
-    size_t effective_bpp = byteorder_details.is_dotstar ? 4 : byteorder_details.bpp; // Always 4 for DotStar
-    size_t bytes = args[ARG_size].u_int * effective_bpp;
-    size_t offset = args[ARG_offset].u_int;
-    mp_buffer_info_t bufinfo, rawbufinfo;
-
-    mp_get_buffer_raise(args[ARG_buf].u_obj, &bufinfo, MP_BUFFER_READ | MP_BUFFER_WRITE);
-    bool two_buffers = args[ARG_rawbuf].u_obj != mp_const_none;
-    if (two_buffers) {
-        mp_get_buffer_raise(args[ARG_rawbuf].u_obj, &rawbufinfo, MP_BUFFER_READ | MP_BUFFER_WRITE);
-        if (rawbufinfo.len != bufinfo.len) {
-            mp_raise_ValueError(translate("rawbuf is not the same size as buf"));
-        }
-    }
-
-    if (bytes + offset > bufinfo.len)
-        mp_raise_ValueError_varg(translate("buf is too small. need %d bytes"), bytes + offset);
-
-    // Validation complete, allocate and populate object.
-    pixelbuf_pixelbuf_obj_t *self = m_new_obj(pixelbuf_pixelbuf_obj_t);
-
-    self->base.type = &pixelbuf_pixelbuf_type;
-    self->pixels = args[ARG_size].u_int;
-    self->bytes = bytes;
-    self->byteorder = byteorder_details;  // Copied because we modify for dotstar
-    self->bytearray = args[ARG_buf].u_obj;
-    self->two_buffers = two_buffers;
-    self->rawbytearray = two_buffers ? args[ARG_rawbuf].u_obj : NULL;
-    self->offset = offset;
-    self->buf = (uint8_t *)bufinfo.buf + offset;
-    self->rawbuf = two_buffers ? (uint8_t *)rawbufinfo.buf + offset : NULL;
-    self->pixel_step = effective_bpp;
-    self->auto_write = args[ARG_auto_write].u_bool;
-
-    if (args[ARG_brightness].u_obj == mp_const_none) {
-        self->brightness = 1.0;
-    } else {
-        self->brightness = mp_obj_get_float(args[ARG_brightness].u_obj);
-        if (self->brightness < 0)
-            self->brightness = 0;
-        else if (self->brightness > 1)
-            self->brightness = 1;
     }
-
-    if (self->byteorder.is_dotstar) {
-        // Initialize the buffer with the dotstar start bytes.
-        // Note: Header and end must be setup by caller
-        for (uint i = 0; i < self->pixels * 4; i += 4) {
-            self->buf[i] = DOTSTAR_LED_START_FULL_BRIGHT;
-            if (two_buffers) {
-                self->rawbuf[i] = DOTSTAR_LED_START_FULL_BRIGHT;
-            }
-        }
-    }
-
-    return MP_OBJ_FROM_PTR(self);
-}
-
-
-// Helper to ensure we have the native super class instead of a subclass.
-static pixelbuf_pixelbuf_obj_t* native_pixelbuf(mp_obj_t pixelbuf_obj) {
-    mp_obj_t native_pixelbuf = mp_instance_cast_to_native_base(pixelbuf_obj, &pixelbuf_pixelbuf_type);
-    mp_obj_assert_native_inited(native_pixelbuf);
-    return MP_OBJ_TO_PTR(native_pixelbuf);
 }
 
 //|   .. attribute:: bpp
@@ -191,8 +161,7 @@ static pixelbuf_pixelbuf_obj_t* native_pixelbuf(mp_obj_t pixelbuf_obj) {
 //|     The number of bytes per pixel in the buffer (read-only)
 //|
 STATIC mp_obj_t pixelbuf_pixelbuf_obj_get_bpp(mp_obj_t self_in) {
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
-    return mp_obj_new_int_from_uint(self->byteorder.bpp);
+    return MP_OBJ_NEW_SMALL_INT(common_hal__pixelbuf_pixelbuf_get_bpp(self_in));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(pixelbuf_pixelbuf_get_bpp_obj, pixelbuf_pixelbuf_obj_get_bpp);
 
@@ -207,30 +176,24 @@ const mp_obj_property_t pixelbuf_pixelbuf_bpp_obj = {
 //|   .. attribute:: brightness
 //|
 //|     Float value between 0 and 1.  Output brightness.
-//|     If the PixelBuf was allocated with two both a buf and a rawbuf,
-//|     setting this value causes a recomputation of the values in buf.
-//|     If only a buf was provided, then the brightness only applies to
-//|     future pixel changes.
-//|     In DotStar mode
+//|
+//|     When brightness is less than 1.0, a second buffer will be used to store the color values
+//|     before they are adjusted for brightness.
 //|
 STATIC mp_obj_t pixelbuf_pixelbuf_obj_get_brightness(mp_obj_t self_in) {
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
-    return mp_obj_new_float(self->brightness);
+    return mp_obj_new_float(common_hal__pixelbuf_pixelbuf_get_brightness(self_in));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(pixelbuf_pixelbuf_get_brightness_obj, pixelbuf_pixelbuf_obj_get_brightness);
 
 
 STATIC mp_obj_t pixelbuf_pixelbuf_obj_set_brightness(mp_obj_t self_in, mp_obj_t value) {
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
-    self->brightness = mp_obj_float_get(value);
-    if (self->brightness > 1)
-        self->brightness = 1;
-    else if (self->brightness < 0)
-        self->brightness = 0;
-    if (self->two_buffers)
-        pixelbuf_recalculate_brightness(self);
-    if (self->auto_write)
-        pixelbuf_call_show(self_in);
+    mp_float_t brightness = mp_obj_float_get(value);
+    if (brightness > 1) {
+        brightness = 1;
+    } else if (brightness < 0) {
+        brightness = 0;
+    }
+    common_hal__pixelbuf_pixelbuf_set_brightness(self_in, brightness);
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(pixelbuf_pixelbuf_set_brightness_obj, pixelbuf_pixelbuf_obj_set_brightness);
@@ -242,37 +205,18 @@ const mp_obj_property_t pixelbuf_pixelbuf_brightness_obj = {
               (mp_obj_t)&mp_const_none_obj},
 };
 
-void pixelbuf_recalculate_brightness(pixelbuf_pixelbuf_obj_t *self) {
-    uint8_t *buf = (uint8_t *)self->buf;
-    uint8_t *rawbuf = (uint8_t *)self->rawbuf;
-    // Compensate for shifted buffer (bpp=3 dotstar)
-    for (uint i = 0; i < self->bytes; i++) {
-        // Don't adjust per-pixel luminance bytes in dotstar mode
-        if (!self->byteorder.is_dotstar || (i % 4 != 0))
-            buf[i] = rawbuf[i] * self->brightness;
-    }
-}
-
-mp_obj_t pixelbuf_call_show(mp_obj_t self_in) {
-    mp_obj_t dest[2];
-    mp_load_method(self_in, MP_QSTR_show, dest);
-    return mp_call_method_n_kw(0, 0, dest);
-}
-
 //|   .. attribute:: auto_write
 //|
 //|     Whether to automatically write the pixels after each update.
 //|
 STATIC mp_obj_t pixelbuf_pixelbuf_obj_get_auto_write(mp_obj_t self_in) {
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
-    return mp_obj_new_bool(self->auto_write);
+    return mp_obj_new_bool(common_hal__pixelbuf_pixelbuf_get_auto_write(self_in));
 }
 MP_DEFINE_CONST_FUN_OBJ_1(pixelbuf_pixelbuf_get_auto_write_obj, pixelbuf_pixelbuf_obj_get_auto_write);
 
 
 STATIC mp_obj_t pixelbuf_pixelbuf_obj_set_auto_write(mp_obj_t self_in, mp_obj_t value) {
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
-    self->auto_write = mp_obj_is_true(value);
+    common_hal__pixelbuf_pixelbuf_set_auto_write(self_in, mp_obj_is_true(value));
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_2(pixelbuf_pixelbuf_set_auto_write_obj, pixelbuf_pixelbuf_obj_set_auto_write);
@@ -284,33 +228,12 @@ const mp_obj_property_t pixelbuf_pixelbuf_auto_write_obj = {
               (mp_obj_t)&mp_const_none_obj},
 };
 
-
-//|   .. attribute:: buf
-//|
-//|     (read-only) bytearray of pixel data after brightness adjustment.  If an offset was provided
-//|     then this bytearray is the subset of the bytearray passed in that represents the
-//|     actual pixels.
-//|
-STATIC mp_obj_t pixelbuf_pixelbuf_obj_get_buf(mp_obj_t self_in) {
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
-    return mp_obj_new_bytearray_by_ref(self->bytes, self->buf);
-}
-MP_DEFINE_CONST_FUN_OBJ_1(pixelbuf_pixelbuf_get_buf_obj, pixelbuf_pixelbuf_obj_get_buf);
-
-const mp_obj_property_t pixelbuf_pixelbuf_buf_obj = {
-    .base.type = &mp_type_property,
-    .proxy = {(mp_obj_t)&pixelbuf_pixelbuf_get_buf_obj,
-              (mp_obj_t)&mp_const_none_obj,
-              (mp_obj_t)&mp_const_none_obj},
-};
-
 //|   .. attribute:: byteorder
 //|
 //|     byteorder string for the buffer (read-only)
 //|
 STATIC mp_obj_t pixelbuf_pixelbuf_obj_get_byteorder(mp_obj_t self_in) {
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
-    return self->byteorder.order;
+    return common_hal__pixelbuf_pixelbuf_get_byteorder_string(self_in);
 }
 MP_DEFINE_CONST_FUN_OBJ_1(pixelbuf_pixelbuf_get_byteorder_str, pixelbuf_pixelbuf_obj_get_byteorder);
 
@@ -322,32 +245,49 @@ const mp_obj_property_t pixelbuf_pixelbuf_byteorder_str = {
 };
 
 STATIC mp_obj_t pixelbuf_pixelbuf_unary_op(mp_unary_op_t op, mp_obj_t self_in) {
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
     switch (op) {
         case MP_UNARY_OP_BOOL: return mp_const_true;
-        case MP_UNARY_OP_LEN: return MP_OBJ_NEW_SMALL_INT(self->pixels);
+        case MP_UNARY_OP_LEN:
+            return MP_OBJ_NEW_SMALL_INT(common_hal__pixelbuf_pixelbuf_get_len(self_in));
         default: return MP_OBJ_NULL; // op not supported
     }
 }
 
 //|   .. method:: show()
 //|
-//|     Must be implemented in subclasses.
+//|     Transmits the color data to the pixels so that they are shown. This is done automatically
+//|     when `auto_write` is True.
 //|
 
 STATIC mp_obj_t pixelbuf_pixelbuf_show(mp_obj_t self_in) {
-    mp_raise_NotImplementedError(NULL);
+    common_hal__pixelbuf_pixelbuf_show(self_in);
+    return mp_const_none;
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(pixelbuf_pixelbuf_show_obj, pixelbuf_pixelbuf_show);
 
+//| .. function:: fill(color)
+//|
+//|   Fills the given pixelbuf with the given color.
+//|
+
+STATIC mp_obj_t pixelbuf_pixelbuf_fill(mp_obj_t self_in, mp_obj_t value) {
+    common_hal__pixelbuf_pixelbuf_fill(self_in, value);
+
+    return mp_const_none;
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_2(pixelbuf_pixelbuf_fill_obj, pixelbuf_pixelbuf_fill);
+
 
 //|   .. method:: __getitem__(index)
 //|
-//|     Returns the pixel value at the given index.
+//|     Returns the pixel value at the given index as a tuple of (Red, Green, Blue[, White]) values
+//|     between 0 and 255.
 //|
 //|   .. method:: __setitem__(index, value)
 //|
-//|     Sets the pixel value at the given index.
+//|     Sets the pixel value at the given index. Value can either be a tuple of (Red, Green, Blue
+//|     [, White]) values between 0 and 255 or an integer where the red, green and blue values are
+//|     packed into the lower three bytes (0xRRGGBB).
 //|
 STATIC mp_obj_t pixelbuf_pixelbuf_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value) {
     if (value == MP_OBJ_NULL) {
@@ -356,32 +296,34 @@ STATIC mp_obj_t pixelbuf_pixelbuf_subscr(mp_obj_t self_in, mp_obj_t index_in, mp
         return MP_OBJ_NULL; // op not supported
     }
 
-    pixelbuf_pixelbuf_obj_t *self = native_pixelbuf(self_in);
-
     if (0) {
 #if MICROPY_PY_BUILTINS_SLICE
     } else if (MP_OBJ_IS_TYPE(index_in, &mp_type_slice)) {
         mp_bound_slice_t slice;
 
-        mp_seq_get_fast_slice_indexes(self->pixels, index_in, &slice);
+        size_t length = common_hal__pixelbuf_pixelbuf_get_len(self_in);
+        mp_seq_get_fast_slice_indexes(length, index_in, &slice);
 
-        if ((slice.stop * self->pixel_step) > self->bytes)
-            mp_raise_IndexError(translate("Range out of bounds"));
-        if (slice.step < 0)
+        if (slice.step < 0) {
             mp_raise_IndexError(translate("Negative step not supported"));
+        }
 
         if (value == MP_OBJ_SENTINEL) { // Get
             size_t len = slice.stop - slice.start;
             if (slice.step > 1) {
                 len = (len / slice.step) + (len % slice.step ? 1 : 0);
             }
-            uint8_t *readbuf = self->two_buffers ? self->rawbuf : self->buf;
-            return pixelbuf_get_pixel_array(readbuf + slice.start, len, &self->byteorder, self->pixel_step, slice.step, self->byteorder.is_dotstar);
+            mp_obj_tuple_t* t = MP_OBJ_TO_PTR(mp_obj_new_tuple(len, NULL));
+            for (uint i = 0; i < len; i++) {
+                t->items[i] = common_hal__pixelbuf_pixelbuf_get_pixel(self_in, i * slice.step);
+            }
+            return MP_OBJ_FROM_PTR(t);
         } else { // Set
             #if MICROPY_PY_ARRAY_SLICE_ASSIGN
 
-            if (!(MP_OBJ_IS_TYPE(value, &mp_type_list) || MP_OBJ_IS_TYPE(value, &mp_type_tuple)))
+            if (!(MP_OBJ_IS_TYPE(value, &mp_type_list) || MP_OBJ_IS_TYPE(value, &mp_type_tuple))) {
                 mp_raise_ValueError(translate("tuple/list required on RHS"));
+            }
 
             size_t dst_len = (slice.stop - slice.start);
             if (slice.step > 1) {
@@ -398,21 +340,12 @@ STATIC mp_obj_t pixelbuf_pixelbuf_subscr(mp_obj_t self_in, mp_obj_t index_in, mp
                 num_items = l->len;
                 src_objs = l->items;
             }
-            if (num_items != dst_len)
+            if (num_items != dst_len) {
                 mp_raise_ValueError_varg(translate("Unmatched number of items on RHS (expected %d, got %d)."),
                                                    dst_len, num_items);
-
-            size_t target_i = slice.start;
-            for (size_t i = slice.start; target_i < slice.stop; i++, target_i += slice.step) {
-                mp_obj_t *item = src_objs[i-slice.start];
-                if (MP_OBJ_IS_TYPE(value, &mp_type_list) || MP_OBJ_IS_TYPE(value, &mp_type_tuple) || MP_OBJ_IS_INT(value)) {
-                    pixelbuf_set_pixel(self->buf + (target_i * self->pixel_step),
-                        self->two_buffers ? self->rawbuf + (i * self->pixel_step) : NULL,
-                        self->brightness, item, &self->byteorder, self->byteorder.is_dotstar);
-                }
             }
-            if (self->auto_write)
-                pixelbuf_call_show(self_in);
+
+            common_hal__pixelbuf_pixelbuf_set_pixels(self_in, slice.start, slice.stop, slice.step, src_objs);
             return mp_const_none;
             #else
             return MP_OBJ_NULL; // op not supported
@@ -420,19 +353,13 @@ STATIC mp_obj_t pixelbuf_pixelbuf_subscr(mp_obj_t self_in, mp_obj_t index_in, mp
         }
 #endif
     } else { // Single index rather than slice.
-        size_t index = mp_get_index(self->base.type, self->pixels, index_in, false);
-        size_t offset = (index * self->pixel_step);
-        if (offset > self->bytes)
-            mp_raise_IndexError(translate("Pixel beyond bounds of buffer"));
+        size_t length = common_hal__pixelbuf_pixelbuf_get_len(self_in);
+        size_t index = mp_get_index(mp_obj_get_type(self_in), length, index_in, false);
 
         if (value == MP_OBJ_SENTINEL) { // Get
-            uint8_t *pixelstart = (uint8_t *)(self->two_buffers ? self->rawbuf : self->buf) + offset;
-            return pixelbuf_get_pixel(pixelstart, &self->byteorder, self->byteorder.is_dotstar);
+            return common_hal__pixelbuf_pixelbuf_get_pixel(self_in, index);
         } else { // Store
-            pixelbuf_set_pixel(self->buf + offset, self->two_buffers ? self->rawbuf + offset : NULL,
-                self->brightness, value, &self->byteorder, self->byteorder.is_dotstar);
-            if (self->auto_write)
-                pixelbuf_call_show(self_in);
+            common_hal__pixelbuf_pixelbuf_set_pixel(self_in, index, value);
             return mp_const_none;
         }
     }
@@ -442,9 +369,9 @@ STATIC const mp_rom_map_elem_t pixelbuf_pixelbuf_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_auto_write), MP_ROM_PTR(&pixelbuf_pixelbuf_auto_write_obj)},
     { MP_ROM_QSTR(MP_QSTR_bpp), MP_ROM_PTR(&pixelbuf_pixelbuf_bpp_obj)},
     { MP_ROM_QSTR(MP_QSTR_brightness), MP_ROM_PTR(&pixelbuf_pixelbuf_brightness_obj)},
-    { MP_ROM_QSTR(MP_QSTR_buf), MP_ROM_PTR(&pixelbuf_pixelbuf_buf_obj)},
     { MP_ROM_QSTR(MP_QSTR_byteorder), MP_ROM_PTR(&pixelbuf_pixelbuf_byteorder_str)},
     { MP_ROM_QSTR(MP_QSTR_show), MP_ROM_PTR(&pixelbuf_pixelbuf_show_obj)},
+    { MP_ROM_QSTR(MP_QSTR_fill), MP_ROM_PTR(&pixelbuf_pixelbuf_fill_obj)},
 };
 
 STATIC MP_DEFINE_CONST_DICT(pixelbuf_pixelbuf_locals_dict, pixelbuf_pixelbuf_locals_dict_table);
diff --git a/shared-bindings/_pixelbuf/PixelBuf.h b/shared-bindings/_pixelbuf/PixelBuf.h
index 691da3cd3..68d6d4eef 100644
--- a/shared-bindings/_pixelbuf/PixelBuf.h
+++ b/shared-bindings/_pixelbuf/PixelBuf.h
@@ -27,27 +27,26 @@
 #ifndef CP_SHARED_BINDINGS_PIXELBUF_PIXELBUF_H
 #define CP_SHARED_BINDINGS_PIXELBUF_PIXELBUF_H
 
-#include "shared-bindings/_pixelbuf/types.h"
+#include "shared-module/_pixelbuf/PixelBuf.h"
 
 const mp_obj_type_t pixelbuf_pixelbuf_type;
 
-typedef struct {
-    mp_obj_base_t base;
-    size_t pixels;
-    size_t bytes;
-    size_t pixel_step;
-    pixelbuf_byteorder_details_t byteorder;
-    mp_obj_t bytearray;
-    mp_obj_t rawbytearray;
-    mp_float_t brightness;
-    bool two_buffers;
-    size_t offset;
-    uint8_t *rawbuf;
-    uint8_t *buf;
-    bool auto_write;
-} pixelbuf_pixelbuf_obj_t;
+void common_hal__pixelbuf_pixelbuf_construct(pixelbuf_pixelbuf_obj_t *self, size_t n,
+    pixelbuf_byteorder_details_t* byteorder, mp_float_t brightness, bool auto_write, uint8_t* header,
+    size_t header_len, uint8_t* trailer, size_t trailer_len);
 
-void pixelbuf_recalculate_brightness(pixelbuf_pixelbuf_obj_t *self);
-mp_obj_t pixelbuf_call_show(mp_obj_t self_in);
+// These take mp_obj_t because they are called on subclasses of PixelBuf.
+uint8_t common_hal__pixelbuf_pixelbuf_get_bpp(mp_obj_t self);
+mp_float_t common_hal__pixelbuf_pixelbuf_get_brightness(mp_obj_t self);
+void common_hal__pixelbuf_pixelbuf_set_brightness(mp_obj_t self, mp_float_t brightness);
+bool common_hal__pixelbuf_pixelbuf_get_auto_write(mp_obj_t self);
+void common_hal__pixelbuf_pixelbuf_set_auto_write(mp_obj_t self, bool auto_write);
+size_t common_hal__pixelbuf_pixelbuf_get_len(mp_obj_t self_in);
+mp_obj_t common_hal__pixelbuf_pixelbuf_get_byteorder_string(mp_obj_t self);
+void common_hal__pixelbuf_pixelbuf_fill(mp_obj_t self, mp_obj_t item);
+void common_hal__pixelbuf_pixelbuf_show(mp_obj_t self);
+mp_obj_t common_hal__pixelbuf_pixelbuf_get_pixel(mp_obj_t self, size_t index);
+void common_hal__pixelbuf_pixelbuf_set_pixel(mp_obj_t self, size_t index, mp_obj_t item);
+void common_hal__pixelbuf_pixelbuf_set_pixels(mp_obj_t self_in, size_t start, size_t stop, size_t step, mp_obj_t* values);
 
 #endif  // CP_SHARED_BINDINGS_PIXELBUF_PIXELBUF_H
diff --git a/shared-bindings/_pixelbuf/__init__.c b/shared-bindings/_pixelbuf/__init__.c
index ed264f3bb..424ed23e4 100644
--- a/shared-bindings/_pixelbuf/__init__.c
+++ b/shared-bindings/_pixelbuf/__init__.c
@@ -29,11 +29,8 @@
 #include "py/runtime.h"
 #include "py/objproperty.h"
 
-#include "types.h"
-#include "__init__.h"
-
-#include "PixelBuf.h"
-#include "../../shared-module/_pixelbuf/PixelBuf.h"
+#include "shared-bindings/_pixelbuf/__init__.h"
+#include "shared-bindings/_pixelbuf/PixelBuf.h"
 
 
 //| :mod:`_pixelbuf` --- Fast RGB(W) pixel buffer and helpers
@@ -82,39 +79,15 @@ const int32_t colorwheel(float pos) {
     }
 }
 
-
-//| .. function:: fill(pixelbuf, color)
-//|
-//|   Fills the given pixelbuf with the given color.
-//|
-
-STATIC mp_obj_t pixelbuf_fill(mp_obj_t pixelbuf_in, mp_obj_t value) {
-    mp_obj_t obj = mp_instance_cast_to_native_base(pixelbuf_in, &pixelbuf_pixelbuf_type);
-    if (obj == MP_OBJ_NULL)
-        mp_raise_TypeError(translate("Expected a PixelBuf instance"));
-    pixelbuf_pixelbuf_obj_t *pixelbuf = MP_OBJ_TO_PTR(obj);
-
-    for (size_t offset = 0; offset < pixelbuf->bytes; offset+= pixelbuf->pixel_step) {
-        pixelbuf_set_pixel(pixelbuf->buf + offset, pixelbuf->two_buffers ? (pixelbuf->rawbuf + offset) : NULL,
-                           pixelbuf->brightness, value, &pixelbuf->byteorder, pixelbuf->byteorder.is_dotstar);
-    }
-    if (pixelbuf->auto_write)
-        pixelbuf_call_show(pixelbuf_in);
-    return mp_const_none;
-}
-STATIC MP_DEFINE_CONST_FUN_OBJ_2(pixelbuf_fill_obj, pixelbuf_fill);
-
-
 STATIC const mp_rom_map_elem_t pixelbuf_module_globals_table[] = {
     { MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR__pixelbuf) },
     { MP_ROM_QSTR(MP_QSTR_PixelBuf), MP_ROM_PTR(&pixelbuf_pixelbuf_type) },
     { MP_ROM_QSTR(MP_QSTR_wheel), MP_ROM_PTR(&pixelbuf_wheel_obj) },
-    { MP_ROM_QSTR(MP_QSTR_fill), MP_ROM_PTR(&pixelbuf_fill_obj) },
 };
 
 STATIC MP_DEFINE_CONST_DICT(pixelbuf_module_globals, pixelbuf_module_globals_table);
 
 const mp_obj_module_t pixelbuf_module = {
-        .base = { &mp_type_module },
-        .globals = (mp_obj_dict_t*)&pixelbuf_module_globals,
+    .base = { &mp_type_module },
+    .globals = (mp_obj_dict_t*)&pixelbuf_module_globals,
 };
diff --git a/shared-bindings/_pixelbuf/__init__.h b/shared-bindings/_pixelbuf/__init__.h
index f70ffe508..0e8c4a37f 100644
--- a/shared-bindings/_pixelbuf/__init__.h
+++ b/shared-bindings/_pixelbuf/__init__.h
@@ -30,6 +30,5 @@
 #include "common-hal/digitalio/DigitalInOut.h"
 
 const int32_t colorwheel(float pos);
-extern void common_hal_neopixel_write(const digitalio_digitalinout_obj_t* gpio, uint8_t *pixels, uint32_t numBytes);
 
 #endif //CP_SHARED_BINDINGS_PIXELBUF_INIT_H
diff --git a/shared-bindings/_pixelbuf/types.h b/shared-bindings/_pixelbuf/types.h
deleted file mode 100644
index f7dc1a109..000000000
--- a/shared-bindings/_pixelbuf/types.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * This file is part of the Circuit Python project, https://github.com/adafruit/circuitpython
- *
- * The MIT License (MIT)
- *
- * Copyright (c) 2018 Roy Hooper
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#ifndef CIRCUITPYTHON_PIXELBUF_TYPES_H
-#define CIRCUITPYTHON_PIXELBUF_TYPES_H
-
-//| :orphan:
-
-typedef struct {
-    uint8_t r;
-    uint8_t g;
-    uint8_t b;
-    uint8_t w;
-} pixelbuf_rgbw_t;
-
-typedef struct {
-    uint8_t bpp;
-    pixelbuf_rgbw_t byteorder;
-    bool has_white;
-    bool is_dotstar;
-    mp_obj_t *order;
-} pixelbuf_byteorder_details_t;
-
-#endif // CIRCUITPYTHON_PIXELBUF_TYPES_H