From 3fad7de8db7f1f317ab6d00a00a82e406ec6fb33 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Thu, 16 May 2019 16:45:38 -0700 Subject: [PATCH 1/3] Rework the pixel computation to use areas This changes the displayio pixel computation from per-pixel to per-area. This is precursor work to updating portions of the screen (#1169). It should provide mild speedups because bounds checks are done once per area rather than once per pixel. Filling by area also allows TileGrid to maintain a row-associative fill pattern even when the display's refresh is orthogonal to it. --- ports/atmel-samd/Makefile | 1 - shared-bindings/displayio/Display.h | 1 + shared-module/displayio/Group.c | 32 ++-- shared-module/displayio/Group.h | 3 +- shared-module/displayio/TileGrid.c | 149 +++++++++++++----- shared-module/displayio/TileGrid.h | 8 +- shared-module/displayio/__init__.c | 225 ++++++++++++++++++---------- shared-module/displayio/__init__.h | 6 +- shared-module/displayio/area.h | 57 +++++++ supervisor/shared/display.c | 14 +- tools/gen_display_resources.py | 10 +- 11 files changed, 353 insertions(+), 153 deletions(-) create mode 100644 shared-module/displayio/area.h diff --git a/ports/atmel-samd/Makefile b/ports/atmel-samd/Makefile index 9b110c1bbcc0c..3a8ad4acc8e95 100644 --- a/ports/atmel-samd/Makefile +++ b/ports/atmel-samd/Makefile @@ -99,7 +99,6 @@ endif #Debugging/Optimization ifeq ($(DEBUG), 1) - # Turn on Python modules useful for debugging (e.g. uheap, ustack). CFLAGS += -ggdb # You may want to disable -flto if it interferes with debugging. CFLAGS += -flto diff --git a/shared-bindings/displayio/Display.h b/shared-bindings/displayio/Display.h index 3136ae88a81d2..6695da32b86a6 100644 --- a/shared-bindings/displayio/Display.h +++ b/shared-bindings/displayio/Display.h @@ -53,6 +53,7 @@ void common_hal_displayio_display_refresh_soon(displayio_display_obj_t* self); bool displayio_display_begin_transaction(displayio_display_obj_t* self); void displayio_display_end_transaction(displayio_display_obj_t* self); +// The second point of the region is exclusive. void displayio_display_set_region_to_update(displayio_display_obj_t* self, uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1); bool displayio_display_frame_queued(displayio_display_obj_t* self); diff --git a/shared-module/displayio/Group.c b/shared-module/displayio/Group.c index 40f112bf175aa..c9be47f9ab348 100644 --- a/shared-module/displayio/Group.c +++ b/shared-module/displayio/Group.c @@ -134,32 +134,28 @@ void displayio_group_construct(displayio_group_t* self, displayio_group_child_t* self->scale = scale; } -bool displayio_group_get_pixel(displayio_group_t *self, int16_t x, int16_t y, uint16_t* pixel) { - x -= self->x; - y -= self->y; - // When we are scaled we need to substract all but one to ensure -scale to 0 divide down to -1. - // Normally -scale to scale both divide down to 0 because 0 is unsigned. - if (x < 0) { - x -= self->scale - 1; - } - if (y < 0) { - y -= self->scale - 1; - } - x /= self->scale; - y /= self->scale; +bool displayio_group_get_area(displayio_group_t *self, displayio_buffer_transform_t* transform, displayio_area_t* area, uint32_t* mask, uint32_t* buffer) { + displayio_area_shift(area, -self->x * transform->scale, -self->y * transform->scale); + transform->scale *= self->scale; + + bool full_coverage = false; for (int32_t i = self->size - 1; i >= 0 ; i--) { mp_obj_t layer = self->children[i].native; if (MP_OBJ_IS_TYPE(layer, &displayio_tilegrid_type)) { - if (displayio_tilegrid_get_pixel(layer, x, y, pixel)) { - return true; + if (displayio_tilegrid_get_area(layer, transform, area, mask, buffer)) { + full_coverage = true; + break; } } else if (MP_OBJ_IS_TYPE(layer, &displayio_group_type)) { - if (displayio_group_get_pixel(layer, x, y, pixel)) { - return true; + if (displayio_group_get_area(layer, transform, area, mask, buffer)) { + full_coverage = true; + break; } } } - return false; + transform->scale /= self->scale; + displayio_area_shift(area, self->x * transform->scale, self->y * transform->scale); + return full_coverage; } bool displayio_group_needs_refresh(displayio_group_t *self) { diff --git a/shared-module/displayio/Group.h b/shared-module/displayio/Group.h index 7826b9e663766..f8fe9be047450 100644 --- a/shared-module/displayio/Group.h +++ b/shared-module/displayio/Group.h @@ -31,6 +31,7 @@ #include #include "py/obj.h" +#include "shared-module/displayio/area.h" typedef struct { mp_obj_t native; @@ -49,7 +50,7 @@ typedef struct { } displayio_group_t; void displayio_group_construct(displayio_group_t* self, displayio_group_child_t* child_array, uint32_t max_size, uint32_t scale, mp_int_t x, mp_int_t y); -bool displayio_group_get_pixel(displayio_group_t *group, int16_t x, int16_t y, uint16_t *pixel); +bool displayio_group_get_area(displayio_group_t *group, displayio_buffer_transform_t* transform, displayio_area_t* area, uint32_t* mask, uint32_t *buffer); bool displayio_group_needs_refresh(displayio_group_t *self); void displayio_group_finish_refresh(displayio_group_t *self); diff --git a/shared-module/displayio/TileGrid.c b/shared-module/displayio/TileGrid.c index 3212dfe8b98ed..6ffc658892228 100644 --- a/shared-module/displayio/TileGrid.c +++ b/shared-module/displayio/TileGrid.c @@ -56,31 +56,40 @@ void common_hal_displayio_tilegrid_construct(displayio_tilegrid_t *self, mp_obj_ self->bitmap_width_in_tiles = bitmap_width_in_tiles; self->width_in_tiles = width; self->height_in_tiles = height; - self->total_width = width * tile_width; - self->total_height = height * tile_height; + self->area.x1 = x; + self->area.y1 = y; + // -1 because areas are inclusive + self->area.x2 = x + width * tile_width - 1; + self->area.y2 = y + height * tile_height - 1; self->tile_width = tile_width; self->tile_height = tile_height; self->bitmap = bitmap; self->pixel_shader = pixel_shader; - self->x = x; - self->y = y; } mp_int_t common_hal_displayio_tilegrid_get_x(displayio_tilegrid_t *self) { - return self->x; + return self->area.x1; } void common_hal_displayio_tilegrid_set_x(displayio_tilegrid_t *self, mp_int_t x) { - self->needs_refresh = self->x != x; - self->x = x; + if (self->area.x1 == x) { + return; + } + self->needs_refresh = true; + self->area.x2 += (self->area.x1 - x); + self->area.x1 = x; } mp_int_t common_hal_displayio_tilegrid_get_y(displayio_tilegrid_t *self) { - return self->y; + return self->area.y1; } void common_hal_displayio_tilegrid_set_y(displayio_tilegrid_t *self, mp_int_t y) { - self->needs_refresh = self->y != y; - self->y = y; + if (self->area.y1 == y) { + return; + } + self->needs_refresh = true; + self->area.y2 += (self->area.y1 - y); + self->area.y1 = y; } mp_obj_t common_hal_displayio_tilegrid_get_pixel_shader(displayio_tilegrid_t *self) { @@ -128,14 +137,11 @@ void common_hal_displayio_tilegrid_set_tile(displayio_tilegrid_t *self, uint16_t void common_hal_displayio_tilegrid_set_top_left(displayio_tilegrid_t *self, uint16_t x, uint16_t y) { self->top_left_x = x; self->top_left_y = y; + self->needs_refresh = true; } -bool displayio_tilegrid_get_pixel(displayio_tilegrid_t *self, int16_t x, int16_t y, uint16_t* pixel) { - x -= self->x; - y -= self->y; - if (y < 0 || y >= self->total_height || x >= self->total_width || x < 0) { - return false; - } +bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_transform_t* transform, displayio_area_t* area, uint32_t* mask, uint32_t *buffer) { + // If no tiles are present we have no impact. uint8_t* tiles = self->tiles; if (self->inline_tiles) { tiles = (uint8_t*) &self->tiles; @@ -143,30 +149,103 @@ bool displayio_tilegrid_get_pixel(displayio_tilegrid_t *self, int16_t x, int16_t if (tiles == NULL) { return false; } - uint16_t tile_location = ((y / self->tile_height + self->top_left_y) % self->height_in_tiles) * self->width_in_tiles + (x / self->tile_width + self->top_left_x) % self->width_in_tiles; - uint8_t tile = tiles[tile_location]; - uint16_t tile_x = tile_x = (tile % self->bitmap_width_in_tiles) * self->tile_width + x % self->tile_width; - uint16_t tile_y = tile_y = (tile / self->bitmap_width_in_tiles) * self->tile_height + y % self->tile_height; - uint32_t value = 0; - if (MP_OBJ_IS_TYPE(self->bitmap, &displayio_bitmap_type)) { - value = common_hal_displayio_bitmap_get_pixel(self->bitmap, tile_x, tile_y); - } else if (MP_OBJ_IS_TYPE(self->bitmap, &displayio_shape_type)) { - value = common_hal_displayio_shape_get_pixel(self->bitmap, tile_x, tile_y); - } else if (MP_OBJ_IS_TYPE(self->bitmap, &displayio_ondiskbitmap_type)) { - value = common_hal_displayio_ondiskbitmap_get_pixel(self->bitmap, tile_x, tile_y); + displayio_area_t overlap; + 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 + }; + if (!displayio_area_compute_overlap(area, &scaled_area, &overlap)) { + return false; } - if (self->pixel_shader == mp_const_none) { - *pixel = value; - return true; - } else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_palette_type) && displayio_palette_get_color(self->pixel_shader, value, pixel)) { - return true; - } else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_colorconverter_type) && common_hal_displayio_colorconverter_convert(self->pixel_shader, value, pixel)) { - return true; + int16_t x_stride = 1; + int16_t y_stride = displayio_area_width(area); + if (transform->transpose_xy) { + x_stride = displayio_area_height(area); + y_stride = 1; + } + uint16_t start = 0; + if (transform->mirror_x) { + start += (area->x2 - area->x1) * x_stride; + x_stride *= -1; + } + if (transform->mirror_y) { + start += (area->y2 - area->y1) * y_stride; + y_stride *= -1; } - return false; + 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): 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. + int16_t y = overlap.y1 - scaled_area.y1; + if (y < 0) { + y = 0; + } + 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++) { + 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++) { + // Compute the destination pixel in the buffer and mask based on the transformations. + uint16_t offset = row_start + (x - x_shift) * x_stride; + + // Check the mask first to see if the pixel has already been set. + if ((mask[offset / 32] & (1 << (offset % 32))) != 0) { + continue; + } + int16_t local_x = x / transform->scale; + uint16_t tile_location = ((local_y / self->tile_height + self->top_left_y) % self->height_in_tiles) * self->width_in_tiles + (local_x / self->tile_width + self->top_left_x) % self->width_in_tiles; + uint8_t tile = tiles[tile_location]; + uint16_t tile_x = (tile % self->bitmap_width_in_tiles) * self->tile_width + local_x % self->tile_width; + uint16_t tile_y = (tile / self->bitmap_width_in_tiles) * self->tile_height + local_y % self->tile_height; + + uint32_t value = 0; + // We always want to read bitmap pixels by row first and then transpose into the destination + // buffer because most bitmaps are row associated. + if (MP_OBJ_IS_TYPE(self->bitmap, &displayio_bitmap_type)) { + value = common_hal_displayio_bitmap_get_pixel(self->bitmap, tile_x, tile_y); + } else if (MP_OBJ_IS_TYPE(self->bitmap, &displayio_shape_type)) { + value = common_hal_displayio_shape_get_pixel(self->bitmap, tile_x, tile_y); + } else if (MP_OBJ_IS_TYPE(self->bitmap, &displayio_ondiskbitmap_type)) { + value = common_hal_displayio_ondiskbitmap_get_pixel(self->bitmap, tile_x, tile_y); + } + + uint16_t* pixel = ((uint16_t*) buffer) + offset; + if (self->pixel_shader == mp_const_none) { + *pixel = value; + 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 + full_coverage = false; + } else if (!always_full_coverage) { + 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 + full_coverage = false; + } else if (!always_full_coverage) { + mask[offset / 32] |= 1 << (offset % 32); + } + } + } + } + + return full_coverage; } bool displayio_tilegrid_needs_refresh(displayio_tilegrid_t *self) { diff --git a/shared-module/displayio/TileGrid.h b/shared-module/displayio/TileGrid.h index 59645553dd1ff..6157dfbe8895e 100644 --- a/shared-module/displayio/TileGrid.h +++ b/shared-module/displayio/TileGrid.h @@ -31,18 +31,16 @@ #include #include "py/obj.h" +#include "shared-module/displayio/area.h" typedef struct { mp_obj_base_t base; mp_obj_t bitmap; mp_obj_t pixel_shader; - uint16_t x; - uint16_t y; + displayio_area_t area; uint16_t bitmap_width_in_tiles; uint16_t width_in_tiles; uint16_t height_in_tiles; - uint16_t total_width; - uint16_t total_height; uint16_t tile_width; uint16_t tile_height; uint16_t top_left_x; @@ -52,7 +50,7 @@ typedef struct { bool inline_tiles; } displayio_tilegrid_t; -bool displayio_tilegrid_get_pixel(displayio_tilegrid_t *self, int16_t x, int16_t y, uint16_t *pixel); +bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_transform_t* transform, displayio_area_t* area, uint32_t* mask, uint32_t *buffer); bool displayio_tilegrid_needs_refresh(displayio_tilegrid_t *self); void displayio_tilegrid_finish_refresh(displayio_tilegrid_t *self); diff --git a/shared-module/displayio/__init__.c b/shared-module/displayio/__init__.c index 156640440eb49..e816fefda4d1c 100644 --- a/shared-module/displayio/__init__.c +++ b/shared-module/displayio/__init__.c @@ -10,6 +10,7 @@ #include "shared-bindings/displayio/Display.h" #include "shared-bindings/displayio/Group.h" #include "shared-bindings/displayio/Palette.h" +#include "shared-module/displayio/area.h" #include "supervisor/shared/autoreload.h" #include "supervisor/shared/display.h" #include "supervisor/memory.h" @@ -17,8 +18,8 @@ primary_display_t displays[CIRCUITPY_DISPLAY_LIMIT]; -static inline void swap(uint16_t* a, uint16_t* b) { - uint16_t temp = *a; +static inline void swap(int16_t* a, int16_t* b) { + int16_t temp = *a; *a = *b; *b = temp; } @@ -56,102 +57,112 @@ void displayio_refresh_displays(void) { continue; } if (displayio_display_refresh_queued(display)) { - // We compute the pixels. r and c are row and column to match the display memory - // structure. x and y match location within the groups. - uint16_t c0 = 0; - uint16_t r0 = 0; - uint16_t c1 = display->width; - uint16_t r1 = display->height; - if (display->transpose_xy) { - swap(&c1, &r1); - } - if (!displayio_display_begin_transaction(display)) { // Can't acquire display bus; skip updating this display. Try next display. continue; } - displayio_display_set_region_to_update(display, c0, r0, c1, r1); displayio_display_end_transaction(display); - uint16_t x0 = 0; - uint16_t x1 = display->width - 1; - uint16_t startx = 0; - int8_t dx = 1; - if (display->mirror_x) { - dx = -1; - startx = x1; - } - uint16_t y0 = 0; - uint16_t y1 = display->height - 1; - uint16_t starty = 0; - int8_t dy = 1; - if (display->mirror_y) { - dy = -1; - starty = y1; - } - - bool transpose = false; + displayio_area_t whole_screen = { + .x1 = 0, + .y1 = 0, + .x2 = display->width - 1, + .y2 = display->height - 1 + }; if (display->transpose_xy) { - transpose = true; - int8_t temp_dx = dx; - dx = dy; - dy = temp_dx; - - swap(&starty, &startx); - swap(&x0, &y0); - swap(&x1, &y1); + swap(&whole_screen.x2, &whole_screen.y2); } - size_t index = 0; - uint16_t buffer_size = 256; + uint16_t buffer_size = 512; + + uint16_t subrectangles = 1; + uint16_t rows_per_buffer = displayio_area_height(&whole_screen); + if (displayio_area_size(&whole_screen) > buffer_size) { + rows_per_buffer = buffer_size / displayio_area_width(&whole_screen); + subrectangles = displayio_area_height(&whole_screen) / rows_per_buffer; + buffer_size = rows_per_buffer * displayio_area_width(&whole_screen); + } uint32_t buffer[buffer_size / 2]; - bool skip_this_display = false; - - for (uint16_t y = starty; y0 <= y && y <= y1; y += dy) { - for (uint16_t x = startx; x0 <= x && x <= x1; x += dx) { - uint16_t* pixel = &(((uint16_t*)buffer)[index]); - *pixel = 0; - - if (display->current_group != NULL) { - if (transpose) { - displayio_group_get_pixel(display->current_group, y, x, pixel); - } else { - displayio_group_get_pixel(display->current_group, x, y, pixel); - } - } - index += 1; - // The buffer is full, send it. - if (index >= buffer_size) { - if (!displayio_display_begin_transaction(display)) { - // Can't acquire display bus; skip the rest of the data. Try next display. - index = 0; - skip_this_display = true; - break; + for (uint16_t j = 0; j < subrectangles; j++) { + 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 + }; + + displayio_display_begin_transaction(display); + displayio_display_set_region_to_update(display, subrectangle.x1, subrectangle.y1, + subrectangle.x2 + 1, subrectangle.y2 + 1); + displayio_display_end_transaction(display); + + // Handle display mirroring and transpose. + displayio_area_t transformed_subrectangle; + 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; + } 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; + } 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; + if (display->transpose_xy) { + int16_t y1 = transformed_subrectangle.y1; + int16_t y2 = transformed_subrectangle.y2; + transformed_subrectangle.y1 = transformed_subrectangle.x1; + transformed_subrectangle.y2 = transformed_subrectangle.x2; + transformed_subrectangle.x1 = y1; + transformed_subrectangle.x2 = y2; + } + transform.transpose_xy = display->transpose_xy; + transform.mirror_x = display->mirror_x; + transform.mirror_y = display->mirror_y; + transform.scale = 1; + + uint32_t mask[(buffer_size / 32) + 1]; + for (uint16_t k = 0; k < (buffer_size / 32) + 1; k++) { + mask[k] = 0x00000000; + } + bool full_coverage = displayio_group_get_area(display->current_group, &transform, &transformed_subrectangle, mask, buffer); + 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++) { + if (index % 32 == 0) { + current_mask = mask[index / 32]; + } + if ((current_mask & (1 << (index % 32))) == 0) { + ((uint16_t*) buffer)[index] = 0x0000; + } + index++; } - displayio_display_send_pixels(display, buffer, buffer_size / 2); - displayio_display_end_transaction(display); - // TODO(tannewt): Make refresh displays faster so we don't starve other - // background tasks. - usb_background(); - index = 0; } } - } - if (skip_this_display) { - // Go on to next display. - continue; - } - // Send the remaining data. - if (index) { if (!displayio_display_begin_transaction(display)) { - // Can't get display bus. Skip the rest of the data. Try next display. - continue; + // Can't acquire display bus; skip the rest of the data. Try next display. + break; } - displayio_display_send_pixels(display, buffer, index * 2); + displayio_display_send_pixels(display, buffer, buffer_size / 2); + displayio_display_end_transaction(display); + + // TODO(tannewt): Make refresh displays faster so we don't starve other + // background tasks. + usb_background(); } - displayio_display_end_transaction(display); } displayio_display_finish_refresh(display); } @@ -220,3 +231,57 @@ void reset_displays(void) { } #endif } + +void displayio_area_shift(displayio_area_t* area, int16_t dx, int16_t dy) { + area->x1 += dx; + area->y1 += dy; + area->x2 += dx; + area->y2 += dy; +} + +bool displayio_area_compute_overlap(const displayio_area_t* a, + const displayio_area_t* b, + displayio_area_t* overlap) { + overlap->x1 = a->x1; + if (b->x1 > overlap->x1) { + overlap->x1 = b->x1; + } + overlap->x2 = a->x2; + if (b->x2 < overlap->x2) { + overlap->x2 = b->x2; + } + if (overlap->x1 > overlap->x2) { + return false; + } + overlap->y1 = a->y1; + if (b->y1 > overlap->y1) { + overlap->y1 = b->y1; + } + overlap->y2 = a->y2; + if (b->y2 < overlap->y2) { + overlap->y2 = b->y2; + } + if (overlap->y1 > overlap->y2) { + return false; + } + return true; +} + +uint16_t displayio_area_width(const displayio_area_t* area) { + return area->x2 - area->x1 + 1; +} + +uint16_t displayio_area_height(const displayio_area_t* area) { + return area->y2 - area->y1 + 1; +} + +uint32_t displayio_area_size(const displayio_area_t* area) { + return displayio_area_width(area) * displayio_area_height(area); +} + +bool displayio_area_equal(const displayio_area_t* a, const displayio_area_t* b) { + return a->x1 == b->x1 && + a->y1 == b->y1 && + a->x2 == b->x2 && + a->y2 == b->y2; +} diff --git a/shared-module/displayio/__init__.h b/shared-module/displayio/__init__.h index 5b56ed55cdfcc..7ffc8eab22293 100644 --- a/shared-module/displayio/__init__.h +++ b/shared-module/displayio/__init__.h @@ -24,8 +24,8 @@ * THE SOFTWARE. */ -#ifndef MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO___INIT___H -#define MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO___INIT___H +#ifndef MICROPY_INCLUDED_SHARED_MODULE_DISPLAYIO___INIT___H +#define MICROPY_INCLUDED_SHARED_MODULE_DISPLAYIO___INIT___H #include "shared-bindings/displayio/Display.h" #include "shared-bindings/displayio/FourWire.h" @@ -47,4 +47,4 @@ extern displayio_group_t circuitpython_splash; void displayio_refresh_displays(void); void reset_displays(void); -#endif // MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO___INIT___H +#endif // MICROPY_INCLUDED_SHARED_MODULE_DISPLAYIO___INIT___H diff --git a/shared-module/displayio/area.h b/shared-module/displayio/area.h new file mode 100644 index 0000000000000..33e41f37d7ed5 --- /dev/null +++ b/shared-module/displayio/area.h @@ -0,0 +1,57 @@ +/* + * This file is part of the Micro Python project, http://micropython.org/ + * + * The MIT License (MIT) + * + * Copyright (c) 2019 Scott Shawcroft for Adafruit Industries + * + * 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 MICROPY_INCLUDED_SHARED_MODULE_DISPLAYIO_AREA_H +#define MICROPY_INCLUDED_SHARED_MODULE_DISPLAYIO_AREA_H + +// Implementations are in __init__.c + +typedef struct { + int16_t x1; + int16_t y1; + int16_t x2; // Second point is inclusive. + int16_t y2; +} displayio_area_t; + +typedef struct { + uint16_t width; + uint16_t height; + uint8_t scale; + bool mirror_x; + bool mirror_y; + bool transpose_xy; +} displayio_buffer_transform_t; + +void displayio_area_shift(displayio_area_t* area, int16_t dx, int16_t dy); +bool displayio_area_compute_overlap(const displayio_area_t* a, + const displayio_area_t* b, + displayio_area_t* overlap); +uint16_t displayio_area_width(const displayio_area_t* area); +uint16_t displayio_area_height(const displayio_area_t* area); +uint32_t displayio_area_size(const displayio_area_t* area); +bool displayio_area_equal(const displayio_area_t* a, const displayio_area_t* b); + +#endif // MICROPY_INCLUDED_SHARED_MODULE_DISPLAYIO_AREA_H diff --git a/supervisor/shared/display.c b/supervisor/shared/display.c index 0b3fbe178f551..cfb9cc1d193aa 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->total_width = width_in_tiles * grid->tile_width; - grid->total_height = height_in_tiles * grid->tile_height; + 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->tiles = tiles; supervisor_terminal.cursor_x = 0; @@ -157,13 +157,15 @@ displayio_tilegrid_t blinka_sprite = { .base = {.type = &displayio_tilegrid_type }, .bitmap = &blinka_bitmap, .pixel_shader = &blinka_palette, - .x = 0, - .y = 0, + .area = { + .x1 = 0, + .y1 = 0, + .x2 = 16, + .y2 = 16 + }, .bitmap_width_in_tiles = 1, .width_in_tiles = 1, .height_in_tiles = 1, - .total_width = 16, - .total_height = 16, .tile_width = 16, .tile_height = 16, .top_left_x = 16, diff --git a/tools/gen_display_resources.py b/tools/gen_display_resources.py index ebea9dccc2c3c..b1fd048316dd0 100644 --- a/tools/gen_display_resources.py +++ b/tools/gen_display_resources.py @@ -120,13 +120,15 @@ def _load_row(self, y, row): .base = {{ .type = &displayio_tilegrid_type }}, .bitmap = (displayio_bitmap_t*) &supervisor_terminal_font_bitmap, .pixel_shader = &supervisor_terminal_color, - .x = 16, - .y = 0, + .area = {{ + .x1 = 16, + .y1 = 0, + .x2 = {1} + 16, + .y2 = {2}, + }}, .bitmap_width_in_tiles = {0}, .width_in_tiles = 1, .height_in_tiles = 1, - .total_width = {1}, - .total_height = {2}, .tile_width = {1}, .tile_height = {2}, .tiles = NULL, From 7a117f52ed81999261fae3d43dfeb572c5e9887e Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 22 May 2019 15:00:47 -0700 Subject: [PATCH 2/3] 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 c9be47f9ab348..f91e2ce3c31d8 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 6ffc658892228..02690b8e64e09 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 e816fefda4d1c..0f2e727dccde1 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 33e41f37d7ed5..9db57e13f8af2 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 cfb9cc1d193aa..5eab74e16b8fe 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; From 5d0791cafbebf6df7692836b60e86bc1238b4d57 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Fri, 31 May 2019 15:50:55 -0700 Subject: [PATCH 3/3] Fix off-by-one error It caused the bottom and right edges to be one pixel short. --- shared-module/displayio/TileGrid.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/shared-module/displayio/TileGrid.c b/shared-module/displayio/TileGrid.c index 02690b8e64e09..078ddf6b1a53c 100644 --- a/shared-module/displayio/TileGrid.c +++ b/shared-module/displayio/TileGrid.c @@ -58,9 +58,8 @@ void common_hal_displayio_tilegrid_construct(displayio_tilegrid_t *self, mp_obj_ self->height_in_tiles = height; self->area.x1 = x; self->area.y1 = y; - // -1 because areas are inclusive - self->area.x2 = x + width * tile_width - 1; - self->area.y2 = y + height * tile_height - 1; + self->area.x2 = x + width * tile_width; + self->area.y2 = y + height * tile_height; self->tile_width = tile_width; self->tile_height = tile_height; self->bitmap = bitmap;