fix(display): a frame the preview throttle skipped still reaches the snapshot (#752)

A screen that draws its card once and holds it no longer leaves the web preview black: DisplayManager remembers a changed frame the snapshot throttle skipped, and the render loop writes it (write_owed_snapshot(), called from _display_once) once the interval has passed. A failed owed write stays owed and is retried.

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

CHANGELOG.md

@@ -19,6 +19,21 @@ accepts both, but the store flags the old spelling as deprecated
 
 ## Unreleased
 
+### Fixed
+
+- The web preview and `/api/v3/display/current` no longer stay black for a
+  whole screen that draws its card once and then holds it. The snapshot is
+  written from `update_display()` at most once per write interval, so a frame
+  pushed inside that interval was skipped and left for the next
+  `update_display()` -- which such a screen never makes. Soccer's
+  recent/upcoming cards skip redundant redraws, and the first one after an
+  on-demand start lands a few milliseconds after the start's clear wrote a
+  black frame: on ledpi the preview showed 0 lit pixels for the whole 15 s
+  while the panel showed the card. `DisplayManager` now remembers a skipped
+  changed frame, and the render loop writes it (`write_owed_snapshot()`)
+  once the interval has passed. The cadence is unchanged, and nothing extra
+  runs when no frame is owed.
+
 ### Cheap per-frame and per-fetch savings
 
 - `BaseOddsManager.get_odds()` no longer pretty-prints every odds response

src/display_controller.py

@@ -1230,6 +1230,13 @@ class DisplayController:
                 note = getattr(self.plugin_manager, 'note_display_duration', None)
                 if note is not None and plugin_id:
                     note(plugin_id, time.monotonic() - started)
+                # A screen that drew once and holds makes no more
+                # update_display() calls, so a frame the preview throttle
+                # skipped would otherwise never reach the snapshot.
+                write_owed = getattr(getattr(self, 'display_manager', None),
+                                     'write_owed_snapshot', None)
+                if write_owed is not None:
+                    write_owed()
 
     def _health_tracker(self):
         """The plugin circuit breaker, or None when it is not enabled."""

src/display_manager.py

@@ -317,6 +317,11 @@ class DisplayManager:
         # is handed to the writer; this only once it has been saved, so an
         # mtime touch never vouches for a frame still waiting to be written.
         self._saved_snapshot_digest: Optional[int] = None
+        # A changed frame reached _write_snapshot_if_due() inside the write
+        # interval and was skipped. Nothing writes it unless update_display()
+        # runs again, and a screen that draws once and holds never calls it
+        # again -- see write_owed_snapshot().
+        self._snapshot_owed = False
         self._snapshot_dir_prepared = False
         # Background writer used mid-scroll; see _write_snapshot_if_due.
         self._snapshot_cond = threading.Condition()
@@ -1788,9 +1793,10 @@ class DisplayManager:
 
             if frame_checksum is not None:
                 digest = frame_checksum
+                frame_changed = digest != self._last_snapshot_digest
                 action = snapshot_policy.decide(
                     now, self._last_snapshot_ts, self._last_snapshot_touch_ts,
-                    viewer_fresh, digest != self._last_snapshot_digest)
+                    viewer_fresh, frame_changed)
             else:
                 # Ask as if the frame had changed before paying to find out.
                 # decide() is monotone in frame_changed -- a SKIP for a
@@ -1802,22 +1808,36 @@ class DisplayManager:
                     now, self._last_snapshot_ts, self._last_snapshot_touch_ts,
                     viewer_fresh, True)
                 if action is snapshot_policy.SnapshotAction.SKIP:
+                    # Not hashed, so not known to be unchanged: owed until a
+                    # later look finds it written or unchanged.
+                    self._snapshot_owed = True
                     return
                 digest = zlib.adler32(self.image.tobytes())
-                if digest == self._last_snapshot_digest:
+                frame_changed = digest != self._last_snapshot_digest
+                if not frame_changed:
                     # Unchanged after all: the decision an unchanged frame gets.
                     action = snapshot_policy.decide(
                         now, self._last_snapshot_ts,
                         self._last_snapshot_touch_ts, viewer_fresh, False)
             if action is snapshot_policy.SnapshotAction.SKIP:
+                # A changed frame inside the write interval stays owed: the
+                # next update_display() would write it, but a static screen
+                # may not make one -- write_owed_snapshot() covers that.
+                self._snapshot_owed = frame_changed
                 return
             if (action is snapshot_policy.SnapshotAction.TOUCH
                     and self._saved_snapshot_digest == digest):
                 # mtime bump only: keeps the health check (snapshot age)
                 # green without paying for a PNG encode of an unchanged frame
