diff --git a/docs/PLUGIN_API_REFERENCE.md b/docs/PLUGIN_API_REFERENCE.md index 0a98d0f1..d96d8259 100644 --- a/docs/PLUGIN_API_REFERENCE.md +++ b/docs/PLUGIN_API_REFERENCE.md @@ -514,21 +514,59 @@ self.display_manager.draw_text_with_icons( For plugins that implement scrolling content, use these methods to coordinate with the display system. -#### `set_scrolling_state(is_scrolling: bool) -> None` +#### `set_scrolling_state(is_scrolling: bool, frame_hold: int = 1) -> None` -Mark the display as scrolling or not scrolling. Call when scrolling starts/stops. +Mark the display as scrolling or not scrolling, and set this scroll's frame +pacing. Call it when a scroll starts (calling it on every scroll frame is fine) +and with `False` when it stops. **Parameters**: - `is_scrolling` (bool): True if currently scrolling, False otherwise +- `frame_hold` (int, default 1): how many panel refreshes each pushed frame is + held for (clamped to 1-255; ignored when `is_scrolling` is False, which + resets it to 1). Pass the `frame_hold` of the settings + `src.common.scroll_config.configure()` returned. Added in core 3.4.0. + +**Why `frame_hold` matters**: `scroll_config.configure()` snaps the speed to +one the panel can show in whole pixels and sets the `ScrollHelper` to advance a +fixed number of pixels on every presented frame -- no clock is consulted. The +panel presents frames at its refresh rate divided by the hold, so the hold is +part of the speed. Omit it and a 50 px/s scroll (1px every 2nd refresh on a +100 Hz panel) runs at 100 px/s. The hold is not applied by `configure()` +because it must not outlive the scroll: plugins share one display manager. **Example**: ```python +from src.common import scroll_config +from src.common.scroll_helper import ScrollHelper + +def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.scroll_helper = ScrollHelper( + self.display_manager.width, self.display_manager.height, self.logger) + # ...later, hand it content with self.scroll_helper.set_scrolling_image(img) + self.scroll_settings = scroll_config.configure( + self.scroll_helper, + plugin_config=self.config, + global_config=self.global_config, + display_manager=self.display_manager, + plugin_logger=self.logger, + ) + def display(self, force_clear=False): - self.display_manager.set_scrolling_state(True) - # Scroll content... - self.display_manager.set_scrolling_state(False) + self.display_manager.set_scrolling_state( + True, frame_hold=self.scroll_settings.frame_hold) + self.scroll_helper.update_scroll_position() + self.display_manager.image = self.scroll_helper.get_visible_portion() + self.display_manager.update_display() + if self.scroll_helper.is_scroll_complete(): + self.display_manager.set_scrolling_state(False) ``` +Don't pace the loop with `time.sleep()`: `update_display()` blocks on the +panel's vsync, which is what paces a scroll. See `docs/SCROLL_PERFORMANCE.md` +for choosing a speed. + #### `is_currently_scrolling() -> bool` Check if the display is currently in a scrolling state. @@ -998,9 +1036,10 @@ if "weather" in enabled_plugins: self.display_manager.update_display() ``` -3. **Handle scrolling state**: If your plugin scrolls, use scrolling state methods +3. **Handle scrolling state**: If your plugin scrolls, use scrolling state methods, + passing the frame hold `scroll_config.configure()` returned ```python - self.display_manager.set_scrolling_state(True) + self.display_manager.set_scrolling_state(True, frame_hold=settings.frame_hold) # Scroll content... self.display_manager.set_scrolling_state(False) ``` diff --git a/docs/SCROLL_PERFORMANCE.md b/docs/SCROLL_PERFORMANCE.md index ba5600d9..82e9f912 100644 --- a/docs/SCROLL_PERFORMANCE.md +++ b/docs/SCROLL_PERFORMANCE.md @@ -83,8 +83,9 @@ second refresh, instead of half a pixel every refresh (which has no good rendering, only a choice between blur and judder). `scroll_config.configure()` snaps the requested speed to the nearest entry on -the ladder and reports the hold that speed needs. It does **not** apply the -hold: the hold belongs to a scroll, not to a plugin's lifetime, and plugins +the ladder, sets the helper to advance that entry's whole-pixel step on every +presented frame (`ScrollHelper.set_pixels_per_frame`), and reports the hold +that speed needs. It does **not** apply the hold: the hold belongs to a scroll, not to a plugin's lifetime, and plugins share one display manager -- one set at construction is reset the moment any other plugin finishes scrolling. Apply it yourself when the scroll starts: @@ -102,10 +103,18 @@ self.display_manager.set_scrolling_state(True, frame_hold=settings.frame_hold) Passing `display_manager` only lets `configure` read the true refresh rate from `display.hardware`, which a plugin config cannot see. Skipping the -`set_scrolling_state` call is the mistake that matters: the speed still -resolves, but the panel keeps presenting a new frame every refresh, so a slow -snapped speed falls back to fractional pixels. Pass `snap_to_crisp=False` to -keep an exact requested speed and accept the artefacts. +`set_scrolling_state(True, frame_hold=...)` call is the mistake that matters. +The helper consults no clock in this mode -- it moves the fixed step once per +`update_scroll_position()` call, and `SwapOnVSync` is what paces those calls -- +so without the hold the panel presents a new frame every refresh and the scroll +runs `frame_hold` times too fast: 50 px/s (hold 2) plays at 100 px/s. + +Pass `snap_to_crisp=False` to keep an exact requested speed and accept the +artefacts. The helper then paces off elapsed time instead of stepping, and the +hold is 1. + +The General tab's `target_fps` ("Scroll Frame Rate") plays no part in any of +this: frames are presented at the panel refresh divided by the hold. Speeds slower than about 20 px/s are stepped no matter what, because a 1-pixel advance at 20 fps is simply a coarse increment. That is the pixel pitch, not a @@ -123,9 +132,12 @@ settings = scroll_config.configure( self.scroll_helper, plugin_config=self.config, global_config=self.global_config, - refresh_hz=scroll_config.refresh_hz_from_config(self.global_config), + display_manager=self.display_manager, plugin_logger=self.logger, ) + +# each frame of a scroll (or at least when it starts): +self.display_manager.set_scrolling_state(True, frame_hold=settings.frame_hold) ``` It resolves every config shape in one place, applies the speed, and returns @@ -154,6 +166,14 @@ it is always present and always wins, so the documented settings become unreachable. That is a real, shipped bug — see [ledmatrix-plugins#408](https://github.com/ChuckBuilds/ledmatrix-plugins/issues/408). +The flip side: a `scroll_pixels_per_second` you add by hand is ignored whenever +the plugin's config also carries the pair, which it does whenever the pair has +a schema default. Set the speed through the pair instead. + +The sports scoreboards (`src.common.sports_scroll`) are the exception to all of +the above: they read `scroll_settings.scroll_speed` per league as px/s directly, +and their `scroll_delay` is kept for compatibility but ignored for pacing. + If you are writing a plugin: do not give a deprecated key a schema default. ## What was actually wrong @@ -198,8 +218,16 @@ zero pixels and rendered an identical frame, which dirty-tracking skipped, so it returned in ~2 ms and the beat repeated. No `scroll_delay` value tunes this out — a shorter delay just trades stalled frames for periodic double-steps. -`ScrollHelper` now accumulates elapsed time in both modes at the same -configured speed, so position stays proportional to real time. +A crisp speed configured through `scroll_config` no longer consults a clock at +all. Once `SwapOnVSync` blocks until the panel has taken the frame, the frame +count is a truer clock than `time.time()`, so the helper advances a fixed whole +number of pixels per presented frame (`set_pixels_per_frame`) and the display +manager holds each frame for `frame_hold` refreshes. Every frame moves the eye +by the same amount. + +The time-based path remains only for callers that set a speed directly or pass +`snap_to_crisp=False`. There, frame-based mode no longer steps either: it +advances by elapsed time at `scroll_speed / scroll_delay` px/s. ## Diagnosing a juddery scroller @@ -222,11 +250,17 @@ p95 10.11ms max 12.03ms min 7.98ms | stalls 0 (0.0%) skips 0 (0.0%) Reading it, on a 100 Hz panel: +A healthy median is the refresh period times the scroll's frame hold: 10 ms +for a hold of 1 (100 px/s), **20 ms for 50 px/s** (hold 2), 30 ms for 33.3 px/s. +A 20 ms median on a 50 px/s scroll is the hold doing its job, not missed +refreshes. The `Scroll configured:` log line gives the hold (`1px every 2 +refreshes`). + | you see | it means | |---|---| -| median 10 ms, p95 within ~0.5 ms of it | healthy — locked to the panel | -| p95 or max at 20/30/50 ms | frames missing refreshes — per-frame work is overrunning, or a background thread is holding the GIL | -| non-zero **skips**, or a median *below* 10 ms | **duplicate frames** — the swap was skipped because the image did not change, so the frame never waited on vsync. The scroller is advancing less than one pixel per frame. | +| median = refresh period × hold, p95 within ~0.5 ms of it | healthy — locked to the panel | +| p95 or max a whole refresh period or more above that median | frames missing refreshes — per-frame work is overrunning, or a background thread is holding the GIL | +| non-zero **skips**, or a median *below* the expected one | **duplicate frames** — the swap was skipped because the image did not change, so the frame never waited on vsync. The scroller is advancing less than one pixel per frame, which a crisp fixed-step scroll never does; look for a plugin pacing off time or not passing the hold. | | non-zero **stalls** | frames past 1.5× the median, which is the measure of judder that survives averaging | `stalls` and `skips` are both counted against that window's own median, so they diff --git a/src/common/scroll_config.py b/src/common/scroll_config.py index 22c0f342..5719f84c 100644 --- a/src/common/scroll_config.py +++ b/src/common/scroll_config.py @@ -22,12 +22,27 @@ shimmer) or repeat frames (which reads as judder). :func:`resolve` warns when the requested speed will not divide evenly, because that is a real display artefact and not a rounding detail. -Speed is always expressed to the helper as pixels per second and applied in -time-based mode. Frame-based stepping gates motion on a wall clock at -``1/scroll_delay`` steps per second; plugins set ``scroll_delay`` to the frame -period, which puts that comparison exactly on its own threshold and makes the -step count flip on sub-millisecond jitter. Accumulating elapsed time keeps -position proportional to real time instead. +How the speed is applied +------------------------ +:func:`configure` snaps the requested speed to a :class:`CrispSpeed` -- a whole +number of pixels per presented frame, with each frame held for ``frame_hold`` +panel refreshes -- and puts the helper in fixed-step mode +(``ScrollHelper.set_pixels_per_frame``). In that mode the helper consults no +clock: every ``update_scroll_position()`` call moves exactly that many pixels. +``SwapOnVSync`` blocks until the panel has taken each frame, so the frame count +is the clock, and the speed on the panel is:: + + px/s = refresh_hz / frame_hold * pixels_per_frame + +That makes the hold part of the speed. The caller must pass +``settings.frame_hold`` to ``display_manager.set_scrolling_state(True, ...)``; +a caller that omits it is presented every refresh and scrolls ``frame_hold`` +times too fast. The refresh is the panel's (``display_manager.refresh_hz``, +i.e. ``display.hardware.limit_refresh_rate_hz``) -- never the global +``target_fps``, which no longer paces anything. + +With ``snap_to_crisp=False`` there is no fixed step: the helper is set to +px/s and advances by elapsed time, and the hold is 1. """ from __future__ import annotations @@ -330,17 +345,18 @@ def configure( ) -> ScrollSettings: """Resolve the config and apply it to ``scroll_helper``. - Applied in time-based mode: see the module docstring for why frame-based - stepping is not used. ``hasattr`` guards keep this usable against older - ScrollHelper builds that a plugin may be running on. + Frame-based mode is switched off, the (snapped) speed is set, and with + ``snap_to_crisp`` the helper steps a fixed whole-pixel amount per presented + frame -- see the module docstring. ``hasattr`` guards keep this usable + against older ScrollHelper builds that a plugin may be running on. :param display_manager: consulted for the panel's refresh rate only (it can see display.hardware; a plugin cannot). The frame hold is NOT applied here -- see the note in the body. The caller must pass ``settings.frame_hold`` to ``display_manager.set_scrolling_state(True, - ...)`` when it starts scrolling, or a sub-refresh speed still presents - a new frame every refresh and the motion falls back to fractional - pixels. + ...)`` when it scrolls. The helper moves its fixed step on every call, + so without the hold each step is presented every refresh and the + scroll runs ``frame_hold`` times faster than the resolved speed. :param snap_to_crisp: move the requested speed to the nearest speed the panel can show in whole pixels. On by default because a speed that does not divide evenly has no good rendering, only a choice of artefacts. @@ -357,7 +373,8 @@ def configure( # refresh to fill in target_fps, pixels_per_frame and the judder warning, # so deriving it afterwards described a 100Hz panel to everyone running at # 60 -- and with snap_to_crisp=False nothing downstream corrected it, so - # set_target_fps() paced the helper to 100 FPS on a 60Hz panel. + # the settings and the helper's (informational) target_fps said 100 FPS on + # a 60Hz panel. hz = _coerce(refresh_hz) if hz is None and display_manager is not None: hz = _coerce(getattr(display_manager, "refresh_hz", None)) diff --git a/src/common/scroll_helper.py b/src/common/scroll_helper.py index 47677ce6..73c6e1e8 100644 --- a/src/common/scroll_helper.py +++ b/src/common/scroll_helper.py @@ -282,7 +282,14 @@ class ScrollHelper: def update_scroll_position(self) -> None: """ - Update scroll position with high FPS control and handle wrap-around. + Advance the scroll by one presented frame and handle wrap-around. + + With a fixed per-frame step (set_pixels_per_frame, which + scroll_config.configure sets for a crisp speed) every call moves + exactly that many pixels and no clock is read; the caller's + vsync-blocking swap, held for ``frame_hold`` refreshes, sets the rate. + Otherwise the position advances by elapsed time at the configured + speed. """ if not self.cached_image: return @@ -823,11 +830,13 @@ class ScrollHelper: def set_scroll_speed(self, speed: float) -> None: """ - Set the scroll speed. - - In time-based mode: pixels per second (typically 10-200) - In frame-based mode: pixels per frame (typically 0.5-5 for smooth scrolling) - + Set the scroll speed, and leave fixed-step mode. + + In time-based mode: pixels per second (clamped to 1-500). + In frame-based mode: pixels per ``scroll_delay`` seconds (clamped to + 0.1-5), still applied by elapsed time as scroll_speed / scroll_delay + px/s. + Args: speed: Scroll speed (interpretation depends on frame_based_scrolling mode) """ diff --git a/src/display_manager.py b/src/display_manager.py index a0b105fa..7014ec6e 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -1415,6 +1415,12 @@ class DisplayManager: pixel every second refresh, which is how a scroll runs at half the refresh rate without fractional pixel positions. + The hold is part of the scroll's speed. A ScrollHelper configured by + ``scroll_config.configure()`` advances a fixed whole-pixel step per + presented frame and reads no clock, so pass the returned + ``settings.frame_hold`` here: a scroll that leaves it at 1 is + presented every refresh and runs ``frame_hold`` times too fast. + The hold is set here rather than once at plugin construction because it must not outlive the scroll that asked for it: plugins share one display manager, so a hold left set by whoever scrolled last would