2 files changed, 104 insertions, 39 deletions

diff --git a/shared-module/framebufferio/FramebufferDisplay.c b/shared-module/framebufferio/FramebufferDisplay.c
index 2a90fa0d4..6b5346877 100644
--- a/shared-module/framebufferio/FramebufferDisplay.c
+++ b/shared-module/framebufferio/FramebufferDisplay.c
@@ -40,6 +40,11 @@
 #include <stdint.h>
 #include <string.h>
 
+#define fb_getter_default(method, default_value) \
+    (self->framebuffer_protocol->method \
+        ? self->framebuffer_protocol->method(self->framebuffer) \
+        : (default_value))
+
 void common_hal_framebufferio_framebufferdisplay_construct(framebufferio_framebufferdisplay_obj_t* self,
         mp_obj_t framebuffer,
         uint16_t rotation,
@@ -51,7 +56,7 @@ void common_hal_framebufferio_framebufferdisplay_construct(framebufferio_framebu
 
     uint16_t ram_width = 0x100;
     uint16_t ram_height = 0x100;
-
+    uint16_t depth = fb_getter_default(get_color_depth, 16);
     displayio_display_core_construct(
         &self->core,
         NULL,
@@ -62,16 +67,29 @@ void common_hal_framebufferio_framebufferdisplay_construct(framebufferio_framebu
         0,
         0,
         rotation,
-        self->framebuffer_protocol->get_color_depth(self->framebuffer),
-        false,
-        false,
-        self->framebuffer_protocol->get_bytes_per_cell(self->framebuffer),
-        false,
-        false);
+        depth,
+        fb_getter_default(get_grayscale, (depth < 8)),
+        fb_getter_default(get_pixels_in_byte_share_row, false),
+        fb_getter_default(get_bytes_per_cell, 2),
+        fb_getter_default(get_reverse_pixels_in_byte, false),
+        fb_getter_default(get_reverse_pixels_in_word, false)
+    );
+
+    self->first_pixel_offset = fb_getter_default(get_first_pixel_offset, 0);
+    self->row_stride = fb_getter_default(get_row_stride, 0);
+    if (self->row_stride == 0) {
+        self->row_stride = self->core.width * self->core.colorspace.depth/8;
+    }
+
+    self->framebuffer_protocol->get_bufinfo(self->framebuffer, &self->bufinfo);
+    size_t framebuffer_size = self->first_pixel_offset + self->row_stride * self->core.height;
+    if (self->bufinfo.len < framebuffer_size) {
+        mp_raise_IndexError_varg(translate("Framebuffer requires %d bytes"), framebuffer_size);
+    }
 
     self->first_manual_refresh = !auto_refresh;
 
-    self->native_frames_per_second = self->framebuffer_protocol->get_native_frames_per_second(self->framebuffer);
+    self->native_frames_per_second = fb_getter_default(get_native_frames_per_second, 60);
     self->native_ms_per_frame = 1000 / self->native_frames_per_second;
 
     supervisor_start_terminal(self->core.width, self->core.height);
@@ -109,7 +127,7 @@ bool common_hal_framebufferio_framebufferdisplay_set_auto_brightness(framebuffer
 }
 
 mp_float_t common_hal_framebufferio_framebufferdisplay_get_brightness(framebufferio_framebufferdisplay_obj_t* self) {
-    if (self->framebuffer_protocol->set_brightness) {
+    if (self->framebuffer_protocol->get_brightness) {
         return self->framebuffer_protocol->get_brightness(self->framebuffer);
     }
     return -1;
@@ -138,7 +156,8 @@ STATIC const displayio_area_t* _get_refresh_areas(framebufferio_framebufferdispl
     return NULL;
 }
 
-STATIC bool _refresh_area(framebufferio_framebufferdisplay_obj_t* self, const displayio_area_t* area) {
+#define MARK_ROW_DIRTY(r) (dirty_row_bitmask[r/8] |= (1 << (r & 7)))
+STATIC bool _refresh_area(framebufferio_framebufferdisplay_obj_t* self, const displayio_area_t* area, uint8_t *dirty_row_bitmask) {
     uint16_t buffer_size = 128; // In uint32_ts
 
     displayio_area_t clipped;
@@ -147,6 +166,14 @@ STATIC bool _refresh_area(framebufferio_framebufferdisplay_obj_t* self, const di
         return true;
     }
     uint16_t subrectangles = 1;
+
+    // If pixels are packed by row then rows are on byte boundaries
+    if (self->core.colorspace.depth < 8 && self->core.colorspace.pixels_in_byte_share_row) {
+        int div = 8 / self->core.colorspace.depth;
+        clipped.x1 = (clipped.x1 / div) * div;
+        clipped.x2 = ((clipped.x2 + div - 1) / div) * div;
+    }
+
     uint16_t rows_per_buffer = displayio_area_height(&clipped);
     uint8_t pixels_per_word = (sizeof(uint32_t) * 8) / self->core.colorspace.depth;
     uint16_t pixels_per_buffer = displayio_area_size(&clipped);
@@ -187,6 +214,7 @@ STATIC bool _refresh_area(framebufferio_framebufferdisplay_obj_t* self, const di
             .x2 = clipped.x2,
             .y2 = clipped.y1 + rows_per_buffer * (j + 1)
         };
+
         if (remaining_rows < rows_per_buffer) {
             subrectangle.y2 = subrectangle.y1 + remaining_rows;
         }
@@ -197,12 +225,18 @@ STATIC bool _refresh_area(framebufferio_framebufferdisplay_obj_t* self, const di
 
         displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer);
 
-        // COULDDO: this arithmetic only supports multiple-of-8 bpp
-        uint8_t *dest = self->bufinfo.buf + (subrectangle.y1 * self->core.width + subrectangle.x1) * (self->core.colorspace.depth / 8);
+        uint8_t *buf = (uint8_t *)self->bufinfo.buf, *endbuf = buf + self->bufinfo.len;
+        (void)endbuf; // Hint to compiler that endbuf is "used" even if NDEBUG
+        buf += self->first_pixel_offset;
+
+        size_t rowstride = self->row_stride;
+        uint8_t *dest = buf + subrectangle.y1 * rowstride + subrectangle.x1 * self->core.colorspace.depth / 8;
         uint8_t *src = (uint8_t*)buffer;
-        size_t rowsize = (subrectangle.x2 - subrectangle.x1) * (self->core.colorspace.depth / 8);
-        size_t rowstride = self->core.width * (self->core.colorspace.depth/8);
+        size_t rowsize = (subrectangle.x2 - subrectangle.x1) * self->core.colorspace.depth / 8;
+
         for (uint16_t i = subrectangle.y1; i < subrectangle.y2; i++) {
+            assert(dest >= buf && dest < endbuf && dest+rowsize <= endbuf);
+            MARK_ROW_DIRTY(i);
             memcpy(dest, src, rowsize);
             dest += rowstride;
             src += rowsize;
@@ -216,15 +250,23 @@ STATIC bool _refresh_area(framebufferio_framebufferdisplay_obj_t* self, const di
 }
 
 STATIC void _refresh_display(framebufferio_framebufferdisplay_obj_t* self) {
-    displayio_display_core_start_refresh(&self->core);
     self->framebuffer_protocol->get_bufinfo(self->framebuffer, &self->bufinfo);
+    if(!self->bufinfo.buf) {
+        return;
+    }
+    displayio_display_core_start_refresh(&self->core);
     const displayio_area_t* current_area = _get_refresh_areas(self);
-    while (current_area != NULL) {
-        _refresh_area(self, current_area);
-        current_area = current_area->next;
+    if (current_area) {
+        uint8_t dirty_row_bitmask[(self->core.height + 7) / 8];
+        memset(dirty_row_bitmask, 0, sizeof(dirty_row_bitmask));
+        self->framebuffer_protocol->get_bufinfo(self->framebuffer, &self->bufinfo);
+        while (current_area != NULL) {
+            _refresh_area(self, current_area, dirty_row_bitmask);
+            current_area = current_area->next;
+        }
+        self->framebuffer_protocol->swapbuffers(self->framebuffer, dirty_row_bitmask);
     }
     displayio_display_core_finish_refresh(&self->core);
-    self->framebuffer_protocol->swapbuffers(self->framebuffer);
 }
 
 void common_hal_framebufferio_framebufferdisplay_set_rotation(framebufferio_framebufferdisplay_obj_t* self, int rotation){
@@ -307,11 +349,7 @@ void release_framebufferdisplay(framebufferio_framebufferdisplay_obj_t* self) {
     common_hal_framebufferio_framebufferdisplay_set_auto_refresh(self, false);
     release_display_core(&self->core);
     self->framebuffer_protocol->deinit(self->framebuffer);
-}
-
-void reset_framebufferdisplay(framebufferio_framebufferdisplay_obj_t* self) {
-    common_hal_framebufferio_framebufferdisplay_set_auto_refresh(self, true);
-    common_hal_framebufferio_framebufferdisplay_show(self, NULL);
+    self->base.type = &mp_type_NoneType;
 }
 
 void framebufferio_framebufferdisplay_collect_ptrs(framebufferio_framebufferdisplay_obj_t* self) {
@@ -320,6 +358,12 @@ void framebufferio_framebufferdisplay_collect_ptrs(framebufferio_framebufferdisp
 }
 
 void framebufferio_framebufferdisplay_reset(framebufferio_framebufferdisplay_obj_t* self) {
-    common_hal_framebufferio_framebufferdisplay_set_auto_refresh(self, true);
-    common_hal_framebufferio_framebufferdisplay_show(self, NULL);
+    mp_obj_type_t *fb_type = mp_obj_get_type(self->framebuffer);
+    if(fb_type != NULL && fb_type != &mp_type_NoneType) {
+        common_hal_framebufferio_framebufferdisplay_set_auto_refresh(self, true);
+        common_hal_framebufferio_framebufferdisplay_show(self, NULL);
+        self->core.full_refresh = true;
+    } else {
+        release_framebufferdisplay(self);
+    }
 }
diff --git a/shared-module/framebufferio/FramebufferDisplay.h b/shared-module/framebufferio/FramebufferDisplay.h
index ca1ab984a..89df0d27c 100644
--- a/shared-module/framebufferio/FramebufferDisplay.h
+++ b/shared-module/framebufferio/FramebufferDisplay.h
@@ -48,44 +48,65 @@ typedef struct {
     uint64_t last_refresh_call;
     uint16_t native_frames_per_second;
     uint16_t native_ms_per_frame;
+    uint16_t first_pixel_offset;
+    uint16_t row_stride;
     bool auto_refresh;
     bool first_manual_refresh;
 } framebufferio_framebufferdisplay_obj_t;
 
 void framebufferio_framebufferdisplay_background(framebufferio_framebufferdisplay_obj_t* self);
 void release_framebufferdisplay(framebufferio_framebufferdisplay_obj_t* self);
-void reset_framebufferdisplay(framebufferio_framebufferdisplay_obj_t* self);
 void framebufferio_framebufferdisplay_reset(framebufferio_framebufferdisplay_obj_t* self);
 
 void framebufferio_framebufferdisplay_collect_ptrs(framebufferio_framebufferdisplay_obj_t* self);
 
 mp_obj_t common_hal_framebufferio_framebufferdisplay_get_framebuffer(framebufferio_framebufferdisplay_obj_t* self);
 
-typedef void (*framebuffer_get_bufinfo_fun)(mp_obj_t, mp_buffer_info_t *bufinfo);
-typedef void (*framebuffer_swapbuffers_fun)(mp_obj_t);
-typedef void (*framebuffer_deinit_fun)(mp_obj_t);
-typedef bool (*framebuffer_set_brightness_fun)(mp_obj_t, mp_float_t);
-typedef mp_float_t (*framebuffer_get_brightness_fun)(mp_obj_t);
-typedef bool (*framebuffer_set_auto_brightness_fun)(mp_obj_t, bool);
 typedef bool (*framebuffer_get_auto_brightness_fun)(mp_obj_t);
-typedef int (*framebuffer_get_width_fun)(mp_obj_t);
-typedef int (*framebuffer_get_height_fun)(mp_obj_t);
-typedef int (*framebuffer_get_color_depth_fun)(mp_obj_t);
+typedef bool (*framebuffer_get_reverse_pixels_in_byte_fun)(mp_obj_t);
+typedef bool (*framebuffer_get_reverse_pixels_in_word_fun)(mp_obj_t);
+typedef bool (*framebuffer_set_auto_brightness_fun)(mp_obj_t, bool);
+typedef bool (*framebuffer_set_brightness_fun)(mp_obj_t, mp_float_t);
 typedef int (*framebuffer_get_bytes_per_cell_fun)(mp_obj_t);
+typedef int (*framebuffer_get_color_depth_fun)(mp_obj_t);
+typedef int (*framebuffer_get_first_pixel_offset_fun)(mp_obj_t);
+typedef int (*framebuffer_get_grayscale_fun)(mp_obj_t);
+typedef int (*framebuffer_get_height_fun)(mp_obj_t);
 typedef int (*framebuffer_get_native_frames_per_second_fun)(mp_obj_t);
+typedef bool (*framebuffer_get_pixels_in_byte_share_row_fun)(mp_obj_t);
+typedef int (*framebuffer_get_row_stride_fun)(mp_obj_t);
+typedef int (*framebuffer_get_width_fun)(mp_obj_t);
+typedef mp_float_t (*framebuffer_get_brightness_fun)(mp_obj_t);
+typedef void (*framebuffer_deinit_fun)(mp_obj_t);
+typedef void (*framebuffer_get_bufinfo_fun)(mp_obj_t, mp_buffer_info_t *bufinfo);
+typedef void (*framebuffer_swapbuffers_fun)(mp_obj_t, uint8_t *dirty_row_bitmask);
 
 typedef struct _framebuffer_p_t {
     MP_PROTOCOL_HEAD // MP_QSTR_protocol_framebuffer
+
+    // Mandatory
     framebuffer_get_bufinfo_fun get_bufinfo;
     framebuffer_swapbuffers_fun swapbuffers;
     framebuffer_deinit_fun deinit;
     framebuffer_get_width_fun get_width;
     framebuffer_get_height_fun get_height;
-    framebuffer_get_color_depth_fun get_color_depth;
-    framebuffer_get_bytes_per_cell_fun get_bytes_per_cell;
-    framebuffer_get_native_frames_per_second_fun get_native_frames_per_second;
+
+    // Optional getters
+    framebuffer_get_bytes_per_cell_fun get_bytes_per_cell; // default: 2
+    framebuffer_get_color_depth_fun get_color_depth; // default: 16
+    framebuffer_get_first_pixel_offset_fun get_first_pixel_offset; // default: 0
+    framebuffer_get_grayscale_fun get_grayscale; // default: grayscale if depth < 8
+    framebuffer_get_native_frames_per_second_fun get_native_frames_per_second; // default: 60
+    framebuffer_get_pixels_in_byte_share_row_fun get_pixels_in_byte_share_row; // default: false
+    framebuffer_get_reverse_pixels_in_byte_fun get_reverse_pixels_in_byte; // default: false
+    framebuffer_get_reverse_pixels_in_word_fun get_reverse_pixels_in_word; // default: false
+    framebuffer_get_row_stride_fun get_row_stride; // default: 0 (no extra row padding)
+
+    // Optional -- default is no brightness control
     framebuffer_get_brightness_fun get_brightness;
     framebuffer_set_brightness_fun set_brightness;
+
+    // Optional -- default is no automatic brightness control
     framebuffer_get_auto_brightness_fun get_auto_brightness;
     framebuffer_set_auto_brightness_fun set_auto_brightness;
 } framebuffer_p_t;