diff --git a/src/display_controller.py b/src/display_controller.py index f924db81..3ba59262 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -2125,10 +2125,23 @@ class DisplayController: # For plugins, call display multiple times to allow game rotation if manager_to_display and hasattr(manager_to_display, 'display'): - # Check if plugin needs high FPS (like stock ticker) - # Always enable high-FPS for static-image plugin (for GIF animation support) + # High-FPS decision, in precedence order: + # 1. A plugin that declares needs_high_fps knows best + # (e.g. static-image sets it False for still PNGs, + # True for animated GIFs). + # 2. Back-compat: older static-image versions without + # the attribute keep the historical forced high-FPS + # (GIF support). + # 3. Otherwise scrolling plugins get high FPS. plugin_id = getattr(manager_to_display, 'plugin_id', None) - if plugin_id == 'static-image': + declared = getattr(manager_to_display, '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) + elif plugin_id == 'static-image': needs_high_fps = True logger.debug("FPS check - static-image plugin: forcing high-FPS mode for GIF support") else: diff --git a/src/display_manager.py b/src/display_manager.py index 420cb3f2..9de558a1 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -33,6 +33,7 @@ else: from contextlib import contextmanager from pathlib import Path from PIL import Image, ImageDraw, ImageFont +import threading import time from collections import OrderedDict from typing import Dict, Any, List, Optional, Tuple @@ -219,6 +220,17 @@ class DisplayManager: # but is never silent: the snapshot's mtime doubles as the web UI's # hardware-liveness signal, so a quiet failure makes health checks lie. self._snapshot_fail_log_ts = 0.0 + # Dirty tracking: (image digest, brightness) of the last frame pushed + # to the panel; update_display() skips identical pushes. Kill switch: + # display.dirty_tracking: false. + self._dirty_tracking_enabled = bool( + self.config.get('display', {}).get('dirty_tracking', True)) + self._last_pushed_digest = None + # Serializes update_display(): plugins can call it directly from + # background threads (see docstring on update_display), not just the + # render loop. RLock in case a caller within the critical section + # ever re-enters (e.g. via a nested draw callback). + self._update_lock = threading.RLock() # Scrolling state tracking for graceful updates self._scrolling_state = { @@ -449,6 +461,10 @@ class DisplayManager: try: # RGBMatrix accepts brightness as a property self.matrix.brightness = brightness + # Brightness applies on the next swap — force a re-push even if + # the image itself is unchanged (belt-and-braces: brightness is + # also part of the dirty-tracking digest when readable). + self._last_pushed_digest = None logger.info(f"[BRIGHTNESS] Display brightness set to {brightness}%") return True except AttributeError as e: @@ -540,33 +556,70 @@ class DisplayManager: return phys def update_display(self): - """Update the display using double buffering with proper sync.""" + """Update the display using double buffering with proper sync. + + Skips the panel push entirely when the frame is byte-identical to + the last pushed one (same image digest AND same brightness) — static + content re-rendered every second, and 125 fps loops between actual + scroll steps, otherwise re-walk the full framebuffer for nothing. + The panel keeps refreshing the current frame from its own thread, + so skipping a swap never blanks or freezes the hardware. + + Correctness hinges on invalidation: clear() resets the digest (it + writes to the matrix directly), and brightness is PART of the digest + so a dim-schedule change is never skipped. Disable via config + ``display.dirty_tracking: false`` if a redraw issue is ever suspected. + + Serialized via ``_update_lock``: plugins can call this directly from + background threads (e.g. sports base classes push an immediate + "live" refresh from inside update()), so without a lock two callers + could both pass the digest check before either writes it back, + double-pushing a frame, or interleave the offscreen/current canvas + swap below. The lock is scoped to this method, so callers never + need to know about it. + """ try: - if self.matrix is None: - # Fallback mode - no actual hardware to update - logger.debug("Update display called in fallback mode (no hardware)") - # Still write a snapshot so the web UI can preview + with self._update_lock: + if self.matrix is None: + # Fallback mode - no actual hardware to update + logger.debug("Update display called in fallback mode (no hardware)") + # Still write a snapshot so the web UI can preview + self._write_snapshot_if_due() + return + + if self._capture_mode_active: + return # Skip hardware write — content is being captured off-screen + + digest = None + if self._dirty_tracking_enabled: + try: + brightness = getattr(self.matrix, 'brightness', None) + except AttributeError: + brightness = None + digest = (zlib.adler32(self.image.tobytes()), brightness) + if digest == self._last_pushed_digest: + # Nothing changed since the last push — the panel is + # already showing exactly this frame. + self._write_snapshot_if_due() + return + + # Copy the current image to the offscreen canvas. In double-sided + # mode the logical screen is first tiled across the full chain. + if self._double_sided is not None: + self.offscreen_canvas.SetImage(self._composite_double_sided()) + else: + self.offscreen_canvas.SetImage(self.image) + + # Swap buffers immediately + self.matrix.SwapOnVSync(self.offscreen_canvas) + + # 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) self._write_snapshot_if_due() - return - - if self._capture_mode_active: - return # Skip hardware write — content is being captured off-screen - - # Copy the current image to the offscreen canvas. In double-sided - # mode the logical screen is first tiled across the full chain. - if self._double_sided is not None: - self.offscreen_canvas.SetImage(self._composite_double_sided()) - else: - self.offscreen_canvas.SetImage(self.image) - - # Swap buffers immediately - self.matrix.SwapOnVSync(self.offscreen_canvas) - - # Swap our canvas references - self.offscreen_canvas, self.current_canvas = self.current_canvas, self.offscreen_canvas - - # Write a snapshot for the web preview (throttled) - self._write_snapshot_if_due() except Exception as e: logger.error(f"Error updating display: {e}") @@ -600,6 +653,9 @@ class DisplayManager: # Clear both canvases and the underlying matrix to ensure no artifacts. # Failures are non-fatal — the image buffer is already black above, so # the next update_display() call will push clean content regardless. + # The matrix content no longer matches the last pushed digest, + # so dirty tracking must not skip the next push. + self._last_pushed_digest = None try: self.offscreen_canvas.Clear() except (RuntimeError, OSError) as e: diff --git a/test/test_display_dirty_tracking.py b/test/test_display_dirty_tracking.py new file mode 100644 index 00000000..d367d2d6 --- /dev/null +++ b/test/test_display_dirty_tracking.py @@ -0,0 +1,162 @@ +"""Tests for update_display dirty tracking (src/display_manager.py). + +Runs against RGBMatrixEmulator (EMULATOR=true), exercising the REAL +DisplayManager — not a mock — so the skip logic, its invalidation hooks, +and the kill switch are verified off-Pi. + +The invariants: +- identical frames are pushed exactly once (SwapOnVSync not re-called) +- ANY pixel change pushes +- clear() and set_brightness() invalidate (the two paths that alter panel + state outside the digest's view) +- the kill switch (display.dirty_tracking: false) restores always-push +""" + +import os +import sys +import time + +os.environ["EMULATOR"] = "true" + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + + +@pytest.fixture(scope="module") +def dm(): + """One real DisplayManager on the emulator (it's a process singleton).""" + from src.display_manager import DisplayManager + DisplayManager._instance = None + DisplayManager._initialized = False + manager = DisplayManager({ + "display": { + "hardware": {"rows": 32, "cols": 64, "chain_length": 2, + "parallel": 1, "brightness": 90}, + "runtime": {"gpio_slowdown": 0}, + }, + }, suppress_test_pattern=True) + yield manager + DisplayManager._instance = None + DisplayManager._initialized = False + + +class _SwapSpy: + """Counts SwapOnVSync calls through the real matrix object.""" + + def __init__(self, matrix): + self.matrix = matrix + self.count = 0 + self._orig = matrix.SwapOnVSync + + def __enter__(self): + def counting(canvas): + self.count += 1 + return self._orig(canvas) + self.matrix.SwapOnVSync = counting + return self + + def __exit__(self, *exc): + self.matrix.SwapOnVSync = self._orig + + +class TestDirtyTracking: + def test_identical_frames_push_once(self, dm): + dm.draw.rectangle([0, 0, 10, 10], fill=(255, 0, 0)) + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + dm.update_display() + dm.update_display() + assert spy.count == 1 + + def test_pixel_change_pushes(self, dm): + dm.update_display() + with _SwapSpy(dm.matrix) as spy: + dm.draw.point((5, 5), fill=(0, 255, 0)) + dm.update_display() + dm.update_display() # unchanged again + assert spy.count == 1 + + def test_clear_invalidates(self, dm): + dm.draw.rectangle([0, 0, 20, 20], fill=(0, 0, 255)) + dm.update_display() + dm.clear() # writes to the matrix directly; digest must reset + with _SwapSpy(dm.matrix) as spy: + dm.update_display() # black frame after clear must still push + assert spy.count == 1 + + def test_brightness_change_forces_push(self, dm): + dm.draw.rectangle([0, 0, 20, 20], fill=(200, 200, 200)) + dm.update_display() + with _SwapSpy(dm.matrix) as spy: + dm.update_display() # identical -> skipped + assert spy.count == 0 + dm.set_brightness(40) # dim schedule scenario + dm.update_display() # same image, new brightness -> push + assert spy.count == 1 + dm.set_brightness(90) + + def test_snapshot_still_written_on_skip(self, dm, tmp_path): + """The web preview mirror must keep working through skipped panel + pushes: _write_snapshot_if_due() still runs on the dirty-tracking + skip path and applies its own write/touch policy rather than being + bypassed entirely (see src/common/snapshot_policy.py — an unchanged + frame is touched, not re-encoded, once TOUCH_INTERVAL elapses).""" + 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.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) + first_mtime = os.path.getmtime(dm._snapshot_path) + + # Age the write/touch bookkeeping past TOUCH_INTERVAL so the next + # identical frame is due for a touch, then push it again: dirty + # tracking must skip the panel write, but the snapshot mirror must + # still get its mtime bumped so the health check doesn't go stale. + from src.common import snapshot_policy + stale_ts = time.time() - snapshot_policy.TOUCH_INTERVAL - 1.0 + dm._last_snapshot_ts = stale_ts + dm._last_snapshot_touch_ts = stale_ts + with _SwapSpy(dm.matrix) as spy: + dm.update_display() # identical frame -> panel push skipped + assert spy.count == 0 + assert os.path.getmtime(dm._snapshot_path) > first_mtime + + +class TestKillSwitch: + def test_dirty_tracking_can_be_disabled(self, dm): + dm._dirty_tracking_enabled = False + try: + dm.draw.rectangle([0, 0, 10, 10], fill=(1, 2, 3)) + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + dm.update_display() + dm.update_display() + assert spy.count == 3 # always-push, exactly the old behavior + finally: + dm._dirty_tracking_enabled = True + dm._last_pushed_digest = None + + def test_config_flag_wires_through(self): + from src.display_manager import DisplayManager + DisplayManager._instance = None + DisplayManager._initialized = False + try: + manager = DisplayManager({ + "display": { + "hardware": {"rows": 32, "cols": 64, "chain_length": 1, + "parallel": 1}, + "runtime": {"gpio_slowdown": 0}, + "dirty_tracking": False, + }, + }, suppress_test_pattern=True) + assert manager._dirty_tracking_enabled is False + finally: + DisplayManager._instance = None + DisplayManager._initialized = False + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-v"]))