diff --git a/docs/CONFIG_REFERENCE.md b/docs/CONFIG_REFERENCE.md index 6e2bf936..829430c5 100644 --- a/docs/CONFIG_REFERENCE.md +++ b/docs/CONFIG_REFERENCE.md @@ -127,6 +127,7 @@ Read by `src/vegas_mode/config.py` (`VegasScrollConfig.from_config`). See | `min_content_separation` | int, `24` | | `min_cut_gap` | int, `6` | | `continuous_scroll` | bool, `true` | +| `offscreen_prefetch` | bool, `true` — render every plugin's ticker content on the background thread, each on its own canvas. `false` restores handing canvas-bound plugins to the render thread, one pause at a time. Temporary; see [OFFSCREEN_RENDERING.md](OFFSCREEN_RENDERING.md) | | `smooth_scroll` | bool, `true` — move a whole number of pixels per panel refresh, locked to vsync. `scroll_speed` is snapped to the nearest speed the panel can show that way (at 95Hz: 95, 47.5, 31.7 px/s…), measured against the panel's real refresh rate once scrolling starts | | `sub_pixel_blend` | bool, `false` — the older smoothing: advance by elapsed time and blend neighbouring pixel columns. Looks anti-aliased in the web preview but shimmers on the panel and is not locked to the refresh. Overrides `smooth_scroll` when on | | `extend_threshold_screens` | float, `2.0` | diff --git a/src/display_manager.py b/src/display_manager.py index 2037ec50..cbcc6f60 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -133,6 +133,81 @@ class _LogicalMatrix: setattr(object.__getattribute__(self, "_matrix"), name, value) +class _OffscreenMatrix(_LogicalMatrix): + """``display_manager.matrix`` as a thread drawing off-screen sees it. + + Reports the surface's size, so plugins that lay out from ``matrix.width`` + follow it, and swallows every write that would reach the hardware. Nothing + drawn off-screen may touch the panel the render loop is driving. + """ + + __slots__ = () + + def SetImage(self, *args: Any, **kwargs: Any) -> None: + """Inert: off-screen drawing never reaches the panel.""" + + def SetPixel(self, *args: Any, **kwargs: Any) -> None: + """Inert: off-screen drawing never reaches the panel.""" + + def Clear(self) -> None: + """Inert: off-screen drawing never reaches the panel.""" + + def Fill(self, *args: Any, **kwargs: Any) -> None: + """Inert: off-screen drawing never reaches the panel.""" + + def SwapOnVSync(self, canvas: Any, *args: Any, **kwargs: Any) -> Any: + """Inert: hands the canvas straight back without waiting on the panel.""" + return canvas + + def __setattr__(self, name: str, value: Any) -> None: + """Inert: brightness and other writes stay off the real matrix.""" + + +class _OffscreenSurface: + """One thread's private canvas while it renders off-screen. + + See :meth:`DisplayManager.offscreen`. + """ + + __slots__ = ("draw", "image", "matrix") + + def __init__(self, width: int, height: int, real_matrix: Any) -> None: + self.image = Image.new('RGB', (width, height)) + self.draw = ImageDraw.Draw(self.image) + self.draw.fontmode = "1" # 1-bit text: the panel has no partial brightness, so AA only smears glyphs. + self.matrix = (_OffscreenMatrix(real_matrix, width, height) + if real_matrix is not None else None) + + +def _per_thread_canvas_attr(name: str) -> property: + """A DisplayManager attribute that resolves per thread. + + A thread inside :meth:`DisplayManager.offscreen` reads and writes its own + surface's ``name``; every other thread reads and writes the shared value, + exactly as when this was a plain attribute. Existing ``self.image = ...`` + assignments therefore keep working and become thread-correct as they are. + """ + shared = "_shared_" + name + + def fget(self: "DisplayManager") -> Any: + surface = self._current_surface() + if surface is not None: + return getattr(surface, name) + try: + return self.__dict__[shared] + except KeyError: + raise AttributeError(name) from None + + def fset(self: "DisplayManager", value: Any) -> None: + surface = self._current_surface() + if surface is not None: + setattr(surface, name, value) + else: + self.__dict__[shared] = value + + return property(fget, fset, doc=f"The plugin-facing ``{name}``, per thread.") + + # Moved to src/display_geometry.py so the web preview, Starlark magnify and # sync handshake compute the display size exactly as DisplayManager does # without importing rgbmatrix. Aliased here for existing callers. @@ -166,6 +241,11 @@ class DisplayManager: cls._instance = super(DisplayManager, cls).__new__(cls) return cls._instance + # The plugin-facing canvas. Per thread: see offscreen(). + image = _per_thread_canvas_attr("image") + draw = _per_thread_canvas_attr("draw") + matrix = _per_thread_canvas_attr("matrix") + def __init__(self, config: Dict[str, Any] = None, force_fallback: bool = False, suppress_test_pattern: bool = False): start_time = time.time() self.config = config or {} @@ -179,6 +259,9 @@ class DisplayManager: # suppress the render loop's own frame pushes for the duration, freezing # the panel exactly when the point was to avoid a freeze. self._capture_state = threading.local() + # Per-thread off-screen surface. While a thread is inside offscreen(), + # image, draw and matrix resolve to its own canvas; see offscreen(). + self._surface_state = threading.local() # Double-sided mode state (resolved in _setup_matrix). When disabled, # the logical image is blitted to the matrix unchanged. self._double_sided = None # dict {copies, axis, logical_width, logical_height} or None @@ -664,11 +747,67 @@ class DisplayManager: Entering this context prevents those writes without affecting the PIL image buffer, which the adapter reads to extract content. """ + # Restore rather than clear: capture_mode() inside offscreen() must not + # switch suppression off for the rest of the off-screen block. + was_active = self._capture_mode_active self._capture_mode_active = True try: yield finally: - self._capture_mode_active = False + self._capture_mode_active = was_active + + def _current_surface(self) -> Optional[_OffscreenSurface]: + """The calling thread's off-screen surface, or None.""" + state = self.__dict__.get('_surface_state') + return getattr(state, 'surface', None) if state is not None else None + + def _writes_suppressed(self) -> bool: + """True when the calling thread must not touch the panel or its pacing.""" + return self._capture_mode_active or self._current_surface() is not None + + @contextmanager + def offscreen(self, width: Optional[int] = None, height: Optional[int] = None): + """Give the calling thread its own canvas to draw on. + + Inside the block, for the calling thread only, ``image``, ``draw`` and + ``matrix`` (and so ``width``/``height``) are a fresh black canvas of the + requested size, and nothing reaches the hardware: ``update_display()`` + and the hardware half of ``clear()`` are skipped, and + ``set_scrolling_state()``/``set_frame_hold()`` cannot re-pace the live + scroll. Every other thread, the render loop above all, keeps seeing the + real canvas. + + That is what lets Vegas mode render a plugin on its background prefetch + thread. The shared canvas used to be the only one, so any plugin that + drew on it (display capture, scroll-content generation, narrowed + rendering) had to be fetched on the render thread, stalling the scroll + for 40-600ms each. See docs/OFFSCREEN_RENDERING.md. + + Blocks nest; each restores the one outside it, also on an exception. + + Args: + width: Width of the surface, clamped to the size this thread sees + now. Defaults to that size. + height: Height, likewise. + + Yields: + The surface. ``surface.image`` is what the plugin drew. + """ + state = self.__dict__.get('_surface_state') + if state is None: + state = self._surface_state = threading.local() + + current_w, current_h = self.width, self.height + target_w = max(1, min(int(width), current_w)) if width else current_w + target_h = max(1, min(int(height), current_h)) if height else current_h + + surface = _OffscreenSurface(target_w, target_h, self.matrix) + previous = getattr(state, 'surface', None) + state.surface = surface + try: + yield surface + finally: + state.surface = previous @contextmanager def render_size(self, width: int, height: Optional[int] = None): @@ -688,18 +827,14 @@ class DisplayManager: indirection that double-sided mode relies on, so plugins see a consistent size from every accessor. - Only meaningful inside :meth:`capture_mode` — this swaps the shared - image buffer, so the render loop must not be writing to it concurrently. + Built on :meth:`offscreen`, so the narrower canvas belongs to the + calling thread alone; the render loop keeps drawing on the real one. Args: width: Logical width to report, clamped to at least 1 and to the real panel width (a larger canvas would overflow the hardware). height: Logical height, defaulting to the current height. """ - real_matrix = self.matrix - prev_image = getattr(self, 'image', None) - prev_draw = getattr(self, 'draw', None) - current_w = self.width current_h = self.height target_w = max(1, min(int(width), current_w)) @@ -710,21 +845,8 @@ class DisplayManager: yield return - try: - if real_matrix is not None: - self.matrix = _LogicalMatrix(real_matrix, target_w, target_h) - # With no hardware, the width/height properties fall through to - # self.image, so swapping the buffer below is enough on its own. - self.image = Image.new('RGB', (target_w, target_h)) - self.draw = ImageDraw.Draw(self.image) - self.draw.fontmode = "1" # 1-bit text: the panel has no partial brightness, so AA only smears glyphs. + with self.offscreen(target_w, target_h): yield - finally: - self.matrix = real_matrix - if prev_image is not None: - self.image = prev_image - if prev_draw is not None: - self.draw = prev_draw def _composite_double_sided(self): """Tile the logical screen across the full physical chain. @@ -769,6 +891,12 @@ class DisplayManager: need to know about it. """ try: + if self._writes_suppressed(): + # This thread is drawing off-screen. Checked before the lock, + # so it never contends with the render loop's swap, and before + # the fallback branch, so captured content never reaches the + # web preview either. + return with self._update_lock: if self.matrix is None: # Fallback mode - no actual hardware to update @@ -777,9 +905,6 @@ class DisplayManager: self._write_snapshot_if_due() return - if self._capture_mode_active: - return # Skip hardware write — content is being captured off-screen - digest = None frame_checksum = None if self._dirty_tracking_enabled: @@ -860,7 +985,7 @@ class DisplayManager: self.draw = ImageDraw.Draw(self.image) self.draw.fontmode = "1" # 1-bit text: the panel has no partial brightness, so AA only smears glyphs. - if not self._capture_mode_active: + if not self._writes_suppressed(): # 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. @@ -1462,6 +1587,8 @@ class DisplayManager: Reset to 1 whenever scrolling stops, so one plugin's pacing cannot leak into the next thing on screen. """ + if self._writes_suppressed(): + return # a plugin drawing off-screen cannot re-pace the live scroll try: value = int(refreshes) except (TypeError, ValueError): @@ -1490,6 +1617,10 @@ class DisplayManager: the lifetime exactly the scroll, and the default of 1 means any caller that does not care gets a new frame every refresh. """ + if self._writes_suppressed(): + # A plugin captured for Vegas calls this from its own display(); + # it must not change the live scroll's state or frame hold. + return current_time = time.time() self._scrolling_state['is_scrolling'] = is_scrolling if is_scrolling: diff --git a/src/plugin_system/testing/visual_display_manager.py b/src/plugin_system/testing/visual_display_manager.py index c5c1f8b4..aed4d568 100644 --- a/src/plugin_system/testing/visual_display_manager.py +++ b/src/plugin_system/testing/visual_display_manager.py @@ -18,7 +18,7 @@ _draw_bdf_text, get_font_height, get_text_width, draw_text, draw_text_with_icons, draw_weather_icon (and the _draw_sun/_draw_cloud/ _draw_rain/_draw_snow/_draw_storm family), format_date_with_ordinal, capture_mode, set_scrolling_state, is_currently_scrolling, -process_deferred_updates, update_display, render_size. A behavior +process_deferred_updates, update_display, render_size, offscreen. A behavior change to any of those in DisplayManager must be mirrored here, or plugin visual tests will pass against stale behavior. """ @@ -237,11 +237,38 @@ class VisualTestDisplayManager: wraps every off-screen content fetch in this context, so the harness must provide it for that code path to be exercisable in tests. """ + was_active = self._capture_mode_active self._capture_mode_active = True try: yield finally: - self._capture_mode_active = False + self._capture_mode_active = was_active + + @contextmanager + def offscreen(self, width: Optional[int] = None, height: Optional[int] = None): + """ + Interface parity with DisplayManager.offscreen(). + + Vegas mode's PluginAdapter draws every plugin on a canvas of its own. + The real display manager keeps that canvas per thread; the harness is + single-threaded, so it swaps a fresh canvas in and restores the old one, + which is all a test can observe. + """ + prev = (self.image, self.draw, self._width, self._height, + self.matrix, self._capture_mode_active) + target_w = max(1, min(int(width), self._width)) if width else self._width + target_h = max(1, min(int(height), self._height)) if height else self._height + try: + self._width, self._height = target_w, target_h + self.matrix = _MatrixProxy(target_w, target_h) + self.image = Image.new('RGB', (target_w, target_h), (0, 0, 0)) + self.draw = ImageDraw.Draw(self.image) + self.draw.fontmode = "1" # Match production: 1-bit text, so goldens show what the panel shows. + self._capture_mode_active = True + yield self + finally: + (self.image, self.draw, self._width, self._height, + self.matrix, self._capture_mode_active) = prev def draw_text(self, text: str, x: Optional[int] = None, y: Optional[int] = None, color: Tuple[int, int, int] = (255, 255, 255), small_font: bool = False, diff --git a/src/vegas_mode/config.py b/src/vegas_mode/config.py index 3137e4da..5f0a28fa 100644 --- a/src/vegas_mode/config.py +++ b/src/vegas_mode/config.py @@ -74,6 +74,14 @@ class VegasModeConfig: # precedence over smooth_scroll's whole-pixel pacing when on. sub_pixel_blend: bool = False + # Render every plugin's ticker content on the background prefetch thread, + # each on a canvas of its own (DisplayManager.offscreen), instead of + # handing plugins that draw on the display canvas to the render thread one + # at a time. Each of those cost the scroll a 40-600ms pause. False restores + # that path; it is kept for one release in case a plugin misbehaves when + # drawn off the render thread. See docs/OFFSCREEN_RENDERING.md. + offscreen_prefetch: bool = True + # Keep one continuous strip, extending it with the next group of plugins as # the scroll approaches the end, instead of composing a fresh strip and # swapping it in. A swap stops the motion, substitutes every pixel at once @@ -203,6 +211,7 @@ class VegasModeConfig: smooth_scroll=vegas_config.get('smooth_scroll', True), sub_pixel_blend=bool(vegas_config.get('sub_pixel_blend', False)), continuous_scroll=vegas_config.get('continuous_scroll', True), + offscreen_prefetch=bool(vegas_config.get('offscreen_prefetch', True)), extend_threshold_screens=float( vegas_config.get('extend_threshold_screens', 2.0)), auto_trim=vegas_config.get('auto_trim', True), @@ -244,6 +253,7 @@ class VegasModeConfig: 'smooth_scroll': self.smooth_scroll, 'sub_pixel_blend': self.sub_pixel_blend, 'continuous_scroll': self.continuous_scroll, + 'offscreen_prefetch': self.offscreen_prefetch, 'extend_threshold_screens': self.extend_threshold_screens, 'auto_trim': self.auto_trim, 'trim_threshold': self.trim_threshold, diff --git a/src/vegas_mode/coordinator.py b/src/vegas_mode/coordinator.py index a7916cc8..5551aea4 100644 --- a/src/vegas_mode/coordinator.py +++ b/src/vegas_mode/coordinator.py @@ -101,7 +101,8 @@ class VegasModeCoordinator: self.plugin_manager = plugin_manager # Initialize components - self.plugin_adapter = PluginAdapter(display_manager, self.vegas_config) + self.plugin_adapter = PluginAdapter( + display_manager, self.vegas_config, plugin_manager=plugin_manager) self.stream_manager = StreamManager( self.vegas_config, plugin_manager, diff --git a/src/vegas_mode/plugin_adapter.py b/src/vegas_mode/plugin_adapter.py index 01ef8cca..e283ecaf 100644 --- a/src/vegas_mode/plugin_adapter.py +++ b/src/vegas_mode/plugin_adapter.py @@ -8,7 +8,7 @@ implement get_vegas_content() and fallback capture of display() output. import logging import threading import time -from contextlib import nullcontext +from contextlib import contextmanager, nullcontext from typing import Optional, List, Any, Tuple, Union, TYPE_CHECKING from PIL import Image @@ -33,7 +33,13 @@ class PluginAdapter: 2. Fallback: Capture display_manager.image after calling plugin.display() """ - def __init__(self, display_manager: Any, config: Optional[Any] = None): + #: How long a background fetch waits for a plugin's update() to finish + #: before skipping the plugin this round. Off the render thread waiting + #: costs nothing visible; it only delays that one plugin's content. + PLUGIN_LOCK_TIMEOUT = 2.0 + + def __init__(self, display_manager: Any, config: Optional[Any] = None, + plugin_manager: Optional[Any] = None): """ Initialize the plugin adapter. @@ -42,8 +48,13 @@ class PluginAdapter: config: VegasModeConfig controlling trim behaviour. When omitted, trimming runs with the dataclass defaults, so existing callers and tests keep working unchanged. + plugin_manager: Source of the per-plugin lock that keeps a + background fetch from running a plugin's display() while its + update() is mid-flight. Optional: without it, fetches take no + lock, as they always did. """ self.display_manager = display_manager + self.plugin_manager = plugin_manager if config is None: from src.vegas_mode.config import VegasModeConfig config = VegasModeConfig() @@ -98,13 +109,14 @@ class PluginAdapter: Args: plugin: Plugin instance to get content from plugin_id: Plugin identifier for logging - offscreen_only: Skip every path that touches the shared display - canvas, for callers running off the render thread. The canvas - and the matrix proxy are process-wide mutable state, so - narrowing or capturing through them from another thread would - corrupt the frame the render loop is pushing. Returns None when - the plugin can only be served that way, leaving the caller to - fetch it on the render thread. + offscreen_only: The caller is off the render thread. Every content + path draws on a canvas of its own (DisplayManager.offscreen), + so all of them are safe there; the fetch also takes the + plugin's lock, waiting up to PLUGIN_LOCK_TIMEOUT for a running + update() to finish. With ``offscreen_prefetch`` switched off, + the old behaviour applies instead: paths that need a canvas + return None, leaving the caller to fetch the plugin on the + render thread. Returns: List of PIL Images representing plugin content, or None if no content @@ -124,11 +136,78 @@ class PluginAdapter: ) return cached + # The old contract, kept behind the switch: background callers may + # not draw, so anything needing a canvas is left for the render thread. + restricted = offscreen_only and not getattr( + self.config, 'offscreen_prefetch', True) + if not offscreen_only or restricted: + return self._fetch_content(plugin, plugin_id, restricted) + + with self._plugin_lock(plugin_id) as acquired: + if not acquired: + logger.warning( + "[%s] update() still running after %.0fs; skipping it this " + "round", plugin_id, self.PLUGIN_LOCK_TIMEOUT + ) + return None + return self._fetch_content(plugin, plugin_id, restricted=False) + + @contextmanager + def _plugin_lock(self, plugin_id: str): + """Hold the plugin's update/display lock, waiting a bounded time. + + Yields whether it was acquired. Yields True, holding nothing, when + there is no plugin manager to ask -- the behaviour before the lock was + taken here at all. + """ + get_lock = getattr(self.plugin_manager, 'get_plugin_lock', None) + if get_lock is None: + yield True + return + lock = get_lock(plugin_id) + acquired = lock.acquire(timeout=self.PLUGIN_LOCK_TIMEOUT) + try: + yield acquired + finally: + if acquired: + lock.release() + + @contextmanager + def _isolated_canvas(self, width: Optional[int] = None): + """A canvas for the plugin to draw on that nothing else sees. + + DisplayManager.offscreen() gives the calling thread its own canvas, so + this is safe on any thread and leaves the shared canvas untouched. + Older display managers and test doubles without it get the previous + behaviour: capture on the shared canvas, narrowed with render_size, + then restore it -- which is only safe on the render thread. + """ + offscreen = getattr(self.display_manager, 'offscreen', None) + if offscreen is not None: + with offscreen(width): + yield + return + + original_image = self.display_manager.image.copy() + try: + with self._capture(), self._render_at(width or self.display_width): + yield + finally: + self.display_manager.image = original_image + + def _fetch_content( + self, plugin: 'BasePlugin', plugin_id: str, restricted: bool + ) -> Optional[List[Image.Image]]: + """Every content path in order: native, scroll helper, display capture. + + ``restricted`` is the pre-offscreen contract for background callers: + skip every path that needs a canvas and return None instead. + """ # Try native Vegas content method first has_native = hasattr(plugin, 'get_vegas_content') logger.debug("[%s] Has get_vegas_content: %s", plugin_id, has_native) if has_native: - content = self._get_native_content(plugin, plugin_id, offscreen_only) + content = self._get_native_content(plugin, plugin_id, restricted) if content: total_width = sum(img.width for img in content) logger.debug( @@ -141,7 +220,7 @@ class PluginAdapter: # Try to get scroll_helper's cached image (for scrolling plugins like stocks/odds) has_scroll_helper = hasattr(plugin, 'scroll_helper') logger.debug("[%s] Has scroll_helper: %s", plugin_id, has_scroll_helper) - content = self._get_scroll_helper_content(plugin, plugin_id, offscreen_only) + content = self._get_scroll_helper_content(plugin, plugin_id, restricted) if content: total_width = sum(img.width for img in content) logger.debug( @@ -152,8 +231,8 @@ class PluginAdapter: if has_scroll_helper: logger.debug("[%s] ScrollHelper content returned None", plugin_id) - if offscreen_only: - # Display capture needs the shared canvas; leave it to the caller. + if restricted: + # Display capture needs a canvas; leave it to the caller. logger.debug( "[%s] Needs display capture, deferring to the render thread", plugin_id @@ -685,7 +764,7 @@ class PluginAdapter: return img.crop((start, 0, end, img.height)) def _get_native_content( - self, plugin: 'BasePlugin', plugin_id: str, offscreen_only: bool = False + self, plugin: 'BasePlugin', plugin_id: str, restricted: bool = False ) -> Optional[List[Image.Image]]: """ Get content via plugin's native get_vegas_content() method. @@ -714,22 +793,21 @@ class PluginAdapter: plugin._vegas_render_width = render_width try: - # capture_mode unconditionally, even at full width. Building - # Vegas content is an off-screen operation, but a plugin is free - # to call update_display() while doing it — and outside - # capture_mode that write lands on the hardware, flashing the - # panel mid-scroll. The narrowing context is separate because it - # is a no-op at full width. - if offscreen_only: - # _render_at swaps the shared canvas, so it is unsafe here. - # _vegas_render_width is set regardless: a plugin reading - # get_vegas_render_width() still gets its narrow size, and - # one that only reads matrix.width renders full width and is - # trimmed instead. + # On a canvas of its own even at full width. Building Vegas + # content is an off-screen operation, but a plugin is free to + # call update_display() while doing it, and on the shared canvas + # that write would land on the hardware, flashing the panel + # mid-scroll. + if restricted: + # Restricted (offscreen_prefetch off): no canvas of our own, + # so no narrowing. _vegas_render_width is set regardless: a + # plugin reading get_vegas_render_width() still gets its + # narrow size, and one that only reads matrix.width renders + # full width and is trimmed instead. with self._capture(): result = plugin.get_vegas_content() else: - with self._capture(), self._render_at(render_width): + with self._isolated_canvas(render_width): result = plugin.get_vegas_content() finally: plugin._vegas_render_width = None @@ -810,7 +888,7 @@ class PluginAdapter: return None def _get_scroll_helper_content( - self, plugin: 'BasePlugin', plugin_id: str, offscreen_only: bool = False + self, plugin: 'BasePlugin', plugin_id: str, restricted: bool = False ) -> Optional[List[Image.Image]]: """ Get content from plugin's scroll_helper if available. @@ -844,7 +922,7 @@ class PluginAdapter: "[%s] scroll_helper.cached_image is None, triggering content generation", plugin_id ) - if offscreen_only: + if restricted: # Generating it calls display(), which needs the canvas. logger.debug( "[%s] scroll_helper cache empty; deferring generation " @@ -994,12 +1072,8 @@ class PluginAdapter: Returns: The generated cached_image or None """ - original_image = None try: - # Save display state to restore after - original_image = self.display_manager.image.copy() - - with self._capture(): + with self._isolated_canvas(): # Method 1: Try _create_scrolling_display (stocks pattern) if hasattr(plugin, '_create_scrolling_display'): logger.debug( @@ -1055,11 +1129,6 @@ class PluginAdapter: logger.exception("[%s] Error triggering scroll content", plugin_id) return None - finally: - # Restore original display state - if original_image is not None: - self.display_manager.image = original_image - def _capture_display_content( self, plugin: 'BasePlugin', plugin_id: str ) -> Optional[List[Image.Image]]: @@ -1073,12 +1142,7 @@ class PluginAdapter: Returns: List with single captured image, or None """ - original_image = None try: - # Save current display state - original_image = self.display_manager.image.copy() - logger.debug("[%s] Fallback: saved original display state", plugin_id) - # Ensure plugin has fresh data before capturing has_update_data = hasattr(plugin, 'update_data') logger.debug("[%s] Fallback: has update_data=%s", plugin_id, has_update_data) @@ -1089,12 +1153,12 @@ class PluginAdapter: except (AttributeError, RuntimeError, OSError): logger.exception("[%s] Fallback: update_data() failed", plugin_id) - # Clear and call plugin display — use capture_mode to suppress hardware writes - # that plugins may trigger internally via update_display(). + # Clear and call plugin display on a canvas of its own: nothing it + # draws, and no update_display() it calls, reaches the panel. # - # render_size narrows the canvas the plugin lays out against, so a - # plugin that spreads across the whole panel produces a compact - # arrangement rather than one that has to be cropped afterwards. + # The canvas is render_width wide, so a plugin that spreads across + # the whole panel produces a compact arrangement rather than one + # that has to be cropped afterwards. render_width = self.resolve_render_width(plugin, plugin_id) if render_width != self.display_width: logger.debug( @@ -1102,7 +1166,7 @@ class PluginAdapter: plugin_id, render_width, self.display_width ) - with self._capture(), self._render_at(render_width): + with self._isolated_canvas(render_width): self.display_manager.clear() logger.debug("[%s] Fallback: display cleared, calling display()", plugin_id) @@ -1136,7 +1200,7 @@ class PluginAdapter: plugin_id ) # Try once more with force_clear=True - with self._capture(), self._render_at(render_width): + with self._isolated_canvas(render_width): self.display_manager.clear() plugin.display(force_clear=True) captured = self.display_manager.image.copy() @@ -1173,12 +1237,6 @@ class PluginAdapter: ) return None - finally: - # Always restore original image to prevent display corruption - if original_image is not None: - self.display_manager.image = original_image - logger.debug("[%s] Fallback: restored original display state", plugin_id) - def _is_blank_image( self, img: Image.Image, return_ratio: bool = False ) -> Union[bool, Tuple[bool, float]]: diff --git a/src/vegas_mode/render_pipeline.py b/src/vegas_mode/render_pipeline.py index 74cf82b1..65f5ab44 100644 --- a/src/vegas_mode/render_pipeline.py +++ b/src/vegas_mode/render_pipeline.py @@ -520,9 +520,12 @@ class RenderPipeline: grouped = [(pid, imgs) for pid, imgs in grouped if imgs] if not grouped: - # Everything in this group is queued; the queue will extend the - # strip as it drains, so this is not a failure. - logger.info("Whole group deferred; strip will extend as it drains") + if deferred: + # Everything in this group is queued; the queue will extend + # the strip as it drains, so this is not a failure. + logger.info("Whole group deferred; strip will extend as it drains") + else: + logger.info("Nothing to show in this group; fetching the next") self.start_prefetch() return bool(deferred) diff --git a/src/vegas_mode/stream_manager.py b/src/vegas_mode/stream_manager.py index cb7cc2bc..c490e3dd 100644 --- a/src/vegas_mode/stream_manager.py +++ b/src/vegas_mode/stream_manager.py @@ -763,6 +763,10 @@ class StreamManager: Ordered list of (plugin_id, images). ``images`` is None when the plugin could not be served under ``offscreen_only``, so the caller can fetch just those on the render thread while keeping the order. + That only happens with ``offscreen_prefetch`` switched off: every + content path now draws on a canvas of its own, so a background + fetch that comes back empty had nothing to show, and ``images`` + is an empty list rather than a request for the render thread. """ if count is None: count = self.config.plugins_per_cycle @@ -780,6 +784,9 @@ class StreamManager: plugins = getattr(self.plugin_manager, 'plugins', {}) group: List[Tuple[str, Optional[List[Image.Image]]]] = [] + # Only the old contract hands anything back to the render thread. + defer_empty = offscreen_only and not getattr( + self.config, 'offscreen_prefetch', True) for plugin_id in ids: plugin = plugins.get(plugin_id) @@ -794,7 +801,10 @@ class StreamManager: continue if images: self.stats['segments_fetched'] += 1 - group.append((plugin_id, images if images else None)) + if images: + group.append((plugin_id, images)) + else: + group.append((plugin_id, None if defer_empty else [])) return group diff --git a/test/test_offscreen_rendering.py b/test/test_offscreen_rendering.py new file mode 100644 index 00000000..866b6121 --- /dev/null +++ b/test/test_offscreen_rendering.py @@ -0,0 +1,360 @@ +"""Per-thread off-screen rendering (DisplayManager.offscreen) and its Vegas use. + +The display canvas used to be one shared object, so any plugin that drew on it +could only be rendered on the render thread, stalling the scroll for 40-600ms +each. offscreen() gives the calling thread a canvas of its own. These tests pin +the property that makes that safe: another thread's drawing never reaches what +the render loop sees, presents or paces by. + +Runs against RGBMatrixEmulator, exercising the real DisplayManager. +""" + +import os +import sys +import threading +from types import SimpleNamespace + +os.environ["EMULATOR"] = "true" + +import pytest +from PIL import Image + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from src.vegas_mode.config import VegasModeConfig # noqa: E402 +from src.vegas_mode.plugin_adapter import PluginAdapter # noqa: E402 +from src.vegas_mode.stream_manager import StreamManager # noqa: E402 + +WIDTH, HEIGHT = 128, 32 +YELLOW = (255, 255, 0) + + +@pytest.fixture(scope="module") +def dm(tmp_path_factory): + 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) + manager._snapshot_path = str( + tmp_path_factory.mktemp("offscreen") / "led_matrix_preview.png") + if manager.matrix is None: + pytest.fail("DisplayManager fell back to matrix=None; see the " + "'Failed to initialize RGB Matrix' log line above.") + yield manager + DisplayManager._instance = None + DisplayManager._initialized = False + + +@pytest.fixture +def fresh(dm): + """A known shared canvas, and scroll state reset around each test.""" + dm.image = Image.new('RGB', (WIDTH, HEIGHT)) + from PIL import ImageDraw + dm.draw = ImageDraw.Draw(dm.image) + dm.set_scrolling_state(False) + yield dm + dm.set_scrolling_state(False) + + +class _Spy: + """Counts calls to one method of one object, and still calls it.""" + + def __init__(self, obj, name): + self.obj, self.name, self.count = obj, name, 0 + self._orig = getattr(obj, name) + + def __enter__(self): + def counting(*args, **kwargs): + self.count += 1 + return self._orig(*args, **kwargs) + setattr(self.obj, self.name, counting) + return self + + def __exit__(self, *exc): + setattr(self.obj, self.name, self._orig) + + +def _in_thread(fn): + """Run fn on another thread and return its result, re-raising its error.""" + box = {} + + def run(): + try: + box["value"] = fn() + except BaseException as exc: # surfaced below + box["error"] = exc + + thread = threading.Thread(target=run) + thread.start() + thread.join(10) + assert not thread.is_alive(), "worker hung" + if "error" in box: + raise box["error"] + return box.get("value") + + +class TestIsolation: + def test_another_threads_drawing_is_invisible_here(self, fresh): + dm = fresh + shared = dm.image + inside, release = threading.Event(), threading.Event() + seen = {} + + def worker(): + with dm.offscreen(40) as surface: + dm.draw.rectangle([0, 0, 39, 31], fill=YELLOW) + seen.update(width=dm.width, matrix_width=dm.matrix.width, + own_image=dm.image is surface.image) + inside.set() + release.wait(5) + seen["restored"] = dm.image is shared + + thread = threading.Thread(target=worker) + thread.start() + try: + assert inside.wait(5) + # While the worker is mid-draw, this thread sees the real canvas. + assert dm.image is shared + assert (dm.width, dm.matrix.width) == (WIDTH, WIDTH) + assert shared.getpixel((0, 0)) == (0, 0, 0) + finally: + release.set() + thread.join(5) + assert seen == {"width": 40, "matrix_width": 40, "own_image": True, + "restored": True} + + def test_the_render_loop_keeps_presenting_while_another_thread_draws(self, fresh): + dm = fresh + inside, release = threading.Event(), threading.Event() + + def worker(): + with dm.offscreen(): + dm.draw.rectangle([0, 0, 10, 10], fill=YELLOW) + dm.update_display() # must not reach the panel + inside.set() + release.wait(5) + + with _Spy(dm.matrix, "SwapOnVSync") as swaps: + thread = threading.Thread(target=worker) + thread.start() + try: + assert inside.wait(5) + dm.draw.point((5, 5), fill=(0, 0, 255)) + dm.update_display() # the render thread's own frame + finally: + release.set() + thread.join(5) + assert swaps.count == 1 + + def test_assignments_go_to_the_callers_canvas(self, fresh): + dm = fresh + shared = dm.image + replacement = Image.new('RGB', (WIDTH, HEIGHT), (1, 2, 3)) + + def worker(): + with dm.offscreen(): + dm.image = replacement # e.g. clear() inside display() + return dm.image is replacement + + assert _in_thread(worker) is True + assert dm.image is shared + + def test_render_size_narrows_only_the_calling_thread(self, fresh): + dm = fresh + inside, release = threading.Event(), threading.Event() + seen = {} + + def worker(): + with dm.render_size(32): + seen["width"] = dm.width + inside.set() + release.wait(5) + + thread = threading.Thread(target=worker) + thread.start() + try: + assert inside.wait(5) + assert dm.width == WIDTH + finally: + release.set() + thread.join(5) + assert seen["width"] == 32 + + +class TestNothingReachesThePanel: + def test_update_display_clear_and_pacing_are_inert_inside(self, fresh): + dm = fresh + real_matrix = dm.matrix + hold_before = dm._frame_hold + with _Spy(real_matrix, "SwapOnVSync") as swaps, \ + _Spy(dm.offscreen_canvas, "Clear") as clears: + with dm.offscreen() as surface: + dm.draw.rectangle([0, 0, 5, 5], fill=YELLOW) + dm.update_display() + drawn = dm.image + dm.clear() + # clear() replaced the surface's image, not the shared one. + assert dm.image is surface.image and dm.image is not drawn + assert dm.image.getpixel((0, 0)) == (0, 0, 0) + dm.set_scrolling_state(True, 3) + dm.set_frame_hold(4) + dm.matrix.SwapOnVSync(object()) # inert through the proxy too + assert swaps.count == 0 + assert clears.count == 0 + assert dm._frame_hold == hold_before + assert dm._scrolling_state['is_scrolling'] is False + + def test_capture_mode_inside_offscreen_does_not_end_suppression(self, fresh): + dm = fresh + real_matrix = dm.matrix + with _Spy(real_matrix, "SwapOnVSync") as swaps: + with dm.offscreen(): + with dm.capture_mode(): + pass + dm.draw.point((1, 1), fill=YELLOW) + dm.update_display() + assert swaps.count == 0 + + def test_nesting_and_exceptions_restore_state(self, fresh): + dm = fresh + shared = dm.image + with pytest.raises(RuntimeError): + with dm.offscreen(100): + with dm.offscreen(50): + assert dm.width == 50 + raise RuntimeError("plugin failed mid-draw") + assert dm.image is shared + assert dm.width == WIDTH + assert not dm._writes_suppressed() + + with dm.offscreen(100) as outer: + with dm.offscreen(50): + pass + assert dm.image is outer.image + assert dm.width == 100 + + +# --- Vegas: the adapter draws plugins off the render thread ------------------ + +class CapturePlugin: + """A plugin with neither get_vegas_content nor a scroll helper: captured.""" + + def __init__(self, display_manager): + self.display_manager = display_manager + self.calls = 0 + + def display(self, force_clear=False): + self.calls += 1 + self.display_manager.draw.rectangle([0, 0, 30, 20], fill=YELLOW) + self.display_manager.update_display() + + +class ScrollingPlugin: + """A ticker whose scroll image only exists once display() has run.""" + + def __init__(self, display_manager): + from src.common.scroll_helper import ScrollHelper + self.display_manager = display_manager + self.scroll_helper = ScrollHelper(WIDTH, HEIGHT) + + def display(self, force_clear=False): + item = Image.new('RGB', (300, HEIGHT), YELLOW) + self.scroll_helper.create_scrolling_image([item], item_gap=0, element_gap=0) + self.display_manager.set_scrolling_state(True, 5) + self.display_manager.update_display() + + +def _adapter(dm, **config): + return PluginAdapter(dm, VegasModeConfig(**config)) + + +def _has_yellow(images): + return any(YELLOW in {img.getpixel((x, y)) for x in range(min(img.width, 40)) + for y in range(img.height)} for img in images) + + +class TestAdapterOffTheRenderThread: + def test_display_capture_runs_on_a_background_thread(self, fresh): + dm = fresh + shared = dm.image + before = shared.tobytes() + plugin = CapturePlugin(dm) + adapter = _adapter(dm) + + with _Spy(dm.matrix, "SwapOnVSync") as swaps: + images = _in_thread( + lambda: adapter.get_content(plugin, "capture-bg", offscreen_only=True)) + + assert images and _has_yellow(images) + assert dm.image is shared and shared.tobytes() == before + assert swaps.count == 0 + + def test_scroll_content_is_generated_on_a_background_thread(self, fresh): + dm = fresh + plugin = ScrollingPlugin(dm) + adapter = _adapter(dm) + hold_before = dm._frame_hold + + images = _in_thread( + lambda: adapter.get_content(plugin, "scroll-bg", offscreen_only=True)) + + assert images and _has_yellow(images) + # The plugin's own set_scrolling_state(True, 5) did not re-pace Vegas. + assert dm._frame_hold == hold_before + assert dm._scrolling_state['is_scrolling'] is False + + def test_with_the_switch_off_background_capture_is_left_for_the_render_thread(self, fresh): + dm = fresh + plugin = CapturePlugin(dm) + adapter = _adapter(dm, offscreen_prefetch=False) + + images = _in_thread( + lambda: adapter.get_content(plugin, "capture-legacy", offscreen_only=True)) + + assert images is None + assert plugin.calls == 0 + + +class TestPluginLock: + def test_a_background_fetch_waits_for_update_then_skips(self, fresh): + dm = fresh + lock = threading.Lock() + plugin = CapturePlugin(dm) + adapter = PluginAdapter(dm, VegasModeConfig(), + plugin_manager=SimpleNamespace( + get_plugin_lock=lambda plugin_id: lock)) + adapter.PLUGIN_LOCK_TIMEOUT = 0.05 + + with lock: # update() in progress + skipped = _in_thread( + lambda: adapter.get_content(plugin, "locked", offscreen_only=True)) + assert skipped is None + assert plugin.calls == 0 + + served = _in_thread( + lambda: adapter.get_content(plugin, "locked", offscreen_only=True)) + assert served and plugin.calls == 1 + assert not lock.locked() + + +class TestStreamDoesNotDeferEmptyResults: + def _group(self, offscreen_prefetch): + config = VegasModeConfig(offscreen_prefetch=offscreen_prefetch) + adapter = SimpleNamespace(get_content=lambda *a, **k: None) + stream = StreamManager(config, SimpleNamespace(plugins={"p": object()}), + adapter) + stream.refresh = lambda: None + stream._ordered_plugins = ["p"] + return stream.take_next_group(count=1, offscreen_only=True) + + def test_nothing_to_show_is_not_sent_to_the_render_thread(self): + assert self._group(offscreen_prefetch=True) == [("p", [])] + + def test_the_old_contract_still_defers_with_the_switch_off(self): + assert self._group(offscreen_prefetch=False) == [("p", None)]