shared-module/busdisplay: merge overlapping dirty rectangles - #11189
shared-module/busdisplay: merge overlapping dirty rectangles#11189lynt-smitka wants to merge 4 commits into
Conversation
Merge dirty rectangles whose bounding box is smaller than the two areas summed, so shared pixels are not sent twice. displayio does not merge them today, so it sends the overlap once per rectangle. Merging never sends more pixels than before. Fixes adafruit#10687
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
What do you think about having Group do this instead? It'll have more context and work for all different display types.
I agree, that will be more versatile. I'll see where it would be best to place it (shared-module/displayio/area.c?) and try to move it there. |
Move the overlap merge from busdisplay into shared code: displayio_area_array_merge_overlapping() in area.c does the greedy fuse, displayio_display_core_merge_refresh_areas() clips and collects the list (raw-list fallback past DISPLAYIO_MAX_MERGE_AREAS), and busdisplay, framebufferio and epaperdisplay all use it.
|
Moved the merge into shared displayio code (area.c + display_core), used by busdisplay, framebufferio and epaperdisplay. Group could still win a different case - combining a moved group's areas at the source (they touch rather than overlap, so list-level merging can't). But that's a bigger change. Should I try to implement it in this PR or make a new one? |
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Please do it in group to prevent duplication. The comment has a link to where I'd do it.
Reduce the refresh list in displayio_group_get_refresh_areas: unlink every area fully contained in another one, so covered pixels are composited and transmitted once instead of once per rectangle. The recursive list builder becomes a static _impl and the public name wraps it, so the reduction runs once on the root group's list and displays need no changes. Only the .next links are rewritten; the item-owned coordinates are never touched. Measured on an ST7789 BusDisplay changing 2-digit scaled text labels: scale 20 (the reported case) 1.75 -> 3.45 fps, scale 8 10.5 -> 19.8 fps; two stacked full-screen bitmaps 1.05 -> 1.75 fps.
|
Moved into displayio_group_get_refresh_areas as requested - the recursive builder became a static _impl, the public name wraps it, and the displays are untouched. The diff is now Group.c only. I first tried merging overlapping rectangles into their union. That can't be done in place - the list nodes belong to the items and modifying their coordinates corrupts item state - so it needed a static scratch buffer, eating .bss on every displayio build. Working through it with Claude Code I ended up with something simpler: no merging, just unlink every area fully contained in another one. The only write is .next, which every provider reassigns at link time anyway. Measured on an ST7789 BusDisplay: scale-20 label 1.75 -> 3.45 fps, scale-8 label 10.5 ->19.8 fps, two full-screen bitmaps 1.05 -> 1.75 fps. |
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Thanks! Much more concise. A couple style things.
If you wanted to introduce additional areas, then you could port_malloc them.
Review: filter_out_redundant_areas (no group_ prefix on a static), and the public wrapper reads better with the built list in a named variable.
|
Thanks for review, I modified it according to your comments. And thank you for port_malloc tip, I wasn't aware of it. This implementation doesn't need it because, in the end, it doesn't have to allocate anything, but I already know where I'll use it in the engine 🙂 |
2 participants
Addresses #10687: when dirty rectangles overlap, displayio resends the shared pixels once per rectangle (and draws them as separate, visible passes). A changing text label is the common trigger.
@tannewt proposed two options in the issue - a quick heuristic (sum the dirty areas; if they exceed the screen, do a full refresh) or the more complex rectangle merge. This PR implements merge. The heuristic only helps when the summed dirty area exceeds the whole screen; it does nothing when the change covers only part of the screen - the common case, e.g. a label that updates but doesn't fill the display. Merge helps there too, and when the change does fill the screen it matches the heuristic (the union is then the whole screen anyway).
Measured on a PicoPad (ST7789 SPI)
What it does
Before each refresh, the clipped dirty areas are copied into a small scratch array and any pair whose bounding box is smaller than the two summed is fused; non-overlapping rectangles stay separate. The motivating case is a changing text label: it dirties a whole-label rectangle plus a nested per-glyph rectangle per character, sending those pixels twice - merge collapses them into one.
MAX_MERGE_AREAS(16); above that, refreshes the raw list unmerged.Fixes #10687