diff --git a/CLAUDE.md b/CLAUDE.md index d0597854..4f8f7bc8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,7 +41,9 @@ The plugin class **inherits from `BasePlugin`** (`src.plugin_system.base_plugin. in the core repo) and is constructed as `__init__(self, plugin_id, config, display_manager, cache_manager, plugin_manager)`. Key methods (see `plugins/hello-world/manager.py` for a minimal reference): -- `update(self)` — fetch/refresh data (called on `update_interval`); never draw here +- `update(self)` — fetch/refresh data (called on `update_interval`); never draw to + `self.display_manager` here. Pre-rendering *offscreen* images into your own cache + does belong here — see "Pre-rendering" below - `display(self, force_clear=False)` — render via `self.display_manager` then call `update_display()` - `validate_config(self)` — call `super().validate_config()` then check plugin-specific keys - `get_info(self)` / `cleanup(self)` — web-UI info and unload teardown @@ -106,6 +108,26 @@ settings). Typical use: size the on-screen time to the width of scrolling conten or extend live games. See `plugins/football-scoreboard/DYNAMIC_DURATION.md` and the `supports_dynamic_duration` implementations in the sports managers. +### Pre-rendering (expensive images belong in `update()`) +"Never draw in `update()`" means never touch `self.display_manager` there — not +that `update()` may not build images. Building a PIL image into the plugin's own +cache is expensive work, and expensive work is exactly what `update()` is for: +it runs on the update worker, while `display()` runs on the render thread, where +a stall shows up as a frozen panel or a stuttering marquee. So a plugin whose +frame costs real time should render offscreen in `update()` and have `display()` +do nothing but paste the result and draw the parts that must be live (a clock, a +countdown). Precedent: `plugins/f1-scoreboard/manager.py` +(`_prepare_scroll_content`, 12.46s of scroll images), `plugins/ledmatrix-elections` +(`_build_scroll_image`), `plugins/geochron` (`_render_for_size`). + +Two things make this safe: +- **Key the cache on `(width, height)`.** Vegas captures plugins at a narrower + width than the panel (`vegas_width_pct`), so the same plugin is asked to render + at two sizes and a single-entry cache thrashes between them. Re-render every + cached size in `update()`. +- **Keep a lazy path in `display()`.** A size that has never been rendered has to + be built on demand; that's a one-off, not the steady state. + ### High-FPS / smooth scrolling Scrolling plugins render far faster than the default loop for smooth motion. Two mechanisms: diff --git a/docs/plugin-development/01-plugin-anatomy.md b/docs/plugin-development/01-plugin-anatomy.md index 2ecaf56a..364f1e3b 100644 --- a/docs/plugin-development/01-plugin-anatomy.md +++ b/docs/plugin-development/01-plugin-anatomy.md @@ -67,13 +67,13 @@ mandatory, but a useful plugin implements at least `update` and `display`. | Method | Signature | When the core calls it | Rule | |--------|-----------|------------------------|------| | `__init__` | `(self, plugin_id, config, display_manager, cache_manager, plugin_manager)` | Once, at load | Call `super().__init__`; set up state; **don't** fetch or draw | -| `update` | `(self)` | Every `update_interval` seconds | Fetch/refresh data only — **never draw** | +| `update` | `(self)` | Every `update_interval` seconds | Fetch/refresh data and pre-render offscreen — **never touch `display_manager`** | | `display` | `(self, force_clear=False)` | Every render turn | Draw via `self.display_manager`, then call `self.display_manager.update_display()` | | `validate_config` | `(self)` | When validating config | Call `super().validate_config()` first, then check your keys; return `bool` | | `get_info` | `(self)` | For the web UI | `info = super().get_info()`; add keys; return the dict | | `cleanup` | `(self)` | On unload/teardown | Release resources; call `super().cleanup()` | -### `update(self)` — fetch, don't draw +### `update(self)` — fetch and prepare, don't touch the display Called on the interval you set via the `update_interval` config key. This is the **only** place you should hit the network or do expensive work. Store results on @@ -81,6 +81,14 @@ Called on the interval you set via the `update_interval` config key. This is the `self.cache_manager` (see [topic 2](./02-core-api.md#cache-manager)) so a restart or a second plugin doesn't re-hit the API. +"Don't draw here" means **don't touch `self.display_manager`** — don't paste into +its image, don't call `update_display()`. It does *not* mean `update()` may not +build images. `update()` runs on the update worker; `display()` runs on the render +thread, where a slow frame freezes the panel and stalls the Vegas marquee. So if +your frame is expensive to build, build it here, offscreen, into your own cache, +and let `display()` paste it. See [pre-rendering](#pre-rendering-expensive-frames) +below. + ```python def update(self): data = self.cache_manager.get(f"{self.plugin_id}:feed", max_age=self.update_interval) @@ -115,6 +123,50 @@ def display(self, force_clear=False): > Returning `False` lets some plugins signal "nothing to show, skip me." See > [`plugins/hockey-scoreboard/manager.py`](../../plugins/hockey-scoreboard/manager.py). +### Pre-rendering expensive frames + +If building your frame costs real time — stitching a long scroll image, drawing a +world map, compositing dozens of cards — build it in `update()` and paste it in +`display()`. The split is about *which thread pays*: `update()` runs on the +update worker, `display()` on the render thread, so an expensive `display()` +freezes the panel and stalls the Vegas marquee for everyone. + +Two details make this work: + +**Key the cache on `(width, height)`.** Vegas captures plugins at a narrower width +than the panel (`vegas_width_pct`, see [topic 3](./03-advanced-features.md)), so the same +plugin gets asked for the same content at two different sizes. A single-entry +cache misses on every switch and rebuilds from scratch each time — the exact +thrash `plugins/geochron` hit, at ~290 ms per pass on the render thread. Cache per +size, and re-render every cached size in `update()`: + +```python +def update(self): + self.data = self._fetch() # size-independent, computed once + sizes = set(self._frame_cache) # every size asked for so far... + sizes.add((self.display_manager.width, self.display_manager.height)) # ...plus the live panel + for size in sizes: + self._render_for_size(size, self.data) + +def display(self, force_clear=False): + size = (self.display_manager.width, self.display_manager.height) + frame = self._frame_cache.get(size) + if frame is None: # never rendered at this size: one-off, not steady state + frame = self._render_for_size(size, self.data) + self.display_manager.image.paste(frame, (0, 0)) + self._draw_clock() # live parts stay in display(), outside the cache + self.display_manager.update_display() +``` + +**Keep live parts out of the cached image.** Anything that must be current on +every frame — a clock, a countdown, a "LIVE" pulse — is drawn in `display()` +*after* the paste. Folding it into the cached image freezes it at render time. + +Worked examples: `plugins/f1-scoreboard/manager.py` (`_prepare_scroll_content`, +which also fingerprints its inputs so it can skip an unchanged rebuild), +`plugins/ledmatrix-elections/manager.py` (`_build_scroll_image`), and +`plugins/geochron/manager.py` (`_render_for_size`). + ### `validate_config(self)` — fail loudly, early Called to check the plugin's config. Convention: call the base first, then diff --git a/plugins.json b/plugins.json index c669b646..5b076472 100644 --- a/plugins.json +++ b/plugins.json @@ -266,7 +266,7 @@ "last_updated": "2026-07-17", "verified": true, "screenshot": "", - "latest_version": "1.0.2", + "latest_version": "1.0.3", "icon": "fa-globe" }, { diff --git a/plugins/geochron/manager.py b/plugins/geochron/manager.py index ac24e498..5bf4e98f 100644 --- a/plugins/geochron/manager.py +++ b/plugins/geochron/manager.py @@ -53,6 +53,11 @@ def __init__(self, plugin_id, config, display_manager, cache_manager, plugin_man self._cached_map = None self._cached_layout = None + # Rendered map per (width, height). Vegas captures at a different + # width than the panel, and a single entry thrashed between the two. + self._map_cache = {} + # Terminator grid, shared across sizes -- it is lat/lon, not pixels. + self._darkness = None self._subsolar_lat = 0.0 self._subsolar_lon = 0.0 self._last_update_utc = None @@ -140,6 +145,21 @@ def _render_base_map(self): # ------------------------------------------------------------------ def update(self): + """Recompute the terminator and re-render every panel size in use. + + The map is cached per (width, height) rather than as a single image. + The Vegas marquee captures this plugin through the display-capture + fallback at a narrower width than the panel -- 153px against 512px on + the rig this was measured on -- and the old single-entry cache + mismatched on every switch. display() then re-rendered inline, which + for a capture means on the render thread: 105ms recomputing a + terminator that does not depend on size at all, plus ~150ms rendering + the map, roughly 290ms of stalled marquee every time round. + + Re-rendering the sizes here, on the update worker, means the render + thread finds a warm entry and pays nothing. The terminator is computed + once and shared across sizes, since it is a lat/lon grid. + """ try: now_utc = datetime.now(timezone.utc) darkness, sub_lat, sub_lon = solar.compute_terminator( @@ -148,17 +168,34 @@ def update(self): self._subsolar_lat = sub_lat self._subsolar_lon = sub_lon self._last_update_utc = now_utc - - dw = self.display_manager.width - dh = self.display_manager.height - layout = gr._layout(dw, dh, map_center_lon=self.map_center_longitude) - self._cached_layout = layout - self._cached_map = gr.render_map_image( - self._base_map, darkness, layout, self.night_brightness, self.colors["night_tint_color"] - ) + self._darkness = darkness + + # Whatever sizes have been asked for so far, plus the live panel. + # Copy first: display() inserts a newly-seen size from the render + # thread, and iterating the live dict could catch it mid-write. + sizes = set(self._map_cache.copy()) + sizes.add((self.display_manager.width, self.display_manager.height)) + for size in sizes: + self._render_for_size(size, darkness) except Exception as e: self.logger.error("Error updating geochron: %s", e, exc_info=True) + def _render_for_size(self, size, darkness): + """Render and cache the map for one panel size.""" + dw, dh = size + layout = gr._layout(dw, dh, map_center_lon=self.map_center_longitude) + image = gr.render_map_image( + self._base_map, darkness, layout, self.night_brightness, + self.colors["night_tint_color"] + ) + self._map_cache[size] = (layout, image) + # Keep the single-entry attributes pointing at the live panel so + # anything still reading them (get_info, tests) sees what is on screen. + if (dw, dh) == (self.display_manager.width, self.display_manager.height): + self._cached_layout = layout + self._cached_map = image + return layout, image + def display(self, force_clear=False): try: if force_clear: @@ -167,20 +204,22 @@ def display(self, force_clear=False): dw = self.display_manager.width dh = self.display_manager.height - layout = self._cached_layout - if ( - layout is None - or self._cached_map is None - or layout["dw"] != dw - or layout["dh"] != dh - ): - self.update() - layout = self._cached_layout - - if layout is None or self._cached_map is None: + cached = self._map_cache.get((dw, dh)) + if cached is None: + # First time at this size. If the terminator has never been + # computed there is nothing to render from, so fall back to a + # full update; otherwise reuse it and render just this size. + if self._darkness is None: + self.update() + cached = self._map_cache.get((dw, dh)) + else: + cached = self._render_for_size((dw, dh), self._darkness) + + if cached is None: return + layout, map_image = cached - self.display_manager.image.paste(self._cached_map, (layout["map_x"], layout["map_y"])) + self.display_manager.image.paste(map_image, (layout["map_x"], layout["map_y"])) draw = self.display_manager.draw if layout["sidebar_w"]: diff --git a/plugins/geochron/manifest.json b/plugins/geochron/manifest.json index e2d092c3..55e22752 100644 --- a/plugins/geochron/manifest.json +++ b/plugins/geochron/manifest.json @@ -1,16 +1,35 @@ { "id": "geochron", "name": "Geochron World Clock", - "version": "1.0.2", + "version": "1.0.3", "author": "ChuckBuilds", "description": "High-fidelity world map clock showing the real-time day/night terminator with civil/nautical/astronomical twilight bands, the subsolar point, configurable city markers, and a digital UTC/local clock - a classic Geochron, reimagined for LED matrix panels of any size.", "entry_point": "manager.py", "class_name": "GeochronPlugin", "category": "time", - "tags": ["clock", "world map", "geochron", "sun", "terminator", "astronomy", "time", "globe"], - "display_modes": ["geochron"], - "compatible_versions": [">=2.0.0"], + "tags": [ + "clock", + "world map", + "geochron", + "sun", + "terminator", + "astronomy", + "time", + "globe" + ], + "display_modes": [ + "geochron" + ], + "compatible_versions": [ + ">=2.0.0" + ], "versions": [ + { + "version": "1.0.3", + "released": "2026-08-14", + "ledmatrix_min_version": "2.0.0", + "notes": "Cache the rendered map per panel size instead of holding one image and re-rendering whenever the size changes. The Vegas marquee captures this plugin at a narrower width than the panel -- 153px against 512px on the rig this was measured on -- so the single entry alternated between the two and every switch re-rendered from scratch on the render thread: 105ms recomputing a terminator that does not depend on size, plus ~150ms drawing the map, about 290ms of stalled marquee each pass. The terminator is now computed once and shared across sizes, and update() refreshes every size in use on the worker thread so the render thread finds a warm image. The clock is untouched -- it is drawn after the map on every frame, so nothing about it goes stale." + }, { "released": "2026-07-17", "version": "1.0.2", diff --git a/plugins/geochron/test_per_size_map_cache.py b/plugins/geochron/test_per_size_map_cache.py new file mode 100644 index 00000000..da3d0673 --- /dev/null +++ b/plugins/geochron/test_per_size_map_cache.py @@ -0,0 +1,187 @@ +#!/usr/bin/env python3 +""" +Tests that the map is cached per panel size instead of thrashing between two. + +Regression under test: the rendered map was held in a single _cached_map, and +display() re-rendered whenever the cached layout's size differed from the +display manager's. The Vegas marquee captures this plugin through the +display-capture fallback at a narrower width than the panel -- 153px against +512px on the rig this was found on -- so the two alternated and every switch +re-rendered from scratch. For a capture that runs on the render thread: +105ms recomputing a terminator that does not depend on size at all, plus +~150ms rendering the map. Measured at 271-639ms of stalled marquee per pass. + +What is deliberately NOT cached is the clock. _draw_readout() runs on every +display() call, after the map is pasted, so a cached map does not freeze the +time -- which is the trade this fix avoids having to make. + +Run: /bin/python plugins/geochron/test_per_size_map_cache.py +""" + +import ast +import sys +from pathlib import Path + +plugin_dir = Path(__file__).parent +sys.path.insert(0, str(plugin_dir)) +for candidate in (Path("/home/rackpi/projects/LEDMatrix"), + plugin_dir.parents[2] / "LEDMatrix"): + if (candidate / "src" / "plugin_system" / "base_plugin.py").exists(): + sys.path.insert(0, str(candidate)) + break + +from manager import GeochronPlugin # noqa: E402 + +failures = [] + + +def check(label, ok): + print((" PASS " if ok else " FAIL ") + label) + if not ok: + failures.append(label) + + +class _DisplayManager: + def __init__(self, width=512, height=64): + self.width = width + self.height = height + + +class _Geo: + """Stand-in carrying only what the cache paths touch.""" + + update = GeochronPlugin.update + _render_for_size = GeochronPlugin._render_for_size + + def __init__(self): + self.display_manager = _DisplayManager() + self._map_cache = {} + self._darkness = None + self._cached_map = None + self._cached_layout = None + self._subsolar_lat = 0.0 + self._subsolar_lon = 0.0 + self._last_update_utc = None + self._base_map = object() + self.night_brightness = 0.35 + self.colors = {"night_tint_color": (0, 0, 40)} + self.map_center_longitude = 0 + self.show_terminator_bands = True + self.logger = _Logger() + self.terminator_calls = 0 + self.render_calls = [] + + +class _Logger: + def error(self, *a, **k): + pass + + def info(self, *a, **k): + pass + + +def _install_stubs(geo): + """Count the two expensive calls without doing their work.""" + import manager as m + + class _Solar: + @staticmethod + def compute_terminator(now, gw, gh, show_bands=True): + geo.terminator_calls += 1 + return ("darkness-%d" % geo.terminator_calls, 12.3, 45.6) + + class _Renderer: + @staticmethod + def _layout(dw, dh, map_center_lon=0): + return {"dw": dw, "dh": dh, "map_x": 0, "map_y": 0, "sidebar_w": 0} + + @staticmethod + def render_map_image(base, darkness, layout, brightness, tint): + geo.render_calls.append((layout["dw"], layout["dh"], darkness)) + return "map-%dx%d-%s" % (layout["dw"], layout["dh"], darkness) + + originals = (m.solar, m.gr) + m.solar, m.gr = _Solar(), _Renderer() + return originals, m + + +def main(): + print("a size that has been rendered is not rendered again") + geo = _Geo() + (orig_solar, orig_gr), m = _install_stubs(geo) + try: + geo.update() + first = len(geo.render_calls) + check("the live panel size is rendered by update()", first == 1) + check("its entry is cached", (512, 64) in geo._map_cache) + + # The Vegas capture asks for a narrower canvas. + geo.display_manager.width = 153 + cached = geo._map_cache.get((153, 64)) + check("the capture size is not cached yet", cached is None) + geo._render_for_size((153, 64), geo._darkness) + check("rendering it caches it", (153, 64) in geo._map_cache) + + before = len(geo.render_calls) + geo._map_cache.get((153, 64)) + check("a second look-up at that size renders nothing", + len(geo.render_calls) == before) + check("the 512px entry survived the switch", (512, 64) in geo._map_cache) + + print("\nupdate() refreshes every size in use, on the worker thread") + terminator_before = geo.terminator_calls + renders_before = len(geo.render_calls) + geo.display_manager.width = 512 + geo.update() + check("both cached sizes were re-rendered", + len(geo.render_calls) - renders_before == 2) + check("the terminator was computed once, not once per size", + geo.terminator_calls - terminator_before == 1) + check("both entries carry the same terminator", + geo._map_cache[(512, 64)][1].split("-")[-1] + == geo._map_cache[(153, 64)][1].split("-")[-1]) + + print("\nthe live panel's single-entry attributes still track it") + check("_cached_map points at the live size", + geo._cached_map == geo._map_cache[(512, 64)][1]) + finally: + m.solar, m.gr = orig_solar, orig_gr + + print("\nthe clock is not cached with the map") + # The whole point of caching the map rather than throttling the refresh: + # the readout is drawn after the paste, every display() call, so a cached + # map cannot freeze the time. + source = (plugin_dir / "manager.py").read_text(encoding="utf-8") + tree = ast.parse(source) + display_fn = next((n for n in ast.walk(tree) + if isinstance(n, ast.FunctionDef) and n.name == "display"), None) + check("display() exists", display_fn is not None) + readout_calls = [n for n in ast.walk(display_fn) + if isinstance(n, ast.Call) + and getattr(n.func, "attr", None) == "_draw_readout"] + check("display() draws the readout on every call", len(readout_calls) == 1) + + readout_fn = next((n for n in ast.walk(tree) + if isinstance(n, ast.FunctionDef) and n.name == "_draw_readout"), None) + now_calls = [n for n in ast.walk(readout_fn) + if isinstance(n, ast.Call) + and getattr(n.func, "attr", None) == "now"] + check("the readout reads the clock fresh, not from cache", + bool(now_calls)) + + print("\nno single-entry size check remains in display()") + # The old bug in one line: re-render when the cached layout's size differs. + stale = [n for n in ast.walk(display_fn) + if isinstance(n, ast.Compare) + and any(isinstance(c, ast.Subscript) + and getattr(getattr(c, "slice", None), "value", None) in ("dw", "dh") + for c in [n.left] + list(n.comparators))] + check("display() no longer compares a cached layout's size", not stale) + + print("\n%s" % ("FAILED: %d" % len(failures) if failures + else "All checks passed")) + return 1 if failures else 0 + + +if __name__ == "__main__": + sys.exit(main())