feat(display): cancel the half-panel scan offset on slower, held-frame scrolls (#711)

Scan-order compensation only ran at one frame per refresh, so a crisp scroll
like 60 px/s on a 120 Hz panel (1px every 2 refreshes) showed a half-pixel
step across the middle of the panel. A held frame is now presented as a
sequence of swaps (scan_order.refresh_plan): the lagging half shows the
previous frame for its first refresh and the new one for the rest, so it
steps one refresh after the rest. Skipped when a blit takes over half a
refresh, since the second blit has to land before the next vsync.

Soaked on ledpi (60 px/s, 120 Hz): 0.16% late frames, as before the change.

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>

docs/SCROLL_PERFORMANCE.md

@@ -521,19 +521,25 @@ On the 2×128×64 chain above, which refreshes at about 130 Hz flat out
 
 ### What the display does about it
 
-At one pixel per refresh, the fastest crisp speed, the step is exactly one
-refresh's worth of motion, so it can be cancelled: show one half of the panel
+The step is the motion of one refresh, so it can be cancelled: show one half of the panel
 a refresh behind the other -- the half whose row at the seam lights at the
 start of each refresh. The two rows either side of the seam then show the same
 moment again. What is left is a
 lean of one pixel per half from top to bottom, continuous across the panel,
 which reads as nothing where the step read as a tear. `DisplayManager` does
-this while something scrolls at one frame per refresh
+this while something scrolls
 (`display.scan_order_compensation`, `"auto"` by default, `"off"` to disable;
 the geometry is in `src/scan_order.py`). The lagging rows come from the
 previous frame the display presented, so it works for Vegas and every plugin
 ticker without knowing how they scroll.
 
+A frame held for several refreshes (any crisp speed below the panel's full
+refresh rate, e.g. 60 px/s at 120 Hz) is presented as two swaps instead of one:
+the lagging half shows the previous frame for the first refresh and the new one
+for the rest, so it steps one refresh after the rest rather than one frame.
+That costs a second blit inside the refresh after the first swap, so it is
+skipped when a blit takes more than half a refresh.
+
 Checked on hdpi (4×128×64 on one chain, rotated 180, 2026-09-24) before it was
 written: `scan_mode: 1` (interlaced) made the step vanish but turned moving
 edges grainy, and halving the speed halved it, so it is the scan and not a torn
@@ -541,9 +547,6 @@ frame. With the compensation the step is gone at 90 px/s.
 
 It is left off where the row order is unknown or the maths does not hold:
 
-- **Slower speeds**, where each frame is held for two or more refreshes. The
-  offset there is half a pixel or less, and cancelling it would need a lag of
-  a fraction of a frame.
 - **Other layouts:** pixel mappers other than a 0 or 180 degree rotation
   (U-mapper, 90/270), non-zero `multiplexing`, interlaced `scan_mode`, and a
   canvas remapped to another height (double-sided mode).

src/display_manager.py

@@ -51,7 +51,7 @@ from src.pi5_matrix_support import is_raspberry_pi_5
 import threading
 import time
 from collections import OrderedDict, deque
-from typing import Dict, Any, Optional, Tuple, TYPE_CHECKING
+from typing import Dict, Any, List, Optional, Tuple, TYPE_CHECKING
 import zlib
 import freetype
 
@@ -223,6 +223,11 @@ def _per_thread_canvas_attr(name: str) -> property:
 
 
 
+#: A held frame is only split when its blit takes less than this share of a
+#: refresh: the second blit has to land before the next vsync.
+_SPLIT_BLIT_FRACTION = 0.5
+
+
 class DisplayManager:
     """
     Singleton hardware abstraction layer for the RGB LED matrix.
@@ -971,28 +976,36 @@ class DisplayManager:
                 # mode the logical screen is first tiled across the full chain.
                 blit_started = time.perf_counter()
                 if self._double_sided is not None:
-                    self.offscreen_canvas.SetImage(self._composite_double_sided())
+                    segments = [(self._composite_double_sided(), self._frame_hold)]
                 else:
-                    self.offscreen_canvas.SetImage(self._scan_compensated(self.image))
-                blit_done = time.perf_counter()
-
-                # Swap buffers immediately. framerate_fraction holds the frame
-                # for N refreshes; SwapOnVSync blocks for all of them, which is
-                # what paces the render loop to the chosen frame rate.
+                    segments = self._scan_segments(self.image)
                 gate = self.render_gate
+                blit_time = swap_time = 0.0
+                # Usually one segment: the frame, held for _frame_hold
+                # refreshes. SwapOnVSync blocks for all of them, which is what
+                # paces the render loop to the chosen frame rate. Scan-order
+                # compensation on a held frame splits it, so the lagging rows
+                # change one refresh after the rest.
                 if gate is not None:
                     gate.before_swap(self._frame_hold)
-                self.matrix.SwapOnVSync(self.offscreen_canvas, self._frame_hold)
+                for index, (shown, hold) in enumerate(segments):
+                    if index:
+                        blit_started = time.perf_counter()
+                    self.offscreen_canvas.SetImage(shown)
+                    blit_done = time.perf_counter()
+                    blit_time += blit_done - blit_started
+                    self.matrix.SwapOnVSync(self.offscreen_canvas, hold)
+                    swap_time += time.perf_counter() - blit_done
+                    # Swap our canvas references
+                    self.offscreen_canvas, self.current_canvas = self.current_canvas, self.offscreen_canvas
                 if gate is not None:
                     gate.after_swap(self._frame_hold)
                 presented_at = time.perf_counter()
+                self._last_blit_seconds = blit_time / len(segments)
                 self.frame_timing.record(
-                    blit_done - blit_started, presented_at - blit_done,
+                    blit_time, swap_time,
                     self._frame_hold, self.is_currently_scrolling(), presented_at)
 
-                # Swap our canvas references
-                self.offscreen_canvas, self.current_canvas = self.current_canvas, self.offscreen_canvas
-
                 self._last_pushed_digest = digest
 
                 # Write a snapshot for the web preview (throttled)
@@ -1038,23 +1051,35 @@ class DisplayManager:
             ", ".join(f"rows {top}-{bottom - 1} show {lag} refresh(es) behind"
                       for top, bottom, lag in bands))
 
-    def _scan_compensated(self, image: Image.Image) -> Image.Image:
-        """The frame to present, with lagging rows taken from earlier frames.
+    def _scan_segments(self, image: Image.Image) -> List[Tuple[Image.Image, int]]:
+        """What to present for this frame: ``[(image, refreshes), ...]``.
 
-        Only mid-scroll at one frame per refresh: that is when consecutive
-        frames are consecutive refreshes. At a longer hold, or on a static
-        screen, the history is dropped and the frame goes out as it is.
+        Mid-scroll with compensation on, lagging rows are taken from earlier
+        refreshes (see src/scan_order.py). At one refresh per frame that is one
+        image. A frame held longer is split at the refresh where the lagging
+        rows catch up, so those rows step a refresh after the rest. The split
+        needs a second blit inside the refresh that follows the first swap, so
+        it is skipped when a blit is too slow to fit. A static screen goes out
+        as it is, and drops the history.
         """
+        hold = self._frame_hold
         bands = getattr(self, '_scan_lag_bands', None)
-        if not bands:
-            return image
-        if self._frame_hold != 1 or not self.is_currently_scrolling():
-            self._scan_history.clear()
-            return image
-        presented = scan_order.compose(image, self._scan_history, bands)
+        if not bands or not self.is_currently_scrolling():
+            if bands:
+                self._scan_history.clear()
+            return [(image, hold)]
+        if hold > 1:
+            blit = getattr(self, '_last_blit_seconds', 0.0)
+            if blit > _SPLIT_BLIT_FRACTION / max(1.0, self.refresh_hz):
+                self._scan_history.clear()
+                return [(image, hold)]
+        segments = [
+            (scan_order.compose(image, self._scan_history, bands, backs), count)
+            for backs, count in scan_order.refresh_plan(bands, hold)
+        ]
         # A copy: plugins draw into the same image object frame after frame.
         self._scan_history.appendleft(image.copy())
-        return presented
+        return segments
 
     def clear(self):
         """Clear the display completely."""

src/scan_order.py

@@ -23,6 +23,10 @@ above it near the end, the section below must show one more refresh of lag to
 stay continuous with it (and one less where the order jumps the other way).
 Stacked parallel chains are lit simultaneously, so each further half adds one.
 
+A frame held for several refreshes (a slower, crisp scroll) is presented as a
+sequence of swaps instead of one long hold, so the lagging half can step one
+refresh after the rest: see :func:`refresh_plan`.
+
 Only layouts whose physical row order is known are compensated: plain chains,
 parallel chains, and a 0 or 180 degree rotation. Other pixel mappers
 (U-mapper, 90/270 rotation, ...), special multiplexing and interlaced scan are
@@ -109,19 +113,47 @@ def scan_lag_bands(hardware: Mapping[str, Any], height: int,
     return [band for band in bands if band[2] > 0] or None
 
 
-def compose(image: Image.Image, history: Sequence[Image.Image],
-            bands: Sequence[Band]) -> Image.Image:
-    """``image`` with each band taken from the frame ``lag`` refreshes back.
+def refresh_plan(bands: Sequence[Band], hold: int) -> List[Tuple[Tuple[int, ...], int]]:
+    """How to present one frame that is held for ``hold`` refreshes.
 
-    ``history[0]`` is the previous frame. A band whose frame is not available
-    yet (the first frames of a scroll) is left current. Returns ``image`` itself
-    when nothing changes, so the caller pays for a copy only when it must.
+    A band lagging ``lag`` refreshes shows, on refresh ``r`` of the frame, what
+    the panel showed ``lag`` refreshes earlier: the current frame once
+    ``r >= lag``, else a frame ``ceil((lag - r) / hold)`` back. At one refresh
+    per frame that is just ``lag`` frames back. Held longer, the lagging band
+    steps one refresh after the rest instead of one frame, which is the only
+    way to cancel the offset: it is a fraction of a frame there.
+
+    Returns ``[(frames_back_per_band, refreshes), ...]`` in order, merging
+    neighbouring refreshes that show the same thing so each costs one swap.
+    """
+    plan: List[Tuple[Tuple[int, ...], int]] = []
+    for r in range(max(1, hold)):
+        backs = tuple(max(0, -((r - lag) // max(1, hold))) for _, _, lag in bands)
+        if plan and plan[-1][0] == backs:
+            plan[-1] = (backs, plan[-1][1] + 1)
+        else:
+            plan.append((backs, 1))
+    return plan
+
+
+def compose(image: Image.Image, history: Sequence[Image.Image],
+            bands: Sequence[Band],
+            backs: Optional[Sequence[int]] = None) -> Image.Image:
+    """``image`` with each band taken from an earlier frame.
+
+    ``history[0]`` is the previous frame. ``backs`` is how many frames back each
+    band is taken from (0 = the current one); by default that is the band's lag,
+    which is right when every frame is held for one refresh. A band whose frame
+    is not available yet (the first frames of a scroll) is left current.
+    Returns ``image`` itself when nothing changes, so the caller pays for a copy
+    only when it must.
     """
     out = image
-    for top, bottom, lag in bands:
-        if lag > len(history):
+    for i, (top, bottom, lag) in enumerate(bands):
+        back = lag if backs is None else backs[i]
+        if back <= 0 or back > len(history):
             continue
-        source = history[lag - 1]
+        source = history[back - 1]
         if source.size != image.size:
             continue
         if out is image:

test/test_scan_order.py

@@ -73,6 +73,27 @@ def _frame(shade):
     return Image.new("RGB", (8, 64), (shade, shade, shade))
 
 
+class TestRefreshPlan:
+    BANDS = [(32, 64, 1)]
+
+    def test_one_refresh_per_frame_is_a_plain_lag(self):
+        assert scan_order.refresh_plan(self.BANDS, 1) == [((1,), 1)]
+
+    def test_a_held_frame_lags_only_its_first_refresh(self):
+        assert scan_order.refresh_plan(self.BANDS, 2) == [((1,), 1), ((0,), 1)]
+        assert scan_order.refresh_plan(self.BANDS, 5) == [((1,), 1), ((0,), 4)]
+
+    def test_a_lag_longer_than_the_hold_reaches_further_back(self):
+        # Three halves down a stack, held for two refreshes.
+        plan = scan_order.refresh_plan([(0, 8, 3)], 2)
+        assert plan == [((2,), 1), ((1,), 1)]
+
+    def test_every_refresh_of_the_frame_is_accounted_for(self):
+        for hold in range(1, 9):
+            plan = scan_order.refresh_plan([(0, 8, 1), (8, 16, 2)], hold)
+            assert sum(count for _, count in plan) == hold
+
+
 class TestCompose:
     def test_a_band_comes_from_the_frame_that_many_refreshes_back(self):
         now, previous = _frame(30), _frame(20)
@@ -133,10 +154,28 @@ class TestUpdateDisplay:
         assert shown.getpixel((0, 0)) == (20, 0, 0)
         assert shown.getpixel((0, 31)) == (10, 0, 0)
 
-    def test_not_on_a_static_screen_or_a_held_frame(self, dm):
+    def test_not_on_a_static_screen(self, dm):
         dm._scan_lag_bands = [(16, 32, 1)]
         self._push(dm, 10)
         assert self._push(dm, 20).getpixel((0, 31)) == (20, 0, 0)   # not scrolling
+
+    def test_a_held_frame_is_split_so_the_lagging_half_steps_a_refresh_late(self, dm):
+        dm._scan_lag_bands = [(16, 32, 1)]
         dm.set_scrolling_state(True, 2)
-        self._push(dm, 30)
-        assert self._push(dm, 40).getpixel((0, 31)) == (40, 0, 0)   # hold 2
+        self._push(dm, 10)
+        before = len(dm._presented)
+        self._push(dm, 20)
+        first, second = dm._presented[before:]
+        assert first.getpixel((0, 0)) == (20, 0, 0)
+        assert first.getpixel((0, 31)) == (10, 0, 0)     # lagging half: still old
+        assert second.getpixel((0, 31)) == (20, 0, 0)    # caught up a refresh later
+
+    def test_a_slow_blit_is_not_split(self, dm):
+        dm._scan_lag_bands = [(16, 32, 1)]
+        dm.set_scrolling_state(True, 3)
+        self._push(dm, 10)
+        dm._last_blit_seconds = 1.0   # far longer than a refresh
+        before = len(dm._presented)
+        self._push(dm, 20)
+        assert len(dm._presented) - before == 1
+        assert dm._presented[-1].getpixel((0, 31)) == (20, 0, 0)