Skip to content

Commit 7a117f5

Browse files
committed
Make point 2 in areas exclusive and simplify full_coverage.
1 parent 3fad7de commit 7a117f5

5 files changed

Lines changed: 39 additions & 32 deletions

File tree

shared-module/displayio/Group.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ bool displayio_group_get_area(displayio_group_t *self, displayio_buffer_transfor
138138
displayio_area_shift(area, -self->x * transform->scale, -self->y * transform->scale);
139139
transform->scale *= self->scale;
140140

141+
// Track if any of the layers finishes filling in the given area. We can ignore any remaining
142+
// layers at that point.
141143
bool full_coverage = false;
142144
for (int32_t i = self->size - 1; i >= 0 ; i--) {
143145
mp_obj_t layer = self->children[i].native;

shared-module/displayio/TileGrid.c

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_tr
154154
displayio_area_t scaled_area = {
155155
.x1 = self->area.x1 * transform->scale,
156156
.y1 = self->area.y1 * transform->scale,
157-
.x2 = (self->area.x2 + 1) * transform->scale - 1, // Second point is inclusive.
158-
.y2 = (self->area.y2 + 1) * transform->scale - 1
157+
.x2 = self->area.x2 * transform->scale,
158+
.y2 = self->area.y2 * transform->scale
159159
};
160160
if (!displayio_area_compute_overlap(area, &scaled_area, &overlap)) {
161161
return false;
@@ -169,19 +169,20 @@ bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_tr
169169
}
170170
uint16_t start = 0;
171171
if (transform->mirror_x) {
172-
start += (area->x2 - area->x1) * x_stride;
172+
start += (area->x2 - area->x1 - 1) * x_stride;
173173
x_stride *= -1;
174174
}
175175
if (transform->mirror_y) {
176-
start += (area->y2 - area->y1) * y_stride;
176+
start += (area->y2 - area->y1 - 1) * y_stride;
177177
y_stride *= -1;
178178
}
179179

180+
// Track if this layer finishes filling in the given area. We can ignore any remaining
181+
// layers at that point.
180182
bool full_coverage = displayio_area_equal(area, &overlap);
181183

182-
// TODO(tannewt): Set full coverage to true if all pixels outside the overlap have already been
183-
// set as well.
184-
bool always_full_coverage = false;
184+
// TODO(tannewt): Skip coverage tracking if all pixels outside the overlap have already been
185+
// set and our palette is all opaque.
185186

186187
// TODO(tannewt): Check to see if the pixel_shader has any transparency. If it doesn't then we
187188
// 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
191192
}
192193
int16_t x_shift = area->x1 - scaled_area.x1;
193194
int16_t y_shift = area->y1 - scaled_area.y1;
194-
for (; y <= overlap.y2 - scaled_area.y1; y++) {
195+
for (; y < overlap.y2 - scaled_area.y1; y++) {
195196
int16_t x = overlap.x1 - scaled_area.x1;
196197
if (x < 0) {
197198
x = 0;
198199
}
199200
int16_t row_start = start + (y - y_shift) * y_stride;
200201
int16_t local_y = y / transform->scale;
201-
for (; x <= overlap.x2 - scaled_area.x1; x++) {
202+
for (; x < overlap.x2 - scaled_area.x1; x++) {
202203
// Compute the destination pixel in the buffer and mask based on the transformations.
203204
uint16_t offset = row_start + (x - x_shift) * x_stride;
204205

206+
// This is super useful for debugging out range accesses. Uncomment to use.
207+
// if (offset < 0 || offset >= displayio_area_size(area)) {
208+
// asm("bkpt");
209+
// }
210+
205211
// Check the mask first to see if the pixel has already been set.
206212
if ((mask[offset / 32] & (1 << (offset % 32))) != 0) {
207213
continue;
@@ -229,22 +235,21 @@ bool displayio_tilegrid_get_area(displayio_tilegrid_t *self, displayio_buffer_tr
229235
return true;
230236
} else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_palette_type)) {
231237
if (!displayio_palette_get_color(self->pixel_shader, value, pixel)) {
232-
// mark the pixel as transparent
238+
// A pixel is transparent so we haven't fully covered the area ourselves.
233239
full_coverage = false;
234-
} else if (!always_full_coverage) {
240+
} else {
235241
mask[offset / 32] |= 1 << (offset % 32);
236242
}
237243
} else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_colorconverter_type)) {
238244
if (!common_hal_displayio_colorconverter_convert(self->pixel_shader, value, pixel)) {
239-
// mark the pixel as transparent
245+
// A pixel is transparent so we haven't fully covered the area ourselves.
240246
full_coverage = false;
241-
} else if (!always_full_coverage) {
247+
} else {
242248
mask[offset / 32] |= 1 << (offset % 32);
243249
}
244250
}
245251
}
246252
}
247-
248253
return full_coverage;
249254
}
250255

