Merge pull request #4432 from jepler/bitmap-dirty-improvements

Bitmap dirty improvements

5 files changed, 71 insertions, 110 deletions

diff --git a/shared-module/bitmaptools/__init__.c b/shared-module/bitmaptools/__init__.c
index 5f105ea33..6fda6195f 100644
--- a/shared-module/bitmaptools/__init__.c
+++ b/shared-module/bitmaptools/__init__.c
@@ -95,10 +95,6 @@ void common_hal_bitmaptools_rotozoom(displayio_bitmap_t *self, int16_t ox, int16
     // #    */
 
 
-    if (self->read_only) {
-        mp_raise_RuntimeError(translate("Read-only object"));
-    }
-
     int16_t x,y;
 
     int16_t minx = dest_clip1_x;
@@ -199,6 +195,9 @@ void common_hal_bitmaptools_rotozoom(displayio_bitmap_t *self, int16_t ox, int16
     float rowu = startu + miny * duCol;
     float rowv = startv + miny * dvCol;
 
+    displayio_area_t dirty_area = {minx, miny, maxx + 1, maxy + 1};
+    displayio_bitmap_set_dirty_area(self, &dirty_area);
+
     for (y = miny; y <= maxy; y++) {
         float u = rowu + minx * duRow;
         float v = rowv + minx * dvRow;
@@ -206,7 +205,7 @@ void common_hal_bitmaptools_rotozoom(displayio_bitmap_t *self, int16_t ox, int16
             if (u >= source_clip0_x && u < source_clip1_x && v >= source_clip0_y && v < source_clip1_y) {
                 uint32_t c = common_hal_displayio_bitmap_get_pixel(source, u, v);
                 if ((skip_index_none) || (c != skip_index)) {
-                    common_hal_displayio_bitmap_set_pixel(self, x, y, c);
+                    displayio_bitmap_write_pixel(self, x, y, c);
                 }
             }
             u += duRow;
@@ -236,34 +235,18 @@ void common_hal_bitmaptools_fill_region(displayio_bitmap_t *destination,
     //
     // input checks should ensure that x1 < x2 and y1 < y2 and are within the bitmap region
 
-    if (destination->read_only) {
-        mp_raise_RuntimeError(translate("Read-only object"));
-    }
-
-    // Ensure x1 < x2 and y1 < y2
-    if (x1 > x2) {
-        int16_t temp = x2;
-        x2 = x1;
-        x1 = temp;
-    }
-    if (y1 > y2) {
-        int16_t temp = y2;
-        y2 = y1;
-        y1 = temp;
-    }
+    displayio_area_t area = { x1, y1, x2, y2 };
+    displayio_area_canon(&area);
 
-    // constrain to bitmap dimensions
-    x1 = constrain(x1, 0, destination->width);
-    x2 = constrain(x2, 0, destination->width);
-    y1 = constrain(y1, 0, destination->height);
-    y2 = constrain(y2, 0, destination->height);
+    displayio_area_t bitmap_area = { 0, 0, destination->width, destination->height };
+    displayio_area_compute_overlap(&area, &bitmap_area, &area);
 
     // update the dirty rectangle
-    displayio_bitmap_set_dirty_area(destination, x1, y1, x2, y2);
+    displayio_bitmap_set_dirty_area(destination, &area);
 
     int16_t x, y;
-    for (x = x1; x < x2; x++) {
-        for (y = y1; y < y2; y++) {
+    for (x = area.x1; x < area.x2; x++) {
+        for (y = area.y1; y < area.y2; y++) {
             displayio_bitmap_write_pixel(destination, x, y, value);
         }
     }
@@ -274,10 +257,6 @@ void common_hal_bitmaptools_draw_line(displayio_bitmap_t *destination,
     int16_t x1, int16_t y1,
     uint32_t value) {
 
-    if (destination->read_only) {
-        mp_raise_RuntimeError(translate("Read-only object"));
-    }
-
     //
     // adapted from Adafruit_CircuitPython_Display_Shapes.Polygon._line
     //
@@ -298,13 +277,11 @@ void common_hal_bitmaptools_draw_line(displayio_bitmap_t *destination,
         ybb0 = y1;
         ybb1 = y0 + 1;
     }
+    displayio_area_t area = { xbb0, ybb0, xbb1, ybb1 };
+    displayio_area_t bitmap_area = { 0, 0, destination->width, destination->height };
+    displayio_area_compute_overlap(&area, &bitmap_area, &area);
 
-    xbb0 = constrain(xbb0, 0, destination->width);
-    xbb1 = constrain(xbb1, 0, destination->width);
-    ybb0 = constrain(ybb0, 0, destination->height);
-    ybb1 = constrain(ybb1, 0, destination->height);
-
-    displayio_bitmap_set_dirty_area(destination, xbb0, ybb0, xbb1, ybb1);
+    displayio_bitmap_set_dirty_area(destination, &area);
 
     int16_t temp, x, y;
 
@@ -401,15 +378,15 @@ void common_hal_bitmaptools_arrayblit(displayio_bitmap_t *self, void *data, int
             }
         }
     }
-    displayio_bitmap_set_dirty_area(self, x1, y1, x2, y2);
+    displayio_area_t area = { x1, y1, x2, y2 };
+    displayio_bitmap_set_dirty_area(self, &area);
 }
 
 void common_hal_bitmaptools_readinto(displayio_bitmap_t *self, pyb_file_obj_t *file, int element_size, int bits_per_pixel, bool reverse_pixels_in_element, bool swap_bytes, bool reverse_rows) {
     uint32_t mask = (1 << common_hal_displayio_bitmap_get_bits_per_value(self)) - 1;
 
-    if (self->read_only) {
-        mp_raise_RuntimeError(translate("Read-only object"));
-    }
+    displayio_area_t a = {0, 0, self->width, self->height};
+    displayio_bitmap_set_dirty_area(self, &a);
 
     size_t elements_per_row = (self->width * bits_per_pixel + element_size * 8 - 1) / (element_size * 8);
     size_t rowsize = element_size * elements_per_row;
@@ -486,6 +463,4 @@ void common_hal_bitmaptools_readinto(displayio_bitmap_t *self, pyb_file_obj_t *f
             displayio_bitmap_write_pixel(self, x, y_draw, value & mask);
         }
     }
-
-    displayio_bitmap_set_dirty_area(self, 0, 0, self->width, self->height);
 }
diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c
index 5da196d9f..872a20b9e 100644
--- a/shared-module/displayio/Bitmap.c
+++ b/shared-module/displayio/Bitmap.c
@@ -105,41 +105,16 @@ uint32_t common_hal_displayio_bitmap_get_pixel(displayio_bitmap_t *self, int16_t
     return 0;
 }
 
-void displayio_bitmap_set_dirty_area(displayio_bitmap_t *self, int16_t x1, int16_t y1, int16_t x2, int16_t y2) {
-    // Update the bitmap's dirty region with the rectangle bounded by (x1,y1) and (x2, y2)
-
-    // Arrange x1 < x2, y1 < y2
-    if (x1 > x2) {
-        int16_t temp = x1;
-        x1 = x2;
-        x2 = temp;
-    }
-    if (y1 > y2) {
-        int16_t temp = y1;
-        y1 = y2;
-        y2 = temp;
+void displayio_bitmap_set_dirty_area(displayio_bitmap_t *self, const displayio_area_t *dirty_area) {
+    if (self->read_only) {
+        mp_raise_RuntimeError(translate("Read-only object"));
     }
 
-    // Update the dirty area.
-    if (self->dirty_area.x1 == self->dirty_area.x2) {
-        self->dirty_area.x1 = x1;
-        self->dirty_area.x2 = x2;
-        self->dirty_area.y1 = y1;
-        self->dirty_area.y2 = y2;
-    } else {
-        if (x1 < self->dirty_area.x1) {
-            self->dirty_area.x1 = x1;
-        }
-        if (x2 > self->dirty_area.x2) {
-            self->dirty_area.x2 = x2;
-        }
-        if (y1 < self->dirty_area.y1) {
-            self->dirty_area.y1 = y1;
-        }
-        if (y2 > self->dirty_area.y2) {
-            self->dirty_area.y2 = y2;
-        }
-    }
+    displayio_area_t area = *dirty_area;
+    displayio_area_canon(&area);
+    displayio_area_union(&area, &self->dirty_area, &area);
+    displayio_area_t bitmap_area = {0, 0, self->width, self->height};
+    displayio_area_compute_overlap(&area, &bitmap_area, &self->dirty_area);
 }
 
 void displayio_bitmap_write_pixel(displayio_bitmap_t *self, int16_t x, int16_t y, uint32_t value) {
@@ -175,10 +150,6 @@ void common_hal_displayio_bitmap_blit(displayio_bitmap_t *self, int16_t x, int16
     // If skip_value is `None`, then all pixels are copied.
     // This function assumes input checks were performed for pixel index entries.
 
-    if (self->read_only) {
-        mp_raise_RuntimeError(translate("Read-only object"));
-    }
-
     // Update the dirty area
     int16_t dirty_x_max = (x + (x2 - x1));
     if (dirty_x_max > self->width) {
@@ -189,7 +160,8 @@ void common_hal_displayio_bitmap_blit(displayio_bitmap_t *self, int16_t x, int16
         dirty_y_max = self->height;
     }
 
-    displayio_bitmap_set_dirty_area(self, x, y, dirty_x_max, dirty_y_max);
+    displayio_area_t a = { x, y, dirty_x_max, dirty_y_max};
+    displayio_bitmap_set_dirty_area(self, &a);
 
     bool x_reverse = false;
     bool y_reverse = false;
@@ -226,12 +198,9 @@ void common_hal_displayio_bitmap_blit(displayio_bitmap_t *self, int16_t x, int16
 }
 
 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"));
-    }
-
     // update the dirty region
-    displayio_bitmap_set_dirty_area(self, x, y, x + 1, y + 1);
+    displayio_area_t a = {x, y, x + 1, y + 1};
+    displayio_bitmap_set_dirty_area(self, &a);
 
     // write the pixel
     displayio_bitmap_write_pixel(self, x, y, value);
@@ -252,14 +221,8 @@ void displayio_bitmap_finish_refresh(displayio_bitmap_t *self) {
 }
 
 void common_hal_displayio_bitmap_fill(displayio_bitmap_t *self, uint32_t value) {
-    if (self->read_only) {
-        mp_raise_RuntimeError(translate("Read-only object"));
-    }
-    // Update the dirty area.
-    self->dirty_area.x1 = 0;
-    self->dirty_area.x2 = self->width;
-    self->dirty_area.y1 = 0;
-    self->dirty_area.y2 = self->height;
+    displayio_area_t a = {0, 0, self->width, self->height};
+    displayio_bitmap_set_dirty_area(self, &a);
 
     // build the packed word
     uint32_t word = 0;
diff --git a/shared-module/displayio/Bitmap.h b/shared-module/displayio/Bitmap.h
index c1ce9612b..82a3de631 100644
--- a/shared-module/displayio/Bitmap.h
+++ b/shared-module/displayio/Bitmap.h
@@ -49,7 +49,7 @@ typedef struct {
 
 void displayio_bitmap_finish_refresh(displayio_bitmap_t *self);
 displayio_area_t *displayio_bitmap_get_refresh_areas(displayio_bitmap_t *self, displayio_area_t *tail);
-void displayio_bitmap_set_dirty_area(displayio_bitmap_t *self, int16_t x1, int16_t y1, int16_t x2, int16_t y2);
+void displayio_bitmap_set_dirty_area(displayio_bitmap_t *self, const displayio_area_t *area);
 void displayio_bitmap_write_pixel(displayio_bitmap_t *self, int16_t x, int16_t y, uint32_t value);
 
 #endif // MICROPY_INCLUDED_SHARED_MODULE_DISPLAYIO_BITMAP_H
diff --git a/shared-module/displayio/__init__.c b/shared-module/displayio/__init__.c
index 3c9c2cc3a..d238c2816 100644
--- a/shared-module/displayio/__init__.c
+++ b/shared-module/displayio/__init__.c
@@ -309,26 +309,46 @@ bool displayio_area_compute_overlap(const displayio_area_t *a,
     return true;
 }
 
+void displayio_copy_coords(const displayio_area_t *src, displayio_area_t *dest) {
+    dest->x1 = src->x1;
+    dest->y1 = src->y1;
+    dest->x2 = src->x2;
+    dest->y2 = src->y2;
+}
+
+bool displayio_area_empty(const displayio_area_t *a) {
+    return (a->x1 == a->x2) || (a->y1 == a->y2);
+}
+
+void displayio_area_canon(displayio_area_t *a) {
+    if (a->x1 > a->x2) {
+        int16_t t = a->x1;
+        a->x1 = a->x2;
+        a->x2 = t;
+    }
+    if (a->y1 > a->y2) {
+        int16_t t = a->y1;
+        a->y1 = a->y2;
+        a->y2 = t;
+    }
+}
+
 void displayio_area_union(const displayio_area_t *a,
     const displayio_area_t *b,
     displayio_area_t *u) {
-    u->x1 = a->x1;
-    if (b->x1 < u->x1) {
-        u->x1 = b->x1;
-    }
-    u->x2 = a->x2;
-    if (b->x2 > u->x2) {
-        u->x2 = b->x2;
-    }
 
-    u->y1 = a->y1;
-    if (b->y1 < u->y1) {
-        u->y1 = b->y1;
+    if (displayio_area_empty(a)) {
+        displayio_copy_coords(b, u);
+        return;
     }
-    u->y2 = a->y2;
-    if (b->y2 > u->y2) {
-        u->y2 = b->y2;
+    if (displayio_area_empty(b)) {
+        displayio_copy_coords(a, u);
+        return;
     }
+    u->x1 = MIN(a->x1, b->x1);
+    u->y1 = MIN(a->y1, b->y1);
+    u->x2 = MAX(a->x2, b->x2);
+    u->y2 = MAX(a->y2, b->y2);
 }
 
 uint16_t displayio_area_width(const displayio_area_t *area) {
diff --git a/shared-module/displayio/area.h b/shared-module/displayio/area.h
index 21bf2f8b2..b6ff1dcd2 100644
--- a/shared-module/displayio/area.h
+++ b/shared-module/displayio/area.h
@@ -53,6 +53,9 @@ typedef struct {
 
 extern displayio_buffer_transform_t null_transform;
 
+bool displayio_area_empty(const displayio_area_t *a);
+void displayio_area_copy_coords(const displayio_area_t *src, displayio_area_t *dest);
+void displayio_area_canon(displayio_area_t *a);
 void displayio_area_union(const displayio_area_t *a,
     const displayio_area_t *b,
     displayio_area_t *u);