Make point 2 in areas exclusive and simplify full_coverage.

4 files changed, 37 insertions, 30 deletions

diff --git a/shared-module/displayio/Group.c b/shared-module/displayio/Group.c
index c9be47f9a..f91e2ce3c 100644
--- a/shared-module/displayio/Group.c
+++ b/shared-module/displayio/Group.c
@@ -138,6 +138,8 @@ bool displayio_group_get_area(displayio_group_t *self, displayio_buffer_transfor
     displayio_area_shift(area, -self->x * transform->scale, -self->y * transform->scale);
     transform->scale *= self->scale;
 
+    // Track if any of the layers finishes filling in the given area. We can ignore any remaining
+    // layers at that point.
     bool full_coverage = false;
     for (int32_t i = self->size - 1; i >= 0 ; i--) {
         mp_obj_t layer = self->children[i].native;
diff --git a/shared-module/displayio/TileGrid.c b/shared-module/displayio/TileGrid.c
index 6ffc65889..02690b8e6 100644
--- a/shared-module/displayio/TileGrid.c
+++ b/shared-module/displayio/TileGrid.c
@@ -154,8 +154,8 @@ bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_tr
     displayio_area_t scaled_area = {
         .x1 = self->area.x1 * transform->scale,
         .y1 = self->area.y1 * transform->scale,
-        .x2 = (self->area.x2 + 1) * transform->scale - 1, // Second point is inclusive.
-        .y2 = (self->area.y2 + 1) * transform->scale - 1
+        .x2 = self->area.x2 * transform->scale,
+        .y2 = self->area.y2 * transform->scale
     };
     if (!displayio_area_compute_overlap(area, &scaled_area, &overlap)) {
         return false;
@@ -169,19 +169,20 @@ bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_tr
     }
     uint16_t start = 0;
     if (transform->mirror_x) {
-        start += (area->x2 - area->x1) * x_stride;
+        start += (area->x2 - area->x1 - 1) * x_stride;
         x_stride *= -1;
     }
     if (transform->mirror_y) {
-        start += (area->y2 - area->y1) * y_stride;
+        start += (area->y2 - area->y1 - 1) * y_stride;
         y_stride *= -1;
     }
 
+    // Track if this layer finishes filling in the given area. We can ignore any remaining
+    // layers at that point.
     bool full_coverage = displayio_area_equal(area, &overlap);
 
-    // TODO(tannewt): Set full coverage to true if all pixels outside the overlap have already been
-    // set as well.
-    bool always_full_coverage = false;
+    // TODO(tannewt): Skip coverage tracking if all pixels outside the overlap have already been
+    // set and our palette is all opaque.
 
     // TODO(tannewt): Check to see if the pixel_shader has any transparency. If it doesn't then we
     // can either return full coverage or bulk update the mask.
@@ -191,17 +192,22 @@ bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_tr
     }
     int16_t x_shift = area->x1 - scaled_area.x1;
     int16_t y_shift = area->y1 - scaled_area.y1;
-    for (; y <= overlap.y2 - scaled_area.y1; y++) {
+    for (; y < overlap.y2 - scaled_area.y1; y++) {
         int16_t x = overlap.x1 - scaled_area.x1;
         if (x < 0) {
             x = 0;
         }
         int16_t row_start = start + (y - y_shift) * y_stride;
         int16_t local_y = y / transform->scale;
-        for (; x <= overlap.x2 - scaled_area.x1; x++) {
+        for (; x < overlap.x2 - scaled_area.x1; x++) {
             // Compute the destination pixel in the buffer and mask based on the transformations.
             uint16_t offset = row_start + (x - x_shift) * x_stride;
 
+            // This is super useful for debugging out range accesses. Uncomment to use.
+            // if (offset < 0 || offset >= displayio_area_size(area)) {
+            //     asm("bkpt");
+            // }
+
             // Check the mask first to see if the pixel has already been set.
             if ((mask[offset / 32] & (1 << (offset % 32))) != 0) {
                 continue;
@@ -229,22 +235,21 @@ bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_tr
                 return true;
             } else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_palette_type)) {
                 if (!displayio_palette_get_color(self->pixel_shader, value, pixel)) {
-                    // mark the pixel as transparent
+                    // A pixel is transparent so we haven't fully covered the area ourselves.
                     full_coverage = false;
-                } else if (!always_full_coverage) {
+                } else {
                     mask[offset / 32] |= 1 << (offset % 32);
                 }
             } else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_colorconverter_type)) {
                 if (!common_hal_displayio_colorconverter_convert(self->pixel_shader, value, pixel)) {
-                    // mark the pixel as transparent
+                    // A pixel is transparent so we haven't fully covered the area ourselves.
                     full_coverage = false;
-                } else if (!always_full_coverage) {
+                } else {
                     mask[offset / 32] |= 1 << (offset % 32);
                 }
             }
         }
     }
-
     return full_coverage;
 }
 
