8000 Un update of a vectorio object palette slow down the microcontroller indefinitely · Issue #9666 · adafruit/circuitpython · GitHub
[go: up one dir, main page]
Skip to content

Un update of a vectorio object palette slow down the microcontroller indefinitely #9666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
CDarius opened this issue Sep 27, 2024 · 3 comments · Fixed by #10356
Closed

Un update of a vectorio object palette slow down the microcontroller indefinitely #9666

CDarius opened this issue Sep 27, 2024 · 3 comments · Fixed by #10356
Labels
bug displayio
Milestone
Long term

Comments

@CDarius
Copy link
CDarius commented Sep 27, 2024 •
edited
Loading

CircuitPython version

Adafruit CircuitPython 9.1.4 on 2024-09-17; M5Stack Dial with ESP32S3

Code/REPL

import time
import board
import displayio
import vectorio

display = board.DISPLAY

def show_and_count_loops_per_second(num_reads: int) -> None:
    time_ms = time.monotonic_ns() // 1e6
    next_lps_time = time_ms + 1000
    lps_counter = 0

    while num_reads > 0:
        lps_counter += 1
        time_ms = time.monotonic_ns() // 1e6
        if time_ms > next_lps_time:
            print(f"Loops per second: {lps_counter}")
            num_reads -= 1
            next_lps_time += 1000
            lps_counter = 0        

print("Empty loop")
show_and_count_loops_per_second(5)

print("\nCreate a vectorio rectangle as big as the screen")
palette = displayio.Palette(1)
palette[0] = 0x125690
rectangle = vectorio.Rectangle(pixel_shader=palette, width=display.width, height=display.height, x=0, y=0)
rectangle_group = displayio.Group()
rectangle_group.append(rectangle)
display.root_group = rectangle_group

show_and_count_loops_per_second(5)

print("\nChange rectangle color")
palette[0] = 0xFFFF00
show_and_count_loops_per_second(5)

Behavior

Empty loop
Loops per second: 64892
Loops per second: 64707
Loops per second: 64781
Loops per second: 64704
Loops per second: 64682

Create a vectorio rectangle as big as the screen
Loops per second: 64931
Loops per second: 74644
Loops per second: 74662
Loops per second: 74627
Loops per second: 74573

Change rectangle color
Loops per second: 8342
Loops per second: 8644
Loops per second: 8801
Loops per second: 8806
Loops per second: 8698

Description

There's something wrong with the vectorio display update, once the palette color change the microcontroller slow down and never recover. To replicate the bug I made the above example.

First the script run an emty loop and measure how many loops are performed in a second. It runs about 65k loops per second,

After the script create a rectangle with vectiorio as big as the screen and measure the looping time again. Now it runs about 75k loops per second. It make sense, the display do not show anymore the console.

Finally the script change the palette color of the rectange and measure for the third time the empty looping time. The loops per second drop below 9k and never recover even after the screen updates are done

Additional information

No response
@CDarius CDarius added the bug label Sep 27, 2024
@tannewt tannewt added this to the Long term milestone Sep 27, 2024
@tannewt
Copy link
Member
tannewt commented Sep 27, 2024

I suspected this was the palette being marked dirty and then never cleared. BUT it looks like it should be cleared. There may be a bug further upstream that is preventing the palette dirty bit from being cleared.

@Neradoc
Copy link
Neradoc commented Apr 6, 2025

I'm learning how this works, but what I see is that when the palette is modified, the shape is not impacted: self->current_area_dirty is not set to true (which seems normal), and its self->ephemeral_dirty_area is empty. It's only redrawn because displayio_palette_needs_refresh returns true on its palette, when vectorio_vector_shape_get_refresh_areas is called.

circuitpython/shared-module/vectorio/VectorShape.c

Lines 502 to 503 in ad73d0b

|| (mp_obj_is_type(self->pixel_shader, &displayio_palette_type) && displayio_palette_needs_refresh(self->pixel_shader))
|| (mp_obj_is_type(self->pixel_shader, &displayio_colorconverter_type) && displayio_colorconverter_needs_refresh(self->pixel_shader))

Causing this section to be executed:

circuitpython/shared-module/vectorio/VectorShape.c

