fix(display): end the scroll state at scroller-to-static handovers (#716)

A static plugin screen that follows a scroller no longer starts with the ticker's lagging rows on scan-compensated panels, and the 1 Hz loop's second frame is no longer recorded as a ~1 s mid-scroll freeze / Render stall. The display controller calls DisplayManager.end_scroll_for_static_screen() before a static screen's first display() (clears the scan history; _scan_segments passes its frames through in one swap) and set_scrolling_state(False) after it; the scroller's hold stays until then, so late-frame counts are unchanged. A screen's first frame is tagged 'handover': gaps of 250 ms or more before it go to the additive handover_freezes (frame_soak prints 'Handover gaps'), not freezes. The display thread is named display-<plugin id>. The WiFi notice and the schedule-off blank are not covered yet (docs list them as a follow-up).

ledpi A B B A soak (20 min each, --preview): main 0.118% / 0.113% late with 6 / 3 freezes; with this and #717 0.107% / 0.104% late, 0 freezes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/common/frame_timing.py

@@ -38,12 +38,20 @@ after the one before it. One that arrives a whole refresh or more after that is
 a visible hitch. ``missed_refreshes`` sums how many refreshes late.
 
 An interval of ``FREEZE_SECONDS`` or more is a **freeze** instead -- a
-recompose, a plugin handover, a blocking call on the render thread. Those are
-counted separately, both because they are a different fault and because
-folding a single 400ms handover into the late count as "40 missed refreshes"
-would drown the jitter the late count exists to measure. ``freeze_by`` splits
-them by length. Intervals of ``GAP_SECONDS`` or more are ignored as not being
-frames of one scroll at all.
+recompose, a plugin handover nobody tagged (see below), a blocking call on the
+render thread. Those are counted separately, both because they are a different
+fault and because folding a single 400ms handover into the late count as "40
+missed refreshes" would drown the jitter the late count exists to measure.
+``freeze_by`` splits them by length. Intervals of ``GAP_SECONDS`` or more are
+ignored as not being frames of one scroll at all.
+
+One kind of freeze is not a scroll stalling at all: the gap from one screen's
+last frame to the next screen's first, while the next screen draws. The
+display controller tags that frame ``handover`` (see "Operations") at the
+start of every turn, the same mode's again included, and a tagged freeze is
+counted in ``handover_freezes`` instead of ``freezes`` and ``freeze_by``.
+Stats written before that field existed have handovers among their freezes,
+so freeze counts from before and after it are not comparable.
 
 A frame that arrives a whole refresh or more *early* means the swap did not
 wait for the panel: the emulator, the fallback display, or a hold that was not
@@ -77,6 +85,12 @@ the work landed in. ``op_frames`` counts timed frames per kind,
 freeze instead, and ``op_bytes`` what the work moved. A kind whose late rate
 sits well above the overall one is the work to look at.
 
+``handover`` (:data:`HANDOVER_OP`) is noted off the render thread: the display
+controller notes it just before it starts a screen's first ``display()``,
+which presents from a thread of its own, and drops the note again with
+:meth:`FrameTimingRecorder.drop_op` once that call returns, so a first
+``display()`` that drew nothing cannot leave the tag for an unrelated frame.
+
 Stall watchdog
 --------------
 Counting a freeze says that it happened, not why. ``StallWatchdog`` watches the
@@ -133,6 +147,11 @@ RESUME_SECONDS = 1.0
 FREEZE_BUCKETS = ((0.5, "<0.5s"), (1.0, "0.5-1s"), (2.0, "1-2s"),
                   (float("inf"), "2s+"))
 
+#: The op the display controller notes before a screen's first frame. A
+#: freeze it ends is a handover, counted apart from the freezes; see
+#: "What is counted".
+HANDOVER_OP = "handover"
+
 #: A window may lower the refresh-period estimate by at most this fraction.
 MAX_REFRESH_DROP = 0.2
 
@@ -302,6 +321,10 @@ class FrameTimingRecorder:
             "freezes": 0,
             "freeze_seconds": 0.0,
             "freeze_by": {label: 0 for _, label in FREEZE_BUCKETS},
+            # Freezes that ended a screen handover rather than stalled a
+            # scroll: in neither of the two above. Additive; see "What is
+            # counted".
+            "handover_freezes": 0,
             "worst_interval_ms": 0.0,
             # Per kind of noted render-thread work; see "Operations".
             "op_frames": {},
@@ -337,7 +360,8 @@ class FrameTimingRecorder:
 
         Render thread only, like :meth:`record`, which consumes the tag: the
         interval the next frame ends is the one this work landed in. Several
-        notes before one frame accumulate, per kind. See "Operations".
+        notes before one frame accumulate, per kind. See "Operations" (and
+        :data:`HANDOVER_OP`, the one note made from another thread).
 
         :param kind: a short name for the work, e.g. ``"extend"``, ``"patch"``.
         :param nbytes: how much the work moved, summed into ``op_bytes``.
@@ -347,6 +371,21 @@ class FrameTimingRecorder:
             ops = self._ops = {}
         ops[kind] = ops.get(kind, 0) + int(nbytes)
 
+    def drop_op(self, kind: str) -> None:
+        """Forget a note of ``kind`` that no frame has carried yet.
+
+        For work that may present nothing: the display controller notes a
+        handover before a screen's first ``display()`` and drops it once that
+        returns. When the call drew a frame, the frame already took the tag
+        and this does nothing; when it drew nothing (no content), the tag
+        would otherwise land on whatever frame came next -- seconds or minutes
+        later, and nothing to do with the handover. Other kinds noted for the
+        same frame are kept.
+        """
+        ops = self._ops
+        if ops is not None:
+            ops.pop(kind, None)
+
     def record(self, blit: float, wait: float, hold: int, scrolling: bool,
                presented_at: float) -> None:
         """One frame reached the panel.
@@ -467,13 +506,19 @@ class FrameTimingRecorder:
                 for kind, nbytes in ops.items():
                     _bump(totals["op_bytes"], kind, nbytes)
             if interval >= FREEZE_SECONDS:
+                for kind in ops or ():
+                    _bump(totals["op_freezes"], kind)
+                if ops and HANDOVER_OP in ops:
+                    # The next screen drawing its first frame, not a scroll
+                    # that stalled: counted apart, so the freezes keep
+                    # meaning the second. See "What is counted".
+                    totals["handover_freezes"] += 1
+                    continue
                 totals["freezes"] += 1
                 totals["freeze_seconds"] += interval
                 label = next(name for limit, name in FREEZE_BUCKETS
                              if interval < limit)
                 totals["freeze_by"][label] += 1
-                for kind in ops or ():
-                    _bump(totals["op_freezes"], kind)
                 continue
             totals["scroll_frames"] += 1
             for name, value in (("blit", blit), ("wait", wait),
@@ -647,11 +692,19 @@ class StallWatchdog:
         return stall_from, dumped
 
     def describe(self, ident: int, age: float, late: float) -> str:
-        """The stack dump: the stalled thread in full, the rest in brief."""
+        """The stack dump: the stalled thread in full, the rest in brief.
+
+        A stall while a ``handover`` note is still waiting for its frame is
+        the next screen's first ``display()`` taking its time, not a scroll
+        that stopped, and is labelled a handover gap.
+        """
         names = {t.ident: t.name for t in threading.enumerate()}
         frames = sys._current_frames()
+        pending = getattr(self.recorder, "_ops", None)
+        where = ("in a handover gap" if pending and HANDOVER_OP in pending
+                 else "mid-scroll")
         lines = [
-            f"Render stall: no frame for {age * 1000.0:.0f}ms mid-scroll "
+            f"Render stall: no frame for {age * 1000.0:.0f}ms {where} "
             f"(watchdog woke {late * 1000.0:.0f}ms late"
             + ("; the interpreter itself was blocked" if late >= age / 2 else "")
             + ")",

src/display_controller.py

@@ -42,6 +42,7 @@ from src.cache_manager import CacheManager
 from src.font_manager import FontManager
 from src.logging_config import get_logger
 from src.exceptions import PluginError
+from src.common.frame_timing import HANDOVER_OP
 from src.common.sync_manager import DisplaySyncManager, SyncRole
 from src.ipc.server import ControlServer, start_control_server
 from src.vegas_mode.render_pipeline import SYNC_SEND_INTERVAL
@@ -2571,6 +2572,70 @@ class DisplayController:
         logger.debug(f"Found plugin manager for mode {mode}: {type(plugin_instance).__name__}")
         return plugin_instance
 
+    def _start_screen_handover(self, plugin, active_mode: str) -> bool:
+        """Before a screen's first dispatch: if the screen is static, keep
+        the last scroll's leftovers off its first frame.
+
+        Nothing else ends a scroll when the rotation moves on: the state
+        expires 2 s after the scroller's last frame. Left to that, a static
+        screen's first frame -- up for a whole second -- went out, on a panel
+        with scan-order compensation, with rows taken from the scroller's
+        last frame (after a held scroll, for its first refresh); and its
+        second frame, 1 s later, was still "mid-scroll", so the frame-timing
+        soak counted a 1-2 s freeze and the stall watchdog logged a "Render
+        stall" at every scroller-to-static handover. See
+        DisplayManager.end_scroll_for_static_screen.
+
+        Returns whether the screen is static, for _finish_screen_handover.
+        False when that cannot be told, which leaves the scroll state as it
+        was before this existed.
+        """
+        try:
+            static_screen = not self._needs_high_fps(plugin, active_mode, log=False)
+        except Exception:  # pylint: disable=broad-except
+            # A plugin property raising. The FPS check after the dispatch is
+            # where that is reported; here it only means "leave it alone".
+            logger.debug("Could not tell whether %s is static before its first frame",
+                         active_mode, exc_info=True)
+            return False
+        if static_screen:
+            end_scroll = getattr(self.display_manager, 'end_scroll_for_static_screen', None)
+            if end_scroll is not None:
+                end_scroll()
+        return static_screen
+
+    def _note_screen_handover(self) -> None:
+        """Tag the frame the first dispatch is about to present.
+
+        The gap from the last screen's final frame to it is the next screen
+        drawing, not a scroll freezing: frame_timing counts it apart from
+        the freezes, and the stall watchdog labels it a handover gap.
+        """
+        recorder = getattr(self.display_manager, 'frame_timing', None)
+        note = getattr(recorder, 'note_op', None)
+        if note is not None:
+            note(HANDOVER_OP)
+
+    def _finish_screen_handover(self, static_screen: bool) -> None:
+        """After a screen's first dispatch, whatever it returned.
+
+        Drops the handover tag if no frame took it (a screen with nothing to
+        show), so it cannot land on an unrelated frame later. For a static
+        screen, also ends the previous scroll now, whether or not it showed
+        anything: its first frame has gone out, and with the state left set
+        its next one -- a second later in the 1 Hz loop -- would be timed as
+        a frame of the old scroll.
+        """
+        dm = self.display_manager
+        recorder = getattr(dm, 'frame_timing', None)
+        drop = getattr(recorder, 'drop_op', None)
+        if drop is not None:
+            drop(HANDOVER_OP)
+        if static_screen:
+            set_scrolling_state = getattr(dm, 'set_scrolling_state', None)
+            if set_scrolling_state is not None:
+                set_scrolling_state(False)
+
     def _dispatch_first_frame(self, plugin, active_mode: str) -> Tuple[bool, bool, bool]:
         """Draw the first frame of a screen through the PluginExecutor.
 
@@ -2592,6 +2657,10 @@ class DisplayController:
         display_failed_due_to_exception = False
         _accepts_display_mode = False
         plugin_id = getattr(plugin, 'plugin_id', active_mode)
+        # Decided before the first frame rather than at run()'s FPS check
+        # after it, by when that frame has gone out with the last scroll's
+        # rows. See _start_screen_handover.
+        static_screen = self._start_screen_handover(plugin, active_mode)
         try:
             logger.debug(f"Calling display() for {active_mode} with force_clear={self.force_change}")
             if plugin_id not in self._plugin_accepts_display_mode:
@@ -2606,6 +2675,10 @@ class DisplayController:
             display_hung = False
             # Set when display() raised inside the executor.
             display_error: Optional[Exception] = None
+            if can_display:
+                # Only when display() will run: a busy plugin
+                # presents nothing for the tag to land on.
+                self._note_screen_handover()
 
             if display_lock is None:
                 # Only when plugin loading failed part-way.
@@ -2729,6 +2802,10 @@ class DisplayController:
             self.force_change = True
             display_result = False
             display_failed_due_to_exception = True
+        # Whatever the dispatch did -- drew, had nothing to show, raised
+        # inside the executor or out here -- and after the health record,
+        # before the 1 Hz loop or the next mode.
+        self._finish_screen_handover(static_screen)
         return display_result, display_failed_due_to_exception, _accepts_display_mode
 
     def _skip_failed_plugin_modes(self, active_mode: str) -> bool:
@@ -2878,7 +2955,7 @@ class DisplayController:
                     return None
         return min_duration, max_duration
 
-    def _needs_high_fps(self, plugin, active_mode: str) -> bool:
+    def _needs_high_fps(self, plugin, active_mode: str, log: bool = True) -> bool:
         """Whether a screen runs the high-FPS (8 ms) loop or the 1 s one.
 
         In precedence order:
@@ -2889,29 +2966,36 @@ class DisplayController:
            the attribute keep the historical forced high-FPS
            (GIF support).
         3. Otherwise scrolling plugins get high FPS.
+
+        ``log=False`` for the look taken before a screen's first dispatch
+        (see _start_screen_handover): the FPS check after it logs the
+        decision, and once per screen is enough.
         """
         plugin_id = getattr(plugin, 'plugin_id', None)
         declared = getattr(plugin, 'needs_high_fps', None)
         if declared is not None:
             needs_high_fps = bool(declared)
-            logger.debug(
-                "[DisplayController] FPS check for %s (plugin=%s) - "
-                "plugin declares needs_high_fps=%s",
-                active_mode, plugin_id, needs_high_fps)
+            if log:
+                logger.debug(
+                    "[DisplayController] FPS check for %s (plugin=%s) - "
+                    "plugin declares needs_high_fps=%s",
+                    active_mode, plugin_id, needs_high_fps)
         elif plugin_id == 'static-image':
             needs_high_fps = True
-            logger.debug("FPS check - static-image plugin: forcing high-FPS mode for GIF support")
+            if log:
+                logger.debug("FPS check - static-image plugin: forcing high-FPS mode for GIF support")
         else:
             has_enable_scrolling = hasattr(plugin, 'enable_scrolling')
             enable_scrolling_value = getattr(plugin, 'enable_scrolling', False)
             needs_high_fps = has_enable_scrolling and enable_scrolling_value
-            logger.info(
-                "FPS check for %s - has_enable_scrolling: %s, enable_scrolling_value: %s, needs_high_fps: %s",
-                active_mode,
-                has_enable_scrolling,
-                enable_scrolling_value,
-                needs_high_fps,
-            )
+            if log:
+                logger.info(
+                    "FPS check for %s - has_enable_scrolling: %s, enable_scrolling_value: %s, needs_high_fps: %s",
+                    active_mode,
+                    has_enable_scrolling,
+                    enable_scrolling_value,
+                    needs_high_fps,
+                )
         return needs_high_fps
 
     def _advance_after_screen(self, active_mode: Optional[str]) -> None:
@@ -3194,6 +3278,9 @@ class DisplayController:
                         continue
                     min_duration, max_duration = bounds
 
+                    # High-FPS decision; see _needs_high_fps for the order.
+                    # Read again here, after the first dispatch, as it always
+                    # was: a plugin may settle it in that display() call.
                     needs_high_fps = self._needs_high_fps(manager_to_display, active_mode)
 
                     target_duration = max_duration

src/display_manager.py

@@ -344,6 +344,10 @@ class DisplayManager:
         # advances a whole pixel every Nth refresh instead of every one.
         # See src/common/scroll_config.py and scripts/scroll_speeds.py.
         self._frame_hold = 1
+        # True while a static screen draws its first frame after a scroll,
+        # whose state is left set until then: those frames go out without
+        # scan-order compensation. See end_scroll_for_static_screen().
+        self._static_handover = False
 
         # A src.common.render_gate.RenderGate while Vegas runs with
         # vegas_scroll.prefetch_gate on: opened around each swap so the
@@ -1060,11 +1064,15 @@ class DisplayManager:
         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.
+        as it is, and drops the history. So does a static screen's first frame
+        after a scroll, while the scroll state is still set (see
+        end_scroll_for_static_screen): one segment, held for the scroll's
+        hold, with no rows from the scroller's frames.
         """
         hold = self._frame_hold
         bands = getattr(self, '_scan_lag_bands', None)
-        if not bands or not self.is_currently_scrolling():
+        if (not bands or not self.is_currently_scrolling()
+                or self._static_handover):
             if bands:
                 self._scan_history.clear()
             return [(image, hold)]
@@ -1524,6 +1532,9 @@ class DisplayManager:
             # A plugin captured for Vegas calls this from its own display();
             # it must not change the live scroll's state or frame hold.
             return
+        # A scroll starting or ending also ends a static screen's handover;
+        # see end_scroll_for_static_screen.
+        self._static_handover = False
         current_time = time.time()
         # Scrolling callers set this every frame; log transitions only.
         changed = self._scrolling_state['is_scrolling'] != is_scrolling
@@ -1536,6 +1547,46 @@ class DisplayManager:
         if changed:
             logger.debug("Scrolling state set to: %s", is_scrolling)
 
+    def end_scroll_for_static_screen(self) -> None:
+        """Ready the panel for a static screen's first frame after a scroll.
+
+        The display controller calls this just before it dispatches the first
+        frame of a screen that runs its 1 Hz loop, and
+        ``set_scrolling_state(False)`` once that dispatch returns. Nothing
+        else ends a scroll at a handover: the state belongs to the screen
+        before, and would only expire 2 s after its last frame.
+
+        Until then, the frames that dispatch presents go out as drawn, not
+        scan-order composed: each as one segment, held for the scroll's hold.
+        With the state still "scrolling", ``_scan_segments`` would take their
+        lagging rows from the frame before: for the first, the scroller's last
+        frame -- the bottom half of the old ticker under the new screen on a
+        96x48 panel. At hold 1 that frame stays up for a whole second; at a
+        longer hold its first refresh flashes the old rows. For a second frame
+        in the same call, the rows would come from the first. Dirty tracking
+        compares frames as drawn, so once the scroll is over it skips every
+        identical 1 Hz redraw of such a frame, and nothing would replace it.
+
+        The rest of that scroll is left on purpose, until the controller ends
+        it:
+
+        * the scroll state, so the gap from the scroller's last frame to this
+          screen's first is still timed by the frame-timing recorder and
+          watched by the stall watchdog, which is where a slow first
+          ``display()`` shows up;
+        * its frame hold. On a frame that stays up for a second it only moves
+          the swap to the scroll's next hold boundary, and it is the pacing
+          that gap is due at: judged at hold 1, a handover that kept the
+          scroller's own schedule would count as frames late.
+
+        The next ``set_scrolling_state()`` call, whoever makes it, ends this.
+        One attribute store, so no lock: ``update_display`` reads it once per
+        frame, under its own, and the history is dropped there.
+        """
+        if self._writes_suppressed():
+            return  # a thread drawing off-screen cannot end the live scroll
+        self._static_handover = True
+
     def is_currently_scrolling(self) -> bool:
         """Check if the display is currently in a scrolling state."""
         current_time = time.time()

src/plugin_system/plugin_executor.py

@@ -59,7 +59,8 @@ class PluginExecutor:
         self,
         operation: Callable[[], Any],
         timeout: Optional[float] = None,
-        plugin_id: Optional[str] = None
+        plugin_id: Optional[str] = None,
+        thread_name: Optional[str] = None
     ) -> Any:
         """
         Execute a plugin operation with timeout.
@@ -68,6 +69,8 @@ class PluginExecutor:
             operation: Function to execute
             timeout: Timeout in seconds (None = use default)
             plugin_id: Optional plugin ID for logging
+            thread_name: Name for the thread the operation runs on (None
+                keeps Python's default). Stack dumps list threads by name.
             
         Returns:
             Result of operation
@@ -93,7 +96,7 @@ class PluginExecutor:
                 result_container['exception'] = e
                 result_container['completed'] = True
         
-        thread = Thread(target=target, daemon=True)
+        thread = Thread(target=target, daemon=True, name=thread_name)
         thread.start()
         thread.join(timeout=timeout)
         
@@ -223,18 +226,24 @@ class PluginExecutor:
                     'display_mode' in inspect.signature(plugin.display).parameters)
             has_display_mode = accepts_display_mode
             
+            # Named for the plugin: this thread presents a screen's first
+            # frame, so the frame-timing stall watchdog's stack dumps name it.
+            thread_name = f"display-{plugin_id}"
+
             # Capture the return value from the plugin's display() method
             if has_display_mode and display_mode:
                 result = self.execute_with_timeout(
                     lambda: plugin.display(display_mode=display_mode, force_clear=force_clear),
                     timeout=timeout,
-                    plugin_id=plugin_id
+                    plugin_id=plugin_id,
+                    thread_name=thread_name
                 )
             else:
                 result = self.execute_with_timeout(
                     lambda: plugin.display(force_clear=force_clear),
                     timeout=timeout,
-                    plugin_id=plugin_id
+                    plugin_id=plugin_id,
+                    thread_name=thread_name
                 )
             
             duration = time.monotonic() - start_time