diff --git a/docs/SCROLL_PERFORMANCE.md b/docs/SCROLL_PERFORMANCE.md new file mode 100644 index 00000000..57176dab --- /dev/null +++ b/docs/SCROLL_PERFORMANCE.md @@ -0,0 +1,266 @@ +# 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.** + +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 + 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` 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 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: + +```python +settings = scroll_config.configure( + self.scroll_helper, + plugin_config=self.config, + global_config=self.global_config, + display_manager=self.display_manager, # supplies the panel refresh rate +) + +# ...then, each time this plugin begins scrolling: +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. + +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 + +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..70a7d2e2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -55,6 +55,18 @@ 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. +# The 3.11.6 floor is CVE-2025-67221: orjson.dumps did not +# limit recursion on deeply nested documents, and the disk +# cache encodes payloads parsed straight from third-party +# APIs. 3.11.6 covers the Python range above. +# pip install 'orjson>=3.11.6,<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..f92a7189 --- /dev/null +++ b/scripts/build_rgbmatrix_nogil.sh @@ -0,0 +1,330 @@ +#!/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, median 10.00ms, p95 10.05ms, 0% stalls +# +# The per-pixel blit (SetPixelsPillow) can also release the GIL and walk the +# Pillow buffer row-major, but that is OFF by default and you almost certainly +# want to leave it that way. Row-major changes what a partially-written frame +# looks like: column-major tearing shows as a vertical seam, row-major tearing +# shows as a horizontal split between the panel's upper and lower halves. On a +# 1/32 scan panel that reads as a one-pixel "fold" across the middle of every +# panel -- reported on hardware, and it went away when the blit was reverted. +# Enable with RGB_PATCH_BLIT=1 only if you have measured that you need it; +# essentially all of the gain above comes from the SwapOnVSync change alone. +# +# 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 + +# Resolve the invoking user's home, not root's. --install runs under sudo, +# where $HOME is /root, so every default path below pointed somewhere the +# build had never written and the install died with "no built module found". +if [ -n "${SUDO_USER:-}" ]; then + OWNER_HOME="$(getent passwd "$SUDO_USER" | cut -d: -f6)" +fi +OWNER_HOME="${OWNER_HOME:-$HOME}" + +SRC_TREE="${RGB_SRC_TREE:-$OWNER_HOME/LEDMatrix/rpi-rgb-led-matrix-master}" +BUILD_DIR="${RGB_BUILD_DIR:-$OWNER_HOME/rgbmatrix-nogil-build}" +VENV="${RGB_CYTHON_VENV:-$OWNER_HOME/.cache/ledmatrix-cython}" +BACKUP="${RGB_BACKUP:-$OWNER_HOME/rgbmatrix-core.so.ORIGINAL}" +PATCH_BLIT="${RGB_PATCH_BLIT:-0}" + +die() { echo "FATAL: $*" >&2; exit 1; } + +# This script runs under `set -uo pipefail` -- no -e -- so an unchecked +# systemctl failure is silently ignored. That matters most for `stop`: leaving +# the old service running means cp overwrites a module the running process has +# mapped, the following `start` succeeds as a no-op, and the health check sees +# an active unit and reports SUCCESS for a binding that was never loaded. +# A machine with no ledmatrix.service at all is a normal build host, so that +# case is skipped rather than treated as a failure. +service_present() { systemctl cat ledmatrix.service >/dev/null 2>&1; } + +service_do() { + local verb="$1" + if ! service_present; then + echo " (no ledmatrix.service installed - skipping $verb)" + return 0 + fi + systemctl "$verb" ledmatrix || die "systemctl $verb ledmatrix failed" +} + +py_site() { + python3 -c 'import rgbmatrix, os; print(os.path.dirname(rgbmatrix.__file__))' 2>/dev/null +} + +# The extension filename the interpreter that builds -- and then loads -- this +# module actually uses, e.g. core.cpython-313-aarch64-linux-gnu.so. The build +# venv is made with --system-site-packages from python3, so the two agree; +# falling back keeps --install working when the venv has been cleaned up. +abi_name() { + local py="$VENV/bin/python" + [ -x "$py" ] || py=python3 + "$py" -c \ + 'import sysconfig; print("core" + sysconfig.get_config_var("EXT_SUFFIX"))' \ + 2>/dev/null +} + +# Exactly the current interpreter's artifact, never merely the first one that +# sorts. Staging copies $SRC_TREE wholesale, so a core.cpython-*.so left in the +# source tree by an earlier build comes along for the ride; build_ext --inplace +# only ever overwrites the current ABI's name, and a glob piped to `head -1` +# sorts cpython-311 ahead of cpython-313. That installed a stale, unpatched +# module as core.so while the GIL check below -- which reads the freshly +# generated core.cpp, not the .so -- still reported success. +abi_so() { + local name path + name="$(abi_name)" || return 1 + [ -n "$name" ] || return 1 + path="$BUILD_DIR/bindings/python/rgbmatrix/$name" + [ -f "$path" ] || return 1 + printf '%s\n' "$path" +} + +do_rollback() { + local dst; dst="$(py_site)" + [ -n "$dst" ] || die "could not locate the installed rgbmatrix package" + [ -f "$BACKUP" ] || die "no backup at $BACKUP" + service_do stop + cp -a "$BACKUP" "$dst/core.so" || die "restore failed" + find "$dst" -name __pycache__ -type d -exec rm -rf {} + 2>/dev/null + service_do start + 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 + + service_do stop + cp "$so" "$dst/core.so" || die "install failed" + find "$dst" -name __pycache__ -type d -exec rm -rf {} + 2>/dev/null + service_do start + + 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" \ + || echo "ROLLBACK FAILED: could not restore $BACKUP -> $dst/core.so" >&2 + if service_present && ! systemctl restart ledmatrix; then + echo "ROLLBACK FAILED: ledmatrix did not restart - the display is" \ + "down; restore manually with 'sudo bash $0 --rollback'" >&2 + fi + 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" + +# Drop any extension artifacts that came across from the source tree. Nothing +# downstream should be able to pick one up, and build_ext --inplace can decide +# a copied .so is already up to date and skip the compile entirely. +find "$BUILD_DIR/bindings/python/rgbmatrix" -maxdepth 1 \ + -name 'core*.so' -delete 2>/dev/null + +echo "==> patching the bindings to release the GIL" +python3 - "$BUILD_DIR" "$PATCH_BLIT" <<'PYEOF' || die "patch failed" +import io +import sys + +base = sys.argv[1] + "/bindings/python/rgbmatrix/" +patch_blit = len(sys.argv) > 2 and sys.argv[2] == "1" + +# --- declare SwapOnVSync as nogil --------------------------------------- +p = base + "cppinc.pxd" +s = io.open(p, encoding="utf-8").read() +OLD_DECL = " FrameCanvas *SwapOnVSync(FrameCanvas*, uint8_t)\n" +NEW_DECL = " FrameCanvas *SwapOnVSync(FrameCanvas*, uint8_t) nogil\n" +if OLD_DECL in s: + io.open(p, "w", encoding="utf-8", newline="\n").write(s.replace(OLD_DECL, NEW_DECL, 1)) + print(" cppinc.pxd: SwapOnVSync declared nogil") +elif NEW_DECL in s: + print(" cppinc.pxd: already nogil") +else: + sys.exit("could not find the SwapOnVSync declaration") + +# --- release the GIL across the vsync wait ------------------------------ +p = base + "core.pyx" +s = io.open(p, encoding="utf-8").read() + +OLD_SWAP = ( + " def SwapOnVSync(self, FrameCanvas newFrame, uint8_t framerate_fraction = 1):\n" + " return __createFrameCanvas(" + "self.__matrix.SwapOnVSync(newFrame.__canvas, framerate_fraction))\n" +) +NEW_SWAP = ( + " def SwapOnVSync(self, FrameCanvas newFrame, uint8_t framerate_fraction = 1):\n" + " # Blocks until the panel's next vertical sync. Holding the GIL\n" + " # across that wait starves every other Python thread for most of\n" + " # each frame. Pointers are hoisted into C locals so the blocking\n" + " # call itself needs no Python state.\n" + " cdef cppinc.RGBMatrix* matrix = self.__matrix\n" + " cdef cppinc.FrameCanvas* frame = newFrame.__canvas\n" + " cdef uint8_t fraction = framerate_fraction\n" + " cdef cppinc.FrameCanvas* swapped\n" + " with nogil:\n" + " swapped = matrix.SwapOnVSync(frame, fraction)\n" + " return __createFrameCanvas(swapped)\n" +) +if OLD_SWAP in s: + s = s.replace(OLD_SWAP, NEW_SWAP, 1) + print(" core.pyx: SwapOnVSync releases the GIL") +elif "swapped = matrix.SwapOnVSync(frame, fraction)" in s: + print(" core.pyx: SwapOnVSync already patched") +else: + sys.exit("could not find the SwapOnVSync body") + +# --- optional: release the GIL across the blit -------------------------- +OLD_BLIT = ( + " buffer = get_pillow_buffer(image_capsule)\n" + "\n" + " for col in range(max(0, -xstart), min(width, frame_width - xstart)):\n" + " for row in range(max(0, -ystart), min(height, frame_height - ystart)):\n" + " pixel = buffer[row][col]\n" + " r = (pixel ) & 0xFF\n" + " g = (pixel >> 8) & 0xFF\n" + " b = (pixel >> 16) & 0xFF\n" + " my_canvas.SetPixel(xstart+col, ystart+row, r, g, b)\n" +) +NEW_BLIT = ( + " buffer = get_pillow_buffer(image_capsule)\n" + "\n" + " # Bounds hoisted so the blit needs no Python state and can run\n" + " # without the GIL: it touches only a C buffer and a C++ canvas.\n" + " # NOTE: row-major order makes a torn frame show as a horizontal\n" + " # split across the panel's halves. See the header before enabling.\n" + " cdef int col_start = max(0, -xstart)\n" + " cdef int col_end = min(width, frame_width - xstart)\n" + " cdef int row_start = max(0, -ystart)\n" + " cdef int row_end = min(height, frame_height - ystart)\n" + "\n" + " with nogil:\n" + " for row in range(row_start, row_end):\n" + " for col in range(col_start, col_end):\n" + " pixel = buffer[row][col]\n" + " r = (pixel ) & 0xFF\n" + " g = (pixel >> 8) & 0xFF\n" + " b = (pixel >> 16) & 0xFF\n" + " my_canvas.SetPixel(xstart+col, ystart+row, r, g, b)\n" +) +if patch_blit: + if OLD_BLIT in s: + s = s.replace(OLD_BLIT, NEW_BLIT, 1) + print(" core.pyx: pixel blit releases the GIL, row-major") + elif "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") +else: + print(" core.pyx: blit left unpatched (RGB_PATCH_BLIT=1 to enable)") + +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)" || true +[ -n "$SO" ] || die "no .so produced - expected $(abi_name) in $BUILD_DIR/bindings/python/rgbmatrix" + +# Verify the GIL really is released before anyone installs this. +EXPECTED=1; [ "$PATCH_BLIT" = "1" ] && EXPECTED=2 +PAIRS=$(grep -c "PyEval_SaveThread\|Py_UNBLOCK_THREADS" \ + "$BUILD_DIR/bindings/python/rgbmatrix/core.cpp") +[ "$PAIRS" -ge "$EXPECTED" ] \ + || die "generated C++ has $PAIRS GIL-release sites, expected >= $EXPECTED" + +echo +echo "BUILT: $SO" +echo " ($PAIRS GIL-release site(s) in the generated C++)" +echo +echo "Install with: sudo bash $0 --install" +echo "Roll back with: sudo bash $0 --rollback" 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/cache/disk_cache.py b/src/cache/disk_cache.py index 03cfb2f1..92c3f465 100644 --- a/src/cache/disk_cache.py +++ b/src/cache/disk_cache.py @@ -5,6 +5,7 @@ """ import json +import math import os import time import tempfile @@ -14,6 +15,11 @@ 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 +46,97 @@ def get_data_type_from_key(self, key: str) -> str: 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") + + +def _replace_nonfinite(obj: Any) -> Any: + """Non-finite floats -> None, matching what ``orjson.dumps`` writes. + + Only reached once a strict pass has proved there is something to replace, + so the ordinary write path never pays for this walk. + """ + if isinstance(obj, float): + return obj if math.isfinite(obj) else None + if isinstance(obj, dict): + return {k: _replace_nonfinite(v) for k, v in obj.items()} + if isinstance(obj, (list, tuple)): + return [_replace_nonfinite(v) for v in obj] + return obj + + +# NON-FINITE FLOATS +# ----------------- +# JSON has no NaN or Infinity. The stdlib emits them anyway as an extension; +# orjson refuses to and writes null. That divergence is not acceptable in a +# cache whose files outlive the decision of which encoder is installed, so the +# policy here is one behaviour on both paths: +# +# writing non-finite floats become null, whichever encoder is in use +# reading files already on disk that carry the stdlib's NaN/Infinity +# tokens stay readable, whichever encoder is in use +# +# Without the write half, installing orjson silently changed cached values. +# Without the read half, installing orjson turned every legacy record holding a +# NaN into a "corrupted cache file" that DiskCache.get logged as an error and +# deleted. Both halves are covered by test/test_cache_nonfinite_floats.py. + + +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: + try: + return orjson.loads(raw) + except orjson.JSONDecodeError: + # Legacy record written by the stdlib path, carrying NaN or + # Infinity. Genuinely malformed files raise again from here, as + # json.JSONDecodeError, which is what DiskCache.get expects. + return json.loads(raw) +else: + def _dumps(data: Any) -> bytes: + try: + return json.dumps(data, cls=DateTimeEncoder, + allow_nan=False).encode("utf-8") + except ValueError: + # allow_nan=False is what detects the non-finite values; the walk + # runs only now that we know there is one to replace. + return json.dumps(_replace_nonfinite(data), cls=DateTimeEncoder, + allow_nan=False).encode("utf-8") + + def _loads(raw: bytes) -> Any: + return json.loads(raw) + + class DiskCache: """Manages persistent disk-based cache.""" @@ -99,8 +189,8 @@ def get(self, key: str, max_age: Optional[int] = 300) -> Optional[Dict[str, Any] 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 +279,12 @@ def set(self, key: str, data: Dict[str, Any]) -> None: # 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 +332,7 @@ def set(self, key: str, data: Dict[str, Any]) -> None: # 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 +350,7 @@ def set(self, key: str, data: Dict[str, Any]) -> None: 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 +380,7 @@ def set(self, key: str, data: Dict[str, Any]) -> None: # 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.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 @@ '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..afaac8e7 --- /dev/null +++ b/src/common/scroll_config.py @@ -0,0 +1,440 @@ +"""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, replace +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 + + +#: 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.""" + + pixels_per_second: float + source: str + 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 + + @property + def frame_hold(self) -> int: + """Refreshes to hold each frame for; pass to set_scrolling_state().""" + return self.crisp.frame_hold if self.crisp else 1 + + 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, + display_manager: Any = None, + snap_to_crisp: bool = True, +) -> 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. + + :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. + :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 + + # Refresh rate, most authoritative first: what the caller passed, then the + # display manager (which can see display.hardware; a plugin cannot), then + # the global config, then the default. + # + # This has to be settled BEFORE resolve(), not after. resolve() uses the + # 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. + hz = _coerce(refresh_hz) + if hz is None and display_manager is not None: + hz = _coerce(getattr(display_manager, "refresh_hz", None)) + if hz is None: + hz = refresh_hz_from_config(global_config) + + settings = resolve( + plugin_config, + global_config, + default_pixels_per_second=default_pixels_per_second, + refresh_hz=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(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) + + # Deliberately NOT applied here. The hold belongs to a scroll, not to a + # plugin's lifetime: plugins share one display manager, and one left set at + # construction is reset the moment any other plugin finishes scrolling. + # Callers pass settings.frame_hold to set_scrolling_state(True, ...) when + # they start scrolling. configure() only reports what is needed. + + 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: + log.debug( + "Scroll needs a frame hold of %d - pass settings.frame_hold to " + "display_manager.set_scrolling_state(True, ...) each scroll", + choice.frame_hold, + ) + else: + 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 + # Each level is checked for being a mapping rather than merely truthy: a + # malformed config where display or display.hardware is a string or a list + # raised AttributeError out of what is meant to be a total function with a + # default, taking down every caller that asked for the refresh rate. + display = global_config.get("display") + if not isinstance(display, dict): + return DEFAULT_REFRESH_HZ + hardware = display.get("hardware") + if not isinstance(hardware, dict): + return DEFAULT_REFRESH_HZ + 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..d5e949d2 100644 --- a/src/common/scroll_helper.py +++ b/src/common/scroll_helper.py @@ -16,6 +16,7 @@ """ import logging +import math import time from typing import Optional, Dict, Any from PIL import Image @@ -29,6 +30,49 @@ HAS_SCIPY = False +def frame_stats(frame_times: list) -> Dict[str, Any]: + """Summary statistics over one window of frame durations (seconds). + + Split out of log_frame_rate() so the arithmetic can be tested without a + clock. Median and p95 are the real ones: the median takes both middle + samples on an even window, and p95 is nearest-rank, so a 100-frame window + reports the 95th sorted sample rather than the 96th. That matters twice + over, because the median is also the threshold the stall and skip counts + are measured against. + """ + window = sorted(frame_times) + n = len(window) + median = (window[n // 2] if n % 2 + else (window[n // 2 - 1] + window[n // 2]) / 2.0) + mean = sum(window) / n + # Anything past 1.5x the median missed a panel refresh; anything under + # half of it never reached the panel at all (dirty tracking skipped the + # swap, so the frame did not wait for vsync). + return { + "frames": n, + "fps": (1.0 / mean) if mean > 0 else 0.0, + "median": median, + "p95": window[max(0, math.ceil(0.95 * n) - 1)], + "max": window[-1], + "min": window[0], + "stalls": sum(1 for f in window if f > median * 1.5), + "skips": sum(1 for f in window if f < median * 0.5), + } + + +def format_frame_stats(frame_times: list) -> str: + """The one-line rendering of frame_stats(), in milliseconds.""" + s = frame_stats(frame_times) + n = s["frames"] + return ( + f"{s['fps']:.1f} fps over {n} frames | " + f"median {s['median'] * 1000:.2f}ms p95 {s['p95'] * 1000:.2f}ms " + f"max {s['max'] * 1000:.2f}ms min {s['min'] * 1000:.2f}ms | " + f"stalls {s['stalls']} ({100.0 * s['stalls'] / n:.1f}%) " + f"skips {s['skips']} ({100.0 * s['skips'] / n:.1f}%)" + ) + + class ScrollHelper: """ Helper class for scrolling text and image content on LED displays. @@ -75,8 +119,19 @@ def __init__(self, display_width: int, display_height: int, # 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 @@ -105,6 +160,9 @@ def __init__(self, display_width: int, display_height: int, self.last_frame_time = time.time() self.last_fps_log_time = time.time() self.frame_times = [] + # Every frame time since the last stats line, so the 5s summary can + # report the tail rather than one arbitrary sample. Cleared on log. + self._window: list = [] # Scrolling state management self.is_scrolling = False @@ -244,19 +302,31 @@ def update_scroll_position(self) -> None: 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 @@ -1017,18 +1087,25 @@ def log_frame_rate(self) -> None: # Keep only last 100 frames for average if len(self.frame_times) > 100: self.frame_times.pop(0) + + # Every frame since the last log, not just the last 100 and not just + # the one that happens to land on the 5s boundary. The old line + # reported a single instantaneous sample -- roughly 1 frame in 500 -- + # which cannot see a stall that hits 1% of frames, and reported it + # next to an average that hides the same stall by construction (a 2ms + # duplicate and a 21ms double-wait mean exactly 10ms). Chasing scroll + # judder needs the tail, so keep the window and report percentiles. + self._window.append(frame_time) # Log FPS every 5 seconds to avoid spam if current_time - self.last_fps_log_time >= 5.0: - avg_frame_time = sum(self.frame_times) / len(self.frame_times) - avg_fps = 1.0 / avg_frame_time if avg_frame_time > 0 else 0 - instant_fps = 1.0 / frame_time if frame_time > 0 else 0 - - self.logger.info(f"Scroll frame stats - Avg FPS: {avg_fps:.1f}, " - f"Current FPS: {instant_fps:.1f}, " - f"Frame time: {frame_time*1000:.2f}ms") + self.logger.info( + "Scroll frame stats - %s", + format_frame_stats(self._window or [frame_time]), + ) self.last_fps_log_time = current_time self.frame_count = 0 + self._window = [] self.last_frame_time = current_time self.frame_count += 1 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 @@ def _should_exit_dynamic(elapsed_time: float) -> bool: ) 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 @@ def _should_exit_dynamic(elapsed_time: float) -> bool: # 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..ed0811a5 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -240,6 +240,14 @@ def __init__(self, config: Dict[str, Any] = None, force_fallback: bool = False, 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, @@ -768,16 +776,33 @@ def update_display(self): 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) - if digest == self._last_pushed_digest: + frame_checksum = zlib.adler32(self.image.tobytes()) + digest = (frame_checksum, brightness) + if digest == self._last_pushed_digest and not self.is_currently_scrolling(): # Nothing changed since the last push — the panel is # already showing exactly this frame. - self._write_snapshot_if_due() + # + # Never taken mid-scroll, and that exception is the + # point. SwapOnVSync is what paces the render loop, so + # skipping it also skips the wait: a duplicate frame + # returns in ~8ms instead of ~10ms on a 100Hz panel, + # advances only 0.8px instead of 1.0px, and so makes + # the *next* frame more likely to repeat as well. That + # is self-sustaining -- measured at ~20% duplicate + # frames mid-scroll on the odds ticker, against + # essentially zero on a lighter plugin with identical + # scroll settings. Swapping an identical frame costs + # one canvas copy and keeps the loop locked to the + # panel; falling out of that lock costs smooth motion. + # Static content is unaffected: is_currently_scrolling() + # expires on its own inactivity threshold. + self._write_snapshot_if_due(frame_checksum) return # Copy the current image to the offscreen canvas. In double-sided @@ -787,8 +812,10 @@ def update_display(self): 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 @@ -796,7 +823,7 @@ def update_display(self): 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}") @@ -1274,12 +1301,65 @@ def format_date_with_ordinal(self, dt): return dt.strftime(f"%b %-d{suffix}") - def set_scrolling_state(self, is_scrolling: bool): - """Set the current scrolling state. Call this when a display starts/stops scrolling.""" + @property + def refresh_hz(self) -> float: + """The panel's refresh rate in Hz, from the hardware config. + + The authoritative place to ask, because a plugin only receives its own + config section and cannot see display.hardware. Scroll pacing needs + this: the speeds a panel can show in whole pixels are refresh_hz + divided by the frame hold, so getting it wrong silently produces + fractional-pixel motion. See src/common/scroll_config.py. + + Note this is the configured *cap*, not necessarily what the panel + achieves -- scripts/scroll_speeds.py --measure reports the real rate. + """ + hardware = (self.config.get('display') or {}).get('hardware') or {} + try: + value = float(hardware.get('limit_refresh_rate_hz') or 0) + except (TypeError, ValueError): + value = 0.0 + return value if value > 0 else 100.0 + + 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, frame_hold: int = 1): + """Set the current scrolling state, and this scroll's frame pacing. + + Call this when a display starts or stops scrolling. ``frame_hold`` is + how many panel refreshes each frame is held for -- 2 gives one whole + pixel every second refresh, which is how a scroll runs at half the + refresh rate without fractional pixel positions. + + 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 + silently re-pace the next plugin. Passing it alongside the state makes + the lifetime exactly the scroll, and the default of 1 means any caller + that does not care gets a new frame every refresh. + """ current_time = time.time() self._scrolling_state['is_scrolling'] = is_scrolling if is_scrolling: self._scrolling_state['last_scroll_activity'] = current_time + self.set_frame_hold(frame_hold) + else: + self._frame_hold = 1 logger.debug(f"Scrolling state set to: {is_scrolling}") def is_currently_scrolling(self) -> bool: @@ -1410,11 +1490,20 @@ def _viewer_is_fresh(self, now: float) -> bool: 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 +1513,8 @@ def _write_snapshot_if_due(self) -> None: 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/src/plugin_system/testing/visual_display_manager.py b/src/plugin_system/testing/visual_display_manager.py index 5211a226..55bb99ea 100644 --- a/src/plugin_system/testing/visual_display_manager.py +++ b/src/plugin_system/testing/visual_display_manager.py @@ -503,9 +503,18 @@ def draw_text_with_icons(self, text: str, icons: List[tuple] = None, # Scrolling state (no-op interface compat) # ------------------------------------------------------------------ - def set_scrolling_state(self, is_scrolling: bool): - """Set the current scrolling state (no-op for testing).""" + def set_scrolling_state(self, is_scrolling: bool, frame_hold: int = 1): + """Set the current scrolling state (no-op for testing). + + ``frame_hold`` mirrors the DisplayManager signature this change adds. + The two are kept in step deliberately: a double that accepts arguments + production does not lets a call pass every harness run and then raise + TypeError on the panel, and a double that lacks one production has + fails every render of a plugin that legitimately paces its scroll. + Plugins begin passing it in ledmatrix-plugins#462. + """ self._scrolling_state['is_scrolling'] = is_scrolling + self._scrolling_state['frame_hold'] = frame_hold if is_scrolling: self._scrolling_state['last_scroll_activity'] = time.time() diff --git a/test/test_cache_nonfinite_floats.py b/test/test_cache_nonfinite_floats.py new file mode 100644 index 00000000..55cdfaa9 --- /dev/null +++ b/test/test_cache_nonfinite_floats.py @@ -0,0 +1,158 @@ +"""Non-finite floats round-trip identically with and without orjson. + +JSON has no NaN or Infinity. The stdlib emits them anyway as an extension; +orjson refuses to and writes null. Cache files outlive the decision of which +encoder is installed, so both halves of that gap are pinned here: + + * writing -- installing orjson must not silently change what gets cached, + so the stdlib path writes null too; + * reading -- records already on disk carrying NaN or Infinity must stay + readable, or installing orjson turns each of them into a "corrupted cache + file" that DiskCache.get logs as an error and deletes. +""" + +import importlib +import json +import math +import sys +from pathlib import Path +from unittest import mock + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from src.cache import disk_cache as disk_cache_module # noqa: E402 + + +@pytest.fixture +def stdlib_cache(): + """The module as it loads on a host with no orjson wheel.""" + # A None entry in sys.modules makes `import orjson` raise ImportError, + # which is the branch we want, whether or not orjson is really installed. + with mock.patch.dict(sys.modules, {"orjson": None}): + module = importlib.reload(disk_cache_module) + assert module.orjson is None + yield module + importlib.reload(disk_cache_module) + + +@pytest.fixture +def orjson_cache(): + """The module as it loads with orjson available.""" + module = importlib.reload(disk_cache_module) + if module.orjson is None: + pytest.skip("orjson is not installed on this host") + return module + + +NON_FINITE = {"nan": float("nan"), "inf": float("inf"), "ninf": float("-inf")} + + +def _reject_constant(name): + """Make json.loads as strict as orjson about NaN/Infinity tokens.""" + raise AssertionError(f"non-spec JSON constant in output: {name}") + + +class TestWritePolicy: + """Non-finite floats become null on whichever encoder is in use.""" + + def _assert_nulled(self, module): + record = module._loads(module._dumps(dict(NON_FINITE, ok=1.5))) + assert record["nan"] is None + assert record["inf"] is None + assert record["ninf"] is None + # Finite values are untouched. + assert record["ok"] == 1.5 + + def test_stdlib_writes_null(self, stdlib_cache): + self._assert_nulled(stdlib_cache) + + def test_orjson_writes_null(self, orjson_cache): + self._assert_nulled(orjson_cache) + + def test_stdlib_emits_spec_compliant_json(self, stdlib_cache): + # The point of the write half: bytes written without orjson must still + # parse once orjson is installed later. json.loads accepts the + # extension tokens, so it cannot show this -- assert on the bytes, and + # on a strict reader when there is one. + raw = stdlib_cache._dumps(dict(NON_FINITE)) + assert b"NaN" not in raw + assert b"Infinity" not in raw + assert json.loads(raw, parse_constant=_reject_constant) == { + "nan": None, "inf": None, "ninf": None} + + def test_nested_non_finite_are_replaced(self, stdlib_cache): + data = {"a": [1.0, float("nan"), {"b": float("inf")}], "c": (float("-inf"),)} + record = stdlib_cache._loads(stdlib_cache._dumps(data)) + assert record["a"] == [1.0, None, {"b": None}] + assert record["c"] == [None] + + def test_finite_payloads_are_byte_identical_to_the_old_encoder(self, stdlib_cache): + # allow_nan=False must not change ordinary output. + data = {"x": 1, "y": [1.5, "s", True, None], "z": {"k": 2.25}} + assert stdlib_cache._dumps(data) == json.dumps( + data, cls=stdlib_cache.DateTimeEncoder).encode("utf-8") + + +class TestReplaceNonFinite: + def test_leaves_ordinary_values_alone(self): + for value in (1, 1.5, "s", True, None, [], {}): + assert disk_cache_module._replace_nonfinite(value) == value + + def test_replaces_every_non_finite_float(self): + for value in NON_FINITE.values(): + assert disk_cache_module._replace_nonfinite(value) is None + + def test_walks_nested_containers(self): + assert disk_cache_module._replace_nonfinite( + {"a": [{"b": float("nan")}]}) == {"a": [{"b": None}]} + + +class TestLegacyRecordsStayReadable: + """Files written before orjson arrived still load.""" + + LEGACY = b'{"timestamp": 1000.0, "value": NaN, "other": Infinity}' + + def test_stdlib_reads_legacy_tokens(self, stdlib_cache): + record = stdlib_cache._loads(self.LEGACY) + assert math.isnan(record["value"]) + assert math.isinf(record["other"]) + + def test_orjson_falls_back_for_legacy_tokens(self, orjson_cache): + record = orjson_cache._loads(self.LEGACY) + assert math.isnan(record["value"]) + assert math.isinf(record["other"]) + + def test_genuinely_malformed_files_still_raise(self, stdlib_cache): + with pytest.raises(json.JSONDecodeError): + stdlib_cache._loads(b'{"a": ') + + def test_orjson_still_raises_for_malformed_files(self, orjson_cache): + with pytest.raises(json.JSONDecodeError): + orjson_cache._loads(b'{"a": ') + + +class TestDiskCacheEndToEnd: + def _cache(self, module, tmp_path): + return module.DiskCache(str(tmp_path)) + + def test_legacy_file_is_not_deleted_as_corrupt(self, orjson_cache, tmp_path): + cache = self._cache(orjson_cache, tmp_path) + path = cache.get_cache_path("legacy") + Path(path).write_bytes( + b'{"timestamp": %d, "value": NaN}' % int(__import__("time").time())) + + record = cache.get("legacy", max_age=None) + + assert record is not None, "legacy NaN record was treated as corrupt" + assert math.isnan(record["value"]) + assert Path(path).exists(), "legacy NaN record was deleted" + + def test_round_trip_through_set_and_get(self, stdlib_cache, tmp_path): + cache = self._cache(stdlib_cache, tmp_path) + cache.set("k", {"timestamp": __import__("time").time(), + "value": float("nan")}) + record = cache.get("k", max_age=None) + assert record is not None + assert record["value"] is None diff --git a/test/test_display_dirty_tracking.py b/test/test_display_dirty_tracking.py index d367d2d6..cf33d12b 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 @@ -109,6 +114,11 @@ def test_snapshot_still_written_on_skip(self, dm, tmp_path): dm.draw.rectangle([0, 0, 30, 8], fill=(255, 255, 0)) dm.update_display() # push + snapshot write (first frame) assert os.path.exists(dm._snapshot_path) + # Backdate the file so the "was it bumped?" check below cannot be + # defeated by filesystem mtime granularity -- on Windows two writes in + # the same tick get identical timestamps, which made this test fail + # roughly two runs in three regardless of the code under test. + os.utime(dm._snapshot_path, (time.time() - 60, time.time() - 60)) first_mtime = os.path.getmtime(dm._snapshot_path) # Age the write/touch bookkeeping past TOUCH_INTERVAL so the next @@ -125,6 +135,62 @@ def test_snapshot_still_written_on_skip(self, dm, tmp_path): assert os.path.getmtime(dm._snapshot_path) > first_mtime +class TestScrollLock: + """Dirty tracking must not skip the panel push while a scroll is running. + + SwapOnVSync is what paces the render loop, so skipping it also skips the + wait for the panel. A duplicate frame therefore returns early -- ~8ms + instead of ~10ms on a 100Hz panel -- which advances the strip only 0.8px + instead of 1.0px, which makes the NEXT frame more likely to be a duplicate + too. That is self-sustaining: measured at ~20% duplicate frames mid-scroll + on the odds ticker against essentially zero on a lighter plugin with + identical scroll settings. Pushing an identical frame costs one canvas + copy; falling out of vsync lock costs smooth motion. + """ + + def test_identical_frames_still_push_while_scrolling(self, dm): + dm.draw.rectangle([0, 0, 12, 12], fill=(0, 0, 255)) + dm.update_display() + dm.set_scrolling_state(True) + try: + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + dm.update_display() + dm.update_display() + assert spy.count == 3, "scrolling must stay locked to the panel" + finally: + dm.set_scrolling_state(False) + + def test_identical_frames_are_skipped_when_not_scrolling(self, dm): + """The optimisation still applies to static content.""" + dm.set_scrolling_state(False) + dm.draw.rectangle([0, 0, 14, 14], fill=(255, 0, 255)) + dm.update_display() + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + dm.update_display() + assert spy.count == 0 + + def test_stale_scrolling_state_stops_forcing_pushes(self, dm): + """A plugin that stops scrolling without saying so must not pin the + panel into always-push forever. is_currently_scrolling() expires on + its own inactivity threshold, and the skip has to come back with it.""" + dm.draw.rectangle([0, 0, 16, 16], fill=(0, 255, 255)) + dm.update_display() + dm.set_scrolling_state(True) + try: + # Backdate the activity marker past the inactivity threshold. + dm._scrolling_state['last_scroll_activity'] = ( + time.time() - dm._scrolling_state['scroll_inactivity_threshold'] - 1.0) + assert dm.is_currently_scrolling() is False + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + dm.update_display() + assert spy.count == 0 + finally: + dm.set_scrolling_state(False) + + class TestKillSwitch: def test_dirty_tracking_can_be_disabled(self, dm): dm._dirty_tracking_enabled = False @@ -160,3 +226,89 @@ def test_config_flag_wires_through(self): 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 + + +class TestFrameHoldLifetime: + """The hold must last exactly as long as the scroll that asked for it. + + Plugins share one display manager. A hold applied at plugin construction is + wiped the moment any *other* plugin finishes scrolling, so by the time the + first plugin renders it is back to one pixel per refresh -- the speed reads + correct in the log and is wrong on the panel. + """ + + def test_scrolling_state_carries_the_hold(self, dm): + dm.set_scrolling_state(True, frame_hold=4) + try: + dm.draw.rectangle([0, 0, 7, 7], fill=(10, 200, 10)) + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + assert spy.last_frame_hold == 4 + finally: + dm.set_scrolling_state(False) + + def test_another_plugin_stopping_does_not_strand_a_hold(self, dm): + dm.set_scrolling_state(True, frame_hold=3) + dm.set_scrolling_state(False) # some other plugin finishes + dm.set_scrolling_state(True) # a plugin that wants no hold + try: + dm.draw.rectangle([0, 0, 6, 6], fill=(200, 10, 10)) + with _SwapSpy(dm.matrix) as spy: + dm.update_display() + assert spy.last_frame_hold == 1 + finally: + dm.set_scrolling_state(False) + + def test_default_keeps_previous_behaviour(self, dm): + """Callers that never heard of frame holds get one frame per refresh.""" + dm.set_scrolling_state(True) + try: + assert dm._frame_hold == 1 + finally: + dm.set_scrolling_state(False) diff --git a/test/test_display_double_parity.py b/test/test_display_double_parity.py new file mode 100644 index 00000000..84506848 --- /dev/null +++ b/test/test_display_double_parity.py @@ -0,0 +1,69 @@ +"""The visual-test double must not drift from the real DisplayManager. + +Both directions of drift are silent and both are damaging: + + * the double accepts an argument production does not -- the call passes + every harness run and raises TypeError on the panel, which is precisely + the failure a safety harness exists to prevent; + * the double lacks an argument production has -- every plugin that + legitimately uses it fails every render, and the harness blames the + plugin. + +`set_scrolling_state` has been each of those in turn across two branches, so +the parity is asserted rather than remembered. + +Read with ast rather than imported: src/display_manager.py imports rgbmatrix +at module scope, which is absent anywhere without the panel library, and this +check should hold on a laptop and in CI as well as on a Pi. +""" + +import ast +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent +REAL = ROOT / "src" / "display_manager.py" +DOUBLE = ROOT / "src" / "plugin_system" / "testing" / "visual_display_manager.py" + +#: Methods a plugin calls on whichever manager it is handed. +SHARED_METHODS = ["set_scrolling_state", "is_currently_scrolling"] + + +def _signature(path: Path, class_hint: str, method: str): + """(name, default-repr) pairs for `method`, or None if it is absent.""" + tree = ast.parse(path.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if not isinstance(node, ast.ClassDef) or class_hint not in node.name: + continue + for item in node.body: + if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) and item.name == method: + args = [a.arg for a in item.args.args if a.arg != "self"] + pad = [None] * (len(args) - len(item.args.defaults)) + defaults = pad + [ast.unparse(d) for d in item.args.defaults] + return list(zip(args, defaults)) + return None + + +@pytest.mark.parametrize("method", SHARED_METHODS) +def test_the_double_matches_production(method): + real = _signature(REAL, "DisplayManager", method) + double = _signature(DOUBLE, "DisplayManager", method) + + assert real is not None, f"DisplayManager lost {method}" + assert double is not None, \ + f"the test double is missing {method}, so every plugin using it fails to render" + assert double == real, ( + f"{method} has drifted: production takes {real}, the double takes {double}. " + "A double that is more permissive hides a production TypeError; one that " + "is less permissive fails plugins that are actually correct." + ) + + +def test_frame_hold_is_accepted_by_both(): + """The specific argument that has drifted twice.""" + for path, label in ((REAL, "DisplayManager"), (DOUBLE, "VisualTestDisplayManager")): + params = dict(_signature(path, "DisplayManager", "set_scrolling_state") or []) + assert "frame_hold" in params, f"{label} does not accept frame_hold" + assert params["frame_hold"] == "1", \ + f"{label} must default frame_hold to 1 so existing callers are unaffected" diff --git a/test/test_scroll_config.py b/test/test_scroll_config.py new file mode 100644 index 00000000..ad0104a5 --- /dev/null +++ b/test/test_scroll_config.py @@ -0,0 +1,467 @@ +"""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_FRAME, + crisp_ladder, + solve_crisp, + 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 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.""" + + 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_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): + 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_reports_the_hold_without_applying_it(self): + """The hold belongs to a scroll, not a plugin's lifetime -- one left set + at construction is wiped as soon as any other plugin stops scrolling. + configure() reports it; the caller passes it to set_scrolling_state.""" + dm = FakeDisplayManager() + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 25.0}}, + display_manager=dm) + assert s.frame_hold == 4, "25px/s at 100Hz is 1px every 4th refresh" + assert dm.hold is None, "must not apply the hold behind the caller's back" + + def test_full_speed_needs_no_hold(self): + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 100.0}}, + display_manager=FakeDisplayManager()) + assert s.frame_hold == 1 + + def test_frame_hold_is_one_when_snapping_is_off(self): + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 50.0}}, + snap_to_crisp=False) + assert s.frame_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}}) + 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 + + @pytest.mark.parametrize("cfg", [ + {"display": "nonsense"}, + {"display": ["nonsense"]}, + {"display": 60}, + {"display": {"hardware": "nonsense"}}, + {"display": {"hardware": ["nonsense"]}}, + {"display": {"hardware": 60}}, + ]) + def test_malformed_nesting_falls_back_rather_than_raising(self, cfg): + # Truthy-but-not-a-mapping at either level used to reach .get() on a + # str/list and raise AttributeError out of a function whose whole + # contract is "a refresh rate, or the default". + 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" + + +class TestRefreshFromDisplayManager: + """A plugin sees only its own config section, so the display manager is the + authoritative source for the panel's refresh rate.""" + + class DM: + def __init__(self, hz): + self.refresh_hz = hz + self.hold = None + + def set_frame_hold(self, refreshes): + self.hold = refreshes + + def test_uses_the_display_manager_rate(self): + dm = self.DM(60.0) + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 30.0}}, + display_manager=dm) + # 30px/s on a 60Hz panel is 1px every 2nd refresh, exactly. + assert s.pixels_per_second == pytest.approx(30.0) + assert s.frame_hold == 2 + + def test_explicit_refresh_hz_wins_over_the_display_manager(self): + dm = self.DM(60.0) + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 25.0}}, + display_manager=dm, refresh_hz=100) + assert s.frame_hold == 4, "25px/s at 100Hz is 1px every 4th refresh" + + def test_missing_attribute_falls_back_to_the_default(self): + class Bare: + def set_frame_hold(self, refreshes): + self.hold = refreshes + + bare = Bare() + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 50.0}}, + display_manager=bare) + assert s.frame_hold == 2, "assumed 100Hz" + + def test_reported_settings_describe_the_display_manager_rate(self): + """Without snapping, nothing downstream corrects a wrong refresh rate. + + The rate used to be read *after* resolve() had already filled in + target_fps, pixels_per_frame and the judder warning from the 100Hz + default -- so on a 60Hz panel every one of those described 100Hz, and + set_target_fps() paced the helper to 100 FPS. + """ + dm = self.DM(60.0) + helper = FakeHelper() + s = configure(helper, {"display_options": {"scroll_pixels_per_second": 30.0}}, + display_manager=dm, snap_to_crisp=False) + + assert s.target_fps == pytest.approx(60.0) + assert s.pixels_per_frame == pytest.approx(0.5), "30px/s over 60 frames" + assert helper.target_fps == pytest.approx(60.0) + + def test_judder_warning_is_computed_at_the_real_refresh_rate(self): + dm = self.DM(60.0) + # 60px/s is exactly 1px per refresh at 60Hz -- crisp, no warning. At + # the 100Hz default it is 0.6px per refresh and would be flagged. + s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 60.0}}, + display_manager=dm, snap_to_crisp=False) + assert s.warning is None, s.warning + + def test_global_config_supplies_the_rate_without_a_display_manager(self): + s = configure( + FakeHelper(), + {"display_options": {"scroll_pixels_per_second": 30.0}}, + {"display": {"hardware": {"limit_refresh_rate_hz": 60}}}, + snap_to_crisp=False, + ) + assert s.target_fps == pytest.approx(60.0) + + +class TestFrameHoldIsReportedNotApplied: + """configure() reports the hold; the caller applies it when it scrolls. + + The hold belongs to a scroll, not to a plugin's lifetime -- plugins share + one display manager, so one set at construction is reset the moment any + other plugin stops scrolling. + """ + + class RecordingDM: + refresh_hz = 100.0 + + def __init__(self): + self.calls = [] + + def set_scrolling_state(self, is_scrolling, frame_hold=1): + self.calls.append((is_scrolling, frame_hold)) + + def set_frame_hold(self, refreshes): + self.calls.append(("set_frame_hold", refreshes)) + + def test_configure_does_not_touch_the_display_manager(self): + dm = self.RecordingDM() + settings = configure( + FakeHelper(), {"display_options": {"scroll_pixels_per_second": 25.0}}, + display_manager=dm) + assert dm.calls == [], "configure() must not apply the hold itself" + assert settings.frame_hold == 4, "but it must report what to apply" diff --git a/test/test_scroll_helper.py b/test/test_scroll_helper.py index 517d3e47..f5290e86 100644 --- a/test/test_scroll_helper.py +++ b/test/test_scroll_helper.py @@ -11,7 +11,11 @@ from unittest.mock import patch from PIL import Image -from src.common.scroll_helper import ScrollHelper +from src.common.scroll_helper import ( + ScrollHelper, + format_frame_stats, + frame_stats, +) DISPLAY_W = 64 @@ -315,3 +319,77 @@ def test_scroll_position_reflected(self, helper): helper.scroll_position = 42.0 info = helper.get_scroll_info() assert info["scroll_position"] == 42.0 + + +class TestFrameStatsPercentiles: + """The stats line is the instrument this whole scroll change is measured + with, so its median and p95 have to be the real ones. + + Both are also thresholds: stalls are counted at 1.5x the median and skips + at 0.5x, so an off-by-one in the median biases the counts as well as the + printed numbers. + """ + + # 100 samples of 1..100ms. True median 50.5ms (the mean of the two middle + # samples, not the upper one at 51ms); nearest-rank p95 is the 95th + # sample at 95ms, not the 96th at 96ms. + HUNDRED = [i / 1000.0 for i in range(1, 101)] + + def test_even_window_median_averages_both_middle_samples(self): + assert frame_stats(self.HUNDRED)["median"] == pytest.approx(0.0505) + + def test_p95_uses_nearest_rank(self): + assert frame_stats(self.HUNDRED)["p95"] == pytest.approx(0.095) + + def test_odd_window_median_is_the_middle_sample(self): + times = [i / 1000.0 for i in range(1, 102)] # 101 samples + assert frame_stats(times)["median"] == pytest.approx(0.051) + + def test_single_sample_window_does_not_index_out_of_range(self): + stats = frame_stats([0.010]) + assert stats["median"] == pytest.approx(0.010) + assert stats["p95"] == pytest.approx(0.010) + assert stats["min"] == stats["max"] == pytest.approx(0.010) + + def test_two_sample_window(self): + stats = frame_stats([0.010, 0.020]) + assert stats["median"] == pytest.approx(0.015) + assert stats["p95"] == pytest.approx(0.020) + + def test_input_order_does_not_matter(self): + assert frame_stats(list(reversed(self.HUNDRED))) == frame_stats(self.HUNDRED) + + def test_caller_window_is_not_mutated(self): + times = [0.030, 0.010, 0.020] + frame_stats(times) + assert times == [0.030, 0.010, 0.020] + + def test_stall_threshold_follows_the_median(self): + # Ten 10ms frames and two 30ms stalls: median 10ms, so >15ms is a + # stall -- exactly the two. + stats = frame_stats([0.010] * 10 + [0.030] * 2) + assert stats["stalls"] == 2 + assert stats["skips"] == 0 + + def test_skips_are_frames_that_never_reached_the_panel(self): + stats = frame_stats([0.010] * 10 + [0.002] * 3) + assert stats["skips"] == 3 + assert stats["stalls"] == 0 + + def test_fps_is_the_reciprocal_of_the_mean(self): + assert frame_stats([0.010] * 50)["fps"] == pytest.approx(100.0) + + def test_formatted_line_reports_the_corrected_values(self): + line = format_frame_stats(self.HUNDRED) + assert "median 50.50ms" in line, line + assert "p95 95.00ms" in line, line + assert "over 100 frames" in line, line + + def test_log_frame_rate_emits_the_line_and_clears_the_window(self, helper): + helper._window = [0.010] * 20 + helper.last_fps_log_time = 0.0 # force the 5s boundary + with patch.object(helper.logger, "info") as info: + helper.log_frame_rate() + assert info.called + assert "Scroll frame stats" in info.call_args[0][0] + assert helper._window == []