Lines 541 to 545 in ad73d0b

} else {
self->current_area.next = tail;
tail = &self->current_area;
VECTORIO_SHAPE_DEBUG("%p get_refresh_area: redrawing current: {(%3d,%3d), (%3d,%3d)}\n", self, self->current_area.x1, self->current_area.y1, self->current_area.x2, self->current_area.y2);
}

However it's vectorio_vector_shape_finish_refresh that resets the dirty bit of the palette, but when it is called it doesn't test the palette, so the function returns early. And so displayio_palette_finish_refresh() is never called, leaving the palette dirty.

circuitpython/shared-module/vectorio/VectorShape.c

Lines 478 to 480 in ad73d0b

if (displayio_area_empty(&self->ephemeral_dirty_area) && !self->current_area_dirty) {
return;
}

I tried adding a test for the palette's "needs refresh" to make vectorio_vector_shape_finish_refresh continue instead of skipping, but if there's 2 things using the same palette, only the first one gets "finish refreshed", potentially leaving bad values in properties that don't get NULL'ed.

So maybe we should set the dirty bit in ...get_refresh_areas when the palette is found to be modified. Does that seem right ? Should the dirty bit maybe be set at some other point earlier ? I assumed not, because of how needs_refresh is tested there anyway.

I tested this and it does fix this issue.

index 75e1de8b96..51e7e0a9da 100644
--- a/shared-module/vectorio/VectorShape.c
+++ b/shared-module/vectorio/VectorShape.c
@@ -539,6 +539,7 @@ displayio_area_t *vectorio_vector_shape_get_refresh_areas(vectorio_vector_shape_
             self->ephemeral_dirty_area.next = tail;
             tail = &self->ephemeral_dirty_area;
         } else {
+            self->current_area_dirty = true;
             self->current_area.next = tail;
             tail = &self->current_area;
             VECTORIO_SHAPE_DEBUG("%p get_refresh_area: redrawing current: {(%3d,%3d), (%3d,%3d)}\n", self, self->current_area.x1, self->current_area.y1, self->current_area.x2, self->current_area.y2);

My test code uses auto refresh False and measures how long display.refresh() takes drawing a big circle before and after changing a value in the palette.

import time
import board
import displayio
import vectorio

display = board.DISPLAY
main_group = displayio.Group()
display.root_group = main_group
display.auto_refresh = False

pal = displayio.Palette(1)
pal[0] = 0xFF0000
shape1 = vectorio.Circle(pixel_shader=pal, x=display.width//2, y=display.height//2, radius=min(display.width,display.height)//2)
main_group.append(shape1)

def measure_sleep(texte):
    t0 = time.monotonic_ns()
    display.refresh()
    t1 = time.monotonic_ns()
    dt = (t1 - t0) // 1_000_000
    print(f"{texte}: {dt:3d} ms")

while True:
    print("\n" + "#"*70)
    measure_sleep("Initial display    ")
    measure_sleep("Refresh no changes ")
    time.sleep(0.5)
    pal[0] = 0x0000FF
    measure_sleep("Switched color     ")
    measure_sleep("Refresh no changes ")
    measure_sleep("Refresh no changes ")
    time.sleep(4)
    # reset
    pal[0] = 0xFF0000
    main_group.remove(shape1)
    main_group.append(shape1)

Before:

######################################################################
Initial display    : 255 ms
Refresh no changes :   0 ms
Switched color     : 127 ms
Refresh no changes : 127 ms
Refresh no changes : 128 ms

After:

######################################################################
Initial display    : 255 ms
Refresh no changes :   0 ms
Switched color     : 127 ms
Refresh no changes :   0 ms
Refresh no changes :   0 ms

@tannewt
Copy link
Member
tannewt commented Apr 7, 2025

Good sleuthing @Neradoc! I think your fix would be great! We need to make sure all users of palette do this too.

@Neradoc Neradoc mentioned this issue May 17, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug displayio
Projects
None yet
Milestone
Long term
Development

Successfully merging a pull request may close this issue.

Fix dirty palette never being cleaned by vector shapes Neradoc/circuitpython
3 participants
@tannewt @Neradoc @CDarius
0