From 9964dd2183cb24ef868e2099745d2be0e0a267d7 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:57:03 -0400 Subject: [PATCH] feat(vegas): render plugin content off the render thread, and keep it off the GIL when the panel needs it (#630) DisplayManager.offscreen() gives a thread its own canvas, so Vegas renders every plugin's ticker content on its prefetch thread instead of pausing the scroll for canvas-bound plugins on the render thread. A render gate (src/common/render_gate.py, vegas_scroll.prefetch_gate, on by default with the GIL-releasing binding) lets the prefetch thread run Python only while the render thread waits in SwapOnVSync: on hdpi, frames 2+ refreshes late fell eightfold and late frames overall from 0.90% to 0.60%. See docs/OFFSCREEN_RENDERING.md. Co-Authored-By: Claude Opus 5.5 --- docs/CONFIG_REFERENCE.md | 3 + docs/OFFSCREEN_RENDERING.md | 388 ++++++++++++++++ src/common/render_gate.py | 244 ++++++++++ src/display_manager.py | 197 +++++++- .../testing/visual_display_manager.py | 32 +- src/vegas_mode/config.py | 31 ++ src/vegas_mode/coordinator.py | 65 ++- src/vegas_mode/plugin_adapter.py | 171 ++++--- src/vegas_mode/render_pipeline.py | 16 +- src/vegas_mode/stream_manager.py | 12 +- test/test_offscreen_rendering.py | 360 +++++++++++++++ test/test_render_gate.py | 432 ++++++++++++++++++ test/test_vegas_switch_interval.py | 41 ++ 13 files changed, 1903 insertions(+), 89 deletions(-) create mode 100644 docs/OFFSCREEN_RENDERING.md create mode 100644 src/common/render_gate.py create mode 100644 test/test_offscreen_rendering.py create mode 100644 test/test_render_gate.py create mode 100644 test/test_vegas_switch_interval.py diff --git a/docs/CONFIG_REFERENCE.md b/docs/CONFIG_REFERENCE.md index 92067cb2..fc9b03f3 100644 --- a/docs/CONFIG_REFERENCE.md +++ b/docs/CONFIG_REFERENCE.md @@ -128,6 +128,9 @@ Read by `src/vegas_mode/config.py` (`VegasScrollConfig.from_config`). See | `min_content_separation` | int, `24` | | `min_cut_gap` | int, `6` | | `continuous_scroll` | bool, `true` | +| `offscreen_prefetch` | bool, `true` — render every plugin's ticker content on the background thread, each on its own canvas. `false` restores handing canvas-bound plugins to the render thread, one pause at a time. Temporary; see [OFFSCREEN_RENDERING.md](OFFSCREEN_RENDERING.md) | +| `prefetch_gate` | bool, `true` — let that background thread run Python only while the render thread is waiting for the panel, so the render thread never waits for the GIL when a refresh comes round. Only takes effect with the rebuilt rgbmatrix binding (`scripts/build_rgbmatrix_nogil.sh`). See [OFFSCREEN_RENDERING.md](OFFSCREEN_RENDERING.md) | +| `switch_interval_ms` | float, `0` — experimental: shorten Python's GIL switch interval to this many ms while Vegas runs. `0` leaves the default (5 ms) alone | | `smooth_scroll` | bool, `true` — move a whole number of pixels per panel refresh, locked to vsync. `scroll_speed` is snapped to the nearest speed the panel can show that way (at 95Hz: 95, 47.5, 31.7 px/s…), measured against the panel's real refresh rate once scrolling starts | | `sub_pixel_blend` | bool, `false` — the older smoothing: advance by elapsed time and blend neighbouring pixel columns. Looks anti-aliased in the web preview but shimmers on the panel and is not locked to the refresh. Overrides `smooth_scroll` when on | | `extend_threshold_screens` | float, `2.0` | diff --git a/docs/OFFSCREEN_RENDERING.md b/docs/OFFSCREEN_RENDERING.md new file mode 100644 index 00000000..b7b7cc65 --- /dev/null +++ b/docs/OFFSCREEN_RENDERING.md @@ -0,0 +1,388 @@ +# Offscreen Rendering + +**Status (2026-09-24):** step 1, offscreen rendering, is implemented +(`DisplayManager.offscreen()`, the adapter on the prefetch thread, the plugin +lock). Steps 2 and 3 are proposed. When all three land, this file becomes the +reference for how plugin content is rendered off the render thread. + +First soak of step 1 on hdpi (50 px/s, `pwm_bits` 7, preview open, 8-minute +runs, A/B/B/A): + +| build | late | by 1 | 2 | 3–5 | 6+ | freezes | render-thread fetches | +|---|---|---|---|---|---|---|---| +| #628 | 0.53% | 82 | 2 | 3 | 2 | 3 | 6 | +| step 1 | 0.63% | 78 | 63 | 17 | 2 | 1 | 0 | +| step 1 | 0.42% | 77 | 23 | 10 | 0 | 0 | 0 | +| #628 | 0.37% | 84 | 5 | 3 | 3 | 2 | 14 | + +It does what it was built to: no plugin is fetched on the render thread, and +freezes fell from 5 to 1. But frames 2–5 refreshes late rose. The rendering +moved to the prefetch thread still needs the GIL, and the render thread waits +for it (risk 5 below). The late rate did not improve overall. The 1–2 s +freezes appear in both builds and have a separate, not yet identified cause. + +The GIL fix, measured on hdpi (90 px/s, `pwm_bits` 8, preview open, 8-minute +runs after a 2-minute warm-up, order A B C C B A, 2026-09-24). Each arm pools +two runs, about 81,000 frames: + +| arm | late | by 1 | 2 | 3–5 | 6+ | 2+ late per 10k frames | freezes | +|---|---|---|---|---|---|---|---| +| A: step 1 as is | 0.90% | 575 | 64 | 91 | 9 | 20.1 | 0 | +| B: `switch_interval_ms` 1 | 0.78% | 510 | 105 | 23 | 2 | 15.8 | 0 | +| C: `prefetch_gate` | **0.60%** | 471 | 11 | 7 | 2 | **2.5** | 0 | + +The gate removes the frames the render thread spent waiting for the GIL, and +it costs the prefetch nothing that shows: it parked the thread for 3–6 s per +run, and the next group was ready at every strip extension in every arm. +`prefetch_gate` is therefore on by default; `switch_interval_ms` stays an +off-by-default experiment. What is left is almost all one refresh late, which +is the per-frame budget (a 6.75 ms p50 blit in a refresh the panel holds at +83–85 Hz while rendering), not contention. + +The runs restart the service, so the hourly sports refresh never fell inside +one. That refresh is its own case: about twenty ESPN chunk-fetch threads at +once, which the gate does not cover (it gates only the prefetch thread). + +## The problem + +Vegas mode builds its ticker from every plugin's content. Most of that work +already happens on a background prefetch thread +(`RenderPipeline.start_prefetch`). But any plugin whose content needs the +**shared display canvas** is deferred to the render thread +(`RenderPipeline.drain_deferred`), one plugin every two seconds. The code's +own comments put each of those at 40–600 ms, and the render thread presents no +frames while one runs. + +On hdpi (Pi 4, 512×64) most plugins take that path: geochron, tide-display, +news, hockey-scoreboard, ledmatrix-stocks, incoming-packages, clock-simple, +countdown, birdnet-go, ledmatrix-music and odds-ticker. They arrive in bursts +("Whole group deferred; strip will extend as it drains") every minute or so, +12 fetches in five minutes. That is the "occasional pause" a viewer sees. + +An 8-minute soak (`scripts/frame_soak.py --preview`) of the #628 build on +hdpi: + +| late by | frames | +|---|---| +| 1 refresh | 238 | +| 2 | 32 | +| 3–5 | 30 | +| 6+ | 5 | +| freezes ≥ 250 ms | 2 (0.97 s total) | + +The 3+ rows and the freezes are the pauses. The single-refresh row is a +separate problem: the blit is 6 ms of a 10 ms refresh, so there is little +slack. It is covered under *What this does not fix*. + +## Why a plugin is canvas-bound + +The plugin-facing canvas is a set of shared attributes on `DisplayManager`: +`image`, `draw`, `matrix`, and the `width`/`height` properties that read from +`matrix`. Three adapter paths (`src/vegas_mode/plugin_adapter.py`) need them, +and each returns `None` under `offscreen_only=True` so the plugin is queued for +the render thread: + +1. **Display capture** (`_capture_display_content`): clear the canvas, call + `plugin.display()`, copy `display_manager.image`. Used by any plugin + without `get_vegas_content()` or a populated `scroll_helper`. +2. **Scroll-content generation** (`_trigger_scroll_content_generation`): a + ticker plugin whose `scroll_helper.cached_image` is empty is made to build + it by calling `display(force_clear=True)` or `_create_scrolling_display()`. + Both draw on the canvas. +3. **Narrowed rendering** (`DisplayManager.render_size`): swaps the shared + `matrix`, `image` and `draw` for a narrower set so the plugin lays out for + `render_width_pct`. The render thread would see the swap mid-frame. + +The render thread keeps the canvas coherent only because nothing else touches +it at the same time. A background thread can't use it. + +## The design: a per-thread render target + +`capture_mode()` is already per-thread (#423 made its state a +`threading.local`, so a background capture no longer suppresses the render +loop's pushes). The same move applies to the canvas itself: + +```python +with display_manager.offscreen(width=None, height=None) as surface: + plugin.display(force_clear=True) + content = surface.image.copy() +``` + +For the **calling thread only**, inside the block: + +| accessor | resolves to | +|---|---| +| `display_manager.image`, `.draw` | the surface's own image and draw: a fresh black canvas, `fontmode = "1"` | +| `display_manager.matrix` | a logical proxy reporting the surface size, so `width`/`height` and plugins that read `matrix.width` follow it. Hardware calls through it (`SetImage`, `SwapOnVSync`, `Clear`, brightness writes) are inert. | +| `update_display()`, `clear()` | canvas-only: the block implies capture mode, which is already per-thread | +| `set_scrolling_state()`, `set_frame_hold()` | no-ops, so a plugin's `display()` cannot re-pace the live scroll. Today it can, when it is captured on the render thread. | + +Every other thread sees the real canvas, unchanged. The render loop in +particular keeps presenting while a plugin draws elsewhere. + +### Implementation sketch + +- `image`, `draw` and `matrix` become properties over `_image`, `_draw` and + `_matrix`, plus a thread-local current surface. The getter returns the + surface's value when the calling thread has one, else the shared one; setters + mirror that. That costs about 0.1 µs per access, and `update_display()` reads + each a handful of times per frame. Every existing `self.image = ...` in + `DisplayManager` (`clear()`, setup, fallback) keeps working and becomes + thread-correct for free. +- `render_size()` is rebuilt on `offscreen()`: it creates or narrows the + calling thread's surface instead of swapping shared state. +- `offscreen()` nests and always restores on exit, including when the plugin + raises. +- `VisualDisplayManager` (the plugin test harness) gets the same method, for + parity. + +### Adapter changes + +- `get_content(offscreen_only=True)` stops returning `None` for the three + paths above. Each runs inside `display_manager.offscreen(render_width)`. +- `_capture_display_content` and `_trigger_scroll_content_generation` drop + their "copy the shared image, restore it afterwards" bookkeeping, since the + shared image is never touched. +- **Take the plugin's lock.** `PluginManager.get_plugin_lock()` keeps + `update()` and `display()` mutually exclusive in normal rotation, but Vegas + never takes it, so today's render-thread captures already race + `update()`. Off the render thread the adapter can afford to wait: blocking + acquire with a timeout (proposed 2 s). On timeout it keeps the cached segment + and tries again next group. +- `drain_deferred()` and the deferred queue are deleted. The only render-thread + fetch left is the inline fallback when no prepared group is ready, which in + practice is the first extension. Prefetching at start removes that too. + +## Keeping live content fresh + +Offscreen rendering is also what makes fresh sports scores possible. Today a +plugin's segment is drawn when its group is prefetched, and the strip carries +7,000–10,000 px of content ahead of the viewport (hdpi logs: "7153px still +ahead", "9842px ahead"). At ~100 px/s, a score drawn now reaches the screen +70–100 seconds later. When a plugin reports new data, Vegas only drops its +cache (`invalidate_pending_updates`), so the change is drawn on the plugin's +*next* turn, several minutes later. A segment already in the strip scrolls by +with the data it was drawn with. + +That was the right trade while every redraw of a canvas-bound plugin stalled +the scroll. Off the render thread a redraw costs the scroll nothing, so the +strip can afford three things. + +### 1. Refresh at the gate + +Before a segment enters the viewport, check whether its plugin has updated +since the segment was drawn. If it has, redraw it offscreen and replace it +while it is still out of sight. Width changes are fine here, because +everything from that segment onward is still invisible. + +The gate sits `lead` pixels ahead of the viewport's right edge: +`lead = max(one screen, speed × (render time + margin))`. The render time is +the plugin's own, measured on each render (sports cards take the longest, +hundreds of ms up to seconds per the prefetch notes). A plugin whose render +does not finish before its segment reaches the viewport keeps the old segment. +The scroll never waits for it. + +Content is then at most `lead / speed` seconds old when it appears, a few +seconds instead of minutes, without changing how far ahead the rotation +fetches. + +### 2. Replace ahead of the screen + +When a plugin reports new data (the Vegas update tick already names them), any +of its segments that are **anywhere ahead of the viewport** are redrawn and +replaced straight away, not only at the gate. That covers the long stretch of +strip between prefetch and the gate. + +### 3. Update on screen + +A segment that is already **visible** is patched in place when the redrawn +version has the same geometry: the same total width, and the same width for +each card (a sports plugin returns one image per game, joined with +`intra_plugin_gap`). Scoreboard cards keep a fixed layout, so a score change +patches in and the digits update as the card scrolls past. The patch is a +pixel copy of one card (a 150×64 card is ~29 KB) applied by the render thread +between frames, so a frame never shows half of a patch. + +When the geometry differs (a game added or dropped, a card that grew), the +visible part cannot change without a jump. Only the cards not yet on screen +are replaced, and only if the geometry up to that point is unchanged. Otherwise +the segment keeps its snapshot until it has scrolled off. + +### Avoiding wasted work + +- **Change detection.** `run_scheduled_updates_with_changes()` names a plugin + whenever its `update()` ran, not when its data changed. On hdpi + `clock-simple` and `ledmatrix-music` are named on every 4-second tick. A + redraw whose pixels hash the same as the segment's is discarded without a + swap. +- **Redraw on real updates only.** Vegas makes no API calls. Each plugin + fetches on its own schedule, and a redraw is triggered only when the + plugin's `update()` has run since its segment was drawn. On hdpi live + football, baseball and hockey poll every 30 s (live odds every 60 s, + everything else hourly), so a live sports card is redrawn once per poll. +- **Floor.** A plugin is redrawn at most once per + `vegas_scroll.refresh_min_interval` (proposed 10 s), and never while its + previous redraw is still running. The floor never holds back a sports card + polling every 30 s. It exists for chatty plugins: `clock-simple` updates + every second and `ledmatrix-music` polls every 2 s. +- **One worker.** Redraws go through the same background worker as prefetch, + one plugin at a time at `nice 10`, under the plugin's lock. + +Data freshness is still bounded by each plugin's own fetch interval (how often +it polls live scores). Drawing faster cannot beat the data source. + +### The strip becomes a list of segments + +All three need the strip to be replaceable by segment. Today it is one +image (`ScrollHelper.cached_array`, 8,000–20,000 px wide, 1.5–3.8 MB), and +`append_content()` rebuilds the whole thing on the render thread for every +appended block. That is also a pause source. + +Proposed `SegmentStrip`, used by Vegas in place of the single image: + +- an ordered list of segments: plugin id, card boundaries, a pixel array, the + render time, and the plugin data version it was drawn from, plus its + x-offset in the strip; +- `visible(x, width)` assembles the viewport by slicing across at most a few + segments: the same ~100 KB copy per frame that slicing the single image + costs today; +- append and trim become O(block) list operations, not a copy of the strip; +- replace swaps one list entry and shifts the offsets of the segments after it + (dozens at most). A same-geometry patch copies pixels into the existing array. + +Every mutation is prepared off the render thread and applied by the render +thread at a frame boundary, so the strip the render loop reads is never +half-changed. + +### Multi-display sync + +The follower renders from its own copy of the strip, offset from the leader's +scroll position. Today the leader sends that copy whole, and only in +`start_new_cycle()` (`send_scroll_image`), plus the scroll position every +frame. Continuous scroll, the default, extends and trims the strip without +starting a new cycle, and nothing sends those changes. From reading the code, +the follower therefore probably falls out of step after the first extension +already, before any of this design. That is untested; it needs a two-Pi rig. + +With a segment strip, keeping the follower identical becomes **replaying the +leader's operations**: + +- Every strip mutation (append, trim, replace, patch) is one operation in + strip coordinates. The leader applies it and sends the same operation to the + follower over the existing TCP channel. Segments are small: a card is ~29 KB + raw and compresses well. +- Operations on off-screen segments apply on arrival. A patch to a segment + that is on either panel carries an *apply at scroll position X* stamp a + couple of hundred milliseconds ahead. Both sides apply it when their scroll + position passes X, so both panels change on the same frame, within the + existing position-sync jitter. +- Each operation carries a sequence number. A follower that sees a gap (a + reconnect, a dropped message) asks for a full snapshot, which is today's + `send_scroll_image` path. + +That also fixes the probable continuous-mode gap as a side effect, since +appends and trims become operations too. Until it is in place, fresh-content +updates are disabled while sync is active. + +## Risks, and what was checked + +1. **Plugins holding their own reference to the shared `draw` or `image`.** + They would keep drawing into the shared canvas, and routing by thread can't + redirect them. A grep of the 49 plugins installed on hdpi found none storing + `display_manager.draw` or `.image` in an attribute (a pattern search, so + indirect aliasing would slip past it). A plugin that did would + draw into an image nobody displays, which trims to a blank segment. That is + not corruption, and it is no worse than today. +2. **Plugins calling the matrix directly.** None in the audit. Inside + `offscreen()` the proxy makes it inert anyway. +3. **Font thread-safety.** `FontManager` shares font objects across plugins. + Measured on Pillow 12.3, two threads rendering text take 1.94× as long as + one, so text rendering holds the GIL and FreeType is never entered + concurrently. Re-check if Pillow changes that. +4. **Plugin thread-safety.** `display()` moves to the prefetch thread. The + plugin lock makes it exclusive with `update()`, which is more protection + than it has today. Threads a plugin starts itself are not covered, as today. +5. **The GIL.** Moving 40–600 ms of plugin rendering off the render thread + removes the pauses, but the work still needs the GIL. Pillow drawing holds + it, and a waiting thread only gets it back after the switch interval + (default 5 ms). Expect some single-refresh late frames while a prefetch + runs. Measure with the soak. A render process separate from plugin work + is the structural answer (the "native presenter" step). Two experiments + get most of the way first (results under Status, above): + - `vegas_scroll.switch_interval_ms` lowers the switch interval for a Vegas + run (1 ms is the obvious try), so the render thread waits at most that + long behind bytecode. It does nothing for a C call that keeps the GIL. + - `vegas_scroll.prefetch_gate` (`src/common/render_gate.py`) lets the + prefetch thread run Python only while the render thread is blocked in + `SwapOnVSync`, up to just before the refresh the swap returns on, and + parks it the rest of the time. That covers C calls too, since the gate is + checked before each one starts. It never parks the thread while it holds + a lock the render thread takes, and never for more than 50 ms. It needs + the rebuilt binding, which releases the GIL during the swap. On by + default. + +## What this does not fix + +- **The blit.** Copying a 512×64 frame into the matrix (`SetImage`) is ~6 ms at + 8 PWM bits on a Pi 4, leaving ~4 ms of slack per refresh. That is the main + source of the single-refresh late frames. Holding frames for two refreshes + (≈50 px/s) doubles the budget. Cutting the blit itself is the native-presenter + step. +- **Live refreshes pushed from `update()`.** Some sports plugins call + `display()` and `update_display()` from inside `update()`, which runs on the + update worker and can push to the panel mid-Vegas. That is a separate + hazard. `offscreen()` gives a tool for it (run the update worker offscreen + while Vegas owns the panel), but it is out of scope here. + +## Test plan + +- **Unit, `DisplayManager`:** one thread inside `offscreen()` draws while + another reads `image`/`draw`/`matrix`/`width`/`height` and sees the real + canvas. Also: `update_display()` and `set_scrolling_state()` are inert inside; + `render_size()` narrows only the calling thread; nesting and exceptions + restore state. +- **Unit, adapter:** a stub display-capture plugin and a stub scroll-helper + plugin both return content with `offscreen_only=True`, and nothing is queued + for the render thread. The plugin lock is taken, and a timeout keeps the cached + segment. +- **Emulator integration:** a stub canvas-bound plugin whose `display()` sleeps + 300 ms. The Vegas render loop never goes a frame without presenting (frame + timing recorder: zero freezes). +- **Unit, `SegmentStrip`:** the viewport assembled across segment boundaries + matches slicing one concatenated image, pixel for pixel. Append, trim, + replace-ahead and same-geometry patch each leave every other column + unchanged. A geometry-changing patch of a visible segment is refused. +- **Freshness:** a stub sports plugin whose score changes every second. The + score on screen is never older than `lead / speed` plus the plugin's fetch + interval. A visible card's digits change without the frame-timing recorder + seeing a late frame. An unchanged redraw is discarded. +- **Hardware:** an hdpi soak, A/B against the #628 build, alternating order. + Targets: no freezes, an empty 6+ bucket, the 3–5 bucket near zero, and the late + rate below 0.66%. Plus, for freshness: log each segment's age when it enters + the viewport, and compare the median and max before and after. + +## Rollout + +Three changes, each soaked on hdpi before the next: + +1. **Offscreen rendering:** `offscreen()`, the adapter on the prefetch thread, + and the plugin lock. Removes the render-thread pauses. +2. **`SegmentStrip`:** Vegas's strip becomes a list of segments. Removes the + whole-strip copy on append. No visible behaviour change. +3. **Fresh content:** refresh at the gate, replace ahead, patch on screen, + with change detection and the rate limit. + +`display.vegas_scroll.offscreen_prefetch` (default `true`) restores today's +deferred path when `false`, and `display.vegas_scroll.live_refresh` (default +`true`) turns off step 3. Keep both for one release, then delete the old paths. + +## Open questions + +1. Keep the kill switch, or ship without one? +2. Plugin lock timeout: skip the plugin and keep its cached segment (proposed), + or wait longer? +3. `refresh_min_interval`: 10 s proposed. It only limits chatty plugins; + live sports are redrawn once per 30 s poll regardless. +4. Multi-display sync: is there a two-Pi rig to test on? Operation replay is + proposed as part of the segment strip (step 2), with fresh content + disabled under sync until it has been verified on real hardware. diff --git a/src/common/render_gate.py b/src/common/render_gate.py new file mode 100644 index 00000000..3c827718 --- /dev/null +++ b/src/common/render_gate.py @@ -0,0 +1,244 @@ +"""Let a background thread run Python only while the render thread waits on vsync. + +With plugin rendering moved to Vegas's prefetch thread (DisplayManager.offscreen, +#630) the render thread no longer stops for it, but it still shares the GIL +with it. The render thread spends most of each refresh inside SwapOnVSync, +which releases the GIL, and needs it back the moment the swap returns. If the +prefetch thread is running Python right then, the render thread waits: up to +the switch interval (5ms) behind bytecode, and for as long as a C call that +keeps the GIL takes. On hdpi that showed up as frames 2-5 refreshes late while +a group was being prepared. + +The gate turns that around. The display manager opens it just before each swap, +with a deadline shortly ahead of the refresh the swap will return on, and +closes it when the swap returns. A thread inside ``gate.yielding()`` checks it on +every Python and C call through a profile hook, and once the window has closed +it parks -- blocked on a condition, GIL released -- until the next swap opens +it. The render thread then finds the GIL free when its refresh arrives, and the +background work runs in time the render thread was only spending waiting. + +Parking a thread is only safe if nothing the render thread needs is stuck +behind it, so it is never parked: + +* while it holds a lock registered with ``guard()`` (the Vegas buffers and + caches the render thread also takes); +* inside logging, threading, importlib or the cache, all of which take locks the + render thread can take too; +* when there is no render loop to protect -- no swap for ``STALE_SECONDS``, as + on a static screen or a stalled frame. + +And a parked thread is never held more than ``MAX_WAIT_SECONDS`` at a time, so +whatever the gate gets wrong costs a frame, not a freeze. The render thread +itself is never gated, whatever it calls. + +It gates the prefetch thread only. Gating the ESPN fetch threads as well was +tried for the hourly sports refresh, twenty-odd of them at once, and measured +worse on hdpi (0.85% late frames without it, 1.14% with it, across a burst every +five minutes): each parked thread has to take the GIL again just to park at the +end of every window, and the fetches ran two to three times as long. + +""" + +from __future__ import annotations + +import math +import sys +import threading +import time +from collections import deque +from typing import Any, Callable, Deque, List, Optional + +#: Park background threads this long before the refresh a swap will return on, +#: so a short C call already under way has finished by then. +MARGIN_SECONDS = 0.002 + +#: The longest a background thread is parked in one go. +MAX_WAIT_SECONDS = 0.05 + +#: No swap for this long means there is no render loop running to protect. +STALE_SECONDS = 0.05 + +#: Swaps needed before the refresh period is trusted enough to open a window. +MIN_SAMPLES = 8 + +#: Parking inside any of these modules could hold a lock the render thread +#: takes: logging handler locks, Condition and Event internals, the module +#: import locks, and the disk and memory cache locks. Matched by module name, +#: not file path: a path can say "cache" or "logging" for reasons of its own -- +#: a virtualenv under ~/.cache, or GitHub's /opt/hostedtoolcache, where every +#: stdlib frame would otherwise count and the gate would never park anything. +_UNSAFE_MODULES = frozenset({ + "logging", "threading", "importlib", "src.cache_manager", "src.cache", +}) +_UNSAFE_PREFIXES = ("logging.", "importlib.", "_frozen_importlib", "src.cache.") + + +def _unsafe(frame: Any, base: Any) -> bool: + """True if a frame above ``base`` comes from somewhere parking could deadlock. + + ``base`` is the frame that entered ``yielding()``; what lies below it (the + thread's own bootstrap in threading.py) holds nothing. + """ + while frame is not None and frame is not base: + name = frame.f_globals.get("__name__") or "" + if name in _UNSAFE_MODULES or name.startswith(_UNSAFE_PREFIXES): + return True + frame = frame.f_back + return False + + +def swap_releases_gil() -> Optional[bool]: + """Whether the loaded rgbmatrix binding releases the GIL, or None if none is loaded. + + The rebuilt binding links PyEval_SaveThread and the stock one never does. + The same test as src.common.frame_timing.binding_releases_gil (#629); one + of the two goes once both have landed. + """ + module = sys.modules.get("rgbmatrix.core") + path = getattr(module, "__file__", None) + if not path: + return None + try: + with open(path, "rb") as handle: + return b"PyEval_SaveThread" in handle.read() + except OSError: + return None + + +def _held(lock: Any) -> bool: + """Is ``lock`` held? RLocks report this thread's ownership; plain locks, anyone's.""" + is_owned = getattr(lock, "_is_owned", None) + if is_owned is not None: + return is_owned() + return lock.locked() + + +class RenderGate: + """Opened by the render thread around each swap; honoured by background threads.""" + + def __init__(self, clock: Callable[[], float] = time.monotonic): + self.clock = clock + self._cond = threading.Condition() + self._generation = 0 + self._open_until = 0.0 + self._last_return: Optional[float] = None + self._periods: Deque[float] = deque(maxlen=64) + self._period: Optional[float] = None + self._guarded: List[Any] = [] + self._local = threading.local() + self._render_ident: Optional[int] = None + #: How often, and for how long in all, background threads were parked. + self.parks = 0 + self.parked_seconds = 0.0 + + def guard(self, *locks: Any) -> None: + """Never park a thread while it holds (or, for a plain Lock, anyone holds) these.""" + self._guarded.extend(lock for lock in locks if lock is not None) + + # -- render thread ----------------------------------------------------- + + def refresh_period(self) -> Optional[float]: + """The panel's refresh period from recent swaps, or None until known. + + The 10th percentile of the gaps between swap returns, each divided by + the hold: a late frame only ever lengthens a gap, so the low end is + the panel's own period. + """ + return self._period + + def before_swap(self, hold: int) -> None: + """The render thread is about to block in SwapOnVSync: open the window.""" + hold = max(1, int(hold)) + period = self._period + now = self.clock() + last = self._last_return + if period and last is not None and now - last < STALE_SECONDS: + # The swap returns on the first refresh boundary after both the + # current frame's hold is up and this frame has been handed over; + # boundaries fall a whole period apart from the last return. + refreshes = max(hold, math.ceil((now - last) / period)) + open_until = last + refreshes * period - MARGIN_SECONDS + else: + open_until = 0.0 # no rhythm to predict from: leave threads be + with self._cond: + self._open_until = open_until + self._generation += 1 + self._cond.notify_all() + + def after_swap(self, hold: int) -> None: + """The swap returned and the render thread needs the GIL: close the window.""" + now = self.clock() + self._open_until = 0.0 + if self._render_ident is None: + # The first thread to swap is the render loop. A plugin pushing a + # live refresh from its update thread swaps too, but must not take + # over its exemption. + self._render_ident = threading.get_ident() + last = self._last_return + if last is not None and now - last < STALE_SECONDS: + self._periods.append((now - last) / max(1, int(hold))) + if len(self._periods) >= MIN_SAMPLES: + ordered = sorted(self._periods) + self._period = ordered[len(ordered) // 10] + self._last_return = now + + # -- background threads ------------------------------------------------ + + def _should_park(self, frame: Any, now: float) -> bool: + if now < self._open_until: + return False # inside the window + last = self._last_return + if last is None or now - last > STALE_SECONDS or self._period is None: + return False # no render loop to protect + for lock in self._guarded: + if _held(lock): + return False + return not _unsafe(frame, getattr(self._local, "base", None)) + + def _hook(self, frame: Any, _event: str, _arg: Any) -> None: + now = self.clock() + if not self._should_park(frame, now): + return + generation = self._generation + with self._cond: + self._cond.wait_for(lambda: self._generation != generation, + timeout=MAX_WAIT_SECONDS) + self.parks += 1 + self.parked_seconds += self.clock() - now + + def yielding(self) -> "_Yielding": + """``with gate.yielding():`` runs the block giving way to the render thread.""" + return _Yielding(self) + + +class _Yielding: + """Installs a gate's profile hook on the thread for the length of a block.""" + + def __init__(self, gate: RenderGate): + self.gate = gate + self._previous: Any = None + self._previous_base: Any = None + self._skipped = False + + def __enter__(self) -> RenderGate: + gate = self.gate + # pylint: disable=protected-access + if threading.get_ident() == gate._render_ident: + self._skipped = True # parking the render thread parks the display + return gate + local = gate._local + self._previous_base = getattr(local, "base", None) + if self._previous_base is None: + # Nested blocks keep the outermost frame, so everything the thread + # entered since it first gave way is still checked for locks. + local.base = sys._getframe(1) + self._previous = sys.getprofile() + sys.setprofile(gate._hook) + return gate + + def __exit__(self, *_exc: Any) -> None: + if self._skipped: + return + sys.setprofile(self._previous) + self.gate._local.base = self._previous_base # pylint: disable=protected-access + diff --git a/src/display_manager.py b/src/display_manager.py index a5d3be18..eb58d206 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -133,6 +133,84 @@ class _LogicalMatrix: setattr(object.__getattribute__(self, "_matrix"), name, value) +class _OffscreenMatrix(_LogicalMatrix): + """``display_manager.matrix`` as a thread drawing off-screen sees it. + + Reports the surface's size, so plugins that lay out from ``matrix.width`` + follow it, and swallows every write that would reach the hardware. Nothing + drawn off-screen may touch the panel the render loop is driving. Method + names mirror the rgbmatrix API they stand in for. + """ + + # pylint: disable=invalid-name + __slots__ = () + + def SetImage(self, *_args: Any, **_kwargs: Any) -> None: + """Inert: off-screen drawing never reaches the panel.""" + + def SetPixel(self, *_args: Any, **_kwargs: Any) -> None: + """Inert: off-screen drawing never reaches the panel.""" + + def Clear(self) -> None: + """Inert: off-screen drawing never reaches the panel.""" + + def Fill(self, *_args: Any, **_kwargs: Any) -> None: + """Inert: off-screen drawing never reaches the panel.""" + + def SwapOnVSync(self, canvas: Any, *_args: Any, **_kwargs: Any) -> Any: + """Inert: hands the canvas straight back without waiting on the panel.""" + return canvas + + def __setattr__(self, name: str, value: Any) -> None: + """Inert: brightness and other writes stay off the real matrix.""" + + +class _OffscreenSurface: + """One thread's private canvas while it renders off-screen. + + See :meth:`DisplayManager.offscreen`. + """ + + __slots__ = ("draw", "image", "matrix") + + def __init__(self, width: int, height: int, real_matrix: Any) -> None: + self.image = Image.new('RGB', (width, height)) + self.draw = ImageDraw.Draw(self.image) + # 1-bit text: the panel has no partial brightness, so AA only smears glyphs. + self.draw.fontmode = "1" + self.matrix = (_OffscreenMatrix(real_matrix, width, height) + if real_matrix is not None else None) + + +def _per_thread_canvas_attr(name: str) -> property: + """A DisplayManager attribute that resolves per thread. + + A thread inside :meth:`DisplayManager.offscreen` reads and writes its own + surface's ``name``; every other thread reads and writes the shared value, + exactly as when this was a plain attribute. Existing ``self.image = ...`` + assignments therefore keep working and become thread-correct as they are. + """ + shared = "_shared_" + name + + def fget(self: "DisplayManager") -> Any: + surface = self._current_surface() # pylint: disable=protected-access + if surface is not None: + return getattr(surface, name) + try: + return self.__dict__[shared] + except KeyError: + raise AttributeError(name) from None + + def fset(self: "DisplayManager", value: Any) -> None: + surface = self._current_surface() # pylint: disable=protected-access + if surface is not None: + setattr(surface, name, value) + else: + self.__dict__[shared] = value + + return property(fget, fset, doc=f"The plugin-facing ``{name}``, per thread.") + + class DisplayManager: """ @@ -160,6 +238,11 @@ class DisplayManager: cls._instance = super(DisplayManager, cls).__new__(cls) return cls._instance + # The plugin-facing canvas. Per thread: see offscreen(). + image = _per_thread_canvas_attr("image") + draw = _per_thread_canvas_attr("draw") + matrix = _per_thread_canvas_attr("matrix") + def __init__(self, config: Dict[str, Any] = None, force_fallback: bool = False, suppress_test_pattern: bool = False): start_time = time.time() self.config = config or {} @@ -173,6 +256,9 @@ class DisplayManager: # suppress the render loop's own frame pushes for the duration, freezing # the panel exactly when the point was to avoid a freeze. self._capture_state = threading.local() + # Per-thread off-screen surface. While a thread is inside offscreen(), + # image, draw and matrix resolve to its own canvas; see offscreen(). + self._surface_state = threading.local() # Double-sided mode state (resolved in _setup_matrix). When disabled, # the logical image is blitted to the matrix unchanged. self._double_sided = None # dict {copies, axis, logical_width, logical_height} or None @@ -238,6 +324,11 @@ class DisplayManager: # See src/common/scroll_config.py and scripts/scroll_speeds.py. self._frame_hold = 1 + # A src.common.render_gate.RenderGate while Vegas runs with + # vegas_scroll.prefetch_gate on: opened around each swap so the + # prefetch thread only runs Python while this thread waits on vsync. + self.render_gate = None + # Timing of every presented frame, whoever drew it, for # scripts/frame_soak.py. See src/common/frame_timing.py. self.frame_timing = FrameTimingRecorder(info=self._frame_timing_info()) @@ -644,7 +735,10 @@ class DisplayManager: @property def _capture_mode_active(self) -> bool: """True while the calling thread is capturing content off-screen.""" - return getattr(self._capture_state, 'active', False) + # Read like _current_surface(): a DisplayManager built without + # __init__ (tests do) has no per-thread state, and captures nothing. + state = self.__dict__.get('_capture_state') + return getattr(state, 'active', False) if state is not None else False @_capture_mode_active.setter def _capture_mode_active(self, value: bool) -> None: @@ -660,11 +754,67 @@ class DisplayManager: Entering this context prevents those writes without affecting the PIL image buffer, which the adapter reads to extract content. """ + # Restore rather than clear: capture_mode() inside offscreen() must not + # switch suppression off for the rest of the off-screen block. + was_active = self._capture_mode_active self._capture_mode_active = True try: yield finally: - self._capture_mode_active = False + self._capture_mode_active = was_active + + def _current_surface(self) -> Optional[_OffscreenSurface]: + """The calling thread's off-screen surface, or None.""" + state = self.__dict__.get('_surface_state') + return getattr(state, 'surface', None) if state is not None else None + + def _writes_suppressed(self) -> bool: + """True when the calling thread must not touch the panel or its pacing.""" + return self._capture_mode_active or self._current_surface() is not None + + @contextmanager + def offscreen(self, width: Optional[int] = None, height: Optional[int] = None): + """Give the calling thread its own canvas to draw on. + + Inside the block, for the calling thread only, ``image``, ``draw`` and + ``matrix`` (and so ``width``/``height``) are a fresh black canvas of the + requested size, and nothing reaches the hardware: ``update_display()`` + and the hardware half of ``clear()`` are skipped, and + ``set_scrolling_state()``/``set_frame_hold()`` cannot re-pace the live + scroll. Every other thread, the render loop above all, keeps seeing the + real canvas. + + That is what lets Vegas mode render a plugin on its background prefetch + thread. The shared canvas used to be the only one, so any plugin that + drew on it (display capture, scroll-content generation, narrowed + rendering) had to be fetched on the render thread, stalling the scroll + for 40-600ms each. See docs/OFFSCREEN_RENDERING.md. + + Blocks nest; each restores the one outside it, also on an exception. + + Args: + width: Width of the surface, clamped to the size this thread sees + now. Defaults to that size. + height: Height, likewise. + + Yields: + The surface. ``surface.image`` is what the plugin drew. + """ + state = self.__dict__.get('_surface_state') + if state is None: + state = self._surface_state = threading.local() + + current_w, current_h = self.width, self.height + target_w = max(1, min(int(width), current_w)) if width else current_w + target_h = max(1, min(int(height), current_h)) if height else current_h + + surface = _OffscreenSurface(target_w, target_h, self.matrix) + previous = getattr(state, 'surface', None) + state.surface = surface + try: + yield surface + finally: + state.surface = previous @contextmanager def render_size(self, width: int, height: Optional[int] = None): @@ -684,18 +834,14 @@ class DisplayManager: indirection that double-sided mode relies on, so plugins see a consistent size from every accessor. - Only meaningful inside :meth:`capture_mode` — this swaps the shared - image buffer, so the render loop must not be writing to it concurrently. + Built on :meth:`offscreen`, so the narrower canvas belongs to the + calling thread alone; the render loop keeps drawing on the real one. Args: width: Logical width to report, clamped to at least 1 and to the real panel width (a larger canvas would overflow the hardware). height: Logical height, defaulting to the current height. """ - real_matrix = self.matrix - prev_image = getattr(self, 'image', None) - prev_draw = getattr(self, 'draw', None) - current_w = self.width current_h = self.height target_w = max(1, min(int(width), current_w)) @@ -706,19 +852,8 @@ class DisplayManager: yield return - try: - if real_matrix is not None: - self.matrix = _LogicalMatrix(real_matrix, target_w, target_h) - # With no hardware, the width/height properties fall through to - # self.image, so swapping the buffer below is enough on its own. - self._new_canvas(target_w, target_h) + with self.offscreen(target_w, target_h): yield - finally: - self.matrix = real_matrix - if prev_image is not None: - self.image = prev_image - if prev_draw is not None: - self.draw = prev_draw def _composite_double_sided(self): """Tile the logical screen across the full physical chain. @@ -763,6 +898,12 @@ class DisplayManager: need to know about it. """ try: + if self._writes_suppressed(): + # This thread is drawing off-screen. Checked before the lock, + # so it never contends with the render loop's swap, and before + # the fallback branch, so captured content never reaches the + # web preview either. + return with self._update_lock: if self.matrix is None: # Fallback mode - no actual hardware to update @@ -771,9 +912,6 @@ class DisplayManager: self._write_snapshot_if_due() return - if self._capture_mode_active: - return # Skip hardware write — content is being captured off-screen - digest = None frame_checksum = None if self._dirty_tracking_enabled: @@ -816,7 +954,12 @@ class DisplayManager: # Swap buffers immediately. framerate_fraction holds the frame # for N refreshes; SwapOnVSync blocks for all of them, which is # what paces the render loop to the chosen frame rate. + gate = self.render_gate + if gate is not None: + gate.before_swap(self._frame_hold) self.matrix.SwapOnVSync(self.offscreen_canvas, self._frame_hold) + if gate is not None: + gate.after_swap(self._frame_hold) presented_at = time.perf_counter() self.frame_timing.record( blit_done - blit_started, presented_at - blit_done, @@ -888,7 +1031,7 @@ class DisplayManager: self._new_canvas(self.matrix.width, self.matrix.height) - if not self._capture_mode_active: + if not self._writes_suppressed(): # Clear both canvases and the underlying matrix to ensure no artifacts. # Failures are non-fatal — the image buffer is already black above, so # the next update_display() call will push clean content regardless. @@ -1480,6 +1623,8 @@ class DisplayManager: Reset to 1 whenever scrolling stops, so one plugin's pacing cannot leak into the next thing on screen. """ + if self._writes_suppressed(): + return # a plugin drawing off-screen cannot re-pace the live scroll try: value = int(refreshes) except (TypeError, ValueError): @@ -1508,6 +1653,10 @@ class DisplayManager: the lifetime exactly the scroll, and the default of 1 means any caller that does not care gets a new frame every refresh. """ + if self._writes_suppressed(): + # A plugin captured for Vegas calls this from its own display(); + # it must not change the live scroll's state or frame hold. + return current_time = time.time() # Scrolling callers set this every frame; log transitions only. changed = self._scrolling_state['is_scrolling'] != is_scrolling diff --git a/src/plugin_system/testing/visual_display_manager.py b/src/plugin_system/testing/visual_display_manager.py index d3e7ba13..4ce4289c 100644 --- a/src/plugin_system/testing/visual_display_manager.py +++ b/src/plugin_system/testing/visual_display_manager.py @@ -18,7 +18,7 @@ get_font_height, get_text_width, draw_text, draw_text_with_icons, draw_weather_icon (and the _draw_sun/_draw_cloud/ _draw_rain/_draw_snow/_draw_storm family), format_date_with_ordinal, capture_mode, set_scrolling_state, is_currently_scrolling, -process_deferred_updates, update_display, render_size. A behavior +process_deferred_updates, update_display, render_size, offscreen. A behavior change to any of those in DisplayManager must be mirrored here, or plugin visual tests will pass against stale behavior. @@ -243,11 +243,39 @@ class VisualTestDisplayManager: wraps every off-screen content fetch in this context, so the harness must provide it for that code path to be exercisable in tests. """ + was_active = self._capture_mode_active self._capture_mode_active = True try: yield finally: - self._capture_mode_active = False + self._capture_mode_active = was_active + + @contextmanager + def offscreen(self, width: Optional[int] = None, height: Optional[int] = None): + """ + Interface parity with DisplayManager.offscreen(). + + Vegas mode's PluginAdapter draws every plugin on a canvas of its own. + The real display manager keeps that canvas per thread; the harness is + single-threaded, so it swaps a fresh canvas in and restores the old one, + which is all a test can observe. + """ + prev = (self.image, self.draw, self._width, self._height, + self.matrix, self._capture_mode_active) + target_w = max(1, min(int(width), self._width)) if width else self._width + target_h = max(1, min(int(height), self._height)) if height else self._height + try: + self._width, self._height = target_w, target_h + self.matrix = _MatrixProxy(target_w, target_h) + self.image = Image.new('RGB', (target_w, target_h), (0, 0, 0)) + self.draw = ImageDraw.Draw(self.image) + # Match production: 1-bit text, so goldens show what the panel shows. + self.draw.fontmode = "1" + self._capture_mode_active = True + yield self + finally: + (self.image, self.draw, self._width, self._height, + self.matrix, self._capture_mode_active) = prev def draw_text(self, text: str, x: Optional[int] = None, y: Optional[int] = None, color: Tuple[int, int, int] = (255, 255, 255), small_font: bool = False, diff --git a/src/vegas_mode/config.py b/src/vegas_mode/config.py index 06e09400..f009bdeb 100644 --- a/src/vegas_mode/config.py +++ b/src/vegas_mode/config.py @@ -74,6 +74,31 @@ class VegasModeConfig: # precedence over smooth_scroll's whole-pixel pacing when on. sub_pixel_blend: bool = False + # Render every plugin's ticker content on the background prefetch thread, + # each on a canvas of its own (DisplayManager.offscreen), instead of + # handing plugins that draw on the display canvas to the render thread one + # at a time. Each of those cost the scroll a 40-600ms pause. False restores + # that path; it is kept for one release in case a plugin misbehaves when + # drawn off the render thread. See docs/OFFSCREEN_RENDERING.md. + offscreen_prefetch: bool = True + + # How long another thread may hold the GIL before the render thread's + # request forces it to yield, in ms, while Vegas runs. CPython's default is + # 5ms. Plugin rendering on the prefetch thread and plugin updates hold the + # GIL in Pillow and Python code, and a frame waiting its turn for 5ms at a + # time misses its refresh. 0 leaves the interpreter default alone. + # Experimental. On hdpi it did less than prefetch_gate (0.90% -> 0.78% late + # against 0.60%; see docs/OFFSCREEN_RENDERING.md), so it stays off. + switch_interval_ms: float = 0.0 + + # Let the prefetch thread run Python only while the render thread is + # blocked waiting for vsync, and park it the rest of the time, so the + # render thread never waits for the GIL when its refresh comes round. Needs + # a binding that releases the GIL in SwapOnVSync; off otherwise. On hdpi + # it cut frames two or more refreshes late eightfold, and late frames + # overall from 0.90% to 0.60%. See src/common/render_gate.py. + prefetch_gate: bool = True + # Keep one continuous strip, extending it with the next group of plugins as # the scroll approaches the end, instead of composing a fresh strip and # swapping it in. A swap stops the motion, substitutes every pixel at once @@ -207,6 +232,9 @@ class VegasModeConfig: smooth_scroll=get('smooth_scroll', d.smooth_scroll), sub_pixel_blend=bool(get('sub_pixel_blend', d.sub_pixel_blend)), continuous_scroll=get('continuous_scroll', d.continuous_scroll), + offscreen_prefetch=bool(get('offscreen_prefetch', d.offscreen_prefetch)), + switch_interval_ms=float(get('switch_interval_ms', d.switch_interval_ms) or 0.0), + prefetch_gate=bool(get('prefetch_gate', d.prefetch_gate)), extend_threshold_screens=float( get('extend_threshold_screens', d.extend_threshold_screens)), auto_trim=get('auto_trim', d.auto_trim), @@ -250,6 +278,9 @@ class VegasModeConfig: 'smooth_scroll': self.smooth_scroll, 'sub_pixel_blend': self.sub_pixel_blend, 'continuous_scroll': self.continuous_scroll, + 'offscreen_prefetch': self.offscreen_prefetch, + 'switch_interval_ms': self.switch_interval_ms, + 'prefetch_gate': self.prefetch_gate, 'extend_threshold_screens': self.extend_threshold_screens, 'auto_trim': self.auto_trim, 'trim_threshold': self.trim_threshold, diff --git a/src/vegas_mode/coordinator.py b/src/vegas_mode/coordinator.py index 1a0f1876..3de38bc8 100644 --- a/src/vegas_mode/coordinator.py +++ b/src/vegas_mode/coordinator.py @@ -13,10 +13,12 @@ Supports three display modes per plugin: import logging import math +import sys import time import threading from typing import Optional, Dict, Any, List, Callable, TYPE_CHECKING +from src.common import render_gate from src.vegas_mode.config import VegasModeConfig from src.vegas_mode.plugin_adapter import PluginAdapter from src.vegas_mode.stream_manager import StreamManager @@ -101,7 +103,8 @@ class VegasModeCoordinator: self.plugin_manager = plugin_manager # Initialize components - self.plugin_adapter = PluginAdapter(display_manager, self.vegas_config) + self.plugin_adapter = PluginAdapter( + display_manager, self.vegas_config, plugin_manager=plugin_manager) self.stream_manager = StreamManager( self.vegas_config, plugin_manager, @@ -276,6 +279,8 @@ class VegasModeCoordinator: # due immediately so the first sample confirms the marquee is up. self._fps_last_health_log = 0.0 self._fps_was_degraded = False + self._apply_switch_interval() + self._install_render_gate() # Line up the next group immediately, so the first extension is already # warm rather than stalling the scroll to fetch it. @@ -298,6 +303,9 @@ class VegasModeCoordinator: self.stats['total_runtime_seconds'] += time.time() - self._start_time self._start_time = None + self._restore_switch_interval() + self._remove_render_gate() + # Cleanup components self.render_pipeline.reset() self.stream_manager.reset() @@ -305,6 +313,57 @@ class VegasModeCoordinator: logger.info("Vegas mode stopped") + def _apply_switch_interval(self) -> None: + """Shorten the GIL switch interval for the run; see VegasModeConfig.""" + ms = self.vegas_config.switch_interval_ms + if not ms or ms <= 0: + return + if getattr(self, '_saved_switch_interval', None) is None: + self._saved_switch_interval = sys.getswitchinterval() + sys.setswitchinterval(ms / 1000.0) + logger.info("Vegas: GIL switch interval %.1fms (was %.1fms)", + ms, self._saved_switch_interval * 1000.0) + + def _restore_switch_interval(self) -> None: + saved = getattr(self, '_saved_switch_interval', None) + if saved is not None: + sys.setswitchinterval(saved) + self._saved_switch_interval = None + + def _install_render_gate(self) -> None: + """Gate the prefetch thread on the render thread's swaps; see VegasModeConfig.""" + if not self.vegas_config.prefetch_gate: + return + if getattr(self.display_manager, 'render_gate', None) is not None: + return + releases = render_gate.swap_releases_gil() + if releases is None: + logger.debug("Vegas: no prefetch gate -- no hardware binding loaded") + return + if not releases: + # On by default, so this is every stock install: say so once per + # run, not as a warning. + logger.info("Vegas: no prefetch gate -- this rgbmatrix binding keeps " + "the GIL in SwapOnVSync (scripts/build_rgbmatrix_nogil.sh)") + return + gate = render_gate.RenderGate() + # Locks the render thread takes too: never park the prefetch holding one. + gate.guard(self._state_lock, + getattr(self.stream_manager, '_buffer_lock', None), + getattr(self.render_pipeline, '_buffer_lock', None), + getattr(self.render_pipeline, '_prefetch_lock', None), + getattr(self.plugin_adapter, '_cache_lock', None)) + self.display_manager.render_gate = gate + logger.info("Vegas: prefetch gated on vsync") + + def _remove_render_gate(self) -> None: + gate = getattr(self.display_manager, 'render_gate', None) + if gate is None: + return + self.display_manager.render_gate = None + logger.info("Vegas: prefetch gate parked the prefetch %d times, %.1fs in all", + gate.parks, gate.parked_seconds) + def pause(self) -> None: """Pause Vegas mode (for live priority interruption).""" with self._state_lock: @@ -524,6 +583,10 @@ class VegasModeCoordinator: fps, target, fps_frame_count, p99 * 1000.0, frame_worst * 1000.0 ) + gate = getattr(self.display_manager, 'render_gate', None) + if gate is not None: + logger.info("Vegas: prefetch parked %d times, %.1fs in all", + gate.parks, gate.parked_seconds) self._fps_last_health_log = current_time else: logger.debug( diff --git a/src/vegas_mode/plugin_adapter.py b/src/vegas_mode/plugin_adapter.py index e7a579b4..fc531379 100644 --- a/src/vegas_mode/plugin_adapter.py +++ b/src/vegas_mode/plugin_adapter.py @@ -8,7 +8,7 @@ implement get_vegas_content() and fallback capture of display() output. import logging import threading import time -from contextlib import nullcontext +from contextlib import contextmanager, nullcontext from typing import Optional, List, Any, Tuple, Union, TYPE_CHECKING from PIL import Image @@ -33,7 +33,13 @@ class PluginAdapter: 2. Fallback: Capture display_manager.image after calling plugin.display() """ - def __init__(self, display_manager: Any, config: Optional[Any] = None): + #: How long a background fetch waits for a plugin's update() to finish + #: before skipping the plugin this round. Off the render thread waiting + #: costs nothing visible; it only delays that one plugin's content. + PLUGIN_LOCK_TIMEOUT = 2.0 + + def __init__(self, display_manager: Any, config: Optional[Any] = None, + plugin_manager: Optional[Any] = None): """ Initialize the plugin adapter. @@ -42,8 +48,13 @@ class PluginAdapter: config: VegasModeConfig controlling trim behaviour. When omitted, trimming runs with the dataclass defaults, so existing callers and tests keep working unchanged. + plugin_manager: Source of the per-plugin lock that keeps a + background fetch from running a plugin's display() while its + update() is mid-flight. Optional: without it, fetches take no + lock, as they always did. """ self.display_manager = display_manager + self.plugin_manager = plugin_manager if config is None: from src.vegas_mode.config import VegasModeConfig config = VegasModeConfig() @@ -91,13 +102,14 @@ class PluginAdapter: Args: plugin: Plugin instance to get content from plugin_id: Plugin identifier for logging - offscreen_only: Skip every path that touches the shared display - canvas, for callers running off the render thread. The canvas - and the matrix proxy are process-wide mutable state, so - narrowing or capturing through them from another thread would - corrupt the frame the render loop is pushing. Returns None when - the plugin can only be served that way, leaving the caller to - fetch it on the render thread. + offscreen_only: The caller is off the render thread. Every content + path draws on a canvas of its own (DisplayManager.offscreen), + so all of them are safe there; the fetch also takes the + plugin's lock, waiting up to PLUGIN_LOCK_TIMEOUT for a running + update() to finish. With ``offscreen_prefetch`` switched off, + the old behaviour applies instead: paths that need a canvas + return None, leaving the caller to fetch the plugin on the + render thread. Returns: List of PIL Images representing plugin content, or None if no content @@ -117,11 +129,77 @@ class PluginAdapter: ) return cached + # The old contract, kept behind the switch: background callers may + # not draw, so anything needing a canvas is left for the render thread. + restricted = offscreen_only and not getattr( + self.config, 'offscreen_prefetch', True) + if not offscreen_only or restricted: + return self._fetch_content(plugin, plugin_id, restricted) + + with self._plugin_lock(plugin_id) as acquired: + if not acquired: + logger.warning( + "[%s] update() still running after %.0fs; skipping it this " + "round", plugin_id, self.PLUGIN_LOCK_TIMEOUT + ) + return None + return self._fetch_content(plugin, plugin_id, restricted=False) + + @contextmanager + def _plugin_lock(self, plugin_id: str): + """Hold the plugin's update/display lock, waiting a bounded time. + + Yields whether it was acquired. Yields True, holding nothing, when + there is no plugin manager to ask -- the behaviour before the lock was + taken here at all. + """ + if not hasattr(self.plugin_manager, 'get_plugin_lock'): + yield True + return + lock = self.plugin_manager.get_plugin_lock(plugin_id) + acquired = lock.acquire(timeout=self.PLUGIN_LOCK_TIMEOUT) + try: + yield acquired + finally: + if acquired: + lock.release() + + @contextmanager + def _isolated_canvas(self, width: Optional[int] = None): + """A canvas for the plugin to draw on that nothing else sees. + + DisplayManager.offscreen() gives the calling thread its own canvas, so + this is safe on any thread and leaves the shared canvas untouched. + Older display managers and test doubles without it get the previous + behaviour: capture on the shared canvas, narrowed with render_size, + then restore it -- which is only safe on the render thread. + """ + offscreen = getattr(self.display_manager, 'offscreen', None) + if offscreen is not None: + with offscreen(width): + yield + return + + original_image = self.display_manager.image.copy() + try: + with self._capture(), self._render_at(width or self.display_width): + yield + finally: + self.display_manager.image = original_image + + def _fetch_content( + self, plugin: 'BasePlugin', plugin_id: str, restricted: bool + ) -> Optional[List[Image.Image]]: + """Every content path in order: native, scroll helper, display capture. + + ``restricted`` is the pre-offscreen contract for background callers: + skip every path that needs a canvas and return None instead. + """ # Try native Vegas content method first has_native = hasattr(plugin, 'get_vegas_content') logger.debug("[%s] Has get_vegas_content: %s", plugin_id, has_native) if has_native: - content = self._get_native_content(plugin, plugin_id, offscreen_only) + content = self._get_native_content(plugin, plugin_id, restricted) if content: total_width = sum(img.width for img in content) logger.debug( @@ -134,7 +212,7 @@ class PluginAdapter: # Try to get scroll_helper's cached image (for scrolling plugins like stocks/odds) has_scroll_helper = hasattr(plugin, 'scroll_helper') logger.debug("[%s] Has scroll_helper: %s", plugin_id, has_scroll_helper) - content = self._get_scroll_helper_content(plugin, plugin_id, offscreen_only) + content = self._get_scroll_helper_content(plugin, plugin_id, restricted) if content: total_width = sum(img.width for img in content) logger.debug( @@ -145,8 +223,8 @@ class PluginAdapter: if has_scroll_helper: logger.debug("[%s] ScrollHelper content returned None", plugin_id) - if offscreen_only: - # Display capture needs the shared canvas; leave it to the caller. + if restricted: + # Display capture needs a canvas; leave it to the caller. logger.debug( "[%s] Needs display capture, deferring to the render thread", plugin_id @@ -682,7 +760,7 @@ class PluginAdapter: return img.crop((start, 0, end, img.height)) def _get_native_content( - self, plugin: 'BasePlugin', plugin_id: str, offscreen_only: bool = False + self, plugin: 'BasePlugin', plugin_id: str, restricted: bool = False ) -> Optional[List[Image.Image]]: """ Get content via plugin's native get_vegas_content() method. @@ -711,22 +789,21 @@ class PluginAdapter: 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. - if offscreen_only: - # _render_at swaps the shared canvas, so it is unsafe here. - # _vegas_render_width is set regardless: a plugin reading - # get_vegas_render_width() still gets its narrow size, and - # one that only reads matrix.width renders full width and is - # trimmed instead. + # On a canvas of its own 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 on the shared canvas + # that write would land on the hardware, flashing the panel + # mid-scroll. + if restricted: + # Restricted (offscreen_prefetch off): no canvas of our own, + # so no narrowing. _vegas_render_width is set regardless: a + # plugin reading get_vegas_render_width() still gets its + # narrow size, and one that only reads matrix.width renders + # full width and is trimmed instead. with self._capture(): result = plugin.get_vegas_content() else: - with self._capture(), self._render_at(render_width): + with self._isolated_canvas(render_width): result = plugin.get_vegas_content() finally: plugin._vegas_render_width = None @@ -807,7 +884,7 @@ class PluginAdapter: return None def _get_scroll_helper_content( - self, plugin: 'BasePlugin', plugin_id: str, offscreen_only: bool = False + self, plugin: 'BasePlugin', plugin_id: str, restricted: bool = False ) -> Optional[List[Image.Image]]: """ Get content from plugin's scroll_helper if available. @@ -841,7 +918,7 @@ class PluginAdapter: "[%s] scroll_helper.cached_image is None, triggering content generation", plugin_id ) - if offscreen_only: + if restricted: # Generating it calls display(), which needs the canvas. logger.debug( "[%s] scroll_helper cache empty; deferring generation " @@ -991,12 +1068,8 @@ class PluginAdapter: Returns: The generated cached_image or None """ - original_image = None try: - # Save display state to restore after - original_image = self.display_manager.image.copy() - - with self._capture(): + with self._isolated_canvas(): # Method 1: Try _create_scrolling_display (stocks pattern) if hasattr(plugin, '_create_scrolling_display'): logger.debug( @@ -1052,11 +1125,6 @@ class PluginAdapter: logger.exception("[%s] Error triggering scroll content", plugin_id) return None - finally: - # Restore original display state - if original_image is not None: - self.display_manager.image = original_image - def _capture_display_content( self, plugin: 'BasePlugin', plugin_id: str ) -> Optional[List[Image.Image]]: @@ -1070,12 +1138,7 @@ class PluginAdapter: Returns: List with single captured image, or None """ - original_image = None try: - # Save current display state - original_image = self.display_manager.image.copy() - logger.debug("[%s] Fallback: saved original display state", plugin_id) - # Ensure plugin has fresh data before capturing has_update_data = hasattr(plugin, 'update_data') logger.debug("[%s] Fallback: has update_data=%s", plugin_id, has_update_data) @@ -1086,12 +1149,12 @@ class PluginAdapter: except (AttributeError, RuntimeError, OSError): logger.exception("[%s] Fallback: update_data() failed", plugin_id) - # Clear and call plugin display — use capture_mode to suppress hardware writes - # that plugins may trigger internally via update_display(). + # Clear and call plugin display on a canvas of its own: nothing it + # draws, and no update_display() it calls, reaches the panel. # - # render_size narrows the canvas the plugin lays out against, so a - # plugin that spreads across the whole panel produces a compact - # arrangement rather than one that has to be cropped afterwards. + # The canvas is render_width wide, so a plugin that spreads across + # the whole panel produces a compact arrangement rather than one + # that has to be cropped afterwards. render_width = self.resolve_render_width(plugin, plugin_id) if render_width != self.display_width: logger.debug( @@ -1099,7 +1162,7 @@ class PluginAdapter: plugin_id, render_width, self.display_width ) - with self._capture(), self._render_at(render_width): + with self._isolated_canvas(render_width): self.display_manager.clear() logger.debug("[%s] Fallback: display cleared, calling display()", plugin_id) @@ -1133,7 +1196,7 @@ class PluginAdapter: plugin_id ) # Try once more with force_clear=True - with self._capture(), self._render_at(render_width): + with self._isolated_canvas(render_width): self.display_manager.clear() plugin.display(force_clear=True) captured = self.display_manager.image.copy() @@ -1170,12 +1233,6 @@ class PluginAdapter: ) return None - finally: - # Always restore original image to prevent display corruption - if original_image is not None: - self.display_manager.image = original_image - logger.debug("[%s] Fallback: restored original display state", plugin_id) - def _is_blank_image( self, img: Image.Image, return_ratio: bool = False ) -> Union[bool, Tuple[bool, float]]: diff --git a/src/vegas_mode/render_pipeline.py b/src/vegas_mode/render_pipeline.py index 7bb0ca12..2c1ebe00 100644 --- a/src/vegas_mode/render_pipeline.py +++ b/src/vegas_mode/render_pipeline.py @@ -10,6 +10,7 @@ import os import time import threading from collections import deque +from contextlib import nullcontext from typing import Optional, List, Any, Dict, Deque from PIL import Image @@ -383,8 +384,12 @@ class RenderPipeline: os.nice(10) except (OSError, AttributeError): pass + # With vegas_scroll.prefetch_gate on, run only while the render + # thread waits on vsync; see src/common/render_gate.py. + gate = getattr(self.display_manager, 'render_gate', None) try: - group = self.stream_manager.take_next_group(offscreen_only=True) + with gate.yielding() if gate is not None else nullcontext(): + group = self.stream_manager.take_next_group(offscreen_only=True) except Exception: logger.exception("Background prefetch failed") group = [] @@ -509,9 +514,12 @@ class RenderPipeline: grouped = [(pid, imgs) for pid, imgs in grouped if imgs] if not grouped: - # Everything in this group is queued; the queue will extend the - # strip as it drains, so this is not a failure. - logger.info("Whole group deferred; strip will extend as it drains") + if deferred: + # Everything in this group is queued; the queue will extend + # the strip as it drains, so this is not a failure. + logger.info("Whole group deferred; strip will extend as it drains") + else: + logger.info("Nothing to show in this group; fetching the next") self.start_prefetch() return bool(deferred) diff --git a/src/vegas_mode/stream_manager.py b/src/vegas_mode/stream_manager.py index 8faaaead..cdf324e0 100644 --- a/src/vegas_mode/stream_manager.py +++ b/src/vegas_mode/stream_manager.py @@ -688,6 +688,10 @@ class StreamManager: Ordered list of (plugin_id, images). ``images`` is None when the plugin could not be served under ``offscreen_only``, so the caller can fetch just those on the render thread while keeping the order. + That only happens with ``offscreen_prefetch`` switched off: every + content path now draws on a canvas of its own, so a background + fetch that comes back empty had nothing to show, and ``images`` + is an empty list rather than a request for the render thread. """ if count is None: count = self.config.plugins_per_cycle @@ -705,6 +709,9 @@ class StreamManager: plugins = getattr(self.plugin_manager, 'plugins', {}) group: List[Tuple[str, Optional[List[Image.Image]]]] = [] + # Only the old contract hands anything back to the render thread. + defer_empty = offscreen_only and not getattr( + self.config, 'offscreen_prefetch', True) for plugin_id in ids: plugin = plugins.get(plugin_id) @@ -719,7 +726,10 @@ class StreamManager: continue if images: self.stats['segments_fetched'] += 1 - group.append((plugin_id, images if images else None)) + if images: + group.append((plugin_id, images)) + else: + group.append((plugin_id, None if defer_empty else [])) return group diff --git a/test/test_offscreen_rendering.py b/test/test_offscreen_rendering.py new file mode 100644 index 00000000..866b6121 --- /dev/null +++ b/test/test_offscreen_rendering.py @@ -0,0 +1,360 @@ +"""Per-thread off-screen rendering (DisplayManager.offscreen) and its Vegas use. + +The display canvas used to be one shared object, so any plugin that drew on it +could only be rendered on the render thread, stalling the scroll for 40-600ms +each. offscreen() gives the calling thread a canvas of its own. These tests pin +the property that makes that safe: another thread's drawing never reaches what +the render loop sees, presents or paces by. + +Runs against RGBMatrixEmulator, exercising the real DisplayManager. +""" + +import os +import sys +import threading +from types import SimpleNamespace + +os.environ["EMULATOR"] = "true" + +import pytest +from PIL import Image + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from src.vegas_mode.config import VegasModeConfig # noqa: E402 +from src.vegas_mode.plugin_adapter import PluginAdapter # noqa: E402 +from src.vegas_mode.stream_manager import StreamManager # noqa: E402 + +WIDTH, HEIGHT = 128, 32 +YELLOW = (255, 255, 0) + + +@pytest.fixture(scope="module") +def dm(tmp_path_factory): + from src.display_manager import DisplayManager + DisplayManager._instance = None + DisplayManager._initialized = False + manager = DisplayManager({ + "display": { + "hardware": {"rows": 32, "cols": 64, "chain_length": 2, + "parallel": 1, "brightness": 90}, + "runtime": {"gpio_slowdown": 0}, + }, + }, suppress_test_pattern=True) + manager._snapshot_path = str( + tmp_path_factory.mktemp("offscreen") / "led_matrix_preview.png") + if manager.matrix is None: + pytest.fail("DisplayManager fell back to matrix=None; see the " + "'Failed to initialize RGB Matrix' log line above.") + yield manager + DisplayManager._instance = None + DisplayManager._initialized = False + + +@pytest.fixture +def fresh(dm): + """A known shared canvas, and scroll state reset around each test.""" + dm.image = Image.new('RGB', (WIDTH, HEIGHT)) + from PIL import ImageDraw + dm.draw = ImageDraw.Draw(dm.image) + dm.set_scrolling_state(False) + yield dm + dm.set_scrolling_state(False) + + +class _Spy: + """Counts calls to one method of one object, and still calls it.""" + + def __init__(self, obj, name): + self.obj, self.name, self.count = obj, name, 0 + self._orig = getattr(obj, name) + + def __enter__(self): + def counting(*args, **kwargs): + self.count += 1 + return self._orig(*args, **kwargs) + setattr(self.obj, self.name, counting) + return self + + def __exit__(self, *exc): + setattr(self.obj, self.name, self._orig) + + +def _in_thread(fn): + """Run fn on another thread and return its result, re-raising its error.""" + box = {} + + def run(): + try: + box["value"] = fn() + except BaseException as exc: # surfaced below + box["error"] = exc + + thread = threading.Thread(target=run) + thread.start() + thread.join(10) + assert not thread.is_alive(), "worker hung" + if "error" in box: + raise box["error"] + return box.get("value") + + +class TestIsolation: + def test_another_threads_drawing_is_invisible_here(self, fresh): + dm = fresh + shared = dm.image + inside, release = threading.Event(), threading.Event() + seen = {} + + def worker(): + with dm.offscreen(40) as surface: + dm.draw.rectangle([0, 0, 39, 31], fill=YELLOW) + seen.update(width=dm.width, matrix_width=dm.matrix.width, + own_image=dm.image is surface.image) + inside.set() + release.wait(5) + seen["restored"] = dm.image is shared + + thread = threading.Thread(target=worker) + thread.start() + try: + assert inside.wait(5) + # While the worker is mid-draw, this thread sees the real canvas. + assert dm.image is shared + assert (dm.width, dm.matrix.width) == (WIDTH, WIDTH) + assert shared.getpixel((0, 0)) == (0, 0, 0) + finally: + release.set() + thread.join(5) + assert seen == {"width": 40, "matrix_width": 40, "own_image": True, + "restored": True} + + def test_the_render_loop_keeps_presenting_while_another_thread_draws(self, fresh): + dm = fresh + inside, release = threading.Event(), threading.Event() + + def worker(): + with dm.offscreen(): + dm.draw.rectangle([0, 0, 10, 10], fill=YELLOW) + dm.update_display() # must not reach the panel + inside.set() + release.wait(5) + + with _Spy(dm.matrix, "SwapOnVSync") as swaps: + thread = threading.Thread(target=worker) + thread.start() + try: + assert inside.wait(5) + dm.draw.point((5, 5), fill=(0, 0, 255)) + dm.update_display() # the render thread's own frame + finally: + release.set() + thread.join(5) + assert swaps.count == 1 + + def test_assignments_go_to_the_callers_canvas(self, fresh): + dm = fresh + shared = dm.image + replacement = Image.new('RGB', (WIDTH, HEIGHT), (1, 2, 3)) + + def worker(): + with dm.offscreen(): + dm.image = replacement # e.g. clear() inside display() + return dm.image is replacement + + assert _in_thread(worker) is True + assert dm.image is shared + + def test_render_size_narrows_only_the_calling_thread(self, fresh): + dm = fresh + inside, release = threading.Event(), threading.Event() + seen = {} + + def worker(): + with dm.render_size(32): + seen["width"] = dm.width + inside.set() + release.wait(5) + + thread = threading.Thread(target=worker) + thread.start() + try: + assert inside.wait(5) + assert dm.width == WIDTH + finally: + release.set() + thread.join(5) + assert seen["width"] == 32 + + +class TestNothingReachesThePanel: + def test_update_display_clear_and_pacing_are_inert_inside(self, fresh): + dm = fresh + real_matrix = dm.matrix + hold_before = dm._frame_hold + with _Spy(real_matrix, "SwapOnVSync") as swaps, \ + _Spy(dm.offscreen_canvas, "Clear") as clears: + with dm.offscreen() as surface: + dm.draw.rectangle([0, 0, 5, 5], fill=YELLOW) + dm.update_display() + drawn = dm.image + dm.clear() + # clear() replaced the surface's image, not the shared one. + assert dm.image is surface.image and dm.image is not drawn + assert dm.image.getpixel((0, 0)) == (0, 0, 0) + dm.set_scrolling_state(True, 3) + dm.set_frame_hold(4) + dm.matrix.SwapOnVSync(object()) # inert through the proxy too + assert swaps.count == 0 + assert clears.count == 0 + assert dm._frame_hold == hold_before + assert dm._scrolling_state['is_scrolling'] is False + + def test_capture_mode_inside_offscreen_does_not_end_suppression(self, fresh): + dm = fresh + real_matrix = dm.matrix + with _Spy(real_matrix, "SwapOnVSync") as swaps: + with dm.offscreen(): + with dm.capture_mode(): + pass + dm.draw.point((1, 1), fill=YELLOW) + dm.update_display() + assert swaps.count == 0 + + def test_nesting_and_exceptions_restore_state(self, fresh): + dm = fresh + shared = dm.image + with pytest.raises(RuntimeError): + with dm.offscreen(100): + with dm.offscreen(50): + assert dm.width == 50 + raise RuntimeError("plugin failed mid-draw") + assert dm.image is shared + assert dm.width == WIDTH + assert not dm._writes_suppressed() + + with dm.offscreen(100) as outer: + with dm.offscreen(50): + pass + assert dm.image is outer.image + assert dm.width == 100 + + +# --- Vegas: the adapter draws plugins off the render thread ------------------ + +class CapturePlugin: + """A plugin with neither get_vegas_content nor a scroll helper: captured.""" + + def __init__(self, display_manager): + self.display_manager = display_manager + self.calls = 0 + + def display(self, force_clear=False): + self.calls += 1 + self.display_manager.draw.rectangle([0, 0, 30, 20], fill=YELLOW) + self.display_manager.update_display() + + +class ScrollingPlugin: + """A ticker whose scroll image only exists once display() has run.""" + + def __init__(self, display_manager): + from src.common.scroll_helper import ScrollHelper + self.display_manager = display_manager + self.scroll_helper = ScrollHelper(WIDTH, HEIGHT) + + def display(self, force_clear=False): + item = Image.new('RGB', (300, HEIGHT), YELLOW) + self.scroll_helper.create_scrolling_image([item], item_gap=0, element_gap=0) + self.display_manager.set_scrolling_state(True, 5) + self.display_manager.update_display() + + +def _adapter(dm, **config): + return PluginAdapter(dm, VegasModeConfig(**config)) + + +def _has_yellow(images): + return any(YELLOW in {img.getpixel((x, y)) for x in range(min(img.width, 40)) + for y in range(img.height)} for img in images) + + +class TestAdapterOffTheRenderThread: + def test_display_capture_runs_on_a_background_thread(self, fresh): + dm = fresh + shared = dm.image + before = shared.tobytes() + plugin = CapturePlugin(dm) + adapter = _adapter(dm) + + with _Spy(dm.matrix, "SwapOnVSync") as swaps: + images = _in_thread( + lambda: adapter.get_content(plugin, "capture-bg", offscreen_only=True)) + + assert images and _has_yellow(images) + assert dm.image is shared and shared.tobytes() == before + assert swaps.count == 0 + + def test_scroll_content_is_generated_on_a_background_thread(self, fresh): + dm = fresh + plugin = ScrollingPlugin(dm) + adapter = _adapter(dm) + hold_before = dm._frame_hold + + images = _in_thread( + lambda: adapter.get_content(plugin, "scroll-bg", offscreen_only=True)) + + assert images and _has_yellow(images) + # The plugin's own set_scrolling_state(True, 5) did not re-pace Vegas. + assert dm._frame_hold == hold_before + assert dm._scrolling_state['is_scrolling'] is False + + def test_with_the_switch_off_background_capture_is_left_for_the_render_thread(self, fresh): + dm = fresh + plugin = CapturePlugin(dm) + adapter = _adapter(dm, offscreen_prefetch=False) + + images = _in_thread( + lambda: adapter.get_content(plugin, "capture-legacy", offscreen_only=True)) + + assert images is None + assert plugin.calls == 0 + + +class TestPluginLock: + def test_a_background_fetch_waits_for_update_then_skips(self, fresh): + dm = fresh + lock = threading.Lock() + plugin = CapturePlugin(dm) + adapter = PluginAdapter(dm, VegasModeConfig(), + plugin_manager=SimpleNamespace( + get_plugin_lock=lambda plugin_id: lock)) + adapter.PLUGIN_LOCK_TIMEOUT = 0.05 + + with lock: # update() in progress + skipped = _in_thread( + lambda: adapter.get_content(plugin, "locked", offscreen_only=True)) + assert skipped is None + assert plugin.calls == 0 + + served = _in_thread( + lambda: adapter.get_content(plugin, "locked", offscreen_only=True)) + assert served and plugin.calls == 1 + assert not lock.locked() + + +class TestStreamDoesNotDeferEmptyResults: + def _group(self, offscreen_prefetch): + config = VegasModeConfig(offscreen_prefetch=offscreen_prefetch) + adapter = SimpleNamespace(get_content=lambda *a, **k: None) + stream = StreamManager(config, SimpleNamespace(plugins={"p": object()}), + adapter) + stream.refresh = lambda: None + stream._ordered_plugins = ["p"] + return stream.take_next_group(count=1, offscreen_only=True) + + def test_nothing_to_show_is_not_sent_to_the_render_thread(self): + assert self._group(offscreen_prefetch=True) == [("p", [])] + + def test_the_old_contract_still_defers_with_the_switch_off(self): + assert self._group(offscreen_prefetch=False) == [("p", None)] diff --git a/test/test_render_gate.py b/test/test_render_gate.py new file mode 100644 index 00000000..bf531520 --- /dev/null +++ b/test/test_render_gate.py @@ -0,0 +1,432 @@ +"""The render gate (src/common/render_gate.py) and its wiring into Vegas.""" + +import os +import sys +import threading +import time +from pathlib import Path + +os.environ.setdefault("EMULATOR", "true") + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from src.common import render_gate # noqa: E402 +from src.common.render_gate import RenderGate # noqa: E402 + +PERIOD = 0.010 + + +class Clock: + def __init__(self): + self.now = 100.0 + + def __call__(self): + return self.now + + +def _running(gate, clock, swaps=render_gate.MIN_SAMPLES + 1, hold=1): + """Drive ``swaps`` on-time swaps through the gate, a refresh apart.""" + for _ in range(swaps): + gate.before_swap(hold) + clock.now += hold * PERIOD + gate.after_swap(hold) + + +class TestWindow: + def test_no_window_until_the_period_is_known(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock, swaps=3) + assert gate.refresh_period() is None + gate.before_swap(1) + assert gate._open_until == 0.0 + # ...and nothing parks meanwhile. + assert not gate._should_park(sys._getframe(), clock.now + 0.009) + + def test_the_period_is_the_low_end_of_the_swap_gaps(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock, swaps=20) + gate.before_swap(1) + clock.now += 0.030 # one late frame lengthens its gap only + gate.after_swap(1) + assert gate.refresh_period() == pytest.approx(PERIOD) + + def test_opens_until_just_before_the_refresh_the_swap_returns_on(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock) + last = clock.now + clock.now += 0.003 # the render thread's own work + gate.before_swap(1) + assert gate._open_until == pytest.approx( + last + PERIOD - render_gate.MARGIN_SECONDS) + + def test_a_held_frame_opens_for_the_whole_hold(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock, hold=2) + last = clock.now + clock.now += 0.003 + gate.before_swap(2) + assert gate._open_until == pytest.approx( + last + 2 * PERIOD - render_gate.MARGIN_SECONDS) + + def test_a_late_frame_opens_until_the_next_boundary(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock) + last = clock.now + clock.now += 0.013 # missed its refresh: returns on the next + gate.before_swap(1) + assert gate._open_until == pytest.approx( + last + 2 * PERIOD - render_gate.MARGIN_SECONDS) + + def test_closed_the_moment_the_swap_returns(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock) + gate.before_swap(1) + clock.now += PERIOD + gate.after_swap(1) + assert gate._open_until == 0.0 + + +class TestWhenToPark: + @pytest.fixture + def gate(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock) + gate.before_swap(1) + return gate + + def test_runs_inside_the_window(self, gate): + assert not gate._should_park(sys._getframe(), gate._open_until - 0.001) + + def test_parks_once_it_has_closed(self, gate): + assert gate._should_park(sys._getframe(), gate._open_until + 0.0001) + + def test_never_with_no_render_loop_to_protect(self, gate): + stale = gate._last_return + render_gate.STALE_SECONDS + 0.001 + assert not gate._should_park(sys._getframe(), stale) + + def test_never_holding_a_guarded_lock(self, gate): + rlock, lock = threading.RLock(), threading.Lock() + gate.guard(rlock, lock, None) + closed = gate._open_until + 0.0001 + with rlock: + assert not gate._should_park(sys._getframe(), closed) + with lock: + assert not gate._should_park(sys._getframe(), closed) + assert gate._should_park(sys._getframe(), closed) + + def test_an_rlock_held_by_another_thread_is_not_this_ones(self, gate): + rlock = threading.RLock() + gate.guard(rlock) + taken, done = threading.Event(), threading.Event() + + def hold(): + with rlock: + taken.set() + done.wait(5) + other = threading.Thread(target=hold) + other.start() + try: + taken.wait(5) + assert gate._should_park(sys._getframe(), gate._open_until + 0.0001) + finally: + done.set() + other.join() + + @staticmethod + def _frame_in(module): + namespace = {"__name__": module} + exec("import sys\ndef here():\n return sys._getframe()\n", namespace) + return namespace["here"]() + + @pytest.mark.parametrize("module", [ + "logging", "logging.handlers", "threading", "importlib", + "_frozen_importlib", "_frozen_importlib_external", + "src.cache_manager", "src.cache.disk_cache", + ]) + def test_never_inside_code_that_takes_shared_locks(self, gate, module): + frame = self._frame_in(module) + assert render_gate._unsafe(frame, None) + assert not gate._should_park(frame, gate._open_until + 0.0001) + + @pytest.mark.parametrize("module", [ + "src.cache_helpers", "plugin_logging_ticker", "src.vegas_mode.render_pipeline", + ]) + def test_modules_that_only_sound_alike_are_fine(self, module): + assert not render_gate._unsafe(self._frame_in(module), None) + + def test_where_python_is_installed_does_not_matter(self): + # GitHub's runners keep Python under /opt/hostedtoolcache; matching + # paths for "cache" made every stdlib frame unsafe there. + namespace = {"__name__": "json.decoder"} + exec(compile("import sys\ndef here():\n return sys._getframe()\n", + "/opt/hostedtoolcache/Python/3.11/lib/json/decoder.py", "exec"), + namespace) + assert not render_gate._unsafe(namespace["here"](), None) + + def test_what_lies_below_the_yielding_block_does_not_count(self): + # A thread's stack always starts in threading.py; only frames above + # the one that entered yielding() matter. + namespace = {"__name__": "threading"} + exec("def bootstrap(fn):\n return fn()\n", namespace) + + def entered(): + base = sys._getframe() + + def work(): + top = sys._getframe() + return render_gate._unsafe(top, None), render_gate._unsafe(top, base) + return work() + assert namespace["bootstrap"](entered) == (True, False) + + +class TestYielding: + """A real background thread, parked and released by the gate.""" + + def _worker(self, gate, stop): + count = [0] + + def step(): + count[0] += 1 + + def work(): + with gate.yielding(): + while not stop.is_set(): + step() + thread = threading.Thread(target=work, daemon=True) + return thread, count + + def test_parks_while_closed_and_runs_while_open(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock) + gate.before_swap(1) + stop = threading.Event() + thread, count = self._worker(gate, stop) + try: + clock.now = gate._open_until + 0.001 # window closed + thread.start() + time.sleep(0.2) + parked_steps = count[0] + # At most one step per MAX_WAIT timeout while parked. + assert parked_steps < 0.2 / render_gate.MAX_WAIT_SECONDS + 5 + assert gate.parks >= 1 + + with gate._cond: # the next swap opens it + gate._open_until = clock.now + 3600.0 + gate._generation += 1 + gate._cond.notify_all() + time.sleep(0.1) + assert count[0] > parked_steps + 1000 + finally: + stop.set() + with gate._cond: + gate._open_until = float("inf") + gate._generation += 1 + gate._cond.notify_all() + thread.join(2) + assert not thread.is_alive() + + def test_runs_freely_once_the_render_loop_stops(self): + clock = Clock() + gate = RenderGate(clock) + _running(gate, clock) + stop = threading.Event() + thread, count = self._worker(gate, stop) + clock.now += render_gate.STALE_SECONDS + 0.01 + thread.start() + time.sleep(0.1) + stop.set() + thread.join(2) + assert count[0] > 1000 + assert gate.parks == 0 + + def test_the_hook_comes_off_when_the_block_ends(self): + gate = RenderGate() + before = sys.getprofile() + with gate.yielding(): + assert sys.getprofile() == gate._hook + assert sys.getprofile() is before + + +class TestWhoIsGated: + def _in_thread(self, fn): + out = [] + thread = threading.Thread(target=lambda: out.append(fn())) + thread.start() + thread.join(5) + return out[0] + + def test_the_render_thread_never_gives_way(self): + gate = RenderGate() + gate.before_swap(1) + gate.after_swap(1) # this thread swaps: it is the render thread + with gate.yielding(): + assert sys.getprofile() is not gate._hook + + def background(): + with gate.yielding(): + return sys.getprofile() == gate._hook + assert self._in_thread(background) + + def test_a_live_refresh_from_another_thread_does_not_take_its_place(self): + gate = RenderGate() + gate.after_swap(1) # the render loop + self._in_thread(lambda: gate.after_swap(1)) # a plugin pushing a frame + with gate.yielding(): + assert sys.getprofile() is not gate._hook + + def test_nested_blocks_keep_the_outer_boundary(self): + gate = RenderGate() + + def nested(): + with gate.yielding(): + outer = gate._local.base + with gate.yielding(): + inner = gate._local.base + after = gate._local.base + still_hooked = sys.getprofile() == gate._hook + return outer is inner is after and still_hooked, sys.getprofile() + kept, final = self._in_thread(nested) + assert kept and final is None + + +class TestDisplayManager: + @pytest.fixture + def dm(self): + from src.display_manager import DisplayManager + DisplayManager._instance = None + DisplayManager._initialized = False + manager = DisplayManager({"display": { + "hardware": {"rows": 32, "cols": 64, "chain_length": 1, "parallel": 1}, + "runtime": {"gpio_slowdown": 0}}}, suppress_test_pattern=True) + yield manager + manager.render_gate = None + manager.set_scrolling_state(False) + DisplayManager._instance = None + DisplayManager._initialized = False + + def test_opens_around_each_swap(self, dm): + calls = [] + + class Spy: + def before_swap(self, hold): + calls.append(("before", hold)) + + def after_swap(self, hold): + calls.append(("after", hold)) + + real_swap = dm.matrix.SwapOnVSync + + def swap(canvas, *args, **kwargs): + calls.append(("swap",)) + return real_swap(canvas, *args, **kwargs) + dm.matrix.SwapOnVSync = swap + dm.render_gate = Spy() + dm.set_scrolling_state(True, 2) + dm.draw.rectangle([0, 0, 3, 3], fill=(255, 0, 0)) + dm.update_display() + assert calls == [("before", 2), ("swap",), ("after", 2)] + + def test_off_screen_drawing_never_touches_it(self, dm): + class Boom: + def before_swap(self, hold): + raise AssertionError("an off-screen frame reached the gate") + after_swap = before_swap + + dm.render_gate = Boom() + with dm.offscreen(): + dm.draw.rectangle([0, 0, 3, 3], fill=(255, 0, 0)) + dm.update_display() + + +class TestVegasWiring: + def _coordinator(self, **config): + from src.vegas_mode.config import VegasModeConfig + from src.vegas_mode.coordinator import VegasModeCoordinator + + class Holder: + def __init__(self): + self._buffer_lock = threading.RLock() + self._prefetch_lock = threading.Lock() + self._cache_lock = threading.Lock() + + c = VegasModeCoordinator.__new__(VegasModeCoordinator) + c.vegas_config = VegasModeConfig(**config) + c._state_lock = threading.Lock() + c.stream_manager = Holder() + c.render_pipeline = Holder() + c.plugin_adapter = Holder() + c.display_manager = type("DM", (), {"render_gate": None})() + return c + + def test_on_by_default(self, monkeypatch): + monkeypatch.setattr(render_gate, "swap_releases_gil", lambda: True) + c = self._coordinator() + c._install_render_gate() + assert isinstance(c.display_manager.render_gate, RenderGate) + + def test_can_be_turned_off(self, monkeypatch): + monkeypatch.setattr(render_gate, "swap_releases_gil", lambda: True) + c = self._coordinator(prefetch_gate=False) + c._install_render_gate() + assert c.display_manager.render_gate is None + + def test_installed_for_the_run_and_removed_after(self, monkeypatch): + monkeypatch.setattr(render_gate, "swap_releases_gil", lambda: True) + c = self._coordinator(prefetch_gate=True) + c._install_render_gate() + gate = c.display_manager.render_gate + assert isinstance(gate, RenderGate) + assert c._state_lock in gate._guarded + assert c.stream_manager._buffer_lock in gate._guarded + assert c.plugin_adapter._cache_lock in gate._guarded + c._remove_render_gate() + assert c.display_manager.render_gate is None + + @pytest.mark.parametrize("releases", [False, None]) + def test_ignored_without_a_binding_that_releases_the_gil(self, monkeypatch, releases): + monkeypatch.setattr(render_gate, "swap_releases_gil", lambda: releases) + c = self._coordinator(prefetch_gate=True) + c._install_render_gate() + assert c.display_manager.render_gate is None + + def test_read_from_config(self): + from src.vegas_mode.config import VegasModeConfig + off = VegasModeConfig.from_config( + {"display": {"vegas_scroll": {"prefetch_gate": False}}}) + assert off.prefetch_gate is False + assert off.to_dict()["prefetch_gate"] is False + assert VegasModeConfig.from_config( + {"display": {"vegas_scroll": {}}}).prefetch_gate is True + + def test_the_prefetch_runs_inside_the_gate(self): + from src.vegas_mode.render_pipeline import RenderPipeline + + gate = RenderGate() + seen = [] + + class Stream: + def take_next_group(self, offscreen_only=False): + seen.append((sys.getprofile() == gate._hook, offscreen_only)) + return ["segment"] + + pipeline = RenderPipeline.__new__(RenderPipeline) + pipeline.config = type("C", (), {"continuous_scroll": True})() + pipeline._prefetch_lock = threading.Lock() + pipeline._prefetch_thread = None + pipeline._prepared_group = None + pipeline.stream_manager = Stream() + pipeline.display_manager = type("DM", (), {"render_gate": gate})() + pipeline.start_prefetch() + pipeline._prefetch_thread.join(5) + assert seen == [(True, True)] + assert pipeline._prepared_group == ["segment"] diff --git a/test/test_vegas_switch_interval.py b/test/test_vegas_switch_interval.py new file mode 100644 index 00000000..4195386d --- /dev/null +++ b/test/test_vegas_switch_interval.py @@ -0,0 +1,41 @@ +"""vegas_scroll.switch_interval_ms: applied for a Vegas run, restored after.""" +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from src.vegas_mode.config import VegasModeConfig # noqa: E402 +from src.vegas_mode.coordinator import VegasModeCoordinator # noqa: E402 + + +def _coordinator(ms): + c = VegasModeCoordinator.__new__(VegasModeCoordinator) + c.vegas_config = VegasModeConfig(switch_interval_ms=ms) + return c + + +def test_applied_for_the_run_and_restored_after(): + before = sys.getswitchinterval() + c = _coordinator(1.0) + try: + c._apply_switch_interval() + assert abs(sys.getswitchinterval() - 0.001) < 1e-9 + c._apply_switch_interval() # a second start must not lose the original + finally: + c._restore_switch_interval() + assert sys.getswitchinterval() == before + + +def test_zero_leaves_the_interpreter_alone(): + before = sys.getswitchinterval() + c = _coordinator(0.0) + c._apply_switch_interval() + c._restore_switch_interval() + assert sys.getswitchinterval() == before + + +def test_read_from_config(): + config = VegasModeConfig.from_config( + {"display": {"vegas_scroll": {"switch_interval_ms": 1}}}) + assert config.switch_interval_ms == 1.0 + assert VegasModeConfig.from_config({"display": {"vegas_scroll": {}}}).switch_interval_ms == 0.0