diff --git a/src/vegas_mode/coordinator.py b/src/vegas_mode/coordinator.py index 6276f3b0..e75e8c51 100644 --- a/src/vegas_mode/coordinator.py +++ b/src/vegas_mode/coordinator.py @@ -307,6 +307,16 @@ class VegasModeCoordinator: self._apply_pending_config() if self.vegas_config.continuous_scroll: + # Drop cached content for plugins whose data just changed, so the + # next time each comes round it is composed from current data. The + # swap path's hot_swap_content() does this via process_updates(), + # but it also rebuilds and repositions the whole strip, which is + # the freeze-and-jump this mode exists to avoid. Without this the + # pending-update flags are never consumed and a segment keeps + # rendering whatever it was first built from — last night's live + # game still shown as live the next morning. + self.render_pipeline.refresh_updated_plugins() + # Extend the strip before the scroll can reach its end, so the next # group arrives from the right and motion never stops. No cycle # boundary, so no freeze, no substitution and no restart with the diff --git a/src/vegas_mode/plugin_adapter.py b/src/vegas_mode/plugin_adapter.py index c722edf0..0da51ce2 100644 --- a/src/vegas_mode/plugin_adapter.py +++ b/src/vegas_mode/plugin_adapter.py @@ -1135,6 +1135,53 @@ class PluginAdapter: else: self._content_cache.clear() + def invalidate_plugin_scroll_cache( + self, plugin: 'BasePlugin', plugin_id: str + ) -> bool: + """ + Drop a plugin's own cached scroll image so its visual is rebuilt. + + Invalidating only this adapter's cache is not enough. A plugin that + composes a scroll strip hands back the *same* image every time until its + own cache is cleared — the sports plugins' ``get_vegas_content()`` + regenerates only "if the cache is empty" — so without this a segment + keeps rendering whatever data it was first built from. That is how a + game that was live last night can still be displayed as live the next + morning. + + Two layouts to cover: a helper directly on the plugin (stocks, news, + odds-ticker) and one owned by a scroll-display manager (the sports + scoreboards). ``cached_image`` and ``cached_array`` must be cleared + together, since the array is the image's numpy mirror and code paths + read whichever is convenient. + + Returns: + True if a cache was found and cleared. + """ + cleared = False + for owner in (plugin, getattr(plugin, '_scroll_manager', None), + getattr(plugin, 'scroll_manager', None)): + if owner is None: + continue + helper = getattr(owner, 'scroll_helper', None) + if helper is None: + continue + try: + if getattr(helper, 'cached_image', None) is not None: + helper.cached_image = None + cleared = True + if getattr(helper, 'cached_array', None) is not None: + helper.cached_array = None + cleared = True + except Exception: # pylint: disable=broad-except + logger.exception( + "[%s] Could not clear scroll cache on %s", + plugin_id, type(owner).__name__ + ) + if cleared: + logger.debug("[%s] Cleared plugin scroll cache", plugin_id) + return cleared + def get_content_type(self, plugin: 'BasePlugin', plugin_id: str) -> str: """ Get the type of content a plugin provides. diff --git a/src/vegas_mode/render_pipeline.py b/src/vegas_mode/render_pipeline.py index 94b1e086..8aceabe1 100644 --- a/src/vegas_mode/render_pipeline.py +++ b/src/vegas_mode/render_pipeline.py @@ -629,6 +629,25 @@ class RenderPipeline: return False + def refresh_updated_plugins(self) -> bool: + """ + Let changed plugin data reach the strip without interrupting motion. + + Used instead of :meth:`hot_swap_content` when scrolling continuously. + The swap rebuilds the whole image and repositions the scroll, which is + visible as a freeze and a jump; the strip is extended here rather than + replaced, so it is enough to drop the stale caches and let the plugin + recompose when it next comes round. + + Returns: + True if any plugin's cached content was dropped. + """ + try: + return bool(self.stream_manager.invalidate_pending_updates()) + except Exception: # pylint: disable=broad-except + logger.exception("Failed to refresh updated plugins") + return False + def hot_swap_content(self) -> bool: """ Hot-swap to new composed content. diff --git a/src/vegas_mode/stream_manager.py b/src/vegas_mode/stream_manager.py index 7f43b24f..a97420c9 100644 --- a/src/vegas_mode/stream_manager.py +++ b/src/vegas_mode/stream_manager.py @@ -201,6 +201,47 @@ class StreamManager: logger.debug("Plugin %s marked for update", plugin_id) + def invalidate_pending_updates(self) -> List[str]: + """ + Drop cached content for plugins whose data changed, without refetching. + + The continuous-scroll counterpart to :meth:`process_updates`. That method + belongs to the swap path: it refetches immediately and merges into the + active buffer, which continuous mode bypasses entirely, and doing that + work on the render thread would hitch the scroll. + + Here it is enough to clear the caches and let the plugin come round in + the rotation, which recomposes it from current data a moment later. Left + uncalled, ``_pending_updates`` simply accumulates and no visual ever + refreshes — a game that was live last night keeps being drawn as live. + + Returns: + The plugin ids whose caches were dropped. + """ + with self._buffer_lock: + if not self._pending_updates: + return [] + updated = list(self._pending_updates.keys()) + self._pending_updates.clear() + + plugins = getattr(self.plugin_manager, 'plugins', {}) + for plugin_id in updated: + try: + self.plugin_adapter.invalidate_cache(plugin_id) + plugin = plugins.get(plugin_id) + if plugin is not None: + self.plugin_adapter.invalidate_plugin_scroll_cache( + plugin, plugin_id) + except Exception: # pylint: disable=broad-except + logger.exception( + "[%s] Could not invalidate cached content", plugin_id) + + logger.info( + "Vegas: dropped cached content for %d updated plugin(s): %s", + len(updated), ', '.join(updated) + ) + return updated + def has_pending_updates(self) -> bool: """Check if any plugins have pending updates awaiting processing.""" with self._buffer_lock: diff --git a/test/test_vegas_continuous_refresh.py b/test/test_vegas_continuous_refresh.py new file mode 100644 index 00000000..422a4921 --- /dev/null +++ b/test/test_vegas_continuous_refresh.py @@ -0,0 +1,227 @@ +""" +Regression tests: changed plugin data must reach the strip in continuous mode. + +Two faults combined to freeze Vegas content indefinitely. + +PR #291 added a call to ``plugin_adapter.invalidate_plugin_scroll_cache()`` so a +plugin's *own* cached scroll image would be rebuilt from fresh data. The method +was never implemented, and ``hot_swap_content()`` wraps the call in a broad +except, so every hot swap raised AttributeError and was silently swallowed. + +Continuous scrolling then removed the only path that reached it at all: +``should_recompose()``/``hot_swap_content()`` are called from the non-continuous +branch, while ``continuous_scroll`` defaults to True. + +Together, a plugin composed its scroll image once and handed back the same +picture forever, because the sports plugins' ``get_vegas_content()`` regenerates +only when its cache is empty. Symptom: a game that was live last night is still +drawn as live the following morning. +""" + +from types import SimpleNamespace +from unittest.mock import MagicMock + +import numpy as np +from PIL import Image + +from src.vegas_mode.config import VegasModeConfig +from src.vegas_mode.plugin_adapter import PluginAdapter +from src.vegas_mode.render_pipeline import RenderPipeline +from src.vegas_mode.stream_manager import StreamManager + + +class FakeDisplayManager: + width = 64 + height = 32 + + +def _helper(): + """A stand-in ScrollHelper holding both halves of its cache.""" + image = Image.new('RGB', (128, 32), (10, 20, 30)) + return SimpleNamespace(cached_image=image, cached_array=np.array(image)) + + +class TestInvalidatePluginScrollCache: + """The method PR #291 called but never defined.""" + + def test_method_exists(self): + # It was called for months without existing; the broad except in + # hot_swap_content() meant nothing ever surfaced. + assert hasattr(PluginAdapter, 'invalidate_plugin_scroll_cache') + + def test_clears_helper_attached_to_the_plugin(self): + adapter = PluginAdapter(FakeDisplayManager(), VegasModeConfig()) + helper = _helper() + plugin = SimpleNamespace(scroll_helper=helper) + + assert adapter.invalidate_plugin_scroll_cache(plugin, 'stocks') is True + assert helper.cached_image is None + assert helper.cached_array is None + + def test_clears_helper_owned_by_a_scroll_manager(self): + # The sports scoreboards keep theirs on _scroll_manager, which is the + # layout that produced the reported stale-scores bug. + adapter = PluginAdapter(FakeDisplayManager(), VegasModeConfig()) + helper = _helper() + plugin = SimpleNamespace(_scroll_manager=SimpleNamespace(scroll_helper=helper)) + + assert adapter.invalidate_plugin_scroll_cache(plugin, 'baseball') is True + assert helper.cached_image is None + assert helper.cached_array is None + + def test_clears_both_halves_together(self): + # cached_array is the image's numpy mirror; leaving one behind lets a + # reader pick up content the other no longer has. + adapter = PluginAdapter(FakeDisplayManager(), VegasModeConfig()) + helper = _helper() + adapter.invalidate_plugin_scroll_cache( + SimpleNamespace(scroll_helper=helper), 'news') + assert (helper.cached_image, helper.cached_array) == (None, None) + + def test_plugin_without_a_helper_is_not_an_error(self): + adapter = PluginAdapter(FakeDisplayManager(), VegasModeConfig()) + assert adapter.invalidate_plugin_scroll_cache(SimpleNamespace(), 'clock') is False + + +class TestInvalidatePendingUpdates: + def _manager(self, plugins): + stream = StreamManager( + VegasModeConfig(), + SimpleNamespace(plugins=plugins), + MagicMock(), + ) + stream.plugin_adapter = MagicMock() + return stream + + def test_drops_caches_for_updated_plugins(self): + helper = _helper() + plugin = SimpleNamespace(scroll_helper=helper) + stream = self._manager({'baseball': plugin}) + stream.mark_plugin_updated('baseball') + + assert stream.invalidate_pending_updates() == ['baseball'] + stream.plugin_adapter.invalidate_cache.assert_called_once_with('baseball') + stream.plugin_adapter.invalidate_plugin_scroll_cache.assert_called_once_with( + plugin, 'baseball') + + def test_pending_flags_are_consumed(self): + # Left unconsumed they accumulate forever and nothing ever refreshes. + stream = self._manager({'baseball': SimpleNamespace()}) + stream.mark_plugin_updated('baseball') + assert stream.has_pending_updates() is True + + stream.invalidate_pending_updates() + assert stream.has_pending_updates() is False + assert stream.invalidate_pending_updates() == [] + + def test_no_pending_updates_does_no_work(self): + stream = self._manager({}) + assert stream.invalidate_pending_updates() == [] + stream.plugin_adapter.invalidate_cache.assert_not_called() + + def test_a_failing_plugin_does_not_stop_the_others(self): + stream = self._manager({'a': SimpleNamespace(), 'b': SimpleNamespace()}) + stream.mark_plugin_updated('a') + stream.mark_plugin_updated('b') + stream.plugin_adapter.invalidate_cache.side_effect = [ + RuntimeError('boom'), None] + + assert sorted(stream.invalidate_pending_updates()) == ['a', 'b'] + assert stream.plugin_adapter.invalidate_cache.call_count == 2 + + +class TestContinuousModeReachesTheRefresh: + def _pipeline(self): + stream = MagicMock() + stream.get_buffer_status.return_value = {'staging_count': 0} + return RenderPipeline(VegasModeConfig(), FakeDisplayManager(), stream), stream + + def test_refresh_delegates_to_the_stream_manager(self): + pipeline, stream = self._pipeline() + stream.invalidate_pending_updates.return_value = ['baseball'] + assert pipeline.refresh_updated_plugins() is True + + def test_refresh_reports_false_when_nothing_changed(self): + pipeline, stream = self._pipeline() + stream.invalidate_pending_updates.return_value = [] + assert pipeline.refresh_updated_plugins() is False + + def test_refresh_never_raises_into_the_render_loop(self): + pipeline, stream = self._pipeline() + stream.invalidate_pending_updates.side_effect = RuntimeError('boom') + assert pipeline.refresh_updated_plugins() is False + + def test_refresh_does_not_reposition_the_scroll(self): + # The whole point of preferring this over hot_swap_content(): that path + # rebuilds and repositions, which reads as a freeze then a jump. + pipeline, stream = self._pipeline() + stream.invalidate_pending_updates.return_value = ['baseball'] + pipeline.scroll_helper.scroll_position = 1234 + + pipeline.refresh_updated_plugins() + + assert pipeline.scroll_helper.scroll_position == 1234 + stream.swap_buffers.assert_not_called() + stream.process_updates.assert_not_called() + + +class TestCoordinatorWiring: + """ + The regression itself: continuous mode has to *call* the refresh. + + should_recompose()/hot_swap_content() sit in the non-continuous branch, and + continuous_scroll defaults to True, so before this fix the refresh was + simply never reached on a default install. + """ + + def _coordinator(self, continuous): + import threading + + from src.vegas_mode.coordinator import VegasModeCoordinator + + config = VegasModeConfig() + config.continuous_scroll = continuous + # Built without __init__ so the test exercises run_frame's branching + # without standing up a display, stream and render stack. + coordinator = VegasModeCoordinator.__new__(VegasModeCoordinator) + coordinator.vegas_config = config + coordinator.render_pipeline = MagicMock() + coordinator.render_pipeline.has_deferred.return_value = False + coordinator.render_pipeline.needs_extension.return_value = False + coordinator.render_pipeline.is_cycle_complete.return_value = False + coordinator.render_pipeline.should_recompose.return_value = False + coordinator.stream_manager = MagicMock() + coordinator.stats = {'cycles_completed': 0} + coordinator._state_lock = threading.Lock() + coordinator._is_active = True + coordinator._is_paused = False + coordinator._should_stop = False + coordinator._pending_config_update = False + coordinator._live_priority_check = None + coordinator._interrupt_check = None + coordinator.sync_manager = None + return coordinator + + def test_continuous_mode_refreshes_updated_plugins_every_frame(self): + coordinator = self._coordinator(continuous=True) + coordinator.run_frame() + coordinator.render_pipeline.refresh_updated_plugins.assert_called_once() + + def test_continuous_mode_does_not_use_the_disruptive_swap(self): + coordinator = self._coordinator(continuous=True) + coordinator.run_frame() + coordinator.render_pipeline.hot_swap_content.assert_not_called() + + def test_swap_mode_still_uses_hot_swap(self): + # The non-continuous path must keep its original behaviour. + coordinator = self._coordinator(continuous=False) + coordinator.render_pipeline.should_recompose.return_value = True + coordinator.run_frame() + coordinator.render_pipeline.hot_swap_content.assert_called_once() + coordinator.render_pipeline.refresh_updated_plugins.assert_not_called() + + def test_a_frame_is_still_rendered_either_way(self): + for continuous in (True, False): + coordinator = self._coordinator(continuous=continuous) + coordinator.run_frame() + coordinator.render_pipeline.render_frame.assert_called_once()