From 6e8a9dd24b268ad461eff435ce68b910a33455a7 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:32:52 -0400 Subject: [PATCH] feat(scroll): let users pick a crisp speed for their own panel Whole-pixel motion was previously only available at multiples of the refresh rate -- 100, 200, 300 px/s on a 100Hz panel. 100 px/s crosses a 256px panel in 2.6s, which is brisk for reading, and everything slower had to blend (blur) or repeat frames unevenly (judder). There was no way to ask for 50 px/s and get clean motion. SwapOnVSync takes a framerate_fraction the display manager never passed. It holds each frame for N panel refreshes; the panel keeps refreshing at its full rate throughout, so holding costs nothing in flicker and only changes how often a NEW image is presented. That turns 50 px/s into one whole pixel every second refresh instead of half a pixel every refresh. The crisp speeds are therefore refresh_hz / hold * pixels_per_frame, and that ladder depends on the panel: a Pi Zero on a long chain has a different set of good speeds from a Pi 4 on a short one. crisp_ladder() enumerates them and solve_crisp() picks the best match for a requested speed. solve_crisp weights motion quality rather than picking the numerically nearest entry, which matters more than it sounds. Asked for 30 px/s, nearest-by-value answers 28.6 -- 2px jumps at 14fps -- over 33.3, which is single-pixel motion at 33fps and obviously better on the panel. The target is also clamped into the ladder's range first, because relative error saturates near 1.0 for a target far outside it and the quality penalty would otherwise answer "10000 px/s" with the slowest entry. configure() snaps to the ladder and applies the hold when given a display manager. Without one the hold silently cannot happen and motion falls back to fractional pixels, so it warns rather than failing quietly. set_frame_hold() resets to 1 when scrolling stops, so one plugin's pacing cannot leak into whatever is on screen next. scripts/scroll_speeds.py is the user-facing part: it prints the ladder for the configured rate, measures what the panel ACTUALLY manages (--measure, for hardware that cannot reach its configured limit), highlights the nearest option to a wanted speed, and demos one live. It never starts or stops the display service itself -- doing that inside a script stranded the panel twice today. Speeds below ~20 px/s remain stepped regardless. That is the pixel pitch, not a software limit. Also fixes the dirty-tracking test spy, which stubbed SwapOnVSync with a single-argument function and would have masked the new call as a failed push, and rewrites a configure() test that had started passing for the wrong reason: it asserted a judder warning, which snapping now prevents, and was matching the unrelated "hold could not be applied" warning instead. 183 tests pass on the Pi. Co-Authored-By: Claude Opus 5 --- docs/SCROLL_PERFORMANCE.md | 78 ++++++++- scripts/scroll_speeds.py | 238 ++++++++++++++++++++++++++++ src/common/scroll_config.py | 193 +++++++++++++++++++++- src/display_manager.py | 34 +++- test/test_display_dirty_tracking.py | 55 ++++++- test/test_scroll_config.py | 147 ++++++++++++++++- 6 files changed, 729 insertions(+), 16 deletions(-) create mode 100644 scripts/scroll_speeds.py diff --git a/docs/SCROLL_PERFORMANCE.md b/docs/SCROLL_PERFORMANCE.md index 479c325b..bb16ddc0 100644 --- a/docs/SCROLL_PERFORMANCE.md +++ b/docs/SCROLL_PERFORMANCE.md @@ -20,8 +20,10 @@ Measured on a Raspberry Pi 4 driving a 2×128×64 chain (256×64 logical) at **Motion is smooth when the strip advances a whole number of pixels per panel refresh.** -On a 100 Hz panel the crisp speeds are 100 px/s, 200 px/s, 300 px/s. A speed -that does not divide evenly has to do one of two bad things: +Advancing one pixel per refresh on a 100 Hz panel gives 100 px/s. Slower crisp +speeds come from holding each frame for several refreshes -- 50 px/s is one +pixel every second refresh -- which is covered under *Choosing a speed* below. +A speed that lands on no such combination has to do one of two bad things: - **blend** two adjacent columns to render a half-step — on pixel-font text this alternates crisp and smeared frames and reads as shimmer, or as the @@ -31,8 +33,76 @@ that does not divide evenly has to do one of two bad things: Neither is tunable away. Pick a speed that divides evenly. -`src.common.scroll_config.resolve()` warns when a configured speed will not, -and names the nearest speed that will. +`src.common.scroll_config` solves this for you: `configure()` snaps a requested +speed to the nearest one the panel can actually show in whole pixels, and +`scripts/scroll_speeds.py` prints the full ladder for your hardware. + +## Choosing a speed + +The crisp speeds are not a fixed list -- they depend on how fast *your* panel +refreshes, which depends on its size, `pwm_bits`, `gpio_slowdown` and the Pi +model. A Pi Zero driving a long chain has a completely different set of good +speeds from a Pi 4 driving a short one. + +```bash +# what can this panel do? (reads your configured refresh rate) +python3 scripts/scroll_speeds.py + +# what does it ACTUALLY manage, rather than what is configured? +sudo systemctl stop ledmatrix +sudo python3 scripts/scroll_speeds.py --measure +sudo systemctl start ledmatrix + +# highlight the closest option to the speed you want +python3 scripts/scroll_speeds.py --want 45 + +# try one on the panel +sudo systemctl stop ledmatrix +sudo python3 scripts/scroll_speeds.py --demo 50 +sudo systemctl start ledmatrix +``` + +Sample ladder for a 100 Hz panel: + +``` + 20.0 px/s (1px every 5 refreshes = 20.0 fps, slightly stepped) + 25.0 px/s (1px every 4 refreshes = 25.0 fps, slightly stepped) + 33.3 px/s (1px every 3 refreshes = 33.3 fps, smooth) + 50.0 px/s (1px every 2 refreshes = 50.0 fps, smooth) + 66.7 px/s (2px every 3 refreshes = 33.3 fps, smooth) + 100.0 px/s (1px every 1 refresh = 100.0 fps, smooth) +``` + +### How a slow speed stays crisp + +`SwapOnVSync(canvas, framerate_fraction)` holds each frame for N panel +refreshes. **The panel keeps refreshing at its full rate either way**, so +holding a frame costs nothing in flicker -- it only changes how often a *new* +image is presented. That is what allows 50 px/s to be one whole pixel every +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 applies the hold, provided it is given the display manager: + +```python +scroll_config.configure( + self.scroll_helper, + plugin_config=self.config, + global_config=self.global_config, + display_manager=self.display_manager, # required for the hold to apply +) +``` + +Without `display_manager` a sub-refresh speed still resolves, but the hold is +never applied and the motion falls back to fractional pixels -- so `configure` +logs a warning rather than failing quietly. Pass `snap_to_crisp=False` to keep +an exact requested speed and accept the artefacts. + +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 +software limit; the only way to move in smaller increments is sub-pixel +blending, which this display does not tolerate (see above). ## Configuring a plugin diff --git a/scripts/scroll_speeds.py b/scripts/scroll_speeds.py new file mode 100644 index 00000000..a142b433 --- /dev/null +++ b/scripts/scroll_speeds.py @@ -0,0 +1,238 @@ +#!/usr/bin/env python3 +"""Show and try the scroll speeds your panel can display cleanly. + +Motion looks smooth when the strip advances a WHOLE number of pixels per panel +refresh. Anything else has to blend two columns (which on pixel-font text reads +as shimmer) or repeat frames unevenly (which reads as judder). So the speeds +worth using are not arbitrary -- they are + + refresh_hz / frame_hold * pixels_per_frame + +for whole numbers of frame_hold and pixels_per_frame, and that ladder depends +on how fast YOUR panel actually refreshes. A Pi Zero driving a big chain will +have a completely different set of good speeds from a Pi 4 driving a small one. + + # what can this panel do? (no hardware needed, uses your configured rate) + python3 scripts/scroll_speeds.py + + # measure what the panel ACTUALLY manages, rather than what is configured + sudo systemctl stop ledmatrix + sudo python3 scripts/scroll_speeds.py --measure + sudo systemctl start ledmatrix + + # what would a 60Hz panel offer? + python3 scripts/scroll_speeds.py --hz 60 + + # try one on the panel + sudo systemctl stop ledmatrix + sudo python3 scripts/scroll_speeds.py --demo 50 + sudo systemctl start ledmatrix + +This script never starts or stops the display service itself -- that is left to +you, so a crash here can never leave the panel dark. +""" +from __future__ import annotations + +import argparse +import json +import os +import sys +import time +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from src.common import scroll_config # noqa: E402 + +CONFIG = Path(__file__).resolve().parent.parent / "config" / "config.json" + + +def load_hardware(): + try: + with open(CONFIG, encoding="utf-8") as handle: + return (json.load(handle).get("display") or {}).get("hardware") or {} + except (OSError, ValueError): + return {} + + +def build_options(hardware, refresh_override=None): + from rgbmatrix import RGBMatrixOptions + + o = RGBMatrixOptions() + o.rows = int(hardware.get("rows", 32)) + o.cols = int(hardware.get("cols", 64)) + o.chain_length = int(hardware.get("chain_length", 1)) + o.parallel = int(hardware.get("parallel", 1)) + 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 open_matrix(hardware, 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") + try: + return RGBMatrix(options=build_options(hardware, refresh_override)) + except Exception as exc: # pragma: no cover - hardware dependent + sys.exit( + "could not open the panel ({}).\n" + "If the display service is running it owns the GPIO - stop it first:\n" + " sudo systemctl stop ledmatrix".format(exc) + ) + + +def measure_refresh(hardware, 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 + runs at exactly the panel's rate. This is what an older Pi or a longer + chain will really give you, as opposed to whatever limit_refresh_rate_hz + optimistically asks for. + """ + matrix = open_matrix(hardware, refresh_override=0) + canvas = matrix.CreateFrameCanvas() + canvas = matrix.SwapOnVSync(canvas) # discard the first, it includes setup + frames = 0 + started = time.perf_counter() + while time.perf_counter() - started < seconds: + canvas = matrix.SwapOnVSync(canvas) + frames += 1 + measured = frames / (time.perf_counter() - started) + matrix.Clear() + return measured + + +def demo(hardware, target, seconds): + """Scroll text at the crisp speed nearest `target`.""" + from PIL import Image, ImageDraw, ImageFont + + hz = float(hardware.get("limit_refresh_rate_hz") or scroll_config.DEFAULT_REFRESH_HZ) + choice = scroll_config.solve_crisp(target, hz) + print("asked for {:.0f} px/s -> {}".format(target, choice.describe())) + + matrix = open_matrix(hardware) + canvas = matrix.CreateFrameCanvas() + W, H = canvas.width, canvas.height + + font = None + for path, size in ( + (str(Path(__file__).resolve().parent.parent / "assets/fonts/PressStart2P-Regular.ttf"), 16), + ("/usr/share/fonts/truetype/dejavu/DejaVuSansMono-Bold.ttf", 26), + ): + try: + font = ImageFont.truetype(path, size) + break + except OSError: + continue + if font is None: + font = ImageFont.load_default() + + text = " {:.0f} px/s *** THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG ***".format( + choice.pixels_per_second) + box = ImageDraw.Draw(Image.new("RGB", (8, 8))).textbbox((0, 0), text, font=font) + tw, th = box[2] - box[0], box[3] - box[1] + reps = max(2, (W * 3) // max(tw, 1) + 1) + strip = Image.new("RGB", (tw * reps, H), (0, 0, 0)) + draw = ImageDraw.Draw(strip) + for i in range(reps): + draw.text((i * tw, (H - th) // 2 - box[1]), text, font=font, fill=(255, 210, 60)) + + offset = 0 + frames = 0 + started = time.time() + while time.time() - started < seconds: + window = strip.crop((offset, 0, offset + W, H)) + if window.width < W: + whole = Image.new("RGB", (W, H), (0, 0, 0)) + head = strip.crop((offset, 0, strip.width, H)) + whole.paste(head, (0, 0)) + whole.paste(strip.crop((0, 0, W - head.width, H)), (head.width, 0)) + window = whole + canvas.SetImage(window) + canvas = matrix.SwapOnVSync(canvas, choice.frame_hold) + offset = (offset + choice.pixels_per_frame) % strip.width + frames += 1 + elapsed = time.time() - started + print(" {} frames in {:.1f}s = {:.1f} fps = {:.1f} px/s actual".format( + frames, elapsed, frames / elapsed, frames * choice.pixels_per_frame / elapsed)) + matrix.Clear() + + +def print_ladder(hz, highlight=None): + print("") + print("Whole-pixel scroll speeds at {:.1f}Hz refresh".format(hz)) + print("(the panel refreshes at {:.0f}Hz for every one of these - holding a " + "frame costs no flicker)".format(hz)) + print("") + for entry in scroll_config.crisp_ladder(hz): + if entry.pixels_per_second > hz * 3: + break + mark = " <-- nearest to {:.0f}".format(highlight) if ( + highlight is not None + and entry.pixels_per_second == scroll_config.solve_crisp(highlight, hz).pixels_per_second + ) 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)) + + +def main(): + ap = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + ap.add_argument("--hz", type=float, + help="refresh rate to compute the ladder for (default: your config)") + ap.add_argument("--measure", action="store_true", + help="measure the panel's real refresh rate (needs root, service stopped)") + ap.add_argument("--demo", type=float, metavar="PXPS", + help="scroll text at the crisp speed nearest this (needs root)") + ap.add_argument("--seconds", type=float, default=15.0, help="demo duration") + ap.add_argument("--want", type=float, metavar="PXPS", + help="highlight the entry nearest this speed") + args = ap.parse_args() + + hardware = load_hardware() + configured = float(hardware.get("limit_refresh_rate_hz") or 0) + + if args.demo is not None: + demo(hardware, args.demo, args.seconds) + return + + if args.measure: + measured = measure_refresh(hardware) + print("measured panel refresh: {:.1f}Hz".format(measured)) + if configured: + print("configured limit_refresh_rate_hz: {:.0f}".format(configured)) + if measured < configured * 0.95: + print(" -> the panel cannot reach the configured rate; the ladder") + print(" below uses what it actually manages") + print_ladder(measured, args.want) + return + + hz = args.hz or configured or scroll_config.DEFAULT_REFRESH_HZ + if not args.hz and not configured: + print("no limit_refresh_rate_hz in config; assuming {:.0f}Hz".format(hz)) + print("run with --measure to find your panel's real rate") + print_ladder(hz, args.want) + + +if __name__ == "__main__": + main() diff --git a/src/common/scroll_config.py b/src/common/scroll_config.py index 9bf38a0d..75d232d9 100644 --- a/src/common/scroll_config.py +++ b/src/common/scroll_config.py @@ -33,7 +33,7 @@ position proportional to real time instead. from __future__ import annotations import logging -from dataclasses import dataclass +from dataclasses import dataclass, replace from typing import Any, Dict, Optional logger = logging.getLogger(__name__) @@ -56,6 +56,132 @@ DEFAULT_REFRESH_HZ = 100.0 _WHOLE_PIXEL_TOLERANCE = 0.05 +#: Longest a frame may be held before motion reads as a slideshow rather than +#: a scroll. 6 refreshes at 100Hz is ~17px/s, already visibly stepped. +MAX_FRAME_HOLD = 8 + +#: Largest whole-pixel jump per presented frame before motion looks like it is +#: teleporting rather than sliding. +MAX_PIXELS_PER_FRAME = 6 + + +@dataclass(frozen=True) +class CrispSpeed: + """A speed the panel can show with whole-pixel motion. + + ``pixels_per_second`` is always ``refresh_hz / frame_hold * pixels_per_frame`` + exactly -- no rounding, no fractional pixel positions, so nothing has to be + blended or repeated unevenly. + + :param frame_hold: refreshes each frame is held for. This is rgbmatrix's + ``SwapOnVSync(canvas, framerate_fraction)``. The panel keeps refreshing + at full rate either way, so holding a frame costs nothing in flicker. + :param pixels_per_frame: whole pixels advanced per presented frame. + """ + + pixels_per_second: float + frame_hold: int + pixels_per_frame: int + refresh_hz: float + + @property + def frames_per_second(self) -> float: + return self.refresh_hz / self.frame_hold + + @property + def steppiness(self) -> str: + """Rough readability hint for this combination.""" + if self.pixels_per_frame > 2: + return "jumpy" + if self.frames_per_second < 20: + return "stepped" + if self.frames_per_second < 30: + return "slightly stepped" + return "smooth" + + def describe(self) -> str: + return ( + f"{self.pixels_per_second:6.1f} px/s " + f"({self.pixels_per_frame}px every {self.frame_hold} refresh" + f"{'es' if self.frame_hold != 1 else ' '} = " + f"{self.frames_per_second:5.1f} fps, {self.steppiness})" + ) + + +def crisp_ladder( + refresh_hz: float = DEFAULT_REFRESH_HZ, + max_frame_hold: int = MAX_FRAME_HOLD, + max_pixels_per_frame: int = MAX_PIXELS_PER_FRAME, +): + """Every whole-pixel speed this panel can show, slowest first. + + Duplicates are collapsed keeping the gentlest option: 100 px/s is reachable + as 1px every refresh or 2px every 2nd refresh, and the former moves in + smaller increments, so that is the one worth offering. + """ + best = {} + for hold in range(1, max_frame_hold + 1): + for ppf in range(1, max_pixels_per_frame + 1): + pps = refresh_hz / hold * ppf + key = round(pps, 3) + candidate = CrispSpeed(pps, hold, ppf, refresh_hz) + incumbent = best.get(key) + if incumbent is None or ppf < incumbent.pixels_per_frame: + best[key] = candidate + return [best[k] for k in sorted(best)] + + +#: How much a bigger pixel step costs, as a fraction of the target speed. +#: Tuned so 66.7px/s (2px at 33fps) beats 50px/s (1px at 50fps) when 60 was +#: asked for, but 33.3px/s (1px, smooth) still beats 28.6px/s (2px at 14fps) +#: when 30 was asked for -- being 11% slow is worth far less than looking bad. +_STEP_PENALTY = 0.05 +_SLOW_FPS_PENALTY = 0.25 # below 20fps +_LOWISH_FPS_PENALTY = 0.10 # below 25fps + + +def _quality_cost(candidate: "CrispSpeed", target: float) -> float: + """Lower is better. Numeric closeness alone picks bad-looking speeds. + + Nearest-by-value would answer "30 px/s" with 28.6 px/s -- which is 2px + jumps at 14fps -- over 33.3 px/s, which is single-pixel motion at 33fps and + obviously better on the panel. Proximity has to be traded against how the + motion actually reads. + """ + error = abs(candidate.pixels_per_second - target) / max(target, 1e-6) + cost = error + _STEP_PENALTY * (candidate.pixels_per_frame - 1) + fps = candidate.frames_per_second + if fps < 20: + cost += _SLOW_FPS_PENALTY + elif fps < 25: + cost += _LOWISH_FPS_PENALTY + return cost + + +def solve_crisp( + target_pixels_per_second: float, + refresh_hz: float = DEFAULT_REFRESH_HZ, + max_frame_hold: int = MAX_FRAME_HOLD, + max_pixels_per_frame: int = MAX_PIXELS_PER_FRAME, +) -> CrispSpeed: + """The whole-pixel speed that will look best for what was asked for. + + Not simply the nearest -- see :func:`_quality_cost`. Ties break toward the + smaller pixel step and the shorter hold. + """ + ladder = crisp_ladder(refresh_hz, max_frame_hold, max_pixels_per_frame) + # Clamp into the ladder's range first. Relative error saturates near 1.0 + # for a target far outside it, so the quality penalty would dominate and + # answer "10000 px/s" with the *slowest* entry -- smooth, and useless. + target = min(max(target_pixels_per_second, ladder[0].pixels_per_second), + ladder[-1].pixels_per_second) + return min( + ladder, + key=lambda c: (round(_quality_cost(c, target), 6), + c.pixels_per_frame, c.frame_hold), + ) + + @dataclass(frozen=True) class ScrollSettings: """The resolved outcome, and which config key produced it.""" @@ -65,6 +191,10 @@ class ScrollSettings: target_fps: Optional[float] = None pixels_per_frame: Optional[float] = None warning: Optional[str] = None + #: The whole-pixel speed actually applied, when snapping was enabled. + crisp: Optional[CrispSpeed] = None + #: What the config asked for, before snapping. + requested_pixels_per_second: Optional[float] = None def describe(self) -> str: text = f"{self.pixels_per_second:.1f} px/s (from {self.source})" @@ -187,6 +317,8 @@ def configure( default_pixels_per_second: float = DEFAULT_PIXELS_PER_SECOND, refresh_hz: Optional[float] = None, plugin_logger: Optional[logging.Logger] = None, + display_manager: Any = None, + snap_to_crisp: bool = True, ) -> ScrollSettings: """Resolve the config and apply it to ``scroll_helper``. @@ -194,6 +326,15 @@ def configure( stepping is not used. ``hasattr`` guards keep this usable against older ScrollHelper builds that a plugin may be running on. + :param display_manager: when given, the frame hold for the chosen speed is + applied to it. Without this a sub-refresh speed still resolves, but the + panel keeps presenting a new frame every refresh, so the motion falls + back to fractional pixels and judders -- the hold is what makes slow + speeds crisp. + :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. + :returns: the settings applied, so the caller can log or assert on them. """ log = plugin_logger or logger @@ -204,13 +345,57 @@ def configure( refresh_hz=refresh_hz, ) + hz = _coerce(refresh_hz) or settings.target_fps or DEFAULT_REFRESH_HZ + applied = settings.pixels_per_second + choice = None + + if snap_to_crisp: + choice = solve_crisp(settings.pixels_per_second, hz) + applied = choice.pixels_per_second + settings = replace( + settings, + pixels_per_second=applied, + requested_pixels_per_second=settings.pixels_per_second, + crisp=choice, + pixels_per_frame=float(choice.pixels_per_frame), + # Snapping resolves the whole-pixel problem the warning describes. + warning=None if settings.warning and "judder" in settings.warning + else settings.warning, + ) + if hasattr(scroll_helper, "set_frame_based_scrolling"): scroll_helper.set_frame_based_scrolling(False) - scroll_helper.set_scroll_speed(settings.pixels_per_second) - if settings.target_fps and hasattr(scroll_helper, "set_target_fps"): + scroll_helper.set_scroll_speed(applied) + 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"): scroll_helper.set_target_fps(settings.target_fps) - log.info("Scroll configured: %s", settings.describe()) + if choice and display_manager is not None and hasattr(display_manager, "set_frame_hold"): + display_manager.set_frame_hold(choice.frame_hold) + + if choice: + requested = settings.requested_pixels_per_second + if abs(requested - applied) > 0.05: + log.info( + "Scroll configured: %s (asked for %.1f px/s from %s; " + "nearest whole-pixel speed on a %.0fHz panel)", + choice.describe(), requested, settings.source, hz, + ) + else: + log.info("Scroll configured: %s (from %s)", + choice.describe(), settings.source) + if choice.frame_hold > 1 and ( + display_manager is None or not hasattr(display_manager, "set_frame_hold") + ): + log.warning( + "Scroll wants a frame hold of %d but no display manager was " + "given to apply it; motion will use fractional pixels and judder", + choice.frame_hold, + ) + else: + log.info("Scroll configured: %s", settings.describe()) + if settings.warning: log.warning("Scroll speed: %s", settings.warning) return settings diff --git a/src/display_manager.py b/src/display_manager.py index e77fcad3..c54b9772 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -240,6 +240,14 @@ class DisplayManager: self._update_lock = threading.RLock() # Scrolling state tracking for graceful updates + # How many panel refreshes each pushed frame is held for. 1 means a new + # frame every refresh. Higher values are how a scroll runs slower than + # one pixel per refresh WITHOUT fractional pixel positions: the panel + # keeps refreshing at full rate (so flicker is unchanged) but motion + # advances a whole pixel every Nth refresh instead of every one. + # See src/common/scroll_config.py and scripts/scroll_speeds.py. + self._frame_hold = 1 + self._scrolling_state = { 'is_scrolling': False, 'last_scroll_activity': 0, @@ -804,8 +812,10 @@ class DisplayManager: else: self.offscreen_canvas.SetImage(self.image) - # Swap buffers immediately - self.matrix.SwapOnVSync(self.offscreen_canvas) + # 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. + self.matrix.SwapOnVSync(self.offscreen_canvas, self._frame_hold) # Swap our canvas references self.offscreen_canvas, self.current_canvas = self.current_canvas, self.offscreen_canvas @@ -1291,12 +1301,32 @@ class DisplayManager: return dt.strftime(f"%b %-d{suffix}") + def set_frame_hold(self, refreshes: int) -> None: + """Hold each pushed frame for this many panel refreshes (>=1). + + Set by the scroll configuration so a plugin can run at, say, 50px/s on + a 100Hz panel as one whole pixel every second refresh, rather than half + a pixel every refresh (which has to be blended or repeated unevenly). + + Reset to 1 whenever scrolling stops, so one plugin's pacing cannot + leak into the next thing on screen. + """ + try: + value = int(refreshes) + except (TypeError, ValueError): + logger.warning("Ignoring unusable frame hold: %r", refreshes) + return + self._frame_hold = max(1, min(255, value)) + def set_scrolling_state(self, is_scrolling: bool): """Set the current scrolling state. Call this when a display starts/stops scrolling.""" current_time = time.time() self._scrolling_state['is_scrolling'] = is_scrolling if is_scrolling: self._scrolling_state['last_scroll_activity'] = current_time + else: + # Whatever pacing the finished scroll asked for must not carry over. + self._frame_hold = 1 logger.debug(f"Scrolling state set to: {is_scrolling}") def is_currently_scrolling(self) -> bool: diff --git a/test/test_display_dirty_tracking.py b/test/test_display_dirty_tracking.py index e6dfb2b4..0c63d5c3 100644 --- a/test/test_display_dirty_tracking.py +++ b/test/test_display_dirty_tracking.py @@ -47,12 +47,17 @@ class _SwapSpy: def __init__(self, matrix): self.matrix = matrix self.count = 0 + self.last_frame_hold = None self._orig = matrix.SwapOnVSync def __enter__(self): - def counting(canvas): + def counting(canvas, *args): + # *args carries framerate_fraction, which display_manager passes so + # a frame can be held for several refreshes. Signature must match + # the real binding or the spy hides a TypeError as a failed push. self.count += 1 - return self._orig(canvas) + self.last_frame_hold = args[0] if args else 1 + return self._orig(canvas, *args) self.matrix.SwapOnVSync = counting return self @@ -221,3 +226,49 @@ class TestKillSwitch: if __name__ == "__main__": sys.exit(pytest.main([__file__, "-v"])) + + +class TestFrameHold: + """Holding a frame for N refreshes is how a scroll runs slower than one + pixel per refresh without fractional pixel positions.""" + + def test_hold_reaches_swap_on_vsync(self, dm): + dm.set_scrolling_state(True) + dm.set_frame_hold(3) + try: + dm.draw.rectangle([0, 0, 9, 9], fill=(120, 0, 200)) + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + assert spy.count == 1 + assert spy.last_frame_hold == 3 + finally: + dm.set_scrolling_state(False) + + def test_default_is_every_refresh(self, dm): + dm.set_scrolling_state(True) + try: + dm.draw.rectangle([0, 0, 11, 11], fill=(0, 120, 200)) + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + assert spy.last_frame_hold == 1 + finally: + dm.set_scrolling_state(False) + + def test_hold_resets_when_scrolling_stops(self, dm): + """One plugin's pacing must not leak into whatever is on screen next.""" + dm.set_scrolling_state(True) + dm.set_frame_hold(5) + dm.set_scrolling_state(False) + dm.set_scrolling_state(True) + try: + dm.draw.rectangle([0, 0, 13, 13], fill=(200, 120, 0)) + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + assert spy.last_frame_hold == 1 + finally: + dm.set_scrolling_state(False) + + @pytest.mark.parametrize("bad,expected", [(0, 1), (-4, 1), (None, 1), ("x", 1)]) + def test_unusable_holds_are_ignored_or_floored(self, dm, bad, expected): + dm.set_frame_hold(bad) + assert dm._frame_hold == expected diff --git a/test/test_scroll_config.py b/test/test_scroll_config.py index f3f1dcc2..ca8f4614 100644 --- a/test/test_scroll_config.py +++ b/test/test_scroll_config.py @@ -10,6 +10,9 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) from src.common.scroll_config import ( # noqa: E402 DEFAULT_PIXELS_PER_SECOND, + MAX_PIXELS_PER_FRAME, + crisp_ladder, + solve_crisp, MAX_PIXELS_PER_SECOND, MIN_PIXELS_PER_SECOND, ScrollSettings, @@ -40,6 +43,16 @@ class FakeHelper: self.target_fps = fps +class FakeDisplayManager: + """Records the frame hold configure() applies.""" + + def __init__(self): + self.hold = None + + def set_frame_hold(self, refreshes): + self.hold = refreshes + + class MinimalHelper: """An older helper exposing only set_scroll_speed.""" @@ -189,11 +202,60 @@ class TestConfigure: configure(helper, {"display_options": {"scroll_pixels_per_second": 100.0}}) assert helper.speed == 100.0 - def test_logs_the_warning_when_speed_will_judder(self, caplog): - helper = FakeHelper() + def test_snapping_removes_the_judder_warning(self, caplog): + """50px/s cannot be shown in whole pixels at 100Hz *per refresh*, but it + can as 1px every 2nd refresh -- so once snapped there is nothing to warn + about. resolve() alone still warns; configure() resolves it.""" + assert "judder" in resolve( + {"display_options": {"scroll_pixels_per_second": 50.0}}, + refresh_hz=100).warning + helper, dm = FakeHelper(), FakeDisplayManager() with caplog.at_level(logging.WARNING): - configure(helper, {"display_options": {"scroll_pixels_per_second": 50.0}}) - assert any("judder" in r.getMessage() for r in caplog.records) + settings = configure( + helper, {"display_options": {"scroll_pixels_per_second": 50.0}}, + display_manager=dm) + assert settings.warning is None + assert not [r for r in caplog.records if "judder" in r.getMessage()] + + def test_warns_when_a_hold_is_needed_but_cannot_be_applied(self, caplog): + """Without a display manager the hold silently does not happen, and the + motion falls back to fractional pixels. That must not pass quietly.""" + with caplog.at_level(logging.WARNING): + configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 50.0}}) + assert any("frame hold" in r.getMessage() for r in caplog.records) + + def test_applies_the_frame_hold_to_the_display_manager(self): + dm = FakeDisplayManager() + configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 25.0}}, + display_manager=dm) + assert dm.hold == 4, "25px/s at 100Hz is 1px every 4th refresh" + + def test_full_speed_needs_no_hold(self): + dm = FakeDisplayManager() + configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 100.0}}, + display_manager=dm) + assert dm.hold == 1 + + def test_snaps_to_the_nearest_crisp_speed_and_reports_both(self): + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 45.0}}, + display_manager=FakeDisplayManager()) + assert s.requested_pixels_per_second == 45.0 + assert s.pixels_per_second == 50.0 + assert s.crisp is not None and s.crisp.pixels_per_frame == 1 + + def test_snapping_can_be_turned_off(self): + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 45.0}}, + snap_to_crisp=False) + assert s.pixels_per_second == 45.0 + assert s.crisp is None + + def test_helper_target_fps_matches_the_presentation_rate(self): + """At a hold of 4 the panel still refreshes at 100Hz, but frames are + presented at 25/s -- that is the rate the helper should pace to.""" + helper = FakeHelper() + configure(helper, {"display_options": {"scroll_pixels_per_second": 25.0}}, + display_manager=FakeDisplayManager()) + assert helper.target_fps == pytest.approx(25.0) def test_returns_settings_describing_the_source(self): s = configure(FakeHelper(), {"display_options": {"scroll_speed": 2.0, "scroll_delay": 0.01}}) @@ -211,3 +273,80 @@ class TestRefreshFromConfig: "nonsense", {"display": {"hardware": {"limit_refresh_rate_hz": None}}}]) def test_falls_back_to_the_default(self, cfg): assert refresh_hz_from_config(cfg) == 100.0 + + +class TestCrispLadder: + """Whole-pixel speeds available on a given panel.""" + + def test_every_entry_is_exactly_reachable(self): + for c in crisp_ladder(100): + assert c.pixels_per_second == pytest.approx( + c.refresh_hz / c.frame_hold * c.pixels_per_frame) + + def test_sorted_slowest_first(self): + speeds = [c.pixels_per_second for c in crisp_ladder(100)] + assert speeds == sorted(speeds) + + def test_no_duplicate_speeds(self): + speeds = [round(c.pixels_per_second, 3) for c in crisp_ladder(100)] + assert len(speeds) == len(set(speeds)) + + def test_duplicates_resolve_to_the_smaller_step(self): + """100px/s is 1px every refresh or 2px every 2nd; prefer the former.""" + entry = next(c for c in crisp_ladder(100) + if c.pixels_per_second == pytest.approx(100.0)) + assert entry.pixels_per_frame == 1 + assert entry.frame_hold == 1 + + def test_ladder_scales_with_the_panel(self): + assert any(c.pixels_per_second == pytest.approx(60.0) + for c in crisp_ladder(60)) + assert any(c.pixels_per_second == pytest.approx(30.0) + for c in crisp_ladder(60)) + + def test_full_refresh_speed_is_present(self): + for hz in (60, 75, 100, 120): + assert any(c.pixels_per_second == pytest.approx(float(hz)) + for c in crisp_ladder(hz)) + + +class TestSolveCrisp: + def test_exact_targets_are_matched_exactly(self): + for target in (100, 50, 25, 20): + assert solve_crisp(target, 100).pixels_per_second == pytest.approx(target) + + def test_prefers_smooth_motion_over_raw_proximity(self): + """30 -> 33.3 (1px, 33fps), not 28.6 (2px at 14fps) which is nearer.""" + got = solve_crisp(30, 100) + assert got.pixels_per_second == pytest.approx(33.333, abs=0.01) + assert got.pixels_per_frame == 1 + + def test_does_not_chase_a_jumpy_exact_match(self): + """45 -> 50 (1px, smooth) beats 42.9 (3px at 14fps).""" + assert solve_crisp(45, 100).pixels_per_frame == 1 + + def test_allows_a_two_pixel_step_when_it_is_clearly_closer(self): + """60 -> 66.7 (2px at 33fps) rather than 50 (1px) which is 17% slow.""" + got = solve_crisp(60, 100) + assert got.pixels_per_second == pytest.approx(66.667, abs=0.01) + assert got.pixels_per_frame == 2 + + def test_result_is_always_on_the_ladder(self): + ladder = {round(c.pixels_per_second, 3) for c in crisp_ladder(100)} + for target in range(5, 205, 5): + assert round(solve_crisp(target, 100).pixels_per_second, 3) in ladder + + def test_adapts_to_a_slower_panel(self): + got = solve_crisp(30, 60) + assert got.pixels_per_second == pytest.approx(30.0) + assert got.pixels_per_frame == 1 + assert got.frame_hold == 2 + + def test_speeds_beyond_the_panel_clamp_to_the_fastest_available(self): + got = solve_crisp(10_000, 100) + assert got.frame_hold == 1 + assert got.pixels_per_frame == MAX_PIXELS_PER_FRAME + + def test_steppiness_labels_are_sane(self): + assert solve_crisp(100, 100).steppiness == "smooth" + assert crisp_ladder(100)[0].steppiness == "stepped"