From 7a117f52ed81999261fae3d43dfeb572c5e9887e Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 22 May 2019 15:00:47 -0700 Subject: [PATCH] Make point 2 in areas exclusive and simplify full_coverage. --- shared-module/displayio/Group.c | 2 ++ shared-module/displayio/TileGrid.c | 33 +++++++++++++++++------------- shared-module/displayio/__init__.c | 30 +++++++++++++-------------- shared-module/displayio/area.h | 2 +- supervisor/shared/display.c | 4 ++-- 5 files changed, 39 insertions(+), 32 deletions(-) diff --git a/shared-module/displayio/Group.c b/shared-module/displayio/Group.c index c9be47f9ab..f91e2ce3c3 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 6ffc658892..02690b8e64 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 e816fefda4..0f2e727dcc 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 33e41f37d7..9db57e13f8 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; diff --git a/supervisor/shared/display.c b/supervisor/shared/display.c index cfb9cc1d19..5eab74e16b 100644 --- a/supervisor/shared/display.c +++ b/supervisor/shared/display.c @@ -70,8 +70,8 @@ void supervisor_start_terminal(uint16_t width_px, uint16_t height_px) { grid->width_in_tiles = width_in_tiles; grid->height_in_tiles = height_in_tiles; - grid->area.x2 = grid->area.x1 + width_in_tiles * grid->tile_width - 1; - grid->area.y2 = grid->area.y1 + height_in_tiles * grid->tile_height - 1; + grid->area.x2 = grid->area.x1 + width_in_tiles * grid->tile_width; + grid->area.y2 = grid->area.y1 + height_in_tiles * grid->tile_height; grid->tiles = tiles; supervisor_terminal.cursor_x = 0;