mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 08:48:05 +00:00
Vegas mode: end cycles before the wrap, keep the width budget honest
Three fixes, the first a regression from lead_in_width defaulting to 0. get_visible_portion wraps: once scroll_position + display_width passes the end of the strip it fills the right of the frame from the *head* of the same strip. So the final display_width of travel showed the cycle's first plugin re-entering on the right while its last plugin exited on the left, and the recompose that followed replaced both at once. On a 512px panel at 50px/s that was 10.2s of two plugins on screen at once, ending in a hard cut — reported as the ticker "switching mid-scroll" from F1 to news. That used to be invisible because the strip began with a full display_width of blank, so the wrapped-in region was black. Removing that blank (it was 10s of dead panel per cycle) exposed the wrap. Cycles now end one display width earlier, before any wrapped content appears, clamped for strips no wider than the display so they don't complete instantly and spin the recompose loop. Verified on hardware: a 3936px strip now completes at 68.5s, exactly (3936 - 512) / 50. Second, auto_trim=False also skipped the width budget, which is an unrelated concern — turning off margin cropping should not let one plugin hold the panel for minutes. Seen in the field: the F1 scoreboard contributed 116 images and 14,848px untouched, giving a 33,821px cycle (11 minutes of content). The budget now applies regardless of trimming; with it restored that cycle is 6,362px. Third, the budget accounted for row gaps using the flat intra_plugin_gap while the compositor had moved to measured separation, so it under-counted by up to (min_content_separation - intra_plugin_gap) per row and a many-row plugin overran its cap. Both now use the same separation_gap() rule, and a test asserts the composed block fits the budget end to end rather than trusting the two paths to agree. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
This commit is contained in:
@@ -12,7 +12,11 @@ from contextlib import nullcontext
|
||||
from typing import Optional, List, Any, Tuple, Union, TYPE_CHECKING
|
||||
from PIL import Image
|
||||
|
||||
from src.vegas_mode.geometry import find_blank_cut, trim_to_content
|
||||
from src.vegas_mode.geometry import (
|
||||
find_blank_cut,
|
||||
separation_gap,
|
||||
trim_to_content,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from src.plugin_system.base_plugin import BasePlugin
|
||||
@@ -166,8 +170,13 @@ class PluginAdapter:
|
||||
Trimmed image list, or None if nothing worth showing remains
|
||||
"""
|
||||
if not self.config.auto_trim:
|
||||
self._cache_content(plugin_id, images)
|
||||
return images
|
||||
# Trimming is off, but the width budget is a separate concern —
|
||||
# turning off margin cropping should not let one plugin hold the
|
||||
# panel for minutes. Skipping it here previously let a 14,848px
|
||||
# segment through untouched.
|
||||
kept = self._apply_width_budget(list(images), plugin_id)
|
||||
self._cache_content(plugin_id, kept)
|
||||
return kept
|
||||
|
||||
original_width = sum(img.width for img in images)
|
||||
kept: List[Image.Image] = []
|
||||
@@ -275,6 +284,20 @@ class PluginAdapter:
|
||||
return self.display_width
|
||||
return max(1, int(self.display_width * pct / 100))
|
||||
|
||||
def _row_gap(self, left: Image.Image, right: Image.Image) -> int:
|
||||
"""
|
||||
Gap the compositor will insert between two of a plugin's rows.
|
||||
|
||||
Mirrors RenderPipeline._join_plugin_rows so the width budget measures
|
||||
what will actually be rendered.
|
||||
"""
|
||||
return separation_gap(
|
||||
left, right,
|
||||
target=max(0, self.config.min_content_separation),
|
||||
minimum=max(0, self.config.intra_plugin_gap),
|
||||
threshold=self.config.trim_threshold,
|
||||
)
|
||||
|
||||
def _width_budget(self) -> int:
|
||||
"""Maximum columns one plugin may occupy in a cycle. 0 means unlimited."""
|
||||
ratio = self.config.max_plugin_width_ratio
|
||||
@@ -304,11 +327,15 @@ class PluginAdapter:
|
||||
"""
|
||||
budget = self._width_budget()
|
||||
|
||||
# Count the gaps the compositor will insert between these rows, not
|
||||
# just the pixels of the rows themselves — otherwise a plugin with many
|
||||
# rows quietly occupies far more of the panel than its budget allows.
|
||||
gap = max(0, self.config.intra_plugin_gap)
|
||||
total = sum(img.width for img in images) + gap * (len(images) - 1)
|
||||
# Count the gaps the compositor will actually insert, not just the
|
||||
# pixels of the rows — otherwise a plugin with many rows quietly
|
||||
# occupies far more of the panel than its budget allows. These must use
|
||||
# the same measured rule as RenderPipeline._join_plugin_rows; assuming
|
||||
# the flat intra_plugin_gap here under-counted by up to
|
||||
# (min_content_separation - intra_plugin_gap) per row.
|
||||
total = sum(img.width for img in images) + sum(
|
||||
self._row_gap(images[i], images[i + 1]) for i in range(len(images) - 1)
|
||||
)
|
||||
|
||||
if not budget or total <= budget:
|
||||
# Fits, so reset rotation — the whole segment is being shown.
|
||||
@@ -327,7 +354,9 @@ class PluginAdapter:
|
||||
# cut never lands in the middle of one.
|
||||
for step in range(len(images)):
|
||||
img = images[(start + step) % len(images)]
|
||||
cost = img.width + (gap if selected else 0)
|
||||
cost = img.width
|
||||
if selected:
|
||||
cost += self._row_gap(selected[-1], img)
|
||||
if selected and used + cost > budget:
|
||||
break
|
||||
selected.append(img)
|
||||
|
||||
@@ -265,21 +265,29 @@ class RenderPipeline:
|
||||
|
||||
# Determine if the cycle is done.
|
||||
#
|
||||
# scroll_helper considers a cycle complete only after
|
||||
# total_distance_scrolled >= total_scroll_width + display_width.
|
||||
# That extra display_width of travel causes a "wrap-around" phase
|
||||
# where scroll_position resets to ~0 and the first plugin's content
|
||||
# re-enters from the right — the user sees this 2-3 s of re-entry
|
||||
# as "a plugin partially displaying before the next one starts."
|
||||
# get_visible_portion wraps: once scroll_position + display_width
|
||||
# passes the end of the strip it fills the right-hand side of the
|
||||
# frame from the *head* of the same strip. So the last
|
||||
# display_width of travel shows the cycle's first plugin re-entering
|
||||
# on the right while its last plugin exits on the left, and the
|
||||
# recompose that follows then replaces both at once. That reads as
|
||||
# the ticker "switching mid-scroll".
|
||||
#
|
||||
# We end the cycle as soon as total_distance_scrolled reaches
|
||||
# total_scroll_width (the wrap-around point), before any second-pass
|
||||
# content becomes visible. The scroll_helper's own is_scroll_complete()
|
||||
# check is kept as a fallback for any edge-cases where that threshold
|
||||
# is never hit.
|
||||
# This used to be hidden because the strip began with a full
|
||||
# display_width of blank, so the wrapped-in region was black.
|
||||
# lead_in_width now defaults to 0 (that blank was 10s of dead panel
|
||||
# at 50px/s), which exposed the wrap — so the cycle has to end
|
||||
# before it, one display width earlier.
|
||||
#
|
||||
# A strip no wider than the display never wraps, and subtracting
|
||||
# would make the cycle complete instantly, so clamp in that case.
|
||||
wrap_point = self.scroll_helper.total_scroll_width
|
||||
if wrap_point > self.display_width:
|
||||
wrap_point -= self.display_width
|
||||
|
||||
at_wrap_point = (
|
||||
not self._cycle_complete and
|
||||
self.scroll_helper.total_distance_scrolled >= self.scroll_helper.total_scroll_width
|
||||
self.scroll_helper.total_distance_scrolled >= wrap_point
|
||||
)
|
||||
|
||||
if at_wrap_point or self.scroll_helper.is_scroll_complete():
|
||||
|
||||
@@ -756,3 +756,175 @@ class TestNewConfigKeys:
|
||||
def test_validate_rejects_out_of_range(self, overrides, bad_key):
|
||||
errors = VegasModeConfig(**overrides).validate()
|
||||
assert any(bad_key in e for e in errors), errors
|
||||
|
||||
|
||||
class TestCycleEndsBeforeWrap:
|
||||
"""
|
||||
get_visible_portion wraps the head of the strip into the right side of the
|
||||
frame once scroll_position + display_width passes the end. With a leading
|
||||
blank that was invisible; with lead_in_width=0 it showed the cycle's first
|
||||
plugin re-entering while the last one exited, then a recompose replaced
|
||||
both — the reported "switched mid-scroll".
|
||||
"""
|
||||
|
||||
def _pipeline(self, strip_width, **cfg):
|
||||
from src.vegas_mode.render_pipeline import RenderPipeline
|
||||
|
||||
class FakeStream:
|
||||
def get_grouped_content_for_composition(self):
|
||||
return [('a', [Image.new('RGB', (strip_width, DISPLAY_H), (255, 255, 255))])]
|
||||
|
||||
def get_active_plugin_ids(self):
|
||||
return ['a']
|
||||
|
||||
class DM:
|
||||
width = DISPLAY_W
|
||||
height = DISPLAY_H
|
||||
|
||||
def __init__(self):
|
||||
self.image = Image.new('RGB', (DISPLAY_W, DISPLAY_H))
|
||||
|
||||
def set_scrolling_state(self, *a):
|
||||
pass
|
||||
|
||||
def update_display(self):
|
||||
pass
|
||||
|
||||
p = RenderPipeline(VegasModeConfig(lead_in_width=0, **cfg), DM(), FakeStream())
|
||||
assert p.compose_scroll_content()
|
||||
return p
|
||||
|
||||
def _advance_to(self, pipeline, distance):
|
||||
pipeline.scroll_helper.total_distance_scrolled = distance
|
||||
pipeline.scroll_helper.scroll_position = float(distance)
|
||||
|
||||
def test_cycle_is_not_complete_before_the_wrap_point(self):
|
||||
p = self._pipeline(2000)
|
||||
self._advance_to(p, 2000 - DISPLAY_W - 1)
|
||||
p.render_frame()
|
||||
assert not p.is_cycle_complete()
|
||||
|
||||
def test_cycle_completes_exactly_at_the_wrap_point(self):
|
||||
p = self._pipeline(2000)
|
||||
self._advance_to(p, 2000 - DISPLAY_W)
|
||||
p.render_frame()
|
||||
assert p.is_cycle_complete()
|
||||
|
||||
def test_completes_a_full_display_width_earlier_than_the_strip_end(self):
|
||||
# The whole point: it must not run to total_scroll_width, which is
|
||||
# where the wrapped content has already been on screen for 10s at
|
||||
# 50px/s on a 512px panel.
|
||||
p = self._pipeline(3000)
|
||||
self._advance_to(p, 3000 - DISPLAY_W - 1)
|
||||
p.render_frame()
|
||||
assert not p.is_cycle_complete()
|
||||
self._advance_to(p, 3000 - DISPLAY_W)
|
||||
p.render_frame()
|
||||
assert p.is_cycle_complete()
|
||||
|
||||
def test_strip_narrower_than_the_display_does_not_complete_instantly(self):
|
||||
# Subtracting the display width would go negative and end the cycle on
|
||||
# the very first frame, spinning the recompose loop.
|
||||
p = self._pipeline(200)
|
||||
self._advance_to(p, 0)
|
||||
p.render_frame()
|
||||
assert not p.is_cycle_complete()
|
||||
|
||||
def test_strip_narrower_than_the_display_still_completes(self):
|
||||
p = self._pipeline(200)
|
||||
self._advance_to(p, 200)
|
||||
p.render_frame()
|
||||
assert p.is_cycle_complete()
|
||||
|
||||
def test_strip_exactly_the_display_width(self):
|
||||
p = self._pipeline(DISPLAY_W)
|
||||
self._advance_to(p, 0)
|
||||
p.render_frame()
|
||||
assert not p.is_cycle_complete()
|
||||
self._advance_to(p, DISPLAY_W)
|
||||
p.render_frame()
|
||||
assert p.is_cycle_complete()
|
||||
|
||||
|
||||
class TestBudgetIndependentOfTrim:
|
||||
"""
|
||||
Turning off margin trimming must not disable the per-plugin width cap —
|
||||
they are unrelated concerns. Found in the field: with auto_trim off, the F1
|
||||
scoreboard contributed 116 images / 14,848px untouched, producing a 33,821px
|
||||
cycle.
|
||||
"""
|
||||
|
||||
def test_budget_still_applies_with_trim_off(self):
|
||||
adapter = adapter_with(auto_trim=False, max_plugin_width_ratio=1.0,
|
||||
intra_plugin_gap=0, min_content_separation=0)
|
||||
items = [canvas([(0, 400)], width=400) for _ in range(10)]
|
||||
images = adapter.get_content(NativePlugin(items), 'f1-scoreboard')
|
||||
assert sum(i.width for i in images) <= DISPLAY_W
|
||||
|
||||
def test_trim_off_still_leaves_content_untrimmed(self):
|
||||
# The margins must survive; only the cap should act.
|
||||
adapter = adapter_with(auto_trim=False, max_plugin_width_ratio=0)
|
||||
images = adapter.get_content(NativePlugin([canvas([(4, 39)])]), 'x')
|
||||
assert images[0].width == DISPLAY_W
|
||||
|
||||
def test_single_oversized_image_capped_with_trim_off(self):
|
||||
adapter = adapter_with(auto_trim=False, max_plugin_width_ratio=1.0)
|
||||
images = adapter.get_content(
|
||||
NativePlugin([canvas([(0, 6898)], width=6898)]), 'leaderboard')
|
||||
assert images[0].width <= DISPLAY_W + DISPLAY_W // 16
|
||||
|
||||
|
||||
class TestBudgetUsesMeasuredGaps:
|
||||
"""
|
||||
The budget must count the gaps the compositor actually inserts. Assuming the
|
||||
flat intra_plugin_gap under-counted by up to
|
||||
(min_content_separation - intra_plugin_gap) per row, so a many-row plugin
|
||||
overran its cap.
|
||||
"""
|
||||
|
||||
def test_flush_rows_are_budgeted_with_the_measured_gap(self):
|
||||
# 6 flush rows of 100px against a 512px budget. With 24px measured gaps
|
||||
# only 4 fit (400 + 3*24 = 472; a 5th would be 596).
|
||||
adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0,
|
||||
intra_plugin_gap=8, min_content_separation=24)
|
||||
rows = [Image.new('RGB', (100, DISPLAY_H), (255, 255, 255)) for _ in range(6)]
|
||||
images = adapter.get_content(NativePlugin(rows), 'rows')
|
||||
n = len(images)
|
||||
assert 100 * n + 24 * (n - 1) <= DISPLAY_W
|
||||
assert n == 4
|
||||
|
||||
def test_larger_separation_fits_fewer_rows(self):
|
||||
rows = [Image.new('RGB', (100, DISPLAY_H), (255, 255, 255)) for _ in range(8)]
|
||||
tight = adapter_with(content_padding=0, max_plugin_width_ratio=1.0,
|
||||
intra_plugin_gap=0, min_content_separation=0)
|
||||
loose = adapter_with(content_padding=0, max_plugin_width_ratio=1.0,
|
||||
intra_plugin_gap=0, min_content_separation=48)
|
||||
assert len(loose.get_content(NativePlugin(rows), 'r')) < \
|
||||
len(tight.get_content(NativePlugin(rows), 'r'))
|
||||
|
||||
def test_composed_block_respects_the_budget_end_to_end(self):
|
||||
# The real invariant: what the compositor produces must fit the cap.
|
||||
from src.vegas_mode.render_pipeline import RenderPipeline
|
||||
|
||||
cfg = dict(content_padding=0, max_plugin_width_ratio=1.0,
|
||||
intra_plugin_gap=8, min_content_separation=24)
|
||||
adapter = adapter_with(**cfg)
|
||||
rows = [Image.new('RGB', (90, DISPLAY_H), (255, 255, 255)) for _ in range(9)]
|
||||
selected = adapter.get_content(NativePlugin(rows), 'rows')
|
||||
|
||||
class FakeStream:
|
||||
def get_grouped_content_for_composition(self):
|
||||
return [('rows', selected)]
|
||||
|
||||
def get_active_plugin_ids(self):
|
||||
return ['rows']
|
||||
|
||||
class DM:
|
||||
width = DISPLAY_W
|
||||
height = DISPLAY_H
|
||||
|
||||
def set_scrolling_state(self, *a):
|
||||
pass
|
||||
|
||||
p = RenderPipeline(VegasModeConfig(lead_in_width=0, **cfg), DM(), FakeStream())
|
||||
assert p._join_plugin_rows(selected).width <= DISPLAY_W
|
||||
|
||||
Reference in New Issue
Block a user