mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 16:58:06 +00:00
Hold capture_mode for every plugin render, not just narrowed ones
The native content path only entered capture_mode when it was also narrowing the canvas, so at full width — which is every plugin without a vegas_width_pct override, i.e. most of them — a plugin calling update_display() while building its Vegas content wrote straight to the hardware. That is a visible flash mid-scroll, and it lines up with the flash reported at cycle transitions, when several plugins are fetched back to back. Suppression is now unconditional; the narrowing context stays separate because it is already a no-op at full width. Both contexts are reached through helpers that degrade to nullcontext when the display manager lacks them. That matters more than it looks: the adapter's handlers are deliberately broad, so an AttributeError from a missing context does not surface as an error — it surfaces as the plugin contributing nothing. Making the call unconditional without this turned 44 tests red for exactly that reason, all of them reporting lost content rather than the real cause. The test double now provides capture_mode and render_size too, so tests exercise the real contexts instead of silently taking the degraded path. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
This commit is contained in:
@@ -224,6 +224,24 @@ class PluginAdapter:
|
|||||||
self._cache_content(plugin_id, kept)
|
self._cache_content(plugin_id, kept)
|
||||||
return kept
|
return kept
|
||||||
|
|
||||||
|
def _capture(self):
|
||||||
|
"""
|
||||||
|
Context manager suppressing hardware writes while plugin render code runs.
|
||||||
|
|
||||||
|
Degrades to a no-op when the display manager predates capture_mode. As
|
||||||
|
with _render_at, losing the suppression risks a visible flash, whereas
|
||||||
|
raising would be swallowed by the broad handlers upstream and drop the
|
||||||
|
plugin's content entirely — much worse.
|
||||||
|
"""
|
||||||
|
capture_mode = getattr(self.display_manager, 'capture_mode', None)
|
||||||
|
if capture_mode is None:
|
||||||
|
logger.debug(
|
||||||
|
"display_manager has no capture_mode(); plugin writes during "
|
||||||
|
"content capture may reach the panel"
|
||||||
|
)
|
||||||
|
return nullcontext()
|
||||||
|
return capture_mode()
|
||||||
|
|
||||||
def _render_at(self, width: int):
|
def _render_at(self, width: int):
|
||||||
"""
|
"""
|
||||||
Context manager narrowing the plugin-facing canvas to ``width``.
|
Context manager narrowing the plugin-facing canvas to ``width``.
|
||||||
@@ -454,17 +472,21 @@ class PluginAdapter:
|
|||||||
# the narrower value with no changes of its own; one that wants to
|
# the narrower value with no changes of its own; one that wants to
|
||||||
# be explicit can read get_vegas_render_width().
|
# be explicit can read get_vegas_render_width().
|
||||||
render_width = self.resolve_render_width(plugin, plugin_id)
|
render_width = self.resolve_render_width(plugin, plugin_id)
|
||||||
plugin._vegas_render_width = render_width
|
if render_width != self.display_width:
|
||||||
try:
|
|
||||||
if render_width == self.display_width:
|
|
||||||
result = plugin.get_vegas_content()
|
|
||||||
else:
|
|
||||||
logger.info(
|
logger.info(
|
||||||
"[%s] Native: requesting %dpx instead of %dpx",
|
"[%s] Native: requesting %dpx instead of %dpx",
|
||||||
plugin_id, render_width, self.display_width
|
plugin_id, render_width, self.display_width
|
||||||
)
|
)
|
||||||
with self.display_manager.capture_mode(), \
|
|
||||||
self._render_at(render_width):
|
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.
|
||||||
|
with self._capture(), self._render_at(render_width):
|
||||||
result = plugin.get_vegas_content()
|
result = plugin.get_vegas_content()
|
||||||
finally:
|
finally:
|
||||||
plugin._vegas_render_width = None
|
plugin._vegas_render_width = None
|
||||||
@@ -727,7 +749,7 @@ class PluginAdapter:
|
|||||||
# Save display state to restore after
|
# Save display state to restore after
|
||||||
original_image = self.display_manager.image.copy()
|
original_image = self.display_manager.image.copy()
|
||||||
|
|
||||||
with self.display_manager.capture_mode():
|
with self._capture():
|
||||||
# Method 1: Try _create_scrolling_display (stocks pattern)
|
# Method 1: Try _create_scrolling_display (stocks pattern)
|
||||||
if hasattr(plugin, '_create_scrolling_display'):
|
if hasattr(plugin, '_create_scrolling_display'):
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -830,8 +852,7 @@ class PluginAdapter:
|
|||||||
plugin_id, render_width, self.display_width
|
plugin_id, render_width, self.display_width
|
||||||
)
|
)
|
||||||
|
|
||||||
with self.display_manager.capture_mode(), \
|
with self._capture(), self._render_at(render_width):
|
||||||
self._render_at(render_width):
|
|
||||||
self.display_manager.clear()
|
self.display_manager.clear()
|
||||||
logger.info("[%s] Fallback: display cleared, calling display()", plugin_id)
|
logger.info("[%s] Fallback: display cleared, calling display()", plugin_id)
|
||||||
|
|
||||||
@@ -865,8 +886,7 @@ class PluginAdapter:
|
|||||||
plugin_id
|
plugin_id
|
||||||
)
|
)
|
||||||
# Try once more with force_clear=True
|
# Try once more with force_clear=True
|
||||||
with self.display_manager.capture_mode(), \
|
with self._capture(), self._render_at(render_width):
|
||||||
self._render_at(render_width):
|
|
||||||
self.display_manager.clear()
|
self.display_manager.clear()
|
||||||
plugin.display(force_clear=True)
|
plugin.display(force_clear=True)
|
||||||
captured = self.display_manager.image.copy()
|
captured = self.display_manager.image.copy()
|
||||||
|
|||||||
+122
-1
@@ -3,6 +3,8 @@ Tests for the Vegas mode density work: dead-space trimming in PluginAdapter
|
|||||||
and the configurable lead-in gap in ScrollHelper.
|
and the configurable lead-in gap in ScrollHelper.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
from contextlib import contextmanager
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from PIL import Image
|
from PIL import Image
|
||||||
|
|
||||||
@@ -16,13 +18,45 @@ DISPLAY_H = 64
|
|||||||
|
|
||||||
|
|
||||||
class FakeDisplayManager:
|
class FakeDisplayManager:
|
||||||
"""Minimal stand-in; the native content path never touches the canvas."""
|
"""Stand-in offering the same contexts the real DisplayManager does.
|
||||||
|
|
||||||
|
capture_mode and render_size must both be present: the adapter degrades
|
||||||
|
gracefully when they are missing, so a fake without them would silently
|
||||||
|
exercise the degraded path instead of the real one.
|
||||||
|
"""
|
||||||
|
|
||||||
width = DISPLAY_W
|
width = DISPLAY_W
|
||||||
height = DISPLAY_H
|
height = DISPLAY_H
|
||||||
|
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
self.image = Image.new('RGB', (DISPLAY_W, DISPLAY_H))
|
self.image = Image.new('RGB', (DISPLAY_W, DISPLAY_H))
|
||||||
|
self.draw = None
|
||||||
|
self._capture_mode_active = False
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def capture_mode(self):
|
||||||
|
self._capture_mode_active = True
|
||||||
|
try:
|
||||||
|
yield
|
||||||
|
finally:
|
||||||
|
self._capture_mode_active = False
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def render_size(self, width, height=None):
|
||||||
|
prev = self.image
|
||||||
|
target_w = max(1, min(int(width), DISPLAY_W))
|
||||||
|
target_h = max(1, min(int(height) if height else DISPLAY_H, DISPLAY_H))
|
||||||
|
try:
|
||||||
|
self.image = Image.new('RGB', (target_w, target_h))
|
||||||
|
yield
|
||||||
|
finally:
|
||||||
|
self.image = prev
|
||||||
|
|
||||||
|
def clear(self):
|
||||||
|
self.image = Image.new('RGB', self.image.size)
|
||||||
|
|
||||||
|
def update_display(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
class NativePlugin:
|
class NativePlugin:
|
||||||
@@ -1098,3 +1132,90 @@ class TestCutsNeverSplitWords:
|
|||||||
out = adapter.get_content(NativePlugin([img]), 'ticker')[0]
|
out = adapter.get_content(NativePlugin([img]), 'ticker')[0]
|
||||||
assert out.width not in mid_word, \
|
assert out.width not in mid_word, \
|
||||||
f"cut at {out.width} is a mid-word position"
|
f"cut at {out.width} is a mid-word position"
|
||||||
|
|
||||||
|
|
||||||
|
class TestCaptureModeAlwaysHeld:
|
||||||
|
"""
|
||||||
|
Building Vegas content is off-screen work, but plugins are free to call
|
||||||
|
update_display() while doing it. Outside capture_mode that write reaches the
|
||||||
|
hardware and flashes the panel mid-scroll, so every path that runs plugin
|
||||||
|
render code must hold capture_mode — including at full width, where the
|
||||||
|
narrowing context is a no-op.
|
||||||
|
"""
|
||||||
|
|
||||||
|
class RecordingDM:
|
||||||
|
width = DISPLAY_W
|
||||||
|
height = DISPLAY_H
|
||||||
|
|
||||||
|
def __init__(self):
|
||||||
|
self.image = Image.new('RGB', (DISPLAY_W, DISPLAY_H))
|
||||||
|
self.draw = None
|
||||||
|
self.capture_depth = 0
|
||||||
|
self.hardware_writes_while_uncaptured = 0
|
||||||
|
|
||||||
|
from contextlib import contextmanager
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def capture_mode(self):
|
||||||
|
self.capture_depth += 1
|
||||||
|
try:
|
||||||
|
yield
|
||||||
|
finally:
|
||||||
|
self.capture_depth -= 1
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def render_size(self, width, height=None):
|
||||||
|
yield
|
||||||
|
|
||||||
|
def clear(self):
|
||||||
|
self.image = Image.new('RGB', (DISPLAY_W, DISPLAY_H))
|
||||||
|
|
||||||
|
def update_display(self):
|
||||||
|
if self.capture_depth == 0:
|
||||||
|
self.hardware_writes_while_uncaptured += 1
|
||||||
|
|
||||||
|
class PushyPlugin:
|
||||||
|
"""A plugin that pushes to the display while building Vegas content."""
|
||||||
|
|
||||||
|
def __init__(self, dm, images):
|
||||||
|
self.display_manager = dm
|
||||||
|
self.config = {}
|
||||||
|
self._images = images
|
||||||
|
|
||||||
|
def get_vegas_content(self):
|
||||||
|
self.display_manager.update_display()
|
||||||
|
return self._images
|
||||||
|
|
||||||
|
def _run(self, **cfg):
|
||||||
|
dm = self.RecordingDM()
|
||||||
|
adapter = PluginAdapter(dm, VegasModeConfig(**cfg))
|
||||||
|
plugin = self.PushyPlugin(dm, [canvas([(100, 300)])])
|
||||||
|
adapter.get_content(plugin, 'pushy')
|
||||||
|
return dm
|
||||||
|
|
||||||
|
def test_no_hardware_write_escapes_at_full_width(self):
|
||||||
|
dm = self._run(render_width_pct=100)
|
||||||
|
assert dm.hardware_writes_while_uncaptured == 0
|
||||||
|
|
||||||
|
def test_no_hardware_write_escapes_when_narrowing(self):
|
||||||
|
dm = self._run(render_width_pct=50)
|
||||||
|
assert dm.hardware_writes_while_uncaptured == 0
|
||||||
|
|
||||||
|
def test_capture_mode_is_released_afterwards(self):
|
||||||
|
dm = self._run(render_width_pct=100)
|
||||||
|
assert dm.capture_depth == 0
|
||||||
|
|
||||||
|
def test_capture_mode_released_even_when_the_plugin_raises(self):
|
||||||
|
dm = self.RecordingDM()
|
||||||
|
adapter = PluginAdapter(dm, VegasModeConfig())
|
||||||
|
|
||||||
|
class Boom:
|
||||||
|
def __init__(self, d):
|
||||||
|
self.display_manager = d
|
||||||
|
self.config = {}
|
||||||
|
|
||||||
|
def get_vegas_content(self):
|
||||||
|
raise ValueError("boom")
|
||||||
|
|
||||||
|
adapter.get_content(Boom(dm), 'boom')
|
||||||
|
assert dm.capture_depth == 0
|
||||||
|
|||||||
Reference in New Issue
Block a user