diff --git a/CHANGELOG.md b/CHANGELOG.md index 30fd4a1e..f77818dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,36 @@ Sports data: to decide whether to submit a season range to the service or fetch it themselves on an older core. +Scrolling: + +- **Scoreboard scroll speed no longer changes with the General tab's "Scroll + Frame Rate" (`target_fps`).** Scoreboards on `src.common.sports_scroll` + computed their speed for that rate while the panel kept presenting at its + real refresh, so on a 100 Hz panel 60 ran a 50 px/s scoreboard at 100 px/s + and 200 ran it at 25 px/s. Speed now comes from `scroll_speed` and the panel + refresh only. The field is labelled legacy: nothing in core scrolling reads + it. Anyone who lowered it will see scoreboards scroll slower than before -- + at the speed they configured. +- `scripts/scroll_speeds.py --measure` / `--demo` open the panel with the + display service's own options (`DisplayManager.apply_matrix_options`), so + `display.runtime.gpio_slowdown`, `rp1_rio`, `panel_type` and orientation are + honoured; the script used to read `gpio_slowdown` from `display.hardware`. + Its closing advice now gives the `scroll_speed` + `scroll_delay` pair + instead of `scroll_pixels_per_second`, which the resolver ignores whenever + the pair is present. +- The frame-stats log no longer opens a scroll with a one-frame window for + scrollers that never call `reset_scroll()`. +- Removed dead scroll code: the optional scipy import (`HAS_SCIPY`), + `ScrollHelper._last_integer_position` and `frame_time_target`. + `ScrollHelper.target_fps` / `set_target_fps()` remain, documented as + informational. +- Docs describe the fixed-step scroll model: `PLUGIN_API_REFERENCE.md` + documents `set_scrolling_state(..., frame_hold)` (omitting the hold runs a + scroll `frame_hold` times too fast), `SCROLL_PERFORMANCE.md` no longer reads a + held 20 ms frame as missed refreshes, and Vegas `frame_based_scrolling` / + `scroll_delay` are described as the speed clamp they are rather than frame + stepping. Scoreboard `scroll_delay` is documented as ignored for pacing. + Web interface: - The plugin settings form honours `"x-display": "hidden"` in config schemas: diff --git a/docs/ADVANCED_FEATURES.md b/docs/ADVANCED_FEATURES.md index d620fef7..abaa17af 100644 --- a/docs/ADVANCED_FEATURES.md +++ b/docs/ADVANCED_FEATURES.md @@ -433,10 +433,14 @@ Vegas mode consists of four core components working together to provide smooth 1 - **Frame Rate Control:** Precise timing to maintain 125 FPS - **Pre-rendered Content:** Plugins pre-render during update() -**Scroll Speed Calculation:** +**Scroll Speed Calculation:** motion is by elapsed time; `target_fps` paces +the render loop, not the speed. ```python -pixels_per_frame = (scroll_speed / target_fps) -scroll_position += pixels_per_frame * elapsed_time +# frame_based_scrolling: false +scroll_position += scroll_speed * elapsed_time # scroll_speed in px/s +# frame_based_scrolling: true (the default) -- not stepping, just a clamp +applied = clamp(scroll_speed * scroll_delay, 0.1, 5) / scroll_delay +scroll_position += applied * elapsed_time ``` #### Component Interactions diff --git a/docs/CONFIG_REFERENCE.md b/docs/CONFIG_REFERENCE.md index 834d1f4b..7906c3fb 100644 --- a/docs/CONFIG_REFERENCE.md +++ b/docs/CONFIG_REFERENCE.md @@ -17,7 +17,7 @@ tooling against it. |---|---|---|---| | `web_display_autostart` | bool, `true` | Whether the web interface service starts with the system | `scripts/utils/start_web_conditionally.py` | | `timezone` | string, `"America/New_York"` | IANA timezone for schedules and displays | `ConfigManager.get_timezone()` | -| `target_fps` | int, `100` | Frame-rate ceiling for plugin rendering | `src/plugin_system/base_plugin.py`, `src/common/sports_scroll.py` | +| `target_fps` | int, `100` | Legacy "Scroll Frame Rate". Core scrolling no longer reads it: scroll frames are presented at `display.hardware.limit_refresh_rate_hz` divided by each scroll's frame hold, and speed comes from each plugin's scroll settings. Still exposed to plugins via `BasePlugin.global_config` | `src/plugin_system/base_plugin.py` | | `location` | object | `city` / `state` / `country`. Supplies the **default** for a plugin's own `location_city` / `location_state` / `location_country` setting, so weather, radar and friends follow this device without being configured twice. A value saved on the plugin itself still overrides it. | `SchemaManager.apply_device_location()`, then plugins via merged config | ## `schedule` — display on/off hours @@ -139,8 +139,8 @@ Read by `src/vegas_mode/config.py` (`VegasScrollConfig.from_config`). See | `dynamic_duration_enabled` | bool, `true` | | `min_cycle_duration` | int, `60` | | `max_cycle_duration` | int, `240` | -| `frame_based_scrolling` | bool, `true` — frame-count-based scroll stepping | -| `scroll_delay` | float, `0.02` — seconds between scroll updates (~50 FPS) | +| `frame_based_scrolling` | bool, `true` — does not step or set a frame rate; motion is by elapsed time either way. When `true`, `scroll_speed` passes through a clamp of 0.1–5 px per `scroll_delay` (see next row) | +| `scroll_delay` | float, `0.02` — not a frame period. Only used with `frame_based_scrolling`: the applied speed is `clamp(scroll_speed × scroll_delay, 0.1, 5) / scroll_delay` px/s, so at `0.02` speeds under 5 px/s run at 5, and at `0.001` nothing runs slower than 100 px/s | | `live_in_ticker` | bool, `false` — keep scrolling during live games instead of handing the display to a full-screen scoreboard | | `live_weight` | int, `3` (1–10) — slots per cycle for a plugin with live content | | `favorite_live_weight` | int, `5` (1–10) — slots per cycle when a plugin reports a favorite team is live | 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/docs/SPORTS_UNIFICATION.md b/docs/SPORTS_UNIFICATION.md index 89592f80..c752337f 100644 --- a/docs/SPORTS_UNIFICATION.md +++ b/docs/SPORTS_UNIFICATION.md @@ -218,6 +218,14 @@ The one behavior the upstreamed version adds is native Part A threaded it through each copy by hand, and this makes that threading legacy compatibility rather than the mechanism. +> **Superseded.** Once presentation became frame-locked (#545) the helper +> steps a fixed whole-pixel amount per presented frame and the panel presents +> at its own refresh, so honouring `target_fps` only turned it into a speed +> multiplier (60 doubled a scoreboard's speed, 200 halved it). `sports_scroll` +> no longer reads it: the crisp-speed ladder uses the panel refresh +> (`display_manager.refresh_hz`), and speed comes from +> `scroll_settings.scroll_speed` alone. See `docs/SCROLL_PERFORMANCE.md`. + ## Phases B0–B3 are merged and shipping in core 3.2.0. Everything that remains is @@ -464,8 +472,9 @@ After adoption plus the frozen legacy copies it was 10,610; removing the dead inline duplication (plugins #252) brought it to roughly 8,620. B6 would take it to about 3,300 including the shared core module — some 2,400 fewer than before this project started. **Until B6 runs, the adoption is net negative on disk**, -and its one delivered user-visible gain is that adopted plugins honour the -global `target_fps` instead of hardcoding ~100 FPS. +and its one delivered user-visible gain was that adopted plugins honoured the +global `target_fps` instead of hardcoding ~100 FPS (since withdrawn: see the +note under the B3 design above). ### Decision: stop adopting further modules until B6 closes diff --git a/scripts/scroll_speeds.py b/scripts/scroll_speeds.py index e824b19f..bdabf023 100644 --- a/scripts/scroll_speeds.py +++ b/scripts/scroll_speeds.py @@ -47,52 +47,47 @@ from src.common import scroll_config # noqa: E402 CONFIG = Path(__file__).resolve().parent.parent / "config" / "config.json" -def load_hardware(): +def load_config(): + """The whole config.json, or {} when it is missing or unreadable.""" try: with open(CONFIG, encoding="utf-8") as handle: - return (json.load(handle).get("display") or {}).get("hardware") or {} + config = json.load(handle) except (OSError, ValueError): return {} + return config if isinstance(config, dict) else {} -def build_options(hardware, refresh_override=None): - from rgbmatrix import RGBMatrixOptions - - o = RGBMatrixOptions() - from src.display_geometry import ( - DEFAULT_CHAIN_LENGTH, DEFAULT_COLS, DEFAULT_PARALLEL, DEFAULT_ROWS, - ) - o.rows = int(hardware.get("rows", DEFAULT_ROWS)) - o.cols = int(hardware.get("cols", DEFAULT_COLS)) - o.chain_length = int(hardware.get("chain_length", DEFAULT_CHAIN_LENGTH)) - o.parallel = int(hardware.get("parallel", DEFAULT_PARALLEL)) - o.brightness = int(hardware.get("brightness", 80)) - o.hardware_mapping = hardware.get("hardware_mapping", "regular") - o.pwm_bits = int(hardware.get("pwm_bits", 11)) - o.pwm_dither_bits = int(hardware.get("pwm_dither_bits", 0)) - o.pwm_lsb_nanoseconds = int(hardware.get("pwm_lsb_nanoseconds", 130)) - o.led_rgb_sequence = hardware.get("led_rgb_sequence", "RGB") - o.scan_mode = int(hardware.get("scan_mode", 0)) - o.row_address_type = int(hardware.get("row_address_type", 0)) - o.multiplexing = int(hardware.get("multiplexing", 0)) - o.gpio_slowdown = int(hardware.get("gpio_slowdown", 2)) - o.limit_refresh_rate_hz = ( - int(refresh_override) if refresh_override is not None - else int(hardware.get("limit_refresh_rate_hz", 0)) - ) - return o +def hardware_of(config): + return (config.get("display") or {}).get("hardware") or {} -def open_matrix(hardware, refresh_override=None): +def build_options(config, refresh_override=None): + """The matrix options the display service would use for this config. + + Built by DisplayManager.apply_matrix_options, not a copy of it, so the + measurement and the demo drive the panel exactly as the service does + (runtime gpio_slowdown, rp1_rio, panel_type, orientation, defaults). + ``refresh_override`` replaces limit_refresh_rate_hz; 0 means uncapped. + """ + from src.display_manager import DisplayManager, RGBMatrixOptions + + options = DisplayManager.apply_matrix_options(RGBMatrixOptions(), config) + if refresh_override is not None: + options.limit_refresh_rate_hz = int(refresh_override) + return options + + +def open_matrix(config, refresh_override=None): """Construct the matrix, or explain why it will not open.""" if os.geteuid() != 0: sys.exit("this needs root for GPIO access - rerun with sudo") try: - from rgbmatrix import RGBMatrix - except ImportError: - sys.exit("rgbmatrix is not installed on this machine") + from src.display_manager import RGBMatrix + except ImportError as exc: + sys.exit("could not load the display stack ({}); is rgbmatrix " + "installed on this machine?".format(exc)) try: - return RGBMatrix(options=build_options(hardware, refresh_override)) + return RGBMatrix(options=build_options(config, refresh_override)) except Exception as exc: # pragma: no cover - hardware dependent sys.exit( "could not open the panel ({}).\n" @@ -101,7 +96,7 @@ def open_matrix(hardware, refresh_override=None): ) -def measure_refresh(hardware, seconds=6.0): +def measure_refresh(config, seconds=6.0): """Actual refresh rate, by running uncapped and timing the swaps. SwapOnVSync blocks until the panel's next refresh, so an unthrottled loop @@ -109,7 +104,7 @@ def measure_refresh(hardware, seconds=6.0): chain will really give you, as opposed to whatever limit_refresh_rate_hz optimistically asks for. """ - matrix = open_matrix(hardware, refresh_override=0) + matrix = open_matrix(config, refresh_override=0) canvas = matrix.CreateFrameCanvas() canvas = matrix.SwapOnVSync(canvas) # discard the first, it includes setup frames = 0 @@ -122,17 +117,17 @@ def measure_refresh(hardware, seconds=6.0): return measured -def demo(hardware, target, seconds): +def demo(config, target, seconds): """Scroll text at the crisp speed nearest `target`.""" from PIL import Image, ImageDraw, ImageFont from src.common.font_layout import load_truetype - hz = float(hardware.get("limit_refresh_rate_hz") or scroll_config.DEFAULT_REFRESH_HZ) + hz = scroll_config.refresh_hz_from_config(config) choice = scroll_config.solve_crisp(target, hz) print("asked for {:.0f} px/s -> {}".format(target, choice.describe())) - matrix = open_matrix(hardware) + matrix = open_matrix(config) canvas = matrix.CreateFrameCanvas() W, H = canvas.width, canvas.height @@ -195,9 +190,38 @@ def print_ladder(hz, highlight=None): ) else "" print(" " + entry.describe() + mark) print("") - print("Set one in config.json as pixels per second, e.g.") - print(' "display_options": {{"scroll_pixels_per_second": {:.0f}}}'.format( - scroll_config.solve_crisp(highlight if highlight else hz / 2, hz).pixels_per_second)) + print_config_advice(scroll_config.solve_crisp(highlight if highlight else hz / 2, hz)) + + +def config_advice(choice): + """The config that selects ``choice``, in the keys the resolver honours. + + Tickers take a ``scroll_speed`` (px per step) + ``scroll_delay`` (seconds) + pair, and scroll_config ranks that pair ABOVE ``scroll_pixels_per_second`` + -- deliberately, because some plugins give the flat key a schema default. + Many schemas default the pair too, so a flat key added by hand is usually + ignored. Advise the pair: pixels_per_frame every frame_hold/refresh + seconds is exactly the crisp speed. + """ + pair = { + "scroll_speed": choice.pixels_per_frame, + "scroll_delay": round(choice.frame_hold / choice.refresh_hz, 6), + } + scoreboard = {"scroll_speed": round(choice.pixels_per_second, 2)} + return pair, scoreboard + + +def print_config_advice(choice): + pair, scoreboard = config_advice(choice) + print("To use {:.1f} px/s, set it where the plugin keeps its scroll speed.".format( + choice.pixels_per_second)) + print("Tickers take a scroll_speed (px per step) + scroll_delay (seconds) pair:") + print(' "display_options": {}'.format(json.dumps(pair))) + print("(some plugins keep the pair at the top level or under \"display\").") + print("The pair outranks scroll_pixels_per_second, which is ignored whenever the") + print("pair is present -- and schema defaults usually put it there.") + print("Sports scoreboards take pixels per second per league instead:") + print(' "scroll_settings": {}'.format(json.dumps(scoreboard))) def main(): @@ -214,15 +238,15 @@ def main(): help="highlight the entry nearest this speed") args = ap.parse_args() - hardware = load_hardware() - configured = float(hardware.get("limit_refresh_rate_hz") or 0) + config = load_config() + configured = float(hardware_of(config).get("limit_refresh_rate_hz") or 0) if args.demo is not None: - demo(hardware, args.demo, args.seconds) + demo(config, args.demo, args.seconds) return if args.measure: - measured = measure_refresh(hardware) + measured = measure_refresh(config) print("measured panel refresh: {:.1f}Hz".format(measured)) if configured: print("configured limit_refresh_rate_hz: {:.0f}".format(configured)) diff --git a/src/common/scroll_config.py b/src/common/scroll_config.py index 4dd7f532..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)) @@ -399,6 +416,9 @@ def configure( if hasattr(scroll_helper, "set_pixels_per_frame"): scroll_helper.set_pixels_per_frame( choice.pixels_per_frame if choice else None) + # Informational only: nothing in the helper paces off target_fps. It is + # still recorded because plugins read it back (ledmatrix-elections' + # test_scroll_pacing.py asserts it equals the crisp presentation rate). if choice and hasattr(scroll_helper, "set_target_fps"): scroll_helper.set_target_fps(choice.frames_per_second) elif settings.target_fps and hasattr(scroll_helper, "set_target_fps"): diff --git a/src/common/scroll_helper.py b/src/common/scroll_helper.py index 71b9080a..73c6e1e8 100644 --- a/src/common/scroll_helper.py +++ b/src/common/scroll_helper.py @@ -22,13 +22,6 @@ from typing import Optional, Dict, Any from PIL import Image import numpy as np -# Try to import scipy for sub-pixel interpolation, fallback to simpler method if not available -try: - from scipy.ndimage import shift - HAS_SCIPY = True -except ImportError: - HAS_SCIPY = False - # How often the frame-stats line is emitted, and therefore also the ceiling # on a believable frame time: a scroll that renders at all cannot take this @@ -138,21 +131,22 @@ class ScrollHelper: # blend (blur) or repeat frames (judder); blending is the worse of the # two here. Vegas mode still opts in via set_sub_pixel_scrolling(). self.sub_pixel_scrolling = False - self._last_integer_position = 0 # Cache for integer position to avoid repeated calculations - + # Frame-based scrolling settings self.frame_based_scrolling = False #: Whole pixels to advance per presented frame, or None to pace #: off elapsed time. See set_pixels_per_frame. - self.fixed_pixels_per_frame = None # If True, use scroll_delay to throttle and move scroll_speed pixels - self.last_step_time = 0.0 # Track last step time for frame-based throttling - + self.fixed_pixels_per_frame = None + self.last_step_time = 0.0 # Time of the last position update + # Time tracking for scroll updates self.last_update_time: Optional[float] = None - - # High FPS settings - self.target_fps = 120 # Target 120 FPS for smooth scrolling - self.frame_time_target = 1.0 / self.target_fps + + #: Informational only: the presentation rate scroll_config chose + #: (panel refresh / frame hold). Nothing paces off it -- the helper + #: steps per call and SwapOnVSync paces the calls. Kept because + #: plugins and their tests read it back. + self.target_fps = 120 # Dynamic duration settings self.dynamic_duration_enabled = True @@ -288,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 @@ -461,10 +462,9 @@ class ScrollHelper: Linear blend between the frames at ``start_x`` and ``start_x + 1``. Implemented with numpy rather than scipy.ndimage.shift: scipy is not - installed on the target devices (HAS_SCIPY is False there), which is why - the pre-existing sub-pixel path was dead code — get_visible_portion never - consulted the flag, and the scipy fallback would not have interpolated - anyway. + installed on the target devices, and the old scipy-based sub-pixel path + was dead code -- get_visible_portion never consulted the flag. The scipy + import was removed with it; installing scipy has no effect. Args: start_x: Left column of the earlier of the two frames @@ -830,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) """ @@ -896,14 +898,19 @@ class ScrollHelper: def set_target_fps(self, fps: float) -> None: """ - Set the target frames per second for scrolling. - + Record the presentation rate, for diagnostics only. + + Nothing paces off this value: with a fixed per-frame step the helper + advances once per call, and without one it advances by elapsed time. + The rate frames are shown at is the panel refresh divided by the frame + hold passed to ``display_manager.set_scrolling_state``. scroll_config + sets it to that rate so it can be read back. + Args: - fps: Target FPS (typically 30-200, default 120) + fps: Frames per second (clamped to 30-200) """ self.target_fps = max(30.0, min(200.0, fps)) - self.frame_time_target = 1.0 / self.target_fps - self.logger.debug(f"Target FPS set to: {self.target_fps} FPS (frame_time_target: {self.frame_time_target:.4f}s)") + self.logger.debug("Target FPS recorded: %s FPS (informational)", self.target_fps) def set_sub_pixel_scrolling(self, enabled: bool) -> None: """ @@ -914,7 +921,7 @@ class ScrollHelper: When disabled, uses integer pixel positioning (faster but may skip pixels). Args: - enabled: True to enable sub-pixel scrolling (default: True) + enabled: True to enable sub-pixel scrolling (default: False) """ self.sub_pixel_scrolling = enabled self.logger.debug(f"Sub-pixel scrolling {'enabled' if enabled else 'disabled'}") @@ -923,10 +930,12 @@ class ScrollHelper: """ Enable or disable frame-based scrolling. - When enabled, update_scroll_position() respects scroll_delay and moves - scroll_speed pixels per step. This provides a "stepped" look similar to - traditional tickers and can be visually smoother on LED matrices. - + This does not step. When enabled, ``scroll_speed`` is read as pixels + per ``scroll_delay`` seconds (set_scroll_speed clamps it to 0.1-5), and + update_scroll_position() still advances by elapsed time at + ``scroll_speed / scroll_delay`` px/s. A fixed per-frame step set by + set_pixels_per_frame() takes precedence over both modes. + Args: enabled: True to enable frame-based scrolling (default: False) """ @@ -985,11 +994,7 @@ class ScrollHelper: # the real stall rates being measured, so the number could not be # trusted at all. Seed the clock and take no sample. if self.last_frame_time is None: - self.last_frame_time = current_time - # Restart the window with the scroll. Otherwise the boundary is - # already long overdue when the second frame arrives, and the new - # scroll opens by reporting a window of exactly one frame. - self.last_fps_log_time = current_time + self._restart_stats_window(current_time) return # Calculate instantaneous frame time @@ -998,9 +1003,10 @@ class ScrollHelper: # A caller that scrolls without ever calling reset_scroll() never arms # the sentinel above, so catch the same gap by its size. Nothing that # renders a scroll produces a frame longer than the log interval; a - # sample that large is an idle period, not a frame. + # sample that large is an idle period, not a frame. It starts a new + # scroll exactly as the sentinel does, window timer included. if frame_time >= FPS_LOG_INTERVAL: - self.last_frame_time = current_time + self._restart_stats_window(current_time) return self.frame_times.append(frame_time) @@ -1034,7 +1040,17 @@ class ScrollHelper: self.last_frame_time = current_time self.frame_count += 1 - + + def _restart_stats_window(self, current_time: float) -> None: + """Seed the frame clock at the start of a scroll, taking no sample. + + The window timer restarts with it. Otherwise the 5s boundary is + already long overdue when the next frame arrives, and the new scroll + opens by reporting a window of exactly one frame. + """ + self.last_frame_time = current_time + self.last_fps_log_time = current_time + def clear_cache(self) -> None: """ Clear the cached scrolling image. diff --git a/src/common/sports_scroll.py b/src/common/sports_scroll.py index 0278a1d3..d32fa76a 100644 --- a/src/common/sports_scroll.py +++ b/src/common/sports_scroll.py @@ -18,10 +18,15 @@ Promoting the content layer would be exactly the mistake ``docs/SPORTS_UNIFICATION.md`` warns against — merging on the intuition that same-named methods are the same method. Same name, different job. -The one behavior this module adds over the plugin copies is native support for -``global_config['target_fps']``: the bundled copies hardcode ~100 FPS via -``scroll_delay=0.01`` and never consult the global smooth-scrolling target. A -plugin inheriting from here gets it for free. +What this module adds over the plugin copies is pacing through +:mod:`src.common.scroll_config`, the resolver every other scroller uses: the +configured px/s is snapped to a whole-pixel speed for the panel's refresh, the +helper steps a fixed number of pixels per presented frame, and each drawn frame +publishes the frame hold to the display manager. Speed depends only on +``scroll_speed`` and the panel refresh +(``display.hardware.limit_refresh_rate_hz``) -- not on the global +``target_fps``, and not on ``scroll_delay``, which is kept in the settings for +compatibility only. Usage:: @@ -94,9 +99,9 @@ class SportsScrollDisplay: :param display_manager: the core display manager :param config: the plugin's configuration :param custom_logger: the plugin's logger, so scroll lines are attributed - :param global_config: the LEDMatrix global config — the source of - ``target_fps``. Optional so an older caller that does not pass it - keeps working at the config-derived pacing. + :param global_config: the LEDMatrix global config, consulted for the + panel refresh when the display manager cannot report one. + Optional so an older caller that does not pass it keeps working. """ self.display_manager = display_manager self.config = config @@ -196,21 +201,6 @@ class SportsScrollDisplay: return {**settings, **override} return settings - def _resolve_target_fps(self) -> Optional[float]: - """The global smooth-scrolling FPS target, or None to keep config pacing. - - Coerced before use: a malformed value in the global config must degrade - to the existing ``scroll_delay`` pacing, never raise on a display path. - """ - raw = self.global_config.get("target_fps") or self.global_config.get( - "scroll_target_fps" - ) - try: - return float(raw) if raw is not None else None - except (TypeError, ValueError): - self.logger.debug("Ignoring unusable target_fps: %r", raw) - return None - def _coerce_float(self, value: Any, default: float) -> float: """A usable float from config, or ``default``. @@ -246,8 +236,8 @@ class SportsScrollDisplay: # settings dict: the two read the same key names with different # meanings, and the collision is a factor of 1/scroll_delay. # - # sports_scroll: scroll_speed is px/SECOND; scroll_delay is only the - # frame period used to reach px/frame. + # sports_scroll: scroll_speed is px/SECOND; scroll_delay is ignored + # for pacing (see _resolve_pixels_per_second). # scroll_config: scroll_speed is px per STEP, so px/s = speed/delay. # # Passing {"scroll_speed": 50.0, "scroll_delay": 0.01} straight through @@ -276,8 +266,13 @@ class SportsScrollDisplay: def _resolve_pixels_per_second(self, settings: Dict[str, Any]) -> float: """This module's config shape, expressed as plain pixels per second. - ``scroll_speed`` is already px/s here. ``scroll_delay`` only matters - when a caller supplied px/frame instead, which the 0 case covers. + ``scroll_speed`` is already px/s here, and it is the whole answer. + ``scroll_delay`` is kept in the settings for compatibility but is + ignored for pacing: the frame rate is the panel refresh divided by the + frame hold scroll_config picks, so no positive delay changes the speed. + The one exception is a delay of exactly 0 (below every scoreboard + schema's minimum), which is read as ``scroll_speed`` being px per + frame at ``ASSUMED_FPS_WHEN_UNPACED``. """ scroll_speed = self._coerce_float(settings.get("scroll_speed"), 50.0) scroll_delay = self._coerce_float(settings.get("scroll_delay"), 0.01) @@ -285,19 +280,29 @@ class SportsScrollDisplay: return scroll_speed * ASSUMED_FPS_WHEN_UNPACED return scroll_speed - def _resolve_refresh_hz(self) -> Optional[float]: + def _resolve_refresh_hz(self) -> float: """The panel refresh the crisp ladder should be computed against. - Prefers the configured hardware refresh. Falls back to the global - ``target_fps``/``scroll_target_fps`` this module has always honoured: - under the old model that key *was* the rate frames were presented at, - so it is the faithful translation for anyone who set it. Returning - None lets scroll_config apply its own default. + This has to be the rate frames are actually presented at, because the + helper advances a fixed number of whole pixels per presented frame and + the display manager holds each frame for ``frame_hold`` refreshes. A + ladder built for any other rate plays back at the wrong speed: built + for 60 Hz and shown on a 100 Hz panel, it runs 100/60 too fast. + + So it is the display manager's ``refresh_hz`` (what the panel is + driven at), then ``display.hardware.limit_refresh_rate_hz`` from the + global config, then scroll_config's default. The global ``target_fps`` + is deliberately NOT consulted: before frame-locked presentation it was + the rate frames were shown at, but now honouring it only turned the + General tab's "Scroll Frame Rate" into a scoreboard speed multiplier. """ - hardware = scroll_config.refresh_hz_from_config(self.global_config) - if hardware and hardware != scroll_config.DEFAULT_REFRESH_HZ: - return hardware - return self._resolve_target_fps() or hardware or None + reported = getattr(self.display_manager, "refresh_hz", None) + # A real number only: a stub or a mock answering for anything must not + # become the refresh rate. + if (isinstance(reported, (int, float)) and not isinstance(reported, bool) + and reported > 0): + return float(reported) + return scroll_config.refresh_hz_from_config(self.global_config) def _scroll_frame_hold(self) -> int: """Refreshes to hold each frame for, from the resolved settings.""" @@ -327,11 +332,12 @@ class SportsScrollDisplay: return False # Tell the core the panel is scrolling, and for how many - # refreshes to hold each frame. Without this the frame hold is - # never applied -- so a speed the ladder made crisp still presents - # a new frame every refresh and judders -- and, because deferred - # updates only run while nothing is scrolling, core would run - # blocking work in the middle of this scroll. + # refreshes to hold each frame. The helper advances a fixed number + # of whole pixels per presented frame, so without the hold a new + # frame is presented every refresh and the scroll runs frame_hold + # times too fast. And because deferred updates only run while + # nothing is scrolling, core would otherwise run blocking work in + # the middle of this scroll. if hasattr(self.display_manager, "set_scrolling_state"): self.display_manager.set_scrolling_state( True, frame_hold=self._scroll_frame_hold()) diff --git a/src/display_manager.py b/src/display_manager.py index a9f5b785..9e0252d5 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -306,50 +306,9 @@ class DisplayManager: raise MatrixSettingsRefused(refused) logger.warning("Emulator mode: continuing, but on a real panel the display would not start. %s", refused) - # Basic hardware settings - options.rows = hardware_config.get('rows', DEFAULT_ROWS) - options.cols = hardware_config.get('cols', DEFAULT_COLS) - options.chain_length = hardware_config.get('chain_length', DEFAULT_CHAIN_LENGTH) - options.parallel = hardware_config.get('parallel', DEFAULT_PARALLEL) - options.hardware_mapping = hardware_config.get('hardware_mapping', 'adafruit-hat-pwm') - - # Performance and stability settings - options.brightness = hardware_config.get('brightness', 90) - options.pwm_bits = hardware_config.get('pwm_bits', 10) - options.pwm_lsb_nanoseconds = hardware_config.get('pwm_lsb_nanoseconds', 150) - options.led_rgb_sequence = hardware_config.get('led_rgb_sequence', 'RGB') - options.pixel_mapper_config = self._build_pixel_mapper_config(hardware_config) - options.row_address_type = hardware_config.get('row_address_type', 0) - options.multiplexing = hardware_config.get('multiplexing', 0) - options.panel_type = hardware_config.get('panel_type', '') - options.disable_hardware_pulsing = hardware_config.get('disable_hardware_pulsing', False) - options.show_refresh_rate = hardware_config.get('show_refresh_rate', False) - options.limit_refresh_rate_hz = hardware_config.get('limit_refresh_rate_hz', 90) - options.gpio_slowdown = runtime_config.get('gpio_slowdown', 3) - - # Disable internal privilege dropping - we manage this via systemd or remain root - # This prevents the library from dropping to 'daemon' user which breaks file permissions - options.drop_privileges = False - - # Additional settings from config - if 'scan_mode' in hardware_config: - options.scan_mode = hardware_config.get('scan_mode') - if 'pwm_dither_bits' in hardware_config: - options.pwm_dither_bits = hardware_config.get('pwm_dither_bits') - if 'inverse_colors' in hardware_config: - options.inverse_colors = hardware_config.get('inverse_colors') - # Pi 5 only: 0=PIO/RP1 coprocessor (default, less CPU), - # 1=RIO/Registered IO (faster; gpio_slowdown effect is inverted in this mode) - if 'rp1_rio' in runtime_config: - if hasattr(options, 'rp1_rio'): - options.rp1_rio = runtime_config.get('rp1_rio') - else: - logger.warning( - "rp1_rio is set in config but the installed rgbmatrix library does " - "not support it — the library was likely built without Pi 5 RP1 " - "support (mmap to 0x3f000000 instead of RP1 chip). " - "Fix: sudo RPI_RGB_FORCE_REBUILD=1 ./first_time_install.sh" - ) + # Every option comes from display.hardware / display.runtime, in + # one place that scripts/scroll_speeds.py shares. + self.apply_matrix_options(options, self.config) logger.info(f"Initializing RGB Matrix with settings: rows={options.rows}, cols={options.cols}, chain_length={options.chain_length}, parallel={options.parallel}, hardware_mapping={options.hardware_mapping}") @@ -1396,6 +1355,68 @@ class DisplayManager: return dt.strftime(f"%b %-d{suffix}") + @classmethod + def apply_matrix_options(cls, options, config: Dict[str, Any]): + """Fill ``options`` (an ``RGBMatrixOptions``) from the LEDMatrix config. + + This is exactly what the display service drives the panel with, so a + tool that opens the matrix itself (``scripts/scroll_speeds.py``) gets + the same panel -- same runtime ``gpio_slowdown``, ``rp1_rio``, + ``panel_type``, orientation and defaults -- rather than a private copy + that drifts. Does not open the matrix. Returns ``options``. + """ + display = config.get('display', {}) if isinstance(config, dict) else {} + hardware_config = display.get('hardware', {}) + runtime_config = display.get('runtime', {}) + + # Basic hardware settings + options.rows = hardware_config.get('rows', DEFAULT_ROWS) + options.cols = hardware_config.get('cols', DEFAULT_COLS) + options.chain_length = hardware_config.get('chain_length', DEFAULT_CHAIN_LENGTH) + options.parallel = hardware_config.get('parallel', DEFAULT_PARALLEL) + options.hardware_mapping = hardware_config.get('hardware_mapping', 'adafruit-hat-pwm') + + # Performance and stability settings + options.brightness = hardware_config.get('brightness', 90) + options.pwm_bits = hardware_config.get('pwm_bits', 10) + options.pwm_lsb_nanoseconds = hardware_config.get('pwm_lsb_nanoseconds', 150) + options.led_rgb_sequence = hardware_config.get('led_rgb_sequence', 'RGB') + # _build_pixel_mapper_config reads only class attributes, so the class + # stands in for an instance here. + options.pixel_mapper_config = cls._build_pixel_mapper_config(cls, hardware_config) + options.row_address_type = hardware_config.get('row_address_type', 0) + options.multiplexing = hardware_config.get('multiplexing', 0) + options.panel_type = hardware_config.get('panel_type', '') + options.disable_hardware_pulsing = hardware_config.get('disable_hardware_pulsing', False) + options.show_refresh_rate = hardware_config.get('show_refresh_rate', False) + options.limit_refresh_rate_hz = hardware_config.get('limit_refresh_rate_hz', 90) + options.gpio_slowdown = runtime_config.get('gpio_slowdown', 3) + + # Disable internal privilege dropping - we manage this via systemd or remain root + # This prevents the library from dropping to 'daemon' user which breaks file permissions + options.drop_privileges = False + + # Additional settings from config + if 'scan_mode' in hardware_config: + options.scan_mode = hardware_config.get('scan_mode') + if 'pwm_dither_bits' in hardware_config: + options.pwm_dither_bits = hardware_config.get('pwm_dither_bits') + if 'inverse_colors' in hardware_config: + options.inverse_colors = hardware_config.get('inverse_colors') + # Pi 5 only: 0=PIO/RP1 coprocessor (default, less CPU), + # 1=RIO/Registered IO (faster; gpio_slowdown effect is inverted in this mode) + if 'rp1_rio' in runtime_config: + if hasattr(options, 'rp1_rio'): + options.rp1_rio = runtime_config.get('rp1_rio') + else: + logger.warning( + "rp1_rio is set in config but the installed rgbmatrix library does " + "not support it — the library was likely built without Pi 5 RP1 " + "support (mmap to 0x3f000000 instead of RP1 chip). " + "Fix: sudo RPI_RGB_FORCE_REBUILD=1 ./first_time_install.sh" + ) + return options + @property def refresh_hz(self) -> float: """The panel's refresh rate in Hz, from the hardware config. @@ -1441,6 +1462,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 diff --git a/src/vegas_mode/config.py b/src/vegas_mode/config.py index f1039f5b..a84b3321 100644 --- a/src/vegas_mode/config.py +++ b/src/vegas_mode/config.py @@ -155,9 +155,14 @@ class VegasModeConfig: target_fps: int = 125 # Target frame rate buffer_ahead: int = 2 # Number of plugins to buffer ahead - # Scroll behavior + # Scroll behavior. Neither key steps the scroll or sets a frame rate: + # motion is always by elapsed time at scroll_speed px/s. With + # frame_based_scrolling the speed is first converted to px per + # scroll_delay and clamped to 0.1-5 (ScrollHelper.set_scroll_speed), so + # the speed actually applied is clamp(scroll_speed * scroll_delay, 0.1, 5) + # / scroll_delay -- at the 0.02 default, speeds under 5 px/s run at 5. frame_based_scrolling: bool = True - scroll_delay: float = 0.02 # 50 FPS effective scroll updates + scroll_delay: float = 0.02 # only feeds the clamp above; not a frame period # Dynamic duration dynamic_duration_enabled: bool = True diff --git a/src/vegas_mode/render_pipeline.py b/src/vegas_mode/render_pipeline.py index 8aceabe1..d9b713f5 100644 --- a/src/vegas_mode/render_pipeline.py +++ b/src/vegas_mode/render_pipeline.py @@ -127,12 +127,14 @@ class RenderPipeline: self.scroll_helper.set_sub_pixel_scrolling(self.config.smooth_scroll) # Config scroll_speed is always pixels per second, but ScrollHelper - # interprets it differently based on frame_based_scrolling mode: - # - Frame-based: pixels per frame step - # - Time-based: pixels per second + # takes it in different units depending on frame_based_scrolling: + # - Frame-based: pixels per scroll_delay seconds (clamped to 0.1-5) + # - Time-based: pixels per second (clamped to 1-500) + # Both modes then advance by elapsed time; frame-based mode does not + # step. So frame-based with scroll_delay only adds the clamp: the + # applied speed is clamp(scroll_speed * scroll_delay, 0.1, 5) / + # scroll_delay px/s. if self.config.frame_based_scrolling: - # Convert pixels/second to pixels/frame - # pixels_per_frame = pixels_per_second * seconds_per_frame pixels_per_frame = self.config.scroll_speed * self.config.scroll_delay self.scroll_helper.set_scroll_speed(pixels_per_frame) else: diff --git a/test/test_scroll_helper.py b/test/test_scroll_helper.py index 8d77edca..a04eabfe 100644 --- a/test/test_scroll_helper.py +++ b/test/test_scroll_helper.py @@ -460,6 +460,19 @@ class TestIdleGapIsNotAFrame: assert not info.called assert len(helper._window) == 1 + def test_a_dropped_gap_restarts_the_window_timer_too(self, helper): + """The size-guard path is the one scrollers that never call + reset_scroll() take, so it must restart the window like the sentinel + does, or their next scroll opens with a one-frame stats line.""" + helper.log_frame_rate() # seeds + helper.last_frame_time = time.time() - 137.0 + helper.last_fps_log_time = 0.0 # boundary long overdue + with patch.object(helper.logger, "info") as info: + helper.log_frame_rate() # the gap, dropped + helper.log_frame_rate() # first real frame + assert not info.called, "a one-frame window was reported" + assert len(helper._window) == 1 + def test_a_normal_frame_still_counts(self, helper): helper.last_frame_time = time.time() - 0.010 helper.log_frame_rate() diff --git a/test/test_scroll_speeds_script.py b/test/test_scroll_speeds_script.py new file mode 100644 index 00000000..6d3b68c9 --- /dev/null +++ b/test/test_scroll_speeds_script.py @@ -0,0 +1,144 @@ +"""scripts/scroll_speeds.py must drive the panel the way the service does. + +--measure and --demo open the matrix themselves. They used to build the +options from a private copy of DisplayManager's builder that had drifted: it +read gpio_slowdown from display.hardware (the service reads display.runtime), +and skipped rp1_rio, panel_type, orientation and more. On a panel that needs a +high slowdown that measured -- or garbled -- a panel the service never drives. +""" +import importlib.util +import os +from pathlib import Path +from unittest.mock import patch + +import pytest + +os.environ.setdefault("EMULATOR", "true") + +import src.display_manager as display_manager # noqa: E402 +from src.display_manager import DisplayManager # noqa: E402 + +SCRIPT = Path(__file__).resolve().parents[1] / "scripts" / "scroll_speeds.py" + + +@pytest.fixture(scope="module") +def script(): + spec = importlib.util.spec_from_file_location("scroll_speeds_script", SCRIPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class _Options: + """Records what is set; declares rp1_rio like a Pi 5-capable binding.""" + + rp1_rio = None + + +CONFIG = { + "display": { + "hardware": { + "rows": 64, "cols": 64, "chain_length": 2, "parallel": 1, + "hardware_mapping": "adafruit-hat-pwm", + "brightness": 70, "pwm_bits": 9, "pwm_lsb_nanoseconds": 130, + "row_address_type": 5, "panel_type": "FM6126A", + "limit_refresh_rate_hz": 120, "orientation": "180", + "pixel_mapper_config": "U-mapper", + "gpio_slowdown": 2, # a stale key in the wrong block + }, + "runtime": {"gpio_slowdown": 7, "rp1_rio": 1}, + } +} + + +def _script_options(script, config, **kwargs): + with patch.object(display_manager, "RGBMatrixOptions", _Options): + return script.build_options(config, **kwargs) + + +def test_runtime_settings_are_used(script): + options = _script_options(script, CONFIG) + assert options.gpio_slowdown == 7 + assert options.rp1_rio == 1 + assert options.panel_type == "FM6126A" + assert options.row_address_type == 5 + assert options.pixel_mapper_config == "U-mapper;Rotate:180" + assert options.limit_refresh_rate_hz == 120 + + +def test_the_options_match_the_display_service(script): + """Same config in, same options out, attribute for attribute.""" + service = DisplayManager.apply_matrix_options(_Options(), CONFIG) + assert vars(_script_options(script, CONFIG)) == vars(service) + + +def test_measure_runs_uncapped(script): + assert _script_options(script, CONFIG, refresh_override=0).limit_refresh_rate_hz == 0 + + +def test_an_empty_config_gets_the_service_defaults(script): + options = _script_options(script, {}) + assert vars(options) == vars(DisplayManager.apply_matrix_options(_Options(), {})) + assert options.gpio_slowdown == 3 + + +def test_the_service_uses_the_shared_builder(test_config): + """DisplayManager._setup_matrix fills its options through the same call.""" + with patch.object(display_manager, "RGBMatrix"), \ + patch.object(display_manager, "RGBMatrixOptions", _Options), \ + patch.object(display_manager, "freetype"), \ + patch.object(DisplayManager, "apply_matrix_options", + wraps=DisplayManager.apply_matrix_options) as shared, \ + patch.dict(os.environ, {"EMULATOR": "false"}): + DisplayManager._instance = None + try: + DisplayManager(test_config) + finally: + DisplayManager._instance = None + shared.assert_called_once() + + +class TestSpeedAdvice: + """The advice must name keys the resolver actually honours.""" + + def _advice(self, script, capsys, hz, want): + script.print_ladder(hz, want) + return capsys.readouterr().out + + def test_advice_is_the_speed_delay_pair(self, script, capsys): + out = self._advice(script, capsys, 100.0, 60) + assert '"scroll_speed": 2' in out + assert '"scroll_delay": 0.03' in out + + @pytest.mark.parametrize("hz,want", [(100.0, 60), (100.0, 50), (120.0, 45), + (60.0, 30), (100.0, None)]) + def test_the_advised_pair_resolves_to_the_advised_speed(self, script, capsys, + hz, want): + """Apply the printed pair over a schema-default pair, as a saved config + would carry it, and the resolver must land on the advertised speed.""" + import json + import re + + from src.common import scroll_config + + out = self._advice(script, capsys, hz, want) + block = re.search(r'"display_options": (\{[^}]*\})', out) + assert block, out + advised = json.loads(block.group(1)) + config = {"display_options": {"scroll_speed": 1.0, "scroll_delay": 0.02, + "scroll_pixels_per_second": 999, + **advised}} + expected = scroll_config.solve_crisp(want if want else hz / 2, hz) + settings = scroll_config.configure( + _Helper(), plugin_config=config, refresh_hz=hz) + assert settings.pixels_per_second == pytest.approx(expected.pixels_per_second) + assert settings.frame_hold == expected.frame_hold + + def test_the_deprecated_key_is_not_recommended(self, script, capsys): + out = self._advice(script, capsys, 100.0, 60) + assert '"scroll_pixels_per_second":' not in out + + +class _Helper: + def set_scroll_speed(self, speed): + pass diff --git a/test/test_sports_scroll.py b/test/test_sports_scroll.py index 893dfa34..f6972883 100644 --- a/test/test_sports_scroll.py +++ b/test/test_sports_scroll.py @@ -6,8 +6,11 @@ is shared, the content layer is not. Two things are worth asserting beyond * ``prepare_scroll_content`` must stay an override point — a base class that quietly rendered *something* would let a plugin ship a blank scroll. -* ``target_fps`` must actually reach the helper. That is the whole reason this - module exists upstream; the bundled plugin copies hardcode ~100 FPS. +* Speed must depend only on ``scroll_speed`` and the panel refresh. The helper + steps a fixed number of whole pixels per presented frame and the panel + presents at its refresh divided by the frame hold, so a ladder computed for + any other rate -- the global ``target_fps`` used to be one -- plays back at + the wrong speed. """ import logging @@ -221,12 +224,14 @@ class TestConfigureScrollHelper: bare = SportsScrollDisplay.__new__(SportsScrollDisplay) assert SportsScrollDisplay._scroll_frame_hold(bare) == 1 - def test_stepping_is_time_based(self, build): - """Frame-based mode gated motion on a wall clock at 1/scroll_delay - steps, with scroll_delay set to the frame period -- putting the - comparison exactly on its own threshold, so it flipped on sub- - millisecond jitter.""" - build().scroll_helper.set_frame_based_scrolling.assert_called_once_with(False) + def test_helper_steps_whole_pixels_per_presented_frame(self, build): + """No wall clock: the helper advances the ladder's whole-pixel step on + every presented frame, and the frame hold sets how often that is.""" + display = build() + helper = display.scroll_helper + helper.set_frame_based_scrolling.assert_called_once_with(False) + helper.set_pixels_per_frame.assert_called_once_with( + display._scroll_settings.crisp.pixels_per_frame) def test_dynamic_duration_settings_are_applied(self, build): display = build({"nhl": {"scroll_settings": { @@ -243,31 +248,65 @@ class TestConfigureScrollHelper: applied = display.scroll_helper.set_scroll_speed.call_args[0][0] assert applied == pytest.approx(100.0, abs=1.0) - def test_global_target_fps_sets_the_refresh_the_ladder_uses(self, build): - """Under the old model this key was the rate frames were presented at, - so it is the faithful translation of it into a panel refresh.""" - display = build(global_config={"target_fps": 60}) - assert display._resolve_refresh_hz() == 60.0 + @pytest.mark.parametrize("global_config", [ + {}, + {"target_fps": 60}, + {"target_fps": 100}, + {"target_fps": 120}, + {"target_fps": 200}, + {"scroll_target_fps": 90}, + {"target_fps": 120, "scroll_target_fps": 90}, + ]) + def test_target_fps_does_not_change_scroll_speed(self, build, global_config): + """The General tab's "Scroll Frame Rate" is not a speed control. - def test_legacy_key_is_honored(self, build): - display = build(global_config={"scroll_target_fps": 90}) - assert display._resolve_refresh_hz() == 90.0 + It used to set the refresh the ladder was computed against, while the + panel kept presenting at its real 100Hz: 60 doubled every scoreboard's + speed and 200 halved it.""" + display = build(global_config=global_config) + assert display._resolve_refresh_hz() == 100.0 + assert display._scroll_settings.pixels_per_second == pytest.approx(50.0) + assert display._scroll_settings.frame_hold == 2 + assert display._scroll_settings.crisp.pixels_per_frame == 1 - def test_modern_key_wins_over_legacy(self, build): - display = build(global_config={"target_fps": 120, "scroll_target_fps": 90}) - assert display._resolve_refresh_hz() == 120.0 - - def test_configured_hardware_refresh_wins_over_the_fps_target(self, build): + def test_configured_hardware_refresh_sets_the_ladder(self, build): display = build(global_config={ "target_fps": 60, "display": {"hardware": {"limit_refresh_rate_hz": 120}}}) assert display._resolve_refresh_hz() == 120.0 + assert display._scroll_settings.crisp.refresh_hz == 120.0 + + def test_display_manager_refresh_wins_over_global_config(self, build, + display_manager): + """The display manager reports the rate the panel is driven at.""" + display_manager.refresh_hz = 60.0 + display = build(global_config={ + "target_fps": 200, + "display": {"hardware": {"limit_refresh_rate_hz": 120}}}) + assert display._resolve_refresh_hz() == 60.0 + + @pytest.mark.parametrize("reported", [True, 0, -5, "100", None]) + def test_unusable_display_manager_refresh_falls_back(self, build, + display_manager, + reported): + display_manager.refresh_hz = reported + display = build(global_config={ + "display": {"hardware": {"limit_refresh_rate_hz": 120}}}) + assert display._resolve_refresh_hz() == 120.0 @pytest.mark.parametrize("bad", ["fast", None, {}, [], "", 0]) - def test_unusable_target_fps_degrades_instead_of_raising(self, build, bad): - """A malformed global config must cost the FPS target, not the display.""" + def test_unusable_target_fps_is_harmless(self, build, bad): + """A malformed global config must not cost the display.""" display = build(global_config={"target_fps": bad}) - assert display._scroll_settings.pixels_per_second > 0 + assert display._scroll_settings.pixels_per_second == pytest.approx(50.0) + + @pytest.mark.parametrize("delay", [0.001, 0.01, 0.05, 0.1]) + def test_scroll_delay_is_ignored_for_pacing(self, build, delay): + """Kept in the settings for compatibility; it does not change speed.""" + display = build({"nhl": {"scroll_settings": { + "scroll_speed": 50.0, "scroll_delay": delay}}}) + assert display._scroll_settings.pixels_per_second == pytest.approx(50.0) + assert display._scroll_settings.frame_hold == 2 @pytest.mark.parametrize("bad", [None, "fast", {}, []]) def test_unusable_scroll_speed_degrades_instead_of_crashing(self, build, bad): @@ -475,7 +514,9 @@ class TestLifecycle: class TestManager: @pytest.fixture def manager(self, display_manager): - return _Manager(display_manager, {}, LOGGER, global_config={"target_fps": 120}) + return _Manager(display_manager, {}, LOGGER, global_config={ + "target_fps": 120, + "display": {"hardware": {"limit_refresh_rate_hz": 120}}}) def test_displays_are_created_lazily_and_reused(self, manager): first = manager.get_scroll_display("live") @@ -487,15 +528,13 @@ class TestManager: "recent") def test_global_config_is_threaded_to_children(self, manager): - """A missed hand-off here is exactly how the plugin copies ended up - never honoring target_fps.""" + """The child needs the global config to find the panel refresh when + the display manager cannot report one.""" child = manager.get_scroll_display("live") - assert child.global_config == {"target_fps": 120} - # The target is now the refresh the crisp ladder is computed against, - # so what proves the hand-off is that the child reasons about 120Hz -- - # not the presentation rate, which the chosen hold divides down. + assert child.global_config["target_fps"] == 120 + # What proves the hand-off is that the child reasons about the 120Hz + # hardware refresh from that config (target_fps plays no part). assert child._resolve_refresh_hz() == 120.0 - child.scroll_helper.set_target_fps.assert_called_once() def test_prepare_sets_the_active_type(self, manager): assert manager.prepare_and_display([{"id": "g1"}], "live", ["nhl"]) is True @@ -619,15 +658,17 @@ class TestAgainstTheRealScrollHelper: def test_configuration_lands_on_the_real_helper(self, real): display = real.get_scroll_display("live") helper = display.scroll_helper - # 500 px/s on a 120Hz panel snaps to a whole number of pixels per - # refresh, and the helper is driven in px/s in time-based mode. + # 500 px/s on the default 100Hz panel is a whole number of pixels per + # refresh, and the helper steps that many pixels per presented frame. assert helper.frame_based_scrolling is False assert helper.scroll_speed == pytest.approx( display._scroll_settings.pixels_per_second) assert helper.scroll_speed == pytest.approx(500.0, abs=25.0) - # target_fps is the presentation rate the chosen hold produces. + assert helper.fixed_pixels_per_frame == ( + display._scroll_settings.crisp.pixels_per_frame) + # target_fps is informational: the presentation rate the hold gives. assert helper.target_fps == pytest.approx( - 120.0 / display._scroll_settings.frame_hold, abs=1.0) + 100.0 / display._scroll_settings.frame_hold, abs=1.0) def test_a_strip_is_built_and_scrolls_to_completion(self, real): import time @@ -638,21 +679,58 @@ class TestAgainstTheRealScrollHelper: # 3 cards of 100px + gaps, so the strip is wider than the 128px panel. assert display.scroll_helper.cached_image.width > 128 - # Frame-based scrolling is wall-clock gated on scroll_delay, so the - # loop has to actually pass time rather than spin. + # The helper steps a fixed whole-pixel amount per presented frame and + # consults no clock, so the scroll completes in a bounded number of + # frames however fast this loop spins. deadline = time.time() + 20 - while not real.is_complete() and time.time() < deadline: + frames = 0 + while not real.is_complete() and time.time() < deadline and frames < 10000: real.display_frame() - time.sleep(0.0012) + frames += 1 - # Asserted separately so a host too slow to sustain the frame rate - # reports a timeout rather than looking like a scrolling defect. - assert time.time() < deadline, ( - "scroll did not finish within 20s — the host may be too slow to " - "sustain the configured frame rate") + assert time.time() < deadline, "scroll did not finish within 20s" assert real.is_complete() is True assert display.scroll_helper.scroll_position > 128 def test_dynamic_duration_is_a_real_number(self, real): real.prepare_and_display([{"id": "a"}], "live", ["nhl"]) assert real.get_scroll_display("live").get_dynamic_duration() > 0 + + @pytest.mark.parametrize("target_fps", [None, 60, 100, 200]) + def test_presented_speed_is_independent_of_target_fps(self, monkeypatch, + target_fps): + """End to end: the px/s the panel actually shows. + + The panel is simulated the way SwapOnVSync paces it: each drawn frame + occupies ``frame_hold`` refreshes of a 100Hz panel. Ten seconds of + refreshes must move the strip 500px at the default 50 px/s, whatever + the General tab's target_fps says.""" + from src.common.scroll_helper import ScrollHelper + monkeypatch.setattr("src.common.sports_scroll.ScrollHelper", ScrollHelper) + + class _Panel: + matrix = None + width, height = 128, 32 + refresh_hz = 100.0 + hold = 1 + image = None + + def set_scrolling_state(self, scrolling, frame_hold=1): + self.hold = frame_hold + + def update_display(self): + pass + + global_config = {"display": {"hardware": {"limit_refresh_rate_hz": 100}}} + if target_fps is not None: + global_config["target_fps"] = target_fps + panel = _Panel() + display = _RealDisplay(panel, {}, LOGGER, global_config=global_config) + display.scroll_helper.set_scrolling_image(Image.new("RGB", (5000, 32))) + + refreshes = 0 + while refreshes < 1000: # ten seconds at 100Hz + assert display.display_scroll_frame() is True + refreshes += panel.hold + moved = display.scroll_helper.total_distance_scrolled + assert moved / 10.0 == pytest.approx(50.0, abs=1.0) diff --git a/web_interface/templates/v3/partials/general.html b/web_interface/templates/v3/partials/general.html index 9627269a..65d8da6f 100644 --- a/web_interface/templates/v3/partials/general.html +++ b/web_interface/templates/v3/partials/general.html @@ -88,9 +88,9 @@
- +