From 7b4b07e3138e10b72b18767e61afe75ef29bdbdb Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:45:45 -0400 Subject: [PATCH] perf(scroll): pace frames to the panel, not to a fixed sleep Scrolling ran at 44-46 fps on a 2x128x64 chain and 14-17% of frames took 41-53ms, which reads as judder. Four independent causes, each measured on the hardware; details and the diagnostic recipe are in docs/SCROLL_PERFORMANCE.md. The high-FPS loop slept a flat 8ms after every render. display() has already blocked on the panel's vsync by then, so that sleep was added to a wait that had happened: ~4ms of render plus 8ms put each iteration at ~12ms against a 10ms refresh grid, so every swap missed a refresh and the loop settled at 50fps while asking for 125 -- with no headroom, so a further 14% of frames slipped again. It now sleeps only the remainder, with a 1ms floor so plugin threads still get the GIL. ScrollHelper stepped position on a wall clock at 1/scroll_delay steps per second. Plugins set scroll_delay to the frame period, so that comparison sat exactly on its own threshold: a frame arriving a hair early moved zero pixels and rendered an identical frame, dirty-tracking skipped the swap, it returned in ~2ms, and the beat repeated. No scroll_delay value tunes that out -- a shorter delay trades stalled frames for periodic double-steps. Both modes now accumulate elapsed time at the same configured speed, so position stays proportional to real time. Sub-pixel blending goes back to off by default. It renders a half-step by mixing two adjacent columns, which on a coarse panel showing pixel-font text alternates crisp and smeared frames and reads as shimmer -- visibly worse than integer stepping on the hardware. Vegas mode still opts in. disk_cache uses orjson when importable, falling back to the stdlib. Encoding a ~1MB record drops from 14.8ms to 5.4ms end-to-end, and that work holds the GIL while a marquee is on screen. display_manager also checksummed the whole framebuffer twice per frame (dirty tracking, then the preview snapshot); the snapshot now takes the checksum the caller already computed. New src/common/scroll_config.py resolves scroll settings in one place. Five ticker plugins each hand-rolled this and disagreed: odds-ticker ranked the deprecated scroll_pixels_per_second above the documented scroll_speed/delay pair, and because that key carries a schema default the documented settings were dead for every user (ChuckBuilds/ledmatrix-plugins#408), while ledmatrix-leaderboard read the same key only as a fallback. The resolver also warns when a speed will not advance a whole number of pixels per refresh, which is the property that actually determines whether a scroll looks smooth. scripts/build_rgbmatrix_nogil.sh rebuilds the rgbmatrix binding so it releases the GIL. Upstream declares SwapOnVSync without nogil, unlike SetPixel/Clear/Fill beside it, so the render thread held the GIL for the whole vsync wait and starved background threads into long uninterruptible bursts. The script patches, builds and self-verifies into a scratch tree; --install backs up the original and rolls back if the service does not come back healthy. Measured after: 100 fps locked, no stalls observed, render thread down from 51% to 19% of one core. Co-Authored-By: Claude Opus 5 --- docs/SCROLL_PERFORMANCE.md | 188 ++++++++++++++++++++++++ requirements.txt | 8 + scripts/build_rgbmatrix_nogil.sh | 242 +++++++++++++++++++++++++++++++ src/cache/disk_cache.py | 60 +++++++- src/common/__init__.py | 12 ++ src/common/scroll_config.py | 224 ++++++++++++++++++++++++++++ src/common/scroll_helper.py | 51 +++++-- src/display_controller.py | 18 ++- src/display_manager.py | 24 ++- test/test_scroll_config.py | 213 +++++++++++++++++++++++++++ 10 files changed, 1011 insertions(+), 29 deletions(-) create mode 100644 docs/SCROLL_PERFORMANCE.md create mode 100644 scripts/build_rgbmatrix_nogil.sh create mode 100644 src/common/scroll_config.py create mode 100644 test/test_scroll_config.py diff --git a/docs/SCROLL_PERFORMANCE.md b/docs/SCROLL_PERFORMANCE.md new file mode 100644 index 00000000..479c325b --- /dev/null +++ b/docs/SCROLL_PERFORMANCE.md @@ -0,0 +1,188 @@ +# Scroll Performance + +How scrolling is paced on this hardware, what was wrong with it, and how to +configure a plugin so its marquee is smooth. + +Measured on a Raspberry Pi 4 driving a 2×128×64 chain (256×64 logical) at +`limit_refresh_rate_hz: 100`. Numbers below come from that panel. + +| | before | after | +|---|---|---| +| scroll frame rate | 44–46 fps | **100 fps, locked** | +| frames ≥ 45 ms | 14–17% | none observed | +| dominant frame time | 20 ms | **10 ms** | +| disk cache write (~1 MB) | 14.8 ms | **5.4 ms** | + +--- + +## The one rule that matters + +**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: + +- **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 + text jumping a pixel ahead of itself; +- **repeat** a frame — the strip stands still, then jumps, which reads as + judder. + +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. + +## Configuring a plugin + +Use the shared resolver rather than reading config keys yourself: + +```python +from src.common import scroll_config + +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), + plugin_logger=self.logger, +) +``` + +It resolves every config shape in one place, applies the speed, and returns +what it did. Precedence, highest first: + +1. `display_options.scroll_speed` + `scroll_delay` — **the recommended form** +2. `display.scroll_speed` + `scroll_delay` — deprecated shape +3. `scroll_speed` + `scroll_delay` at the root — legacy flat +4. `scroll_pixels_per_second` — deprecated +5. the global `display` block +6. the built-in default (100 px/s) + +`scroll_speed` is pixels per frame and `scroll_delay` is the frame period in +seconds, so the pair means `scroll_speed / scroll_delay` px/s. The recommended +config for a 100 Hz panel: + +```json +"display_options": { "scroll_speed": 1.0, "scroll_delay": 0.01 } +``` + +### Why the deprecated key ranks below the explicit pair + +Because some plugins give `scroll_pixels_per_second` a **schema default**, and +schema defaults are merged into plugin config. Ranking it above the pair means +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). + +If you are writing a plugin: do not give a deprecated key a schema default. + +## What was actually wrong + +Four independent faults, each found by measurement. + +### 1. The frame loop slept on top of a wait it had already done + +`display_controller.py` ran the high-FPS loop as `render → SwapOnVSync (blocks +to the panel's refresh) → time.sleep(0.008) → plugin ticks`. The sleep was +unconditional and added to a wait that had already happened. Render work +measured ~4 ms, so each iteration cost ~12 ms against a 10 ms refresh grid — +every swap missed a refresh and landed on the next one. The loop settled at +exactly 50 fps while asking for 125, with no headroom, so ~14% of frames +slipped a further refresh. + +Now the loop sleeps only the remainder of the frame budget, with a 1 ms floor +so plugin threads still get the GIL. + +### 2. `SwapOnVSync` held the GIL while blocking + +The rgbmatrix binding declares it without `nogil` (unlike `SetPixel`, `Clear` +and `Fill` immediately above it in `cppinc.pxd`), so the render thread held the +GIL for the entire vsync wait — most of every frame. Background threads were +starved into long uninterruptible bursts; a 1.5 MB API response costs ~17 ms to +parse and ~18 ms to re-encode for the cache, and `json.raw_decode` cannot be +preempted mid-document. Those bursts are what the render loop then waited on. + +Fixed by rebuilding the binding: `scripts/build_rgbmatrix_nogil.sh`. + +### 3. Sub-pixel blending was wrong for this display + +Enabling it made things worse, not better — see the rule at the top. It is off +by default and only Vegas mode opts in via `set_sub_pixel_scrolling(True)`. + +### 4. Frame-based stepping raced the vsync clock + +Frame-based mode gated 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: a frame arriving a hair early moved +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. + +## Diagnosing a juddery scroller + +**`Avg FPS` will lie to you.** It is a 100-frame moving average, and a 2 ms +duplicate frame plus a 21 ms double-wait average to exactly 10 ms. A ticker +that is stalling on half its frames still reports a healthy `100.0`. + +Look at the **distribution** instead: + +```bash +journalctl -u ledmatrix --since "-10min" --no-pager \ + | grep -oE "Frame time: [0-9.]+ms" | awk '{print $3}' | sed 's/ms//' \ + | awk '{printf "%.0f\n", $1}' | sort -n | uniq -c +``` + +Reading it, on a 100 Hz panel: + +| you see | it means | +|---|---| +| everything at 10 ms | healthy | +| a mode at ~2 ms | **duplicate frames** — the swap was skipped because the image did not change. The scroller is advancing less than one pixel per frame. | +| a mode at 20/30/50 ms | frames missing refreshes — per-frame work is overrunning, or a background thread is holding the GIL | +| `Avg FPS` above 100 | duplicates present, unless the scroll cycle has completed and is idling | + +Then confirm what the plugin actually loaded — config edits do not always reach +the running code: + +```bash +journalctl -u ledmatrix --since "-5min" --no-pager | grep -iE "px/s|px/frame" +``` + +If a plugin logs its scroll config **twice** with different modes, the second +line is what is running. + +## Rebuilding the binding + +```bash +bash scripts/build_rgbmatrix_nogil.sh # build into a scratch dir +sudo bash scripts/build_rgbmatrix_nogil.sh --install +sudo bash scripts/build_rgbmatrix_nogil.sh --rollback +``` + +The build never touches the installed module. `--install` backs up the original +to `~/rgbmatrix-core.so.ORIGINAL` first, and rolls back automatically if the +service does not come back healthy. Requires `build-essential`; Cython is +installed into a cached venv under `~/.cache/ledmatrix-cython`. + +Re-run it after upgrading `rpi-rgb-led-matrix`, since a library upgrade +replaces the patched binding. + +## Faster JSON + +`src/cache/disk_cache.py` uses `orjson` when it is importable and falls back to +the stdlib otherwise, so it is optional: + +```bash +sudo pip3 install --break-system-packages orjson +``` + +Encoding is where it pays — about 7× on this hardware. Decoding gains far less +(~1.3× on large payloads) because the cost there is building Python objects, +not scanning text. That is also why moving parsing to a subprocess does not +help: `pickle.loads` of the same payload costs 8.1 ms against `json.loads` at +10.9 ms, so the work just moves rather than disappearing. diff --git a/requirements.txt b/requirements.txt index 760dc596..2d0350bf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -55,6 +55,14 @@ packaging>=23.0,<27.0 # range as a hard dependency — keep the two in sync. # pip install 'psutil>=6.0.0,<7.0.0' # +# orjson — faster JSON for the disk cache +# (src/cache/disk_cache.py). Encoding a ~1MB cache +# record drops from ~12ms to ~1.6ms on a Pi 4, which +# matters because that work holds the GIL and stalls +# the render thread mid-scroll. Falls back to the +# stdlib json when missing — see docs/SCROLL_PERFORMANCE.md. +# pip install 'orjson>=3.9,<4.0' +# # Flask-Limiter — request rate limiting in web_interface/app.py # (accidental-abuse protection, not security). The # web interface starts without rate limiting when diff --git a/scripts/build_rgbmatrix_nogil.sh b/scripts/build_rgbmatrix_nogil.sh new file mode 100644 index 00000000..959a6c4d --- /dev/null +++ b/scripts/build_rgbmatrix_nogil.sh @@ -0,0 +1,242 @@ +#!/usr/bin/env bash +# +# Rebuild the rgbmatrix Python binding so it releases the GIL. +# +# WHY THIS EXISTS +# --------------- +# The upstream binding declares FrameCanvas::SwapOnVSync WITHOUT `nogil` +# (cppinc.pxd), unlike SetPixel/Clear/Fill on the lines just above it. +# SwapOnVSync blocks until the panel's next vertical sync -- up to a full +# refresh period on every frame -- so the render thread was holding the GIL +# for most of every frame. Background threads (API fetches, JSON parsing, +# image decode) were starved into long uninterruptible bursts, which in turn +# made the render loop miss refreshes. +# +# Measured on a Pi 4 driving a 2x128x64 chain at limit_refresh_rate_hz=100: +# +# before ~44 fps average, 14-17% of frames 41-53ms +# after 100 fps locked, no stalls observed +# +# This script also releases the GIL across the per-pixel blit +# (SetPixelsPillow) and walks the Pillow buffer row-major instead of +# column-major so each row is contiguous. +# +# SAFETY +# ------ +# Builds into a scratch directory; touches the installed module only in the +# --install step, and backs up the original first. Roll back at any time with: +# +# sudo bash scripts/build_rgbmatrix_nogil.sh --rollback +# +# USAGE +# bash scripts/build_rgbmatrix_nogil.sh # build only +# sudo bash scripts/build_rgbmatrix_nogil.sh --install +# sudo bash scripts/build_rgbmatrix_nogil.sh --rollback +# +set -uo pipefail + +SRC_TREE="${RGB_SRC_TREE:-$HOME/LEDMatrix/rpi-rgb-led-matrix-master}" +BUILD_DIR="${RGB_BUILD_DIR:-$HOME/rgbmatrix-nogil-build}" +VENV="${RGB_CYTHON_VENV:-$HOME/.cache/ledmatrix-cython}" +BACKUP="${RGB_BACKUP:-$HOME/rgbmatrix-core.so.ORIGINAL}" + +die() { echo "FATAL: $*" >&2; exit 1; } + +py_site() { + python3 -c 'import rgbmatrix, os; print(os.path.dirname(rgbmatrix.__file__))' 2>/dev/null +} + +abi_so() { + ls "$BUILD_DIR"/bindings/python/rgbmatrix/core.cpython-*.so 2>/dev/null | head -1 +} + +do_rollback() { + local dst; dst="$(py_site)" || die "rgbmatrix not importable" + [ -n "$dst" ] || die "could not locate the installed rgbmatrix package" + [ -f "$BACKUP" ] || die "no backup at $BACKUP" + systemctl stop ledmatrix 2>/dev/null + cp -a "$BACKUP" "$dst/core.so" || die "restore failed" + find "$dst" -name __pycache__ -type d -exec rm -rf {} + 2>/dev/null + systemctl start ledmatrix 2>/dev/null + echo "rolled back to the original core.so" + exit 0 +} + +do_install() { + local so dst + so="$(abi_so)"; [ -n "$so" ] || die "no built module found - run the build first" + dst="$(py_site)"; [ -n "$dst" ] || die "could not locate the installed rgbmatrix package" + + if [ ! -f "$BACKUP" ]; then + cp -a "$dst/core.so" "$BACKUP" || die "could not back up the original" + echo "backed up original core.so -> $BACKUP" + else + echo "backup already present at $BACKUP (keeping the true original)" + fi + + systemctl stop ledmatrix 2>/dev/null + cp "$so" "$dst/core.so" || die "install failed" + find "$dst" -name __pycache__ -type d -exec rm -rf {} + 2>/dev/null + systemctl start ledmatrix 2>/dev/null + + echo "waiting 25s for the display to come back..." + sleep 25 + local healthy=1 + systemctl is-active --quiet ledmatrix || healthy=0 + if journalctl -u ledmatrix --since "40 sec ago" --no-pager \ + | grep -qiE "Traceback|ImportError|Segmentation fault|undefined symbol"; then + healthy=0 + fi + if [ "$healthy" = "1" ]; then + echo "SUCCESS - running on the rebuilt binding" + else + echo "UNHEALTHY - rolling back" + cp -a "$BACKUP" "$dst/core.so" + systemctl restart ledmatrix + journalctl -u ledmatrix --since "90 sec ago" --no-pager | tail -25 + exit 1 + fi + exit 0 +} + +case "${1:-}" in + --rollback) do_rollback ;; + --install) do_install ;; + "" ) ;; + *) die "unknown option: $1" ;; +esac + +# ---------------------------------------------------------------- build ---- +[ -d "$SRC_TREE" ] || die "matrix source tree not found at $SRC_TREE (set RGB_SRC_TREE)" +command -v g++ >/dev/null || die "g++ not installed (apt install build-essential)" + +echo "==> staging a scratch copy at $BUILD_DIR" +rm -rf "$BUILD_DIR" +cp -r "$SRC_TREE" "$BUILD_DIR" || die "copy failed" + +echo "==> patching the bindings to release the GIL" +python3 - "$BUILD_DIR" <<'PYEOF' || die "patch failed" +import io, sys +base = sys.argv[1] + "/bindings/python/rgbmatrix/" + +p = base + "cppinc.pxd" +s = io.open(p, encoding="utf-8").read() +old = " FrameCanvas *SwapOnVSync(FrameCanvas*, uint8_t)\n" +new = " FrameCanvas *SwapOnVSync(FrameCanvas*, uint8_t) nogil\n" +if old in s: + io.open(p, "w", encoding="utf-8", newline="\n").write(s.replace(old, new, 1)) + print(" cppinc.pxd: SwapOnVSync declared nogil") +elif new in s: + print(" cppinc.pxd: already nogil") +else: + sys.exit("could not find the SwapOnVSync declaration") + +p = base + "core.pyx" +s = io.open(p, encoding="utf-8").read() + +old = """ def SwapOnVSync(self, FrameCanvas newFrame, uint8_t framerate_fraction = 1): + return __createFrameCanvas(self.__matrix.SwapOnVSync(newFrame.__canvas, framerate_fraction)) +""" +new = """ def SwapOnVSync(self, FrameCanvas newFrame, uint8_t framerate_fraction = 1): + # Blocks until the panel's next vertical sync. Holding the GIL across + # that wait starves every other Python thread for most of each frame. + cdef cppinc.RGBMatrix* matrix = self.__matrix + cdef cppinc.FrameCanvas* frame = newFrame.__canvas + cdef uint8_t fraction = framerate_fraction + cdef cppinc.FrameCanvas* swapped + with nogil: + swapped = matrix.SwapOnVSync(frame, fraction) + return __createFrameCanvas(swapped) +""" +if old in s: + s = s.replace(old, new, 1) + print(" core.pyx: SwapOnVSync releases the GIL") +elif "with nogil:\n swapped = matrix.SwapOnVSync" in s: + print(" core.pyx: SwapOnVSync already patched") +else: + sys.exit("could not find the SwapOnVSync body") + +old = """ buffer = get_pillow_buffer(image_capsule) + + for col in range(max(0, -xstart), min(width, frame_width - xstart)): + for row in range(max(0, -ystart), min(height, frame_height - ystart)): + pixel = buffer[row][col] + r = (pixel ) & 0xFF + g = (pixel >> 8) & 0xFF + b = (pixel >> 16) & 0xFF + my_canvas.SetPixel(xstart+col, ystart+row, r, g, b) +""" +new = """ buffer = get_pillow_buffer(image_capsule) + + # Bounds hoisted so the blit needs no Python state and can run without + # the GIL: it touches only a C buffer and a C++ canvas. Row-major order + # walks each row contiguously; col-outer re-strided the whole buffer. + cdef int col_start = max(0, -xstart) + cdef int col_end = min(width, frame_width - xstart) + cdef int row_start = max(0, -ystart) + cdef int row_end = min(height, frame_height - ystart) + + with nogil: + for row in range(row_start, row_end): + for col in range(col_start, col_end): + pixel = buffer[row][col] + r = (pixel ) & 0xFF + g = (pixel >> 8) & 0xFF + b = (pixel >> 16) & 0xFF + my_canvas.SetPixel(xstart+col, ystart+row, r, g, b) +""" +if old in s: + s = s.replace(old, new, 1) + print(" core.pyx: pixel blit releases the GIL, row-major") +elif "with nogil:\n for row in range(row_start, row_end):" in s: + print(" core.pyx: blit already patched") +else: + sys.exit("could not find the SetPixelsPillow loop") + +io.open(p, "w", encoding="utf-8", newline="\n").write(s) +PYEOF + +echo "==> building librgbmatrix.a (this takes a few minutes)" +nice -n 10 make -C "$BUILD_DIR/lib" -j2 >/dev/null 2>&1 \ + || die "library build failed - rerun 'make -C $BUILD_DIR/lib' to see why" +[ -f "$BUILD_DIR/lib/librgbmatrix.a" ] || die "librgbmatrix.a was not produced" + +echo "==> preparing Cython" +[ -d "$VENV" ] || python3 -m venv --system-site-packages "$VENV" || die "venv failed" +"$VENV/bin/pip" install --quiet cython || die "cython install failed" + +cat > "$BUILD_DIR/bindings/python/setup.py" <<'EOF' +from setuptools import setup, Extension +from Cython.Build import cythonize + +core = Extension( + "rgbmatrix.core", + sources=["rgbmatrix/core.pyx", "rgbmatrix/shims/pillow.c"], + include_dirs=["../../include", "rgbmatrix/shims"], + extra_objects=["../../lib/librgbmatrix.a"], + language="c++", + extra_compile_args=["-O3", "-Wall", "-fno-exceptions", "-std=c++11"], + extra_link_args=["-lrt", "-lm", "-lpthread"], +) + +setup(name="rgbmatrix", + ext_modules=cythonize([core], language_level="3str", + compiler_directives={"binding": False})) +EOF + +echo "==> compiling the extension" +( cd "$BUILD_DIR/bindings/python" && "$VENV/bin/python" setup.py build_ext --inplace ) \ + >/dev/null 2>&1 || die "extension build failed" + +SO="$(abi_so)"; [ -n "$SO" ] || die "no .so produced" + +# Verify the GIL really is released before anyone installs this. +PAIRS=$(grep -c "PyEval_SaveThread\|Py_UNBLOCK_THREADS" "$BUILD_DIR/bindings/python/rgbmatrix/core.cpp") +[ "$PAIRS" -ge 2 ] || die "generated C++ has only $PAIRS GIL releases, expected >= 2" + +echo +echo "BUILT: $SO" +echo " ($PAIRS GIL-release sites in the generated C++)" +echo +echo "Install with: sudo bash $0 --install" +echo "Roll back with: sudo bash $0 --rollback" diff --git a/src/cache/disk_cache.py b/src/cache/disk_cache.py index 03cfb2f1..30ffd1ad 100644 --- a/src/cache/disk_cache.py +++ b/src/cache/disk_cache.py @@ -14,6 +14,11 @@ import zlib from typing import Dict, Any, Optional, Protocol from datetime import datetime +try: # optional: large speedup on the cache write path, see _dumps below + import orjson +except ImportError: # pragma: no cover - exercised on hosts without the wheel + orjson = None + # How old an abandoned write's temp file must be before the sweep removes it. # A real write holds its temp file for milliseconds, so an hour is far beyond # any in-flight write while still clearing the same day's debris. Deliberately @@ -40,13 +45,52 @@ class CacheStrategyProtocol(Protocol): class DateTimeEncoder(json.JSONEncoder): - """JSON encoder that handles datetime objects.""" + """JSON encoder that handles datetime objects. + + Retained for the stdlib fallback path and for any caller importing it. + """ def default(self, obj: Any) -> Any: if isinstance(obj, datetime): return obj.isoformat() return super().default(obj) +def _datetime_default(obj: Any) -> Any: + """Serialise datetimes exactly as DateTimeEncoder did.""" + if isinstance(obj, datetime): + return obj.isoformat() + raise TypeError(f"Object of type {type(obj).__name__} is not JSON serializable") + + +if orjson is not None: + # Encoding the cache record dominated the background fetch worker: on a + # Pi 4, stdlib json.dumps runs ~12ms per MB and holds the GIL for all of + # it, which stalls the render thread mid-scroll. orjson measures ~7x + # faster on the same payloads (11.9ms -> 1.6ms for 985KB). Decoding gains + # far less (~1.3x on large payloads) because the cost there is building + # the Python objects, not scanning the text, but it is still free to take. + # + # OPT_NON_STR_KEYS: stdlib json coerces int/float dict keys to strings; + # orjson raises without this, and cache records do carry numeric keys. + # OPT_PASSTHROUGH_DATETIME: orjson would otherwise emit its own RFC 3339 + # form for datetimes instead of calling default(). Routing them through + # _datetime_default keeps byte-for-byte parity with the records already + # on disk. + _DUMPS_OPTS = orjson.OPT_NON_STR_KEYS | orjson.OPT_PASSTHROUGH_DATETIME + + def _dumps(data: Any) -> bytes: + return orjson.dumps(data, default=_datetime_default, option=_DUMPS_OPTS) + + def _loads(raw: bytes) -> Any: + return orjson.loads(raw) +else: + def _dumps(data: Any) -> bytes: + return json.dumps(data, cls=DateTimeEncoder).encode("utf-8") + + def _loads(raw: bytes) -> Any: + return json.loads(raw) + + class DiskCache: """Manages persistent disk-based cache.""" @@ -99,8 +143,8 @@ class DiskCache: try: with self._lock: - with open(cache_path, 'r', encoding='utf-8') as f: - record = json.load(f) + with open(cache_path, 'rb') as f: + record = _loads(f.read()) # Determine record timestamp (prefer embedded, else file mtime) record_ts = None @@ -189,12 +233,12 @@ class DiskCache: # write path below, and cache files are machine-read only — indenting # them just multiplied the bytes written to the SD card. try: - payload = json.dumps(data, cls=DateTimeEncoder) + payload = _dumps(data) except (TypeError, ValueError) as e: self.logger.warning("Cache data for key '%s' not serializable: %s", key, e) return - digest = zlib.adler32(payload.encode('utf-8')) + digest = zlib.adler32(payload) try: # Atomic write to avoid partial/corrupt files @@ -242,7 +286,7 @@ class DiskCache: # wear source (dozens of fsyncs/min on API-heavy # installs) for data that can be re-downloaded. try: - with os.fdopen(fd, 'w', encoding='utf-8') as tmp_file: + with os.fdopen(fd, 'wb') as tmp_file: tmp_file.write(payload) os.replace(tmp_path, cache_path) self._write_digests[key] = digest @@ -260,7 +304,7 @@ class DiskCache: else: # Fallback: direct write (not atomic, but better than failing) try: - with open(cache_path, 'w', encoding='utf-8') as cache_file: + with open(cache_path, 'wb') as cache_file: cache_file.write(payload) self._write_digests[key] = digest # Set proper permissions: 660 (rw-rw----) for group-readable cache files @@ -290,7 +334,7 @@ class DiskCache: # is a different path, so future sets must keep # retrying the primary location. fallback_path = os.path.join(fallback_dir, os.path.basename(cache_path)) - with open(fallback_path, 'w', encoding='utf-8') as tmp_file: + with open(fallback_path, 'wb') as tmp_file: tmp_file.write(payload) # Set proper permissions: 660 (rw-rw----) for group-readable cache files try: diff --git a/src/common/__init__.py b/src/common/__init__.py index 4b6cb383..03588175 100644 --- a/src/common/__init__.py +++ b/src/common/__init__.py @@ -23,6 +23,13 @@ from src.common.error_handler import ( ) from src.common.api_helper import APIHelper from src.common.scroll_helper import ScrollHelper +from src.common import scroll_config +from src.common.scroll_config import ( + ScrollSettings, + configure as configure_scroll, + resolve as resolve_scroll_settings, + refresh_hz_from_config, +) from src.common.logo_helper import LogoHelper from src.common.text_helper import TextHelper @@ -60,6 +67,11 @@ __all__ = [ 'log_and_raise', 'APIHelper', 'ScrollHelper', + 'scroll_config', + 'ScrollSettings', + 'configure_scroll', + 'resolve_scroll_settings', + 'refresh_hz_from_config', 'LogoHelper', 'TextHelper', # adaptive layout & images diff --git a/src/common/scroll_config.py b/src/common/scroll_config.py new file mode 100644 index 00000000..9bf38a0d --- /dev/null +++ b/src/common/scroll_config.py @@ -0,0 +1,224 @@ +"""One place that turns plugin config into a configured ScrollHelper. + +Five ticker plugins each hand-rolled this resolution (odds-ticker, news and +ledmatrix-leaderboard reference the deprecated ``scroll_pixels_per_second`` +key 16-18 times apiece), and they disagreed in ways that were invisible until +someone watched the panel: + +* odds-ticker read ``scroll_pixels_per_second`` on the *recommended* config + path and let it override ``scroll_speed``/``scroll_delay``. Because that key + carries a schema default, the documented settings were dead for every user + -- see ChuckBuilds/ledmatrix-plugins#408. +* ledmatrix-leaderboard read the same key only as a fallback, so identical + config produced different speeds in the two plugins. +* stock-news derived px/frame from it via its own arithmetic. + +What matters on the hardware +---------------------------- +Motion is smooth when the strip advances a **whole number of pixels per panel +refresh**. On a 100Hz panel that means 100 px/s, 200 px/s, and so on. Anything +else has to either blend adjacent columns (which on pixel-font text reads as +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. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from typing import Any, Dict, Optional + +logger = logging.getLogger(__name__) + +#: Speed used when a plugin supplies nothing usable. One pixel per refresh on a +#: 100Hz panel, which is the slowest crisp scroll that hardware can show. +DEFAULT_PIXELS_PER_SECOND = 100.0 + +#: Bounds accepted from config. Below the floor a marquee appears frozen; +#: above the ceiling it outruns any panel's refresh and tears. +MIN_PIXELS_PER_SECOND = 1.0 +MAX_PIXELS_PER_SECOND = 500.0 + +#: Assumed refresh when the caller does not say. Matches the usual +#: ``display.hardware.limit_refresh_rate_hz``. +DEFAULT_REFRESH_HZ = 100.0 + +#: How far px/s may sit from a whole number of pixels per refresh before it is +#: worth warning about. 0.05px per frame is invisible; a third of a pixel is not. +_WHOLE_PIXEL_TOLERANCE = 0.05 + + +@dataclass(frozen=True) +class ScrollSettings: + """The resolved outcome, and which config key produced it.""" + + pixels_per_second: float + source: str + target_fps: Optional[float] = None + pixels_per_frame: Optional[float] = None + warning: Optional[str] = None + + def describe(self) -> str: + text = f"{self.pixels_per_second:.1f} px/s (from {self.source})" + if self.pixels_per_frame is not None: + text += f" = {self.pixels_per_frame:.2f} px/frame" + if self.target_fps: + text += f" at {self.target_fps:.0f} fps" + return text + + +def _coerce(value: Any) -> Optional[float]: + """A positive float, or None. Config reaches us with nulls and strings.""" + if value is None or isinstance(value, bool): + return None + try: + number = float(value) + except (TypeError, ValueError): + return None + return number if number > 0 else None + + +def _from_speed_and_delay(block: Any) -> Optional[float]: + """px/s from a ``scroll_speed`` (px/frame) + ``scroll_delay`` (s) pair.""" + if not isinstance(block, dict): + return None + speed = _coerce(block.get("scroll_speed")) + delay = _coerce(block.get("scroll_delay")) + if speed is None or delay is None: + return None + return speed / delay + + +def resolve( + plugin_config: Optional[Dict[str, Any]] = None, + global_config: Optional[Dict[str, Any]] = None, + default_pixels_per_second: float = DEFAULT_PIXELS_PER_SECOND, + refresh_hz: Optional[float] = None, +) -> ScrollSettings: + """Resolve one scroll speed from the several shapes plugins accept. + + Precedence, highest first. The deprecated flat key sits *below* the + explicit pairs deliberately: it carries schema defaults in some plugins, so + ranking it above them silently disables the documented settings. + + 1. ``display_options.scroll_speed`` + ``scroll_delay`` (current) + 2. ``display.scroll_speed`` + ``scroll_delay`` (deprecated shape) + 3. ``scroll_speed`` + ``scroll_delay`` at the root (legacy flat) + 4. ``scroll_pixels_per_second``, nested or flat (deprecated) + 5. the global ``display`` block + 6. ``default_pixels_per_second`` + + :param refresh_hz: panel refresh, used only to check whether the resolved + speed lands on whole pixels per frame and to fill in ``target_fps``. + """ + plugin_config = plugin_config or {} + global_config = global_config or {} + refresh = _coerce(refresh_hz) or DEFAULT_REFRESH_HZ + + display_options = plugin_config.get("display_options") + display_block = plugin_config.get("display") + + candidates = [ + (_from_speed_and_delay(display_options), "display_options.scroll_speed/delay"), + (_from_speed_and_delay(display_block), "display.scroll_speed/delay"), + (_from_speed_and_delay(plugin_config), "scroll_speed/delay (root)"), + ] + for block, label in ( + (display_options, "display_options.scroll_pixels_per_second"), + (display_block, "display.scroll_pixels_per_second"), + (plugin_config, "scroll_pixels_per_second"), + ): + if isinstance(block, dict): + candidates.append((_coerce(block.get("scroll_pixels_per_second")), label)) + + global_display = global_config.get("display") + candidates.append((_from_speed_and_delay(global_display), "global display.scroll_speed/delay")) + + pixels_per_second = None + source = "default" + for value, label in candidates: + if value is not None: + pixels_per_second, source = value, label + break + if pixels_per_second is None: + pixels_per_second = default_pixels_per_second + + clamped = max(MIN_PIXELS_PER_SECOND, min(MAX_PIXELS_PER_SECOND, pixels_per_second)) + warning = None + if clamped != pixels_per_second: + warning = ( + f"scroll speed {pixels_per_second:.1f} px/s out of range, " + f"clamped to {clamped:.1f}" + ) + pixels_per_second = clamped + + pixels_per_frame = pixels_per_second / refresh if refresh > 0 else None + if warning is None and pixels_per_frame is not None: + offset = abs(pixels_per_frame - round(pixels_per_frame)) + if pixels_per_frame < 1.0 - _WHOLE_PIXEL_TOLERANCE or offset > _WHOLE_PIXEL_TOLERANCE: + suggestion = max(1.0, round(pixels_per_frame)) * refresh + warning = ( + f"{pixels_per_second:.1f} px/s is {pixels_per_frame:.2f} px per " + f"refresh at {refresh:.0f}Hz, so some frames repeat and the " + f"scroll will judder; {suggestion:.0f} px/s divides evenly" + ) + + return ScrollSettings( + pixels_per_second=pixels_per_second, + source=source, + target_fps=refresh, + pixels_per_frame=pixels_per_frame, + warning=warning, + ) + + +def configure( + scroll_helper: Any, + plugin_config: Optional[Dict[str, Any]] = None, + global_config: Optional[Dict[str, Any]] = None, + default_pixels_per_second: float = DEFAULT_PIXELS_PER_SECOND, + refresh_hz: Optional[float] = None, + plugin_logger: Optional[logging.Logger] = None, +) -> 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. + + :returns: the settings applied, so the caller can log or assert on them. + """ + log = plugin_logger or logger + settings = resolve( + plugin_config, + global_config, + default_pixels_per_second=default_pixels_per_second, + refresh_hz=refresh_hz, + ) + + 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_target_fps(settings.target_fps) + + log.info("Scroll configured: %s", settings.describe()) + if settings.warning: + log.warning("Scroll speed: %s", settings.warning) + return settings + + +def refresh_hz_from_config(global_config: Optional[Dict[str, Any]]) -> float: + """The panel's refresh cap from the global config, or the default.""" + if not isinstance(global_config, dict): + return DEFAULT_REFRESH_HZ + hardware = (global_config.get("display") or {}).get("hardware") or {} + return _coerce(hardware.get("limit_refresh_rate_hz")) or DEFAULT_REFRESH_HZ diff --git a/src/common/scroll_helper.py b/src/common/scroll_helper.py index 6e2f4218..5b34cfe0 100644 --- a/src/common/scroll_helper.py +++ b/src/common/scroll_helper.py @@ -75,8 +75,19 @@ class ScrollHelper: # Pre-allocated buffer for output frame (reused to avoid allocations) self._frame_buffer: Optional[np.ndarray] = None - # Sub-pixel scrolling settings (disabled - using high FPS integer scrolling instead) - self.sub_pixel_scrolling = False # Disabled - use high frame rate for smoothness + # Sub-pixel scrolling: OFF by default, and that is deliberate. + # Blending renders a half-step by mixing two adjacent columns 50/50. + # On a high-resolution screen that reads as smooth motion; on a coarse + # LED matrix showing pixel-font text it does not. A one-pixel stroke + # becomes two half-brightness pixels, so frames alternate between crisp + # and smeared and the text appears to shimmer and jump a pixel ahead -- + # tested on a 2x128x64 panel and clearly worse than integer stepping. + # + # The rule this display obeys: motion is smooth when it advances a + # whole number of pixels per refresh. Anything slower must either + # 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 @@ -244,19 +255,31 @@ class ScrollHelper: if self.last_step_time == 0.0: self.last_step_time = current_time - # Check if scroll_delay has passed - time_since_last_step = current_time - self.last_step_time - if time_since_last_step >= self.scroll_delay: - # Move pixels (can move multiple steps if lag occurred, but cap to prevent huge jumps) - steps = int(time_since_last_step / self.scroll_delay) - # Cap at reasonable number to prevent huge jumps from lag - max_steps = max(1, int(0.04 / self.scroll_delay)) # Limit to 0.04s (2 steps at 50 FPS) for smoother scrolling - steps = min(steps, max_steps) - pixels_to_move = self.scroll_speed * steps - # Update last_step_time, preserving fractional delay for smooth timing - self.last_step_time = current_time - (time_since_last_step % self.scroll_delay) + # Frame-based mode advances by elapsed time, exactly like the + # time-based branch below, at the same configured speed + # (scroll_speed px per scroll_delay seconds). + # + # It used to step discretely: 0, 1 or 2 whole pixels depending on + # whether a wall clock had passed scroll_delay. Plugins set + # scroll_delay to the target frame period, so that comparison sits + # exactly on its own threshold and the decision flips on sub- + # millisecond jitter -- a frame a hair early moved nothing and + # rendered an identical frame, a frame a hair late moved two + # pixels. Rounding the step count fixed the stalls but still + # discarded the remainder, so the error never corrected. + # + # Accumulating elapsed time keeps position exactly proportional to + # real time: jitter shifts a pixel boundary by a fraction of a + # frame instead of flipping a whole step, and nothing is lost or + # gained. This is what the one visibly smooth scroller on the + # hardware (the stock ticker) was already doing by virtue of never + # enabling frame-based mode. + if self.scroll_delay > 0: + pixels_per_second = self.scroll_speed / self.scroll_delay else: - pixels_to_move = 0.0 + pixels_per_second = self.scroll_speed * 100.0 + pixels_to_move = pixels_per_second * delta_time + self.last_step_time = current_time else: # Time-based: move based on time delta (correct speed over time) # scroll_speed is pixels per second diff --git a/src/display_controller.py b/src/display_controller.py index bb650698..d68e2e97 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -2405,6 +2405,7 @@ class DisplayController: ) while True: + _frame_start = time.perf_counter() try: with self._display_lock_or_skip(plugin_id) as can_display: if can_display: @@ -2425,11 +2426,26 @@ class DisplayController: # Multi-display sync: send follower frame after each render self._send_follower_frame(manager_to_display) - time.sleep(display_interval) self._tick_plugin_updates() self._poll_on_demand_requests() self._check_on_demand_expiration() + # Pace to the frame deadline rather than sleeping a flat + # interval on top of the work. display() has already + # blocked on the panel's vsync by this point, so an + # unconditional sleep is added to a wait that already + # happened. Measured on a 2x128x64 chain at + # limit_refresh_rate_hz=100: ~4ms of render plus a flat + # 8ms put each iteration at ~12ms against a 10ms refresh + # grid, so every swap missed a refresh and the loop + # settled at 50fps where display_interval asks for 125 -- + # and with zero headroom, ~14% of frames slipped a + # further refresh, which is what reads as scroll stutter. + _remaining = display_interval - (time.perf_counter() - _frame_start) + # Yield even when the frame overran its budget, so plugin + # update threads and the web UI are not starved of the GIL. + time.sleep(_remaining if _remaining > 0 else 0.001) + if self.current_display_mode != active_mode: logger.debug("Mode changed during high-FPS loop, breaking early") break diff --git a/src/display_manager.py b/src/display_manager.py index 9cc7f622..af23f880 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -768,16 +768,18 @@ class DisplayManager: return # Skip hardware write — content is being captured off-screen digest = None + frame_checksum = None if self._dirty_tracking_enabled: try: brightness = getattr(self.matrix, 'brightness', None) except AttributeError: brightness = None - digest = (zlib.adler32(self.image.tobytes()), brightness) + frame_checksum = zlib.adler32(self.image.tobytes()) + digest = (frame_checksum, brightness) if digest == self._last_pushed_digest: # Nothing changed since the last push — the panel is # already showing exactly this frame. - self._write_snapshot_if_due() + self._write_snapshot_if_due(frame_checksum) return # Copy the current image to the offscreen canvas. In double-sided @@ -796,7 +798,7 @@ class DisplayManager: self._last_pushed_digest = digest # Write a snapshot for the web preview (throttled) - self._write_snapshot_if_due() + self._write_snapshot_if_due(frame_checksum) except Exception as e: logger.error(f"Error updating display: {e}") @@ -1410,11 +1412,20 @@ class DisplayManager: self._viewer_fresh = False return self._viewer_fresh - def _write_snapshot_if_due(self) -> None: + def _write_snapshot_if_due(self, frame_checksum: Optional[int] = None) -> None: """Mirror the current frame to the preview snapshot when the policy says it's worth it — see src/common/snapshot_policy.py. Unchanged frames are never re-encoded; without viewers the cadence drops to - the idle keepalive.""" + the idle keepalive. + + Args: + frame_checksum: adler32 of the current frame, when the caller has + already computed one. Dirty tracking checksums every frame a + few lines above the call site, and re-deriving it here meant a + second tobytes() plus a second pass over the whole framebuffer + on every single frame — ~0.17ms per frame of the two combined + at 256x64, paid 100 times a second to reach the same number. + """ try: now = time.time() viewer_fresh = self._viewer_is_fresh(now) @@ -1424,7 +1435,8 @@ class DisplayManager: self._last_snapshot_ts = 0.0 self._viewer_was_fresh = viewer_fresh - digest = zlib.adler32(self.image.tobytes()) + digest = (frame_checksum if frame_checksum is not None + else zlib.adler32(self.image.tobytes())) action = snapshot_policy.decide( now, self._last_snapshot_ts, self._last_snapshot_touch_ts, viewer_fresh, digest != self._last_snapshot_digest) diff --git a/test/test_scroll_config.py b/test/test_scroll_config.py new file mode 100644 index 00000000..f3f1dcc2 --- /dev/null +++ b/test/test_scroll_config.py @@ -0,0 +1,213 @@ +"""Tests for the shared scroll configuration resolver.""" + +import logging +import sys +from pathlib import Path + +import pytest + +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_SECOND, + MIN_PIXELS_PER_SECOND, + ScrollSettings, + configure, + refresh_hz_from_config, + resolve, +) + + +class FakeHelper: + """Records what configure() applied.""" + + def __init__(self, with_optional=True): + self.speed = None + self.frame_based = None + self.target_fps = None + if not with_optional: + del FakeHelper.set_frame_based_scrolling + del FakeHelper.set_target_fps + + def set_scroll_speed(self, speed): + self.speed = speed + + def set_frame_based_scrolling(self, enabled): + self.frame_based = enabled + + def set_target_fps(self, fps): + self.target_fps = fps + + +class MinimalHelper: + """An older helper exposing only set_scroll_speed.""" + + def __init__(self): + self.speed = None + + def set_scroll_speed(self, speed): + self.speed = speed + + +class TestPrecedence: + def test_display_options_pair_wins(self): + s = resolve({"display_options": {"scroll_speed": 1.0, "scroll_delay": 0.01}}) + assert s.pixels_per_second == 100.0 + assert s.source == "display_options.scroll_speed/delay" + + def test_display_block_used_when_options_absent(self): + s = resolve({"display": {"scroll_speed": 2.0, "scroll_delay": 0.01}}) + assert s.pixels_per_second == 200.0 + assert s.source == "display.scroll_speed/delay" + + def test_root_pair_used_when_both_blocks_absent(self): + s = resolve({"scroll_speed": 1.0, "scroll_delay": 0.02}) + assert s.pixels_per_second == 50.0 + + def test_pixels_per_second_used_when_no_pair_given(self): + s = resolve({"display_options": {"scroll_pixels_per_second": 120.0}}) + assert s.pixels_per_second == 120.0 + assert s.source == "display_options.scroll_pixels_per_second" + + def test_global_display_is_the_last_resort_before_default(self): + s = resolve({}, {"display": {"scroll_speed": 1.0, "scroll_delay": 0.005}}) + assert s.pixels_per_second == 200.0 + + def test_default_when_nothing_configured(self): + s = resolve({}, {}) + assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND + assert s.source == "default" + + +class TestDeprecatedKeyCannotOverrideExplicitPair: + """Regression for ChuckBuilds/ledmatrix-plugins#408. + + odds-ticker ranked scroll_pixels_per_second above the documented + scroll_speed/scroll_delay pair. Because that key carries a schema default, + the documented settings became unreachable for every user and the ticker + silently ran at the default speed. The pair must win. + """ + + def test_pair_beats_pixels_per_second_in_the_same_block(self): + s = resolve( + { + "display_options": { + "scroll_speed": 1.0, + "scroll_delay": 0.01, + "scroll_pixels_per_second": 50.0, # schema default + } + } + ) + assert s.pixels_per_second == 100.0 + assert "scroll_speed/delay" in s.source + + def test_pair_beats_pixels_per_second_in_an_outer_block(self): + s = resolve( + { + "display_options": {"scroll_speed": 1.0, "scroll_delay": 0.01}, + "scroll_pixels_per_second": 50.0, + } + ) + assert s.pixels_per_second == 100.0 + + +class TestMalformedValues: + @pytest.mark.parametrize("bad", [None, "fast", "", {}, [], float("nan")]) + def test_unusable_speed_falls_through(self, bad): + s = resolve({"display_options": {"scroll_speed": bad, "scroll_delay": 0.01}}) + assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND + + @pytest.mark.parametrize("bad", [0, -5, 0.0]) + def test_non_positive_values_fall_through(self, bad): + s = resolve({"display_options": {"scroll_pixels_per_second": bad}}) + assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND + + def test_booleans_are_not_treated_as_numbers(self): + s = resolve({"display_options": {"scroll_pixels_per_second": True}}) + assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND + + def test_zero_delay_does_not_divide_by_zero(self): + s = resolve({"display_options": {"scroll_speed": 1.0, "scroll_delay": 0}}) + assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND + + def test_non_dict_blocks_are_ignored(self): + s = resolve({"display_options": "nonsense", "display": 5}) + assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND + + +class TestClamping: + def test_absurdly_fast_is_clamped(self): + s = resolve({"display_options": {"scroll_pixels_per_second": 100000.0}}) + assert s.pixels_per_second == MAX_PIXELS_PER_SECOND + assert s.warning and "clamped" in s.warning + + def test_absurdly_slow_is_clamped(self): + s = resolve({"display_options": {"scroll_pixels_per_second": 0.01}}) + assert s.pixels_per_second == MIN_PIXELS_PER_SECOND + + +class TestWholePixelWarning: + """The display rule: whole pixels per refresh, or it judders.""" + + def test_no_warning_when_speed_divides_evenly(self): + s = resolve({"display_options": {"scroll_pixels_per_second": 100.0}}, refresh_hz=100) + assert s.warning is None + assert s.pixels_per_frame == pytest.approx(1.0) + + def test_no_warning_at_an_integer_multiple(self): + s = resolve({"display_options": {"scroll_pixels_per_second": 200.0}}, refresh_hz=100) + assert s.warning is None + + def test_warns_on_a_half_pixel_per_frame(self): + s = resolve({"display_options": {"scroll_pixels_per_second": 50.0}}, refresh_hz=100) + assert s.warning is not None + assert "judder" in s.warning + assert "100 px/s" in s.warning + + def test_warns_on_a_fractional_multiple(self): + s = resolve({"display_options": {"scroll_pixels_per_second": 130.0}}, refresh_hz=100) + assert s.warning is not None + + def test_respects_a_non_default_refresh_rate(self): + s = resolve({"display_options": {"scroll_pixels_per_second": 150.0}}, refresh_hz=150) + assert s.warning is None + assert s.pixels_per_frame == pytest.approx(1.0) + + +class TestConfigure: + def test_applies_time_based_mode_and_speed(self): + helper = FakeHelper() + s = configure(helper, {"display_options": {"scroll_speed": 1.0, "scroll_delay": 0.01}}) + assert helper.speed == 100.0 + assert helper.frame_based is False, "must not use the wall-clock step gate" + assert helper.target_fps == 100.0 + assert s.pixels_per_second == 100.0 + + def test_works_against_a_helper_without_optional_methods(self): + helper = MinimalHelper() + 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() + 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) + + def test_returns_settings_describing_the_source(self): + s = configure(FakeHelper(), {"display_options": {"scroll_speed": 2.0, "scroll_delay": 0.01}}) + assert isinstance(s, ScrollSettings) + assert "200.0 px/s" in s.describe() + + +class TestRefreshFromConfig: + def test_reads_the_hardware_limit(self): + assert refresh_hz_from_config( + {"display": {"hardware": {"limit_refresh_rate_hz": 150}}} + ) == 150.0 + + @pytest.mark.parametrize("cfg", [None, {}, {"display": {}}, {"display": {"hardware": {}}}, + "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