diff --git a/src/vegas_mode/plugin_adapter.py b/src/vegas_mode/plugin_adapter.py index 0da51ce2..a86cf8fc 100644 --- a/src/vegas_mode/plugin_adapter.py +++ b/src/vegas_mode/plugin_adapter.py @@ -68,6 +68,21 @@ class PluginAdapter: # always the same opening items. self._item_offsets: dict = {} + # What the matching entry in _item_offsets is an offset *into*, as + # (kind, size). An offset only means anything against the content it + # was derived from, and there are three incompatible kinds: + # + # ('rows', n) index into a list of n images + # ('cuts', n) index into the n item boundaries of one image + # ('cols', w) pixel column in a w-wide image with no item boundaries + # + # Without this the offsets were reused across kinds — a plugin that + # returned one wide image on one fetch and several rows on the next had + # a pixel column of 1400 read back as a row index — and across content + # changes, where a column recorded against a 9,793px news strip pointed + # into unrelated headlines once the strip refreshed to 9,505px. + self._offset_shapes: dict = {} + logger.info( "PluginAdapter initialized: display=%dx%d", self.display_width, self.display_height @@ -398,6 +413,88 @@ class PluginAdapter: return 0 return int(self.display_width * ratio) + def _resume_offset(self, plugin_id: str, shape: Tuple[str, int]) -> int: + """ + The plugin's stored rotation offset, if it still applies. + + An offset is only meaningful against content shaped the way it was + when the offset was recorded. When the shape has changed — a different + number of rows, a re-rendered strip with different item boundaries — + the stored value points somewhere arbitrary, so rotation restarts. + + Args: + plugin_id: Plugin identifier + shape: (kind, size) describing what an offset would index into now + + Returns: + The stored offset, or 0 when it no longer applies + """ + if self._offset_shapes.get(plugin_id) != shape: + if plugin_id in self._item_offsets: + logger.info( + "[%s] Content is %s now, was %s — restarting the rotation " + "rather than resuming at a position that no longer means " + "anything", plugin_id, shape, + self._offset_shapes.get(plugin_id)) + self._item_offsets.pop(plugin_id, None) + self._offset_shapes[plugin_id] = shape + return 0 + return self._item_offsets.get(plugin_id, 0) + + def _record_offset( + self, plugin_id: str, offset: int, shape: Tuple[str, int] + ) -> None: + """Store where the next window should resume, with what it indexes.""" + if offset: + self._item_offsets[plugin_id] = offset + self._offset_shapes[plugin_id] = shape + else: + # A wrapped-to-zero rotation is the same as no state at all, and + # keeping the key would report a window as active when the next + # pass starts from the top anyway. + self._item_offsets.pop(plugin_id, None) + self._offset_shapes.pop(plugin_id, None) + + def _clear_offset(self, plugin_id: str) -> None: + """Forget any rotation state for a plugin.""" + self._item_offsets.pop(plugin_id, None) + self._offset_shapes.pop(plugin_id, None) + + def _merge_trailing_runt(self, end: int, width: int, budget: int) -> int: + """ + Extend a window to the end of the content when what would be left over + is too small to be worth its own pass. + + Windows were placed by walking forward from the last one, which makes + the final window whatever happens to remain. Measured on a live panel + that produced a 1,840px stocks ticker splitting 1,492 + 348 — the + second pass showing seven seconds of content before cutting, which + reads as the display failing rather than as a rotation. + + Absorbing the remainder overruns the budget by less than one window + floor, which is a better trade than a fragment: the budget is a guard + against one plugin holding the panel for minutes, not a hard limit. + + Args: + end: Column the window would otherwise end at + width: Full content width + budget: Width budget being applied + + Returns: + ``end``, or ``width`` when the remainder is below the floor + """ + remainder = width - end + # Measured against the budget rather than the panel: snapping to item + # boundaries means an ordinary window already lands short of the budget + # (a 512px budget over 182px-pitch items yields 348px windows), so an + # absolute floor would merge windows that were never fragments. Half a + # budget separates "a short last pass" from "a sliver", and caps the + # overrun this can cause at 1.5 budgets. + floor = budget // 2 + if 0 < remainder < floor: + return width + return end + def _apply_width_budget( self, images: List[Image.Image], plugin_id: str, plugin: Optional['BasePlugin'] = None @@ -435,19 +532,20 @@ class PluginAdapter: if not budget or total <= budget: # Fits, so reset rotation — the whole segment is being shown. - self._item_offsets.pop(plugin_id, None) + self._clear_offset(plugin_id) return images if len(images) == 1: return [self._crop_to_budget(images[0], budget, plugin_id, mode)] + shape = ('rows', len(images)) if mode == 'truncate': # Ordered content: always show from the top. Deliberately does not # advance the offset, so the same opening items appear every time # rather than the viewer being shown the middle of a ranked list. start = 0 else: - start = self._item_offsets.get(plugin_id, 0) % len(images) + start = self._resume_offset(plugin_id, shape) % len(images) selected: List[Image.Image] = [] used = 0 consumed = 0 @@ -472,7 +570,8 @@ class PluginAdapter: plugin_id, budget, len(selected), len(images), used ) else: - self._item_offsets[plugin_id] = (start + consumed) % len(images) + self._record_offset( + plugin_id, (start + consumed) % len(images), shape) logger.info( "[%s] Width budget %dpx: showing %d of %d row(s) (%dpx incl. gaps) " "from offset %d; remainder deferred to a later cycle", @@ -490,16 +589,13 @@ class PluginAdapter: The cut is snapped to the nearest blank column so it does not slice through a glyph or logo and leave half a character at the panel edge. - """ - if mode == 'truncate': - # Always the start of the strip, so a ranked table is never entered - # from the middle. - offset = 0 - else: - offset = self._item_offsets.get(plugin_id, 0) - if offset >= img.width: - offset = 0 + Rotation is tracked as an index into the strip's item boundaries rather + than as a pixel column, because a ticker re-renders between fetches. A + column recorded against one render points at unrelated content in the + next as soon as anything ahead of it changes width — a digit in a + price, a shorter headline. The Nth boundary stays the Nth boundary. + """ # Cut only where the plugin left a real gap between items. Snapping to # any blank column used to pick the single-column gaps between # characters, splitting a word and orphaning its tail into the next @@ -514,9 +610,17 @@ class PluginAdapter: # budget exactly. The gap rule exists to protect discrete items # (words, ticker entries); it would be wrong to let a solid image # escape the cap in its name. - end = min(offset + budget, img.width) + # + # With no items to index, the offset here has to stay a column, so + # it is only reusable while the image keeps its width. + shape = ('cols', img.width) + offset = 0 if mode == 'truncate' else self._resume_offset( + plugin_id, shape) + end = self._merge_trailing_runt( + min(offset + budget, img.width), img.width, budget) if mode != 'truncate': - self._item_offsets[plugin_id] = 0 if end >= img.width else end + self._record_offset( + plugin_id, 0 if end >= img.width else end, shape) logger.info( "[%s] Width budget %dpx: cropped continuous %dpx image to " "[%d:%d] (no item gaps of %dpx+ to align to)%s", @@ -528,8 +632,15 @@ class PluginAdapter: # Cut mid-gap so the content either side keeps some breathing room. cuts = sorted({0, img.width} | {(a + b) // 2 for a, b in gaps}) - start = max((c for c in cuts if c <= offset), default=0) - later = [c for c in cuts if c > start] + shape = ('cuts', len(cuts)) + index = 0 if mode == 'truncate' else self._resume_offset( + plugin_id, shape) + # Clamped rather than wrapped: a stale index past the end means the + # strip shrank, and restarting reads better than landing near the end. + start_index = index if 0 <= index < len(cuts) - 1 else 0 + start = cuts[start_index] + + later = cuts[start_index + 1:] if not later: end = img.width else: @@ -537,15 +648,22 @@ class PluginAdapter: # No boundary inside the budget: take the next one and overrun, # because the alternative is cutting through an item. end = max(within) if within else min(later) + end = self._merge_trailing_runt(end, img.width, budget) + # Every candidate for `end` came from `cuts` (which includes img.width), + # so this always resolves; the fallback is defensive only. + end_index = cuts.index(end) if end in cuts else len(cuts) - 1 if mode != 'truncate': - # Next cycle resumes where this one stopped; wrap when the strip ends. - self._item_offsets[plugin_id] = 0 if end >= img.width else end + # Next cycle resumes at the boundary this one stopped on; wrap when + # the strip ends. + self._record_offset( + plugin_id, 0 if end >= img.width else end_index, shape) logger.info( "[%s] Width budget %dpx: cropped single %dpx image to [%d:%d] " - "(%dpx) at item boundaries, %s", + "(%dpx) at item boundaries %d-%d of %d, %s", plugin_id, budget, img.width, start, end, end - start, + start_index, end_index, len(cuts) - 1, "showing the start only (overflow=truncate)" if mode == 'truncate' else "window advances next cycle" ) diff --git a/test/test_vegas_density.py b/test/test_vegas_density.py index f0c1fd72..b41b1570 100644 --- a/test/test_vegas_density.py +++ b/test/test_vegas_density.py @@ -1643,3 +1643,178 @@ class TestPerPluginWidthBudget: strip = canvas([(0, 5000)], width=5000) adapter.get_content(NativePlugin([strip]), 'ticker') assert adapter._item_offsets.get('ticker', 0) > 0 + + +def ticker(item_widths, gap=32, height=DISPLAY_H): + """ + A strip of discrete items separated by real gaps, like a news or stocks + ticker. Wide enough gaps that blank_runs() sees item boundaries, which is + what puts _crop_to_budget on its item-aligned path rather than treating the + strip as one continuous block. + """ + width = sum(item_widths) + gap * (len(item_widths) - 1) + spans, x = [], 0 + for w in item_widths: + spans.append((x, x + w)) + x += w + gap + return canvas(spans, width=width, height=height) + + +class TestTrailingRuntWindow: + """ + A rotation's last window used to be whatever happened to be left over. + Measured on a live 512px panel, a 1,840px stocks ticker against a 1,536px + budget split 1,492 + 348 — the second pass showed seven seconds and cut. + """ + + def test_a_barely_oversized_strip_is_shown_whole(self): + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0) + # 1.2 budgets wide: splitting it can only ever produce a fragment. + strip = ticker([180] * 12) # 2160 + 352 gaps = 2512px vs 512 budget + assert strip.width > DISPLAY_W + + adapter = adapter_with(content_padding=0, + max_plugin_width_ratio=strip.width / DISPLAY_W * 0.9) + shown = adapter.get_content(NativePlugin([strip]), 'stocks')[0] + assert shown.width == strip.width, "should absorb the runt, not split" + assert 'stocks' not in adapter._item_offsets + + def test_no_window_in_a_rotation_is_a_fragment(self): + # Walk a long ticker all the way round; every pass must be worth + # showing rather than one of them being a leftover sliver. + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0) + strip = ticker([150] * 40) + plugin = NativePlugin([strip]) + + widths, seen_offsets = [], set() + for _ in range(20): + adapter.invalidate_cache('news') + widths.append(adapter.get_content(plugin, 'news')[0].width) + offset = adapter._item_offsets.get('news', 0) + if offset in seen_offsets: + break + seen_offsets.add(offset) + + assert len(widths) > 1, "a strip this long must take several passes" + # Item snapping means an ordinary window lands short of the budget, so + # the bar is "not a sliver" rather than "a full budget". + assert min(widths) >= DISPLAY_W // 2, ( + "no window should be a fragment, got %r" % widths) + assert max(widths) <= DISPLAY_W * 1.5, ( + "absorbing a runt must stay bounded, got %r" % widths) + + def test_a_continuous_image_also_absorbs_its_runt(self): + # The no-item-gaps path had the same leftover problem. + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0) + solid = canvas([(0, 700)], width=700) # 512 budget -> 512 + 188 runt + first = adapter.get_content(NativePlugin([solid]), 'chart')[0] + assert first.width == 700, "188px tail is not worth its own pass" + assert 'chart' not in adapter._item_offsets + + def test_the_reported_stocks_case(self): + # The exact numbers logged on a 512px panel: an 1,840px stocks ticker + # against a 1,536px budget split 1,492 + 348, so every other appearance + # showed seven seconds of stocks and cut. It should now come through in + # one piece, 20% over budget being the better of the two outcomes. + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=3.0) + # 10 items of 152px with 32px gaps = 1520 + 288 = 1808, near enough. + strip = ticker([152] * 10) + assert DISPLAY_W * 3 < strip.width < DISPLAY_W * 4 + + widths = [] + for _ in range(3): + adapter.invalidate_cache('stocks') + widths.append(adapter.get_content( + NativePlugin([strip]), 'stocks')[0].width) + + assert widths == [strip.width] * 3, ( + "a strip this close to the budget should be shown whole every " + "time, not split into a big pass and a sliver; got %r" % widths) + + def test_a_genuinely_long_strip_still_gets_capped(self): + # Absorbing runts must not become "never cap anything". + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0) + strip = ticker([150] * 60) + shown = adapter.get_content(NativePlugin([strip]), 'long')[0] + assert shown.width < strip.width + assert shown.width <= DISPLAY_W * 2 + + +class TestOffsetOutlivesItsContent: + """ + A rotation offset only means something against the content it was recorded + against. news re-rendered 9,793px -> 9,505px mid-rotation while its stored + column kept advancing, so the window pointed into unrelated headlines. + """ + + def test_rotation_survives_items_changing_width(self): + # Same items, each a little wider — a price gaining a digit. The window + # should resume at the same *item*, not at a now-meaningless column. + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0) + adapter.get_content(NativePlugin([ticker([150] * 40)]), 'stocks') + first = adapter._item_offsets.get('stocks') + assert first, "the first pass should leave a resume point" + + adapter.invalidate_cache('stocks') + adapter.get_content(NativePlugin([ticker([158] * 40)]), 'stocks') + assert adapter._item_offsets.get('stocks', 0) > first, ( + "same item count means the offset still applies and should advance") + + def test_rotation_restarts_when_the_item_count_changes(self): + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0) + adapter.get_content(NativePlugin([ticker([150] * 40)]), 'news') + assert adapter._item_offsets.get('news', 0) > 0 + + # A fresh headline set with fewer entries: the old position is + # meaningless, so the next pass starts at the top. + adapter.invalidate_cache('news') + shown = adapter.get_content(NativePlugin([ticker([150] * 25)]), 'news')[0] + expected = adapter.get_content( + NativePlugin([ticker([150] * 25)]), 'fresh')[0] + assert shown.width == expected.width + + def test_a_row_index_is_never_read_back_as_a_pixel_column(self): + # The unit collision: _apply_width_budget stores an index into a list + # of rows, _crop_to_budget a column in one image, under the same key. + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0, + intra_plugin_gap=0, min_content_separation=0) + rows = [canvas([(0, 200)], width=200) for _ in range(8)] + adapter.get_content(NativePlugin(rows), 'mixed') + assert adapter._item_offsets.get('mixed', 0) > 0 + assert adapter._offset_shapes['mixed'][0] == 'rows' + + # Now the same plugin returns one wide strip instead. The row index + # must not be read as a column into it: the strip is entered at the + # top, exactly as it would be for a plugin with no history at all. + strip = ticker([150] * 40) + adapter.invalidate_cache('mixed') + carried = adapter.get_content(NativePlugin([strip]), 'mixed')[0] + assert adapter._offset_shapes['mixed'][0] == 'cuts' + + clean = adapter_with(content_padding=0, max_plugin_width_ratio=1.0, + intra_plugin_gap=0, min_content_separation=0) + assert carried.tobytes() == clean.get_content( + NativePlugin([strip]), 'clean')[0].tobytes() + + def test_a_stale_index_past_the_end_restarts(self): + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0) + strip = ticker([150] * 40) + adapter.get_content(NativePlugin([strip]), 'news') + # Force an index far beyond anything the current strip has, keeping the + # shape intact so the guard does not catch it first. + shape = adapter._offset_shapes['news'] + adapter._item_offsets['news'] = 10_000 + adapter.invalidate_cache('news') + shown = adapter.get_content(NativePlugin([strip]), 'news')[0] + assert shown.width > 0 + assert adapter._offset_shapes['news'] == shape + + def test_content_that_fits_clears_both_offset_and_shape(self): + adapter = adapter_with(content_padding=0, max_plugin_width_ratio=1.0) + adapter.get_content(NativePlugin([ticker([150] * 40)]), 'shrink') + assert 'shrink' in adapter._offset_shapes + + adapter.invalidate_cache('shrink') + adapter.get_content(NativePlugin([canvas([(0, 100)], width=100)]), 'shrink') + assert 'shrink' not in adapter._item_offsets + assert 'shrink' not in adapter._offset_shapes