From 85ab02bb7e3bc13996e3d8f5011d764fa6f6cb24 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:55:51 -0400 Subject: [PATCH] fix(display): keep the panel swap locked to vsync while scrolling Dirty tracking skipped SwapOnVSync for byte-identical frames. That is the right call for static content, but SwapOnVSync is also what paces the render loop, so skipping it skips the wait for the panel: a duplicate frame returns in ~8ms instead of ~10ms on a 100Hz panel, advances the strip only 0.8px instead of 1.0px, and so makes the next frame more likely to repeat as well. The effect sustains itself once it starts. Measured over 20 minutes on a 2x128x64 chain, both scrollers configured identically at 100 px/s: leaderboard 10ms x35, 11ms x3 (clean) odds-ticker 10ms x26, 8ms x7, 15ms x5 (~20% duplicates mid-scroll) The duplicates were not end-of-cycle idling -- 38% of fast frames fell within 90s of a scroll completion against 35% of normal frames, a null result. The trigger is per-frame work: odds does more of it, and more variably, so it is first to land a frame that advances less than a whole pixel. Pushing an identical frame costs one canvas copy. Falling out of vsync lock costs smooth motion. Static content is untouched, because is_currently_scrolling() expires on its own inactivity threshold -- covered by test_stale_scrolling_state_stops_forcing_pushes so a plugin that stops scrolling without saying so cannot pin the panel into always-push. Also de-flakes test_snapshot_still_written_on_skip, which asserted a strict mtime increase between two writes that can land in the same filesystem tick; it failed about two runs in three on Windows regardless of the code under test. The file is now backdated before the check. 156 tests pass on the Pi. Not yet confirmed by eye on the panel. Co-Authored-By: Claude Opus 5 --- src/display_manager.py | 17 +++++++- test/test_display_dirty_tracking.py | 61 +++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/display_manager.py b/src/display_manager.py index af23f880..e77fcad3 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -776,9 +776,24 @@ class DisplayManager: brightness = None frame_checksum = zlib.adler32(self.image.tobytes()) digest = (frame_checksum, brightness) - if digest == self._last_pushed_digest: + if digest == self._last_pushed_digest and not self.is_currently_scrolling(): # Nothing changed since the last push — the panel is # already showing exactly this frame. + # + # Never taken mid-scroll, and that exception is the + # point. SwapOnVSync is what paces the render loop, so + # skipping it also skips the wait: a duplicate frame + # returns in ~8ms instead of ~10ms on a 100Hz panel, + # advances only 0.8px instead of 1.0px, and so makes + # the *next* frame more likely to repeat as well. That + # is self-sustaining -- measured at ~20% duplicate + # frames mid-scroll on the odds ticker, against + # essentially zero on a lighter plugin with identical + # scroll settings. Swapping an identical frame costs + # one canvas copy and keeps the loop locked to the + # panel; falling out of that lock costs smooth motion. + # Static content is unaffected: is_currently_scrolling() + # expires on its own inactivity threshold. self._write_snapshot_if_due(frame_checksum) return diff --git a/test/test_display_dirty_tracking.py b/test/test_display_dirty_tracking.py index d367d2d6..e6dfb2b4 100644 --- a/test/test_display_dirty_tracking.py +++ b/test/test_display_dirty_tracking.py @@ -109,6 +109,11 @@ class TestDirtyTracking: dm.draw.rectangle([0, 0, 30, 8], fill=(255, 255, 0)) dm.update_display() # push + snapshot write (first frame) assert os.path.exists(dm._snapshot_path) + # Backdate the file so the "was it bumped?" check below cannot be + # defeated by filesystem mtime granularity -- on Windows two writes in + # the same tick get identical timestamps, which made this test fail + # roughly two runs in three regardless of the code under test. + os.utime(dm._snapshot_path, (time.time() - 60, time.time() - 60)) first_mtime = os.path.getmtime(dm._snapshot_path) # Age the write/touch bookkeeping past TOUCH_INTERVAL so the next @@ -125,6 +130,62 @@ class TestDirtyTracking: assert os.path.getmtime(dm._snapshot_path) > first_mtime +class TestScrollLock: + """Dirty tracking must not skip the panel push while a scroll is running. + + SwapOnVSync is what paces the render loop, so skipping it also skips the + wait for the panel. A duplicate frame therefore returns early -- ~8ms + instead of ~10ms on a 100Hz panel -- which advances the strip only 0.8px + instead of 1.0px, which makes the NEXT frame more likely to be a duplicate + too. That is self-sustaining: measured at ~20% duplicate frames mid-scroll + on the odds ticker against essentially zero on a lighter plugin with + identical scroll settings. Pushing an identical frame costs one canvas + copy; falling out of vsync lock costs smooth motion. + """ + + def test_identical_frames_still_push_while_scrolling(self, dm): + dm.draw.rectangle([0, 0, 12, 12], fill=(0, 0, 255)) + dm.update_display() + dm.set_scrolling_state(True) + try: + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + dm.update_display() + dm.update_display() + assert spy.count == 3, "scrolling must stay locked to the panel" + finally: + dm.set_scrolling_state(False) + + def test_identical_frames_are_skipped_when_not_scrolling(self, dm): + """The optimisation still applies to static content.""" + dm.set_scrolling_state(False) + dm.draw.rectangle([0, 0, 14, 14], fill=(255, 0, 255)) + dm.update_display() + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + dm.update_display() + assert spy.count == 0 + + def test_stale_scrolling_state_stops_forcing_pushes(self, dm): + """A plugin that stops scrolling without saying so must not pin the + panel into always-push forever. is_currently_scrolling() expires on + its own inactivity threshold, and the skip has to come back with it.""" + dm.draw.rectangle([0, 0, 16, 16], fill=(0, 255, 255)) + dm.update_display() + dm.set_scrolling_state(True) + try: + # Backdate the activity marker past the inactivity threshold. + dm._scrolling_state['last_scroll_activity'] = ( + time.time() - dm._scrolling_state['scroll_inactivity_threshold'] - 1.0) + assert dm.is_currently_scrolling() is False + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + dm.update_display() + assert spy.count == 0 + finally: + dm.set_scrolling_state(False) + + class TestKillSwitch: def test_dirty_tracking_can_be_disabled(self, dm): dm._dirty_tracking_enabled = False