shared-module/displayio/__init__.c

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ void displayio_refresh_displays(void) {
6666
displayio_area_t whole_screen = {
6767
.x1 = 0,
6868
.y1 = 0,
69-
.x2 = display->width - 1,
70-
.y2 = display->height - 1
69+
.x2 = display->width,
70+
.y2 = display->height
7171
};
7272
if (display->transpose_xy) {
7373
swap(&whole_screen.x2, &whole_screen.y2);
@@ -88,36 +88,36 @@ void displayio_refresh_displays(void) {
8888
displayio_area_t subrectangle = {
8989
.x1 = 0,
9090
.y1 = rows_per_buffer * j,
91-
.x2 = displayio_area_width(&whole_screen) - 1,
92-
.y2 = rows_per_buffer * (j + 1) - 1
91+
.x2 = displayio_area_width(&whole_screen),
92+
.y2 = rows_per_buffer * (j + 1)
9393
};
9494

9595
displayio_display_begin_transaction(display);
9696
displayio_display_set_region_to_update(display, subrectangle.x1, subrectangle.y1,
97-
subrectangle.x2 + 1, subrectangle.y2 + 1);
97+
subrectangle.x2, subrectangle.y2);
9898
displayio_display_end_transaction(display);
9999

100100
// Handle display mirroring and transpose.
101101
displayio_area_t transformed_subrectangle;
102102
displayio_buffer_transform_t transform;
103103
if (display->mirror_x) {
104104
uint16_t width = displayio_area_width(&whole_screen);
105-
transformed_subrectangle.x1 = width - subrectangle.x2 - 1;
106-
transformed_subrectangle.x2 = width - subrectangle.x1 - 1;
105+
transformed_subrectangle.x1 = width - subrectangle.x2;
106+
transformed_subrectangle.x2 = width - subrectangle.x1;
107107
} else {
108108
transformed_subrectangle.x1 = subrectangle.x1;
109109
transformed_subrectangle.x2 = subrectangle.x2;
110110
}
111111
if (display->mirror_y != display->transpose_xy) {
112112
uint16_t height = displayio_area_height(&whole_screen);
113-
transformed_subrectangle.y1 = height - subrectangle.y2 - 1;
114-
transformed_subrectangle.y2 = height - subrectangle.y1 - 1;
113+
transformed_subrectangle.y1 = height - subrectangle.y2;
114+
transformed_subrectangle.y2 = height - subrectangle.y1;
115115
} else {
116116
transformed_subrectangle.y1 = subrectangle.y1;
117117
transformed_subrectangle.y2 = subrectangle.y2;
118118
}
119-
transform.width = transformed_subrectangle.x2 - transformed_subrectangle.x1 + 1;
120-
transform.height = transformed_subrectangle.y2 - transformed_subrectangle.y1 + 1;
119+
transform.width = transformed_subrectangle.x2 - transformed_subrectangle.x1;
120+
transform.height = transformed_subrectangle.y2 - transformed_subrectangle.y1;
121121
if (display->transpose_xy) {
122122
int16_t y1 = transformed_subrectangle.y1;
123123
int16_t y2 = transformed_subrectangle.y2;
@@ -139,8 +139,8 @@ void displayio_refresh_displays(void) {
139139
if (!full_coverage) {
140140
uint32_t index = 0;
141141
uint32_t current_mask = 0;
142-
for (int16_t y = subrectangle.y1; y <= subrectangle.y2; y++) {
143-
for (int16_t x = subrectangle.x1; x <= subrectangle.x2; x++) {
142+
for (int16_t y = subrectangle.y1; y < subrectangle.y2; y++) {
143+
for (int16_t x = subrectangle.x1; x < subrectangle.x2; x++) {
144144
if (index % 32 == 0) {
145145
current_mask = mask[index / 32];
146146
}
@@ -268,11 +268,11 @@ bool displayio_area_compute_overlap(const displayio_area_t* a,
268268
}
269269

270270
uint16_t displayio_area_width(const displayio_area_t* area) {
271-
return area->x2 - area->x1 + 1;
271+
return area->x2 - area->x1;
272272
}
273273

274274
uint16_t displayio_area_height(const displayio_area_t* area) {
275-
return area->y2 - area->y1 + 1;
275+
return area->y2 - area->y1;
276276
}
277277

278278
uint32_t displayio_area_size(const displayio_area_t* area) {

shared-module/displayio/area.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
typedef struct {
3333
int16_t x1;
3434
int16_t y1;
35-
int16_t x2; // Second point is inclusive.
35+
int16_t x2; // Second point is exclusive.
3636
int16_t y2;
3737
} displayio_area_t;
3838

supervisor/shared/display.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ void supervisor_start_terminal(uint16_t width_px, uint16_t height_px) {
7070

7171
grid->width_in_tiles = width_in_tiles;
7272
grid->height_in_tiles = height_in_tiles;
73-
grid->area.x2 = grid->area.x1 + width_in_tiles * grid->tile_width - 1;
74-
grid->area.y2 = grid->area.y1 + height_in_tiles * grid->tile_height - 1;
73+
grid->area.x2 = grid->area.x1 + width_in_tiles * grid->tile_width;
74+
grid->area.y2 = grid->area.y1 + height_in_tiles * grid->tile_height;
7575
grid->tiles = tiles;
7676

7777
supervisor_terminal.cursor_x = 0;

0 commit comments

Comments
 (0)