diff --git a/shared-module/displayio/__init__.c b/shared-module/displayio/__init__.c
index e816fefda..0f2e727dc 100644
--- a/shared-module/displayio/__init__.c
+++ b/shared-module/displayio/__init__.c
@@ -66,8 +66,8 @@ void displayio_refresh_displays(void) {
             displayio_area_t whole_screen = {
                 .x1 = 0,
                 .y1 = 0,
-                .x2 = display->width - 1,
-                .y2 = display->height - 1
+                .x2 = display->width,
+                .y2 = display->height
             };
             if (display->transpose_xy) {
                 swap(&whole_screen.x2, &whole_screen.y2);
@@ -88,13 +88,13 @@ void displayio_refresh_displays(void) {
                 displayio_area_t subrectangle = {
                     .x1 = 0,
                     .y1 = rows_per_buffer * j,
-                    .x2 = displayio_area_width(&whole_screen) - 1,
-                    .y2 = rows_per_buffer * (j + 1) - 1
+                    .x2 = displayio_area_width(&whole_screen),
+                    .y2 = rows_per_buffer * (j + 1)
                 };
 
                 displayio_display_begin_transaction(display);
                 displayio_display_set_region_to_update(display, subrectangle.x1, subrectangle.y1,
-                                                                subrectangle.x2 + 1, subrectangle.y2 + 1);
+                                                                subrectangle.x2, subrectangle.y2);
                 displayio_display_end_transaction(display);
 
                 // Handle display mirroring and transpose.
@@ -102,22 +102,22 @@ void displayio_refresh_displays(void) {
                 displayio_buffer_transform_t transform;
                 if (display->mirror_x) {
                     uint16_t width = displayio_area_width(&whole_screen);
-                    transformed_subrectangle.x1 = width - subrectangle.x2 - 1;
-                    transformed_subrectangle.x2 = width - subrectangle.x1 - 1;
+                    transformed_subrectangle.x1 = width - subrectangle.x2;
+                    transformed_subrectangle.x2 = width - subrectangle.x1;
                 } else {
                     transformed_subrectangle.x1 = subrectangle.x1;
                     transformed_subrectangle.x2 = subrectangle.x2;
                 }
                 if (display->mirror_y != display->transpose_xy) {
                     uint16_t height = displayio_area_height(&whole_screen);
-                    transformed_subrectangle.y1 = height - subrectangle.y2 - 1;
-                    transformed_subrectangle.y2 = height - subrectangle.y1 - 1;
+                    transformed_subrectangle.y1 = height - subrectangle.y2;
+                    transformed_subrectangle.y2 = height - subrectangle.y1;
                 } else {
                     transformed_subrectangle.y1 = subrectangle.y1;
                     transformed_subrectangle.y2 = subrectangle.y2;
                 }
-                transform.width = transformed_subrectangle.x2 - transformed_subrectangle.x1 + 1;
-                transform.height = transformed_subrectangle.y2 - transformed_subrectangle.y1 + 1;
+                transform.width = transformed_subrectangle.x2 - transformed_subrectangle.x1;
+                transform.height = transformed_subrectangle.y2 - transformed_subrectangle.y1;
                 if (display->transpose_xy) {
                     int16_t y1 = transformed_subrectangle.y1;
                     int16_t y2 = transformed_subrectangle.y2;
@@ -139,8 +139,8 @@ void displayio_refresh_displays(void) {
                 if (!full_coverage) {
                     uint32_t index = 0;
                     uint32_t current_mask = 0;
-                    for (int16_t y = subrectangle.y1; y <= subrectangle.y2; y++) {
-                        for (int16_t x = subrectangle.x1; x <= subrectangle.x2; x++) {
+                    for (int16_t y = subrectangle.y1; y < subrectangle.y2; y++) {
+                        for (int16_t x = subrectangle.x1; x < subrectangle.x2; x++) {
                             if (index % 32 == 0) {
                                 current_mask = mask[index / 32];
                             }
@@ -268,11 +268,11 @@ bool displayio_area_compute_overlap(const displayio_area_t* a,
 }
 
 uint16_t displayio_area_width(const displayio_area_t* area) {
-    return area->x2 - area->x1 + 1;
+    return area->x2 - area->x1;
 }
 
 uint16_t displayio_area_height(const displayio_area_t* area) {
-    return area->y2 - area->y1 + 1;
+    return area->y2 - area->y1;
 }
 
 uint32_t displayio_area_size(const displayio_area_t* area) {
diff --git a/shared-module/displayio/area.h b/shared-module/displayio/area.h
index 33e41f37d..9db57e13f 100644
--- a/shared-module/displayio/area.h
+++ b/shared-module/displayio/area.h
@@ -32,7 +32,7 @@
 typedef struct {
     int16_t x1;
     int16_t y1;
-    int16_t x2; // Second point is inclusive.
+    int16_t x2; // Second point is exclusive.
     int16_t y2;
 } displayio_area_t;