+                # (this frame is already on disk, so nothing is owed).
+                self._snapshot_owed = False
                 os.utime(self._snapshot_path, None)
                 self._last_snapshot_touch_ts = now
                 return
+            # Owed until the write below succeeds: if it raises, the frame
+            # stays owed and write_owed_snapshot() retries it, rather than a
+            # held screen leaving the preview stale after one failed write.
+            self._snapshot_owed = True
             # (A TOUCH for a frame that isn't on disk yet -- still queued, or
             # its write failed -- is written instead: touching would make the
             # older file on disk look current.)
@@ -1842,9 +1862,39 @@ class DisplayManager:
             self._last_snapshot_ts = now
             self._last_snapshot_touch_ts = now
             self._last_snapshot_digest = digest
+            self._snapshot_owed = False
         except Exception as e:
             self._log_snapshot_failure(e)
 
+    def write_owed_snapshot(self) -> None:
+        """Write a frame the snapshot throttle skipped, once it is due.
+
+        The preview snapshot is only ever written from update_display(), and
+        at most once per write interval (snapshot_policy). A frame pushed
+        inside that interval is skipped, and is written by the next
+        update_display() that comes after it -- but a screen that draws its
+        card once and then holds it makes no further call. Its frame was on
+        the panel and never in the preview: soccer's recent/upcoming cards
+        skip redundant redraws, and the first one after an on-demand start
+        (pushed a few milliseconds after the controller's clear) left
+        /api/v3/display/current and the web preview black for the whole
+        screen while the panel showed the card.
+
+        The render loop calls this after each frame. Cheap when nothing is
+        owed (one attribute read); otherwise the usual policy decides, so
+        the write still waits out the interval and an unchanged frame is
+        never re-encoded.
+        """
+        if not self._snapshot_owed:
+            return
+        try:
+            if self._writes_suppressed():
+                return
+            with self._update_lock:
+                self._write_snapshot_if_due()
+        except Exception as e:  # pylint: disable=broad-except
+            self._log_snapshot_failure(e)
+
     def _log_snapshot_failure(self, error: Exception) -> None:
         # Snapshot failures must never break display — but they must not
         # be silent either: the snapshot's mtime is the web UI's display

test/test_snapshot_owed_frame.py

