From 596809acc34605817e012ed6392a8302fd680b4a Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Wed, 30 Sep 2026 20:50:34 -0400 Subject: [PATCH] perf(scroll): build the strip's PIL image only when something reads it (#695) * perf(timing): say which render-thread work a late frame followed The soak already says how often a moving frame reached the panel late, but not what the render thread was doing just before it. Vegas does two kinds of work there between frames -- building its strip (compose, extend) and, with live elements, patching changed pixels into it -- and deciding whether either is affordable needs their own numbers. - FrameTimingRecorder.note_op(kind, nbytes) tags the next presented frame. Totals gain op_frames, late_op_frames, op_freezes and op_bytes per kind; aggregate() still takes frames without ops. The file schema is unchanged. - Vegas tags compose and every strip extension (with the bytes it copied). - frame_soak prints an "after work" table: frames, late %, freezes and MB moved per kind, only when something tagged its work. - render_bench gains --strip-screens (Vegas-sized strips), --patch-bytes / --patch-every / --patch-where (in-place column writes, as a live element update does) and --extend-every-screens / --extend-width (append + trim on a fixed cadence that holds the strip's width). No runtime behaviour changes: this is the measurement gate for live Vegas elements. Co-Authored-By: Claude Opus 5.5 * docs(changelog): note the frame-op attribution and bench modes Co-Authored-By: Claude Opus 5.5 * perf(scroll): build the strip's PIL image only when something reads it Every Vegas strip extension rebuilt ScrollHelper.cached_image from cached_array in full, twice (append, then trim), on the render thread: Image.fromarray is 1.7ms for an 8,000px strip and 3.8ms for 20,000px on a Pi 4 (measured on ledpi), about two thirds of an extension's render-thread cost. Nothing on the frame path reads the image's pixels; every frame is cut from the array. cached_image is now a property. append_content and drop_scrolled_prefix defer it; the first read builds it from the array it started with and keeps it only if the strip has not changed meanwhile, so a sync push racing an extension cannot leave a stale image cached. Assigning cached_image stores exactly what was assigned, as before. has_strip() says whether there is a strip without building its image; the helper's frame path, Vegas and the adapter's scroll-cache invalidation use it. The strip is also no longer held in memory twice. In Vegas the image is now built only by a multi-display sync push. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 14 ++ src/common/scroll_helper.py | 89 ++++++++--- src/vegas_mode/plugin_adapter.py | 9 +- src/vegas_mode/render_pipeline.py | 21 +-- test/test_scroll_helper_lazy_image.py | 218 ++++++++++++++++++++++++++ 5 files changed, 322 insertions(+), 29 deletions(-) create mode 100644 test/test_scroll_helper_lazy_image.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d86e82b..2fbd7e78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -332,6 +332,20 @@ read any of them: worker, which applies the latest one as soon as the lock frees, and before the plugin's next update() at the latest. The plugin API is unchanged. +### Scrolling + +- A Vegas strip extension costs the render thread about a third of what it + did. Appending the next group and trimming what has scrolled past each + rebuilt the strip's PIL image from its numpy array in full + (`Image.fromarray`: 1.7ms for an 8,000px strip, 3.8ms for 20,000px, on a + Pi 4 -- twice per extension), though every frame is cut from the array and + nothing on the frame path reads the image's pixels. `ScrollHelper` now + builds `cached_image` only when something reads it, which in Vegas means + only a multi-display sync push, and the strip is no longer held in memory + twice. Assigning `cached_image` still stores exactly what was assigned. + New `ScrollHelper.has_strip()` says whether there is a strip without + building its image; the frame path and Vegas use it. + ### Tooling - The frame-timing recorder says which render-thread work a late frame diff --git a/src/common/scroll_helper.py b/src/common/scroll_helper.py index 21f694b4..2072a532 100644 --- a/src/common/scroll_helper.py +++ b/src/common/scroll_helper.py @@ -111,7 +111,7 @@ class ScrollHelper: self.total_distance_scrolled = 0.0 # Track total distance including wrap-arounds self.scroll_speed = 1.0 self.scroll_delay = 0.001 # Minimal delay for high FPS (1ms) - self.cached_image: Optional[Image.Image] = None + self.cached_image = None # see the property below self.cached_array: Optional[np.ndarray] = None # Numpy array cache for fast operations self.total_scroll_width = 0 @@ -172,7 +172,53 @@ class ScrollHelper: # Scrolling state management self.is_scrolling = False self.scroll_complete = False - + + # -- the strip as a PIL image --------------------------------------------- + # + # Every frame is cut from cached_array; nothing on the frame path reads the + # PIL image's pixels. Extending and trimming a strip (append_content, + # drop_scrolled_prefix) used to rebuild that image in full each time + # anyway: Image.fromarray of a Vegas-sized strip is 1.7-3.8ms on a Pi 4, + # twice per extension, on the render thread. Those two now leave it to be + # built from the array on first read, which in Vegas means only by a + # multi-display sync push -- and the strip is not held twice in memory. + # + # Assigning cached_image still stores exactly what was assigned; a lazy + # image is only ever one the helper derived from its own array. + + @property + def cached_image(self) -> Optional[Image.Image]: + """The strip as a PIL image, built from ``cached_array`` if deferred.""" + image = self.__dict__.get('_cached_image') + if image is not None: + return image + source = self.__dict__.get('_image_source') + if source is None: + return None + # Built from the array this read started with. Another thread (the + # sync push) may read while the render thread extends the strip; it + # then gets the strip as it was, as it did when the image was built + # eagerly, and the stale build is not kept. + image = Image.fromarray(source) + if self.__dict__.get('_image_source') is source: + self._cached_image = image + return image + + @cached_image.setter + def cached_image(self, image: Optional[Image.Image]) -> None: + self._cached_image = image + self._image_source = None + + def _defer_image(self) -> None: + """The array just changed under the image: rebuild it only if read.""" + self._cached_image = None + self._image_source = self.cached_array + + def has_strip(self) -> bool: + """Whether there is a strip (an image, or one deferred), not reading it.""" + return (self.__dict__.get('_cached_image') is not None + or self.__dict__.get('_image_source') is not None) + def create_scrolling_image(self, content_items: list, item_gap: int = 32, element_gap: int = 16, @@ -283,7 +329,7 @@ class ScrollHelper: Otherwise the position advances by elapsed time at the configured speed. """ - if not self.cached_image: + if not self.has_strip(): return # Calculate frame time for consistent scroll speed regardless of FPS @@ -427,7 +473,7 @@ class ScrollHelper: Returns: PIL Image showing the visible portion, or None if no cached image """ - if not self.cached_image or self.cached_array is None: + if self.cached_array is None or not self.has_strip(): return None start_x_int = int(self.scroll_position) @@ -501,7 +547,7 @@ class ScrollHelper: slices (128×32 = 12 KB) used here. """ _size = (self.display_width, self.display_height) - img_w = self.cached_image.width + img_w = self.cached_array.shape[1] if end_x <= img_w: # Normal case: single contiguous slice (fastest path) @@ -646,7 +692,7 @@ class ScrollHelper: if not content_items: return False - if self.cached_image is None or self.cached_array is None: + if self.cached_array is None or not self.has_strip(): # Nothing to extend yet — this is just the first build. self.create_scrolling_image( content_items, item_gap=item_gap, element_gap=element_gap, lead_gap=0) @@ -666,13 +712,14 @@ class ScrollHelper: addition.paste(img, (x, 0)) x += img.width + element_gap - # numpy concatenate then one conversion back, rather than allocating a - # full-width PIL image and pasting twice: the strip can be tens of - # thousands of columns wide and this runs on the render path. + # numpy concatenate, and no conversion back: the strip can be tens of + # thousands of columns wide and this runs on the render path. The PIL + # image is built from the array only if something reads it (see the + # cached_image property). self.cached_array = np.concatenate( (self.cached_array, np.array(addition)), axis=1) - self.cached_image = Image.fromarray(self.cached_array) - self.total_scroll_width = self.cached_image.width + self._defer_image() + self.total_scroll_width = self.cached_array.shape[1] self.scroll_complete = False self.logger.info( @@ -699,14 +746,15 @@ class ScrollHelper: Returns: Number of columns actually removed """ - if self.cached_image is None or self.cached_array is None: + if self.cached_array is None or not self.has_strip(): return 0 + strip_width = self.cached_array.shape[1] # While the viewport wraps, get_visible_portion fills its right-hand side # from the *head* of the strip, so trimming the head would change what # is on screen. Continuous mode extends before ever reaching that state; # refusing here keeps "trimming is invisible" true unconditionally. - if self.scroll_position + self.display_width > self.cached_image.width: + if self.scroll_position + self.display_width > strip_width: return 0 cut = int(self.scroll_position) - max(0, keep_before) @@ -714,15 +762,15 @@ class ScrollHelper: return 0 # Never trim so far that the remaining strip is narrower than the # viewport, or get_visible_portion has nothing to slice. - cut = min(cut, max(0, self.cached_image.width - self.display_width)) + cut = min(cut, max(0, strip_width - self.display_width)) if cut <= 0: return 0 # .copy() so the original buffer is released rather than kept alive by - # a numpy view. + # a numpy view. The PIL image is deferred, as in append_content. self.cached_array = self.cached_array[:, cut:].copy() - self.cached_image = Image.fromarray(self.cached_array) - self.total_scroll_width = self.cached_image.width + self._defer_image() + self.total_scroll_width = self.cached_array.shape[1] self.scroll_position -= cut self.total_distance_scrolled = max(0.0, self.total_distance_scrolled - cut) @@ -734,7 +782,7 @@ class ScrollHelper: def remaining_unscrolled(self) -> int: """Columns of strip still to the right of the viewport.""" - if self.cached_image is None: + if not self.has_strip(): return 0 return max(0, self.total_scroll_width - int(self.scroll_position) - self.display_width) @@ -1082,5 +1130,8 @@ class ScrollHelper: 'elapsed_time': (time.time() - self.scroll_start_time) if self.scroll_start_time else None, - 'cached_image_size': (self.cached_image.width, self.cached_image.height) if self.cached_image else None + # From the array: reading cached_image would build a deferred one. + 'cached_image_size': ((self.cached_array.shape[1], self.cached_array.shape[0]) + if self.cached_array is not None and self.has_strip() + else None) } diff --git a/src/vegas_mode/plugin_adapter.py b/src/vegas_mode/plugin_adapter.py index 8dd62187..131938a1 100644 --- a/src/vegas_mode/plugin_adapter.py +++ b/src/vegas_mode/plugin_adapter.py @@ -12,6 +12,7 @@ from contextlib import contextmanager, nullcontext from typing import Optional, List, Any, Tuple, Union, TYPE_CHECKING from PIL import Image +from src.common.scroll_helper import ScrollHelper from src.vegas_mode.geometry import ( blank_runs, separation_gap, @@ -1354,7 +1355,13 @@ class PluginAdapter: if helper is None: continue try: - if getattr(helper, 'cached_image', None) is not None: + # has_strip() rather than reading cached_image, which would + # build a deferred image only to throw it away. + if isinstance(helper, ScrollHelper): + has_image = helper.has_strip() + else: + has_image = getattr(helper, 'cached_image', None) is not None + if has_image: helper.cached_image = None cleared = True if getattr(helper, 'cached_array', None) is not None: diff --git a/src/vegas_mode/render_pipeline.py b/src/vegas_mode/render_pipeline.py index 8558a0a8..449e4e91 100644 --- a/src/vegas_mode/render_pipeline.py +++ b/src/vegas_mode/render_pipeline.py @@ -323,7 +323,7 @@ class RenderPipeline: ) # Verify scroll image was created successfully - if not self.scroll_helper.cached_image: + if not self.scroll_helper.has_strip(): logger.error("ScrollHelper failed to create cached image") return False @@ -341,7 +341,7 @@ class RenderPipeline: "Composed scroll image: %dx%d, %d plugin block(s), %d rows, " "separator=%dpx between plugins, rows spaced to %dpx of ink " "(min added %dpx)", - self.scroll_helper.cached_image.width if self.scroll_helper.cached_image else 0, + self.scroll_helper.total_scroll_width if self.scroll_helper.has_strip() else 0, self.display_height, len(blocks), total_rows, @@ -429,7 +429,7 @@ class RenderPipeline: Cheap enough to call every frame: it is arithmetic over cached state. """ - if not self.config.continuous_scroll or not self.scroll_helper.cached_image: + if not self.config.continuous_scroll or not self.scroll_helper.has_strip(): return False threshold = int(self.display_width * self.config.extend_threshold_screens) return self.scroll_helper.remaining_unscrolled() <= threshold @@ -599,8 +599,10 @@ class RenderPipeline: 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) + # From the helper's own bookkeeping, never cached_image: reading + # that would build the full PIL strip the helper now defers. + strip_end = (self.scroll_helper.total_scroll_width + if self.scroll_helper.has_strip() 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 @@ -636,7 +638,7 @@ class RenderPipeline: total_rows += len(images) blocks.append(self._join_plugin_rows(images)) - had_strip = self.scroll_helper.cached_image is not None + had_strip = self.scroll_helper.has_strip() appended = self.scroll_helper.append_content( content_items=blocks, item_gap=self.config.separator_width, @@ -740,7 +742,7 @@ class RenderPipeline: frame_start = time.time() try: - if not self.scroll_helper.cached_image: + if not self.scroll_helper.has_strip(): return False # Update scroll position @@ -980,10 +982,11 @@ class RenderPipeline: self.sync_manager.send_new_cycle() # Push the actual scroll image over TCP so follower has identical pixels. # Done in a background thread to not block the render loop (~15ms transfer). - if self.scroll_helper.cached_image is not None: + image = self.scroll_helper.cached_image + if image is not None: threading.Thread( target=self.sync_manager.send_scroll_image, - args=(self.scroll_helper.cached_image,), + args=(image,), daemon=True, name="sync-image-push" ).start() diff --git a/test/test_scroll_helper_lazy_image.py b/test/test_scroll_helper_lazy_image.py new file mode 100644 index 00000000..e49bc5f0 --- /dev/null +++ b/test/test_scroll_helper_lazy_image.py @@ -0,0 +1,218 @@ +"""ScrollHelper builds the strip's PIL image only when something reads it. + +Vegas extends and trims one long strip on the render thread. Each of those used +to rebuild ``cached_image`` from ``cached_array`` in full -- 1.7-3.8ms apiece on +a Pi 4 for a Vegas-sized strip, twice per extension -- though nothing on the +frame path reads the image's pixels. These tests pin that the frame path never +builds it, that a read still gets the right pixels, and that the two threads +which do read it (a multi-display sync push, the render thread) cannot leave a +stale image behind. +""" +import sys +from pathlib import Path + +import numpy as np +import pytest +from PIL import Image + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from src.common import scroll_helper as scroll_helper_module # noqa: E402 +from src.common.scroll_helper import ScrollHelper # noqa: E402 + +W, H = 64, 16 + + +def _block(width, seed): + # frombytes, not fromarray: the no_fromarray fixture refuses the latter + # everywhere, and a test block is not the strip. + rng = np.random.default_rng(seed) + pixels = rng.integers(0, 255, (H, width, 3), dtype=np.uint8) + return Image.frombytes("RGB", (width, H), pixels.tobytes()) + + +def _helper(width=400): + helper = ScrollHelper(W, H) + helper.set_scrolling_image(_block(width, 0)) + return helper + + +@pytest.fixture +def no_fromarray(monkeypatch): + """Fail if the helper builds a PIL image from its array.""" + def refuse(*_a, **_k): + raise AssertionError("the strip's PIL image was built") + monkeypatch.setattr(scroll_helper_module.Image, "fromarray", refuse) + + +def test_append_and_trim_do_not_build_the_image(no_fromarray): + helper = _helper() + helper.scroll_position = 300.0 + assert helper.append_content([_block(200, 1)], item_gap=8) + assert helper.drop_scrolled_prefix(keep_before=W) > 0 + assert helper.__dict__["_cached_image"] is None + + +def test_the_frame_path_never_builds_the_image(no_fromarray): + helper = _helper() + helper.set_pixels_per_frame(2) + for i in range(400): + if helper.remaining_unscrolled() <= 2 * W: + helper.append_content([_block(150, i)], item_gap=8) + helper.drop_scrolled_prefix(keep_before=W) + helper.update_scroll_position() + frame = helper.get_visible_portion() + assert frame is not None and frame.size == (W, H) + helper.get_scroll_info() + assert not helper.is_scroll_complete() + + +def test_reading_the_image_gives_the_strip_as_it_is(): + helper = _helper() + helper.scroll_position = 250.0 + helper.append_content([_block(120, 7)], item_gap=8) + helper.drop_scrolled_prefix(keep_before=W) + image = helper.cached_image + assert image.size == (helper.cached_array.shape[1], H) + assert np.array_equal(np.asarray(image), helper.cached_array) + assert helper.total_scroll_width == image.width + + +def test_the_built_image_is_kept_until_the_strip_changes(): + helper = _helper() + helper.append_content([_block(50, 1)], item_gap=0) + first = helper.cached_image + assert helper.cached_image is first + helper.append_content([_block(50, 2)], item_gap=0) + assert helper.cached_image is not first + assert helper.cached_image.width == first.width + 50 + + +def test_an_assigned_image_is_kept_exactly(): + # Plugins and the multi-display follower assign cached_image themselves. + helper = _helper() + helper.append_content([_block(50, 1)], item_gap=0) + mine = _block(99, 3) + helper.cached_image = mine + assert helper.cached_image is mine + helper.cached_image = None + assert helper.cached_image is None + + +def test_frames_are_the_same_as_with_an_eager_image(): + lazy = _helper() + eager = _helper() + blocks = [_block(90, 10 + i) for i in range(6)] + for i, block in enumerate(blocks): + for helper in (lazy, eager): + helper.scroll_position = 60.0 * (i + 1) + helper.append_content([block], item_gap=8) + helper.drop_scrolled_prefix(keep_before=W) + eager.cached_image = Image.fromarray(eager.cached_array) # the old way + for x in range(0, lazy.cached_array.shape[1] - W, 7): + lazy.scroll_position = eager.scroll_position = float(x) + assert lazy.get_visible_portion().tobytes() == \ + eager.get_visible_portion().tobytes() + + +def test_a_read_racing_an_extension_does_not_keep_a_stale_image(monkeypatch): + # The sync push reads the image on its own thread. If the render thread + # extends the strip while that read is building the image, the read gets + # the strip as it was, and the next read must not be handed it again. + helper = _helper() + helper.append_content([_block(40, 1)], item_gap=0) + real = Image.fromarray + + def build_while_the_strip_changes(array, *a, **k): + monkeypatch.setattr(scroll_helper_module.Image, "fromarray", real) + helper.append_content([_block(40, 2)], item_gap=0) # "render thread" + return real(array, *a, **k) + + monkeypatch.setattr(scroll_helper_module.Image, "fromarray", + build_while_the_strip_changes) + before_width = helper.cached_array.shape[1] + raced = helper.cached_image + assert raced.width == before_width + fresh = helper.cached_image + assert fresh.width == before_width + 40 + assert np.array_equal(np.asarray(fresh), helper.cached_array) + + +def test_clearing_the_cache_forgets_a_deferred_image(): + helper = _helper() + helper.append_content([_block(40, 1)], item_gap=0) + helper.clear_cache() + assert helper.cached_image is None + assert not helper.has_strip() + assert helper.get_visible_portion() is None + assert helper.remaining_unscrolled() == 0 + + +def test_a_helper_with_no_strip_has_nothing_to_scroll(): + helper = ScrollHelper(W, H) + assert not helper.has_strip() + helper.update_scroll_position() + assert helper.scroll_position == 0.0 + assert helper.get_scroll_info()["cached_image_size"] is None + + +def test_dropping_a_plugins_scroll_cache_does_not_build_it_first(no_fromarray): + # Vegas clears a plugin's own scroll cache on the render thread whenever + # the plugin updates; that must not build a deferred image to discard it. + from types import SimpleNamespace + + from src.vegas_mode.plugin_adapter import PluginAdapter + + helper = _helper() + helper.append_content([_block(40, 1)], item_gap=0) + dm = SimpleNamespace(width=W, height=H, image=Image.new("RGB", (W, H))) + adapter = PluginAdapter(dm) + assert adapter.invalidate_plugin_scroll_cache( + SimpleNamespace(scroll_helper=helper), "p") + assert helper.cached_array is None and not helper.has_strip() + + +def test_vegas_extends_without_building_the_image(no_fromarray): + from src.vegas_mode.config import VegasModeConfig + from src.vegas_mode.render_pipeline import RenderPipeline + + groups = [[("a", [_block(300, 1)])]] + [[(f"p{i}", [_block(300, i + 2)])] + for i in range(6)] + + class Stream: + plugin_manager = type("PM", (), {"plugins": {}})() + plugin_adapter = None + i = 0 + + def get_grouped_content_for_composition(self): + return groups[0] + + def get_active_plugin_ids(self): + return ["a"] + + def take_next_group(self, count=None, offscreen_only=False): + self.i += 1 + return groups[self.i] if self.i < len(groups) else [] + + class DM: + width, height = W, H + + def __init__(self): + self.image = Image.new("RGB", (W, H)) + + def set_scrolling_state(self, *a): + pass + + def update_display(self): + pass + + p = RenderPipeline(VegasModeConfig(lead_in_width=0, continuous_scroll=True), + DM(), Stream()) + # compose pastes into a new image and converts that to the array once; + # it never needs fromarray either. + assert p.compose_scroll_content() + for _ in range(5): + p.scroll_helper.scroll_position += 250 + assert p.extend_scroll_content() + assert p.render_frame() + assert p.scroll_helper.__dict__["_cached_image"] is None