fix(vegas): pause for STATIC plugins where their turn falls in the strip (#651)

The static trigger peeked at the front of StreamManager's segment buffer,
which continuous scrolling (the default) never advances -- it extends the
strip with take_next_group() -- so the same first segment was examined on
every frame. A STATIC plugin paused the scroll only if it was first, once,
at startup; otherwise it scrolled past as ordinary content. Swap mode had
the same problem for any STATIC plugin not first in its cycle.

The render pipeline now records a marker (strip column, plugin id) for
each STATIC plugin where the strip is built -- composition and every
extension -- shifts the markers when the scrolled prefix is trimmed, and
clears them on reset. The coordinator pauses when the scroll reaches the
next marker: a tuple comparison per frame instead of a lock, a plugin
lookup and a get_vegas_display_mode() call. take_next_group() no longer
renders STATIC plugins' content. The pause calls display() under the
plugin lock and is timed with the monotonic clock.

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

src/vegas_mode/coordinator.py

@@ -23,7 +23,6 @@ from src.vegas_mode.config import VegasModeConfig
 from src.vegas_mode.plugin_adapter import PluginAdapter
 from src.vegas_mode.stream_manager import StreamManager
 from src.vegas_mode.render_pipeline import RenderPipeline
-from src.plugin_system.base_plugin import VegasDisplayMode
 
 if TYPE_CHECKING:
     from src.plugin_system.plugin_manager import PluginManager
@@ -78,6 +77,10 @@ class VegasModeCoordinator:
     - Provide status and control interface
     """
 
+    #: How long a STATIC pause waits for the plugin's lock (held while its
+    #: update() runs) before skipping that turn.
+    STATIC_LOCK_TIMEOUT = 1.0
+
     # Class-level so coordinators built without __init__ (tests) have it.
     _last_live_check: float = float('-inf')
 
@@ -542,8 +545,7 @@ class VegasModeCoordinator:
                 if not self._handle_static_pause(static_plugin):
                     # Static pause was interrupted
                     return False
-                # After static pause, skip this segment and continue
-                self.stream_manager.get_next_segment()  # Consume the segment
+                # The trigger consumed the plugin's marker; carry on scrolling.
                 continue
 
             # Run frame
@@ -822,32 +824,30 @@ class VegasModeCoordinator:
         """
         Check if a STATIC mode plugin should take over display.
 
-        Called during iteration to detect when scroll should pause
-        for a static plugin display.
+        Called every frame. The render pipeline marks where each STATIC
+        plugin's turn falls in the strip, and this reports the one the scroll
+        has just reached.
+
+        This used to peek at the front of the stream manager's segment
+        buffer, which continuous scrolling (the default) never advances: it
+        extends the strip with take_next_group() instead. The same first
+        segment was examined on every frame, so a STATIC plugin paused the
+        scroll only if it happened to be first, once, at startup -- and
+        otherwise just scrolled past as ordinary content. Swap mode fared no
+        better: nothing advanced the buffer mid-cycle either.
 
         Returns:
             Plugin instance if static pause should begin, None otherwise
         """
-        # Get the next plugin that would be displayed
-        next_segment = self.stream_manager.peek_next_segment()
-        if not next_segment:
+        plugin_id = self.render_pipeline.next_static_trigger()
+        if not plugin_id:
             return None
-
-        plugin_id = next_segment.plugin_id
         plugin = self.plugin_manager.get_plugin(plugin_id)
-
         if not plugin:
+            logger.debug("[%s] STATIC turn reached, but the plugin is no longer loaded",
+                         plugin_id)
             return None
-
-        # Check if this plugin is configured for STATIC mode
-        try:
-            display_mode = plugin.get_vegas_display_mode()
-            if display_mode == VegasDisplayMode.STATIC:
-                return plugin
-        except (AttributeError, TypeError):
-            logger.exception("Error checking vegas mode for %s", plugin_id)
-
-        return None
+        return plugin
 
     def _handle_static_pause(self, plugin: 'BasePlugin') -> bool:
         """
@@ -877,15 +877,32 @@ class VegasModeCoordinator:
         self.display_manager.set_scrolling_state(False)
 
         try:
-            # Display the plugin using its standard display() method
-            plugin.display(force_clear=True)
+            # Display the plugin using its standard display() method, under
+            # its plugin lock like every other display() call: without it this
+            # could draw while the update worker is inside the plugin's
+            # update(). If update() holds the lock past the wait, skip this
+            # turn rather than stall the marquee.
+            get_lock = getattr(self.plugin_manager, 'get_plugin_lock', None)
+            plugin_lock = get_lock(plugin_id) if get_lock else None
+            if plugin_lock is not None and not plugin_lock.acquire(
+                    timeout=self.STATIC_LOCK_TIMEOUT):
+                logger.info("Static pause skipped for %s: its update() is still running",
+                            plugin_id)
+                return True
+            try:
+                plugin.display(force_clear=True)
+            finally:
+                if plugin_lock is not None:
+                    plugin_lock.release()
             self.display_manager.update_display()
 
-            # Wait for the plugin's display duration
+            # Wait for the plugin's display duration. Monotonic, like the
+            # iteration clock: an NTP step on an RTC-less Pi would otherwise
+            # end the pause at once or stretch it by the correction.
             duration = plugin.get_display_duration()
-            start = time.time()
+            start = time.monotonic()
 
-            while time.time() - start < duration:
+            while time.monotonic() - start < duration:
                 # Check for interruptions
                 if self._should_stop:
                     logger.info("Static pause interrupted by stop request")
@@ -905,7 +922,7 @@ class VegasModeCoordinator:
 
             logger.info(
                 "Static pause completed for %s after %.1fs",
-                plugin_id, time.time() - start
+                plugin_id, time.monotonic() - start
             )
 
         except Exception:

src/vegas_mode/render_pipeline.py

@@ -11,7 +11,7 @@ import time
 import threading
 from collections import deque
 from contextlib import nullcontext
-from typing import Optional, List, Any, Dict, Deque
+from typing import Optional, List, Any, Dict, Deque, Tuple
 from PIL import Image
 
 from src.common.scroll_config import solve_crisp
@@ -58,6 +58,14 @@ class RenderPipeline:
     # pipelines built without __init__ (tests) still have it.
     _prefetch_generation = 0
 
+    # Where each STATIC plugin's turn falls in the strip, as (x, plugin_id)
+    # in ascending strip columns: x is the end of the content before it. When
+    # that column reaches the right edge of the viewport, the coordinator
+    # pauses the scroll for the plugin (next_static_trigger). Always replaced,
+    # never mutated, so the class-level default is safe for pipelines built
+    # without __init__ (tests).
+    _static_markers: Tuple[Tuple[int, str], ...] = ()
+
     def __init__(
         self,
         config: VegasModeConfig,
@@ -282,6 +290,7 @@ class RenderPipeline:
             # Content grouped by plugin, so a separator can be placed at the
             # plugin boundaries only.
             grouped = self.stream_manager.get_grouped_content_for_composition()
+            self._static_markers = ()
 
             if not grouped:
                 logger.warning("No content available for composition")
@@ -317,6 +326,8 @@ class RenderPipeline:
                 logger.error("ScrollHelper failed to create cached image")
                 return False
 
+            self._static_markers = self._markers_for_composition(blocks)
+
             # Track which plugins are in this scroll (get safely via buffer status)
             self._segments_in_scroll = self.stream_manager.get_active_plugin_ids()
 
@@ -344,6 +355,56 @@ class RenderPipeline:
             logger.exception("Error composing scroll content")
             return False
 
+    def _markers_for_composition(self, blocks: List[Image.Image]) -> Tuple[Tuple[int, str], ...]:
+        """Static markers for a strip just built by create_scrolling_image.
+
+        Mirrors its layout: lead_in_width, then each block followed by
+        separator_width except the last. A STATIC plugin's marker is the end
+        of whatever content precedes it (the lead-in, if nothing does).
+        """
+        layout_fn = getattr(self.stream_manager, 'get_static_layout', None)
+        if layout_fn is None:
+            return ()
+        layout = layout_fn()
+        if not any(is_static for _pid, is_static in layout):
+            return ()
+        markers = []
+        end = max(0, int(self.config.lead_in_width))
+        x = end
+        block_index = 0
+        for plugin_id, is_static in layout:
+            if is_static:
+                markers.append((end, plugin_id))
+                continue
+            if block_index >= len(blocks):
+                break
+            end = x + blocks[block_index].width
+            x = end + self.config.separator_width
+            block_index += 1
+        return tuple(markers)
+
+    def _add_static_markers(self, markers: List[Tuple[int, str]]) -> None:
+        if markers:
+            self._static_markers = tuple(
+                sorted(self._static_markers + tuple(markers), key=lambda m: m[0]))
+
+    def next_static_trigger(self) -> Optional[str]:
+        """
+        The STATIC plugin whose turn the scroll has just reached, if any.
+
+        Called every frame by the coordinator, so it is a comparison against
+        the first marker and nothing more. The marker is consumed: a plugin
+        pauses the scroll once per place it holds in the strip.
+        """
+        markers = self._static_markers
+        if not markers:
+            return None
+        x, plugin_id = markers[0]
+        if x > self.scroll_helper.scroll_position + self.display_width:
+            return None
+        self._static_markers = markers[1:]
+        return plugin_id
+
     def needs_extension(self) -> bool:
         """
         Whether the strip should be extended with the next group of plugins.
@@ -507,6 +568,21 @@ class RenderPipeline:
                 logger.warning("No content available to extend the scroll strip")
                 return False
 
+            # STATIC plugins pause the scroll instead of joining the strip.
+            # Note each one's place -- how many of this group's blocks come
+            # before it -- and take it out before anything else sees it.
+            is_static = getattr(self.stream_manager, 'is_static_plugin', None)
+            statics: List[Tuple[int, str]] = []
+            content: List[Tuple[str, Optional[List[Image.Image]]]] = []
+            for pid, images in grouped:
+                if is_static is not None and is_static(pid):
+                    statics.append((sum(1 for _p, imgs in content if imgs), pid))
+                else:
+                    content.append((pid, images))
+            grouped = content
+            strip_end = (self.scroll_helper.cached_image.width
+                         if self.scroll_helper.cached_image is not None else 0)
+
             # Plugins the background thread had to defer need the shared canvas,
             # so they can only be fetched here. Queue them rather than doing all
             # of them now: measured, six in one go held the render thread for
@@ -523,6 +599,9 @@ class RenderPipeline:
             grouped = [(pid, imgs) for pid, imgs in grouped if imgs]
 
             if not grouped:
+                # Nothing is appended, so each STATIC turn falls at the end
+                # of the strip as it stands.
+                self._add_static_markers([(strip_end, pid) for _n, pid in statics])
                 if deferred:
                     # Everything in this group is queued; the queue will extend
                     # the strip as it drains, so this is not a failure.
@@ -538,6 +617,7 @@ class RenderPipeline:
                 total_rows += len(images)
                 blocks.append(self._join_plugin_rows(images))
 
+            had_strip = self.scroll_helper.cached_image is not None
             appended = self.scroll_helper.append_content(
                 content_items=blocks,
                 item_gap=self.config.separator_width,
@@ -546,8 +626,25 @@ class RenderPipeline:
             if not appended:
                 return False
 
+            if statics:
+                # Where each block ends, laid out as append_content does: a
+                # separator before every block, or -- when there was no strip
+                # to extend -- as create_scrolling_image does with no lead-in.
+                gap = max(0, self.config.separator_width)
+                ends = []
+                x = strip_end if had_strip else -gap
+                for block in blocks:
+                    x += gap + block.width
+                    ends.append(x)
+                self._add_static_markers([
+                    (ends[n - 1] if n > 0 else strip_end, pid) for n, pid in statics
+                ])
+
             # Keep a screen's worth behind the viewport as a safety margin.
-            self.scroll_helper.drop_scrolled_prefix(keep_before=self.display_width)
+            cut = self.scroll_helper.drop_scrolled_prefix(keep_before=self.display_width)
+            if cut and self._static_markers:
+                self._static_markers = tuple(
+                    (max(0, x - cut), pid) for x, pid in self._static_markers)
 
             self._segments_in_scroll = [pid for pid, _ in grouped]
             self.stats['composition_count'] += 1
@@ -935,6 +1032,7 @@ class RenderPipeline:
             self._prefetch_generation += 1
             self._prepared_group = None
             self._deferred_queue = []
+        self._static_markers = ()
 
         self.display_manager.set_scrolling_state(False)
 

src/vegas_mode/stream_manager.py

@@ -667,6 +667,37 @@ class StreamManager:
                 grouped.append((segment.plugin_id, list(segment.images)))
         return grouped
 
+    def get_static_layout(self) -> List[Tuple[str, bool]]:
+        """
+        The buffer's composition order, with STATIC segments kept in place.
+
+        get_grouped_content_for_composition() drops STATIC segments because
+        they contribute no columns; this says where they sat. Each entry is
+        (plugin_id, is_static), and only segments that composition keeps or
+        that are STATIC are listed, so the non-static entries line up one to
+        one with get_grouped_content_for_composition()'s groups.
+        """
+        layout: List[Tuple[str, bool]] = []
+        with self._buffer_lock:
+            for segment in self._active_buffer:
+                if segment.display_mode == VegasDisplayMode.STATIC:
+                    layout.append((segment.plugin_id, True))
+                elif segment.images:
+                    layout.append((segment.plugin_id, False))
+        return layout
+
+    def is_static_plugin(self, plugin_id: str) -> bool:
+        """Whether a loaded plugin asks Vegas to pause for it (STATIC mode)."""
+        plugin = getattr(self.plugin_manager, 'plugins', {}).get(plugin_id)
+        if plugin is None:
+            return False
+        try:
+            return plugin.get_vegas_display_mode() == VegasDisplayMode.STATIC
+        except Exception:
+            logger.debug("[%s] get_vegas_display_mode() failed; treating as not STATIC",
+                         plugin_id, exc_info=True)
+            return False
+
     def take_next_group(
         self, count: Optional[int] = None, offscreen_only: bool = False
     ) -> List[Tuple[str, Optional[List[Image.Image]]]]:
@@ -692,6 +723,8 @@ class StreamManager:
             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.
+            A STATIC plugin is also returned with an empty list, unfetched: it
+            pauses the scroll instead of adding to it (see is_static_plugin).
         """
         if count is None:
             count = self.config.plugins_per_cycle
@@ -717,6 +750,12 @@ class StreamManager:
             plugin = plugins.get(plugin_id)
             if not plugin:
                 continue
+            if self.is_static_plugin(plugin_id):
+                # A STATIC plugin pauses the scroll rather than scrolling by,
+                # so it contributes no columns. It keeps its place in the
+                # group (empty) so the pipeline can mark where its turn falls.
+                group.append((plugin_id, []))
+                continue
             try:
                 images = self.plugin_adapter.get_content(
                     plugin, plugin_id, offscreen_only=offscreen_only)