@@ -0,0 +1,183 @@
+"""A frame the preview throttle skipped still reaches the snapshot.
+
+The preview snapshot (/api/v3/display/current, the web UI's live preview) is
+only written from update_display(), at most once per write interval. A screen
+that draws its card once and then holds it -- soccer's recent/upcoming cards
+skip redundant redraws -- pushes exactly one frame. When that push lands inside
+the interval, e.g. a few milliseconds after the on-demand start's clear wrote a
+black frame, the throttle skips it and nothing ever writes it: on ledpi the
+preview stayed black for soccer's whole 15 s screen while the panel showed the
+card, and the next screen "rendered immediately".
+
+Runs the real DisplayManager on the emulator, like test_display_dirty_tracking.
+"""
+
+import os
+import sys
+import types
+
+os.environ["EMULATOR"] = "true"
+
+import pytest
+from PIL import Image
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+
+
+@pytest.fixture(scope="module")
+def dm(tmp_path_factory):
+    from src.display_manager import DisplayManager
+    DisplayManager._instance = None
+    manager = DisplayManager({
+        "display": {
+            "hardware": {"rows": 32, "cols": 64, "chain_length": 2,
+                         "parallel": 1, "brightness": 90},
+            "runtime": {"gpio_slowdown": 0},
+        },
+    }, suppress_test_pattern=True)
+    manager._snapshot_path = str(
+        tmp_path_factory.mktemp("owed_snapshot") / "led_matrix_preview.png")
+    yield manager
+    DisplayManager._instance = None
+
+
+@pytest.fixture
+def viewer(dm, monkeypatch, tmp_path):
+    """A preview is open (1 s write interval); fresh snapshot bookkeeping."""
+    monkeypatch.setattr(dm, "_viewer_is_fresh", lambda now: True)
+    dm._viewer_was_fresh = True
+    dm._snapshot_path = str(tmp_path / "snap.png")
+    dm._last_snapshot_ts = 0.0
+    dm._last_snapshot_touch_ts = 0.0
+    dm._last_snapshot_digest = None
+    dm._saved_snapshot_digest = None
+    dm._snapshot_owed = False
+    dm.set_scrolling_state(False)
+    return dm
+
+
+def _lit(path):
+    with Image.open(path) as img:
+        return sum(1 for p in img.convert("RGB").getdata() if max(p) > 20)
+
+
+def _age_last_write(dm, seconds=2.0):
+    """As if `seconds` had passed since the last snapshot write."""
+    dm._last_snapshot_ts -= seconds
+    dm._last_snapshot_touch_ts -= seconds
+
+
+def _clear_then_draw_card(dm):
+    """The on-demand start's clear, then the card a few ms later."""
+    dm.clear()
+    dm.update_display()                       # black frame: written
+    assert _lit(dm._snapshot_path) == 0
+    dm.draw.rectangle([4, 4, 40, 20], fill=(255, 255, 0))
+    dm.update_display()                       # the card: inside the interval
+
+
+def _controller(dm):
+    from src import display_controller as dc_module
+    controller = dc_module.DisplayController.__new__(dc_module.DisplayController)
+    controller.plugin_manager = None
+    controller.display_manager = dm
+    return controller
+
+
+class _HoldingPlugin:
+    """Already showing its card: display() returns True and draws nothing."""
+
+    plugin_id = "holding"
+
+    def __init__(self):
+        self.calls = 0
+
+    def display(self, display_mode=None, force_clear=False):
+        self.calls += 1
+        return True
+
+
+def test_a_held_card_reaches_the_preview_on_the_next_frame(viewer):
+    dm = viewer
+    _clear_then_draw_card(dm)
+    assert _lit(dm._snapshot_path) == 0       # the throttle skipped the card
+
+    controller = _controller(dm)
+    plugin = _HoldingPlugin()
+    _age_last_write(dm)
+    # The render loop's next frame: the plugin draws nothing and makes no
+    # update_display() call, as soccer's switch cards do.
+    assert controller._display_once(plugin, "soccer_eng.1_recent", True) is True
+    assert plugin.calls == 1
+    assert _lit(dm._snapshot_path) > 0
+
+
+def test_the_owed_write_still_waits_out_the_interval(viewer, monkeypatch):
+    dm = viewer
+    _clear_then_draw_card(dm)
+    saves = []
+    monkeypatch.setattr(dm, "_save_snapshot", lambda image: saves.append(image))
+    dm.write_owed_snapshot()                  # still inside the interval
+    assert saves == []
+    _age_last_write(dm)
+    dm.write_owed_snapshot()
+    assert len(saves) == 1
+    # Written: nothing is owed, so later frames do no work and the unchanged
+    # frame is not encoded again.
+    assert dm._snapshot_owed is False
+    _age_last_write(dm)
+    dm.write_owed_snapshot()
+    assert len(saves) == 1
+
+
+def test_a_failed_owed_write_stays_owed_and_is_retried(viewer, monkeypatch):
+    dm = viewer
+    _clear_then_draw_card(dm)
+    _age_last_write(dm)
+    attempts = []
+
+    def failing_save(image):
+        attempts.append(image)
+        raise OSError("disk full")
+
+    monkeypatch.setattr(dm, "_save_snapshot", failing_save)
+    dm.write_owed_snapshot()                  # the write fails
+    assert len(attempts) == 1
+    assert dm._snapshot_owed is True          # still owed: a held screen
+    saves = []                                # makes no update_display()
+    monkeypatch.setattr(dm, "_save_snapshot", lambda image: saves.append(image))
+    dm.write_owed_snapshot()                  # retried on the next frame
+    assert len(saves) == 1
+    assert dm._snapshot_owed is False
+
+
+def test_nothing_owed_after_a_frame_that_was_written(viewer, monkeypatch):
+    dm = viewer
+    dm.draw.rectangle([0, 0, 8, 8], fill=(0, 255, 0))
+    dm.update_display()                       # due: written at once
+    assert dm._snapshot_owed is False
+    calls = []
+    monkeypatch.setattr(dm, "_write_snapshot_if_due",
+                        lambda *a, **k: calls.append(a))
+    dm.write_owed_snapshot()
+    assert calls == []
+
+
+def test_an_unchanged_frame_inside_the_interval_is_not_owed(viewer):
+    dm = viewer
+    dm.draw.rectangle([0, 0, 8, 8], fill=(0, 0, 255))
+    dm.update_display()
+    dm.update_display()                       # same frame, inside the interval
+    assert dm._snapshot_owed is False
+
+
+def test_a_controller_without_the_hook_still_draws():
+    """Controllers built without a display manager (tests) are unaffected."""
+    from src import display_controller as dc_module
+    controller = dc_module.DisplayController.__new__(dc_module.DisplayController)
+    controller.plugin_manager = None
+    plugin = _HoldingPlugin()
+    assert controller._display_once(plugin, "x", True) is True
+    controller.display_manager = types.SimpleNamespace()
+    assert controller._display_once(plugin, "x", True) is True
+    assert plugin.calls == 2