镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

feat(scroll): show which scroll speeds are smooth on this panel - #710

Merged
ChuckBuilds merged 3 commits into
mainfrom
claude/ledpi-scrolling-performance-4c4c05
Oct 1, 2026
Merged

ChuckBuilds merged 3 commits into
mainfrom
claude/ledpi-scrolling-performance-4c4c05

Conversation

@ChuckBuilds

@ChuckBuilds ChuckBuilds commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

Why

On a Pi 4 / 120 Hz panel the default Vegas speed of 50 px/s silently snapped to 48 px/s (2 px every 5 refreshes, 24 fps): visibly stepped, with the only evidence a log line. No frames were late (frame_soak.py: ~0.2%).

What

  • solve_crisp(): raise the sub-25 fps penalty so 50 px/s on 120 Hz resolves to a smooth 60 or 40 px/s. 100 Hz results are unchanged (existing tests pass).
  • scroll_config.speed_advice() + GET /api/v3/config/scroll-speed-advice: what the panel will really run, plus the nearest smooth options. Uses the refresh the display measured (frame-stats file), falling back to the configured cap; an implausible stale measurement is ignored.
  • Vegas Scroll Speed slider: live hint under it ("50 px/s will run as 48 px/s (…stepped). Smooth speeds: 40 px/s 60 px/s" with click-to-apply). Slider step 5 → 1 so the suggestions are reachable.
  • CHANGELOG and docs/SCROLL_PERFORMANCE.md notes.

Test plan

  • pytest test -k "scroll or vegas or sports or crisp" — 1737 passed
  • new tests for speed_advice and the endpoint (measured vs cap, stale measurement, bad input)
  • inline JS syntax-checked with node --check
  • eyeball the hint in the web UI on ledpi

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • The Vegas scroll-speed slider now supports 1 px/s adjustments and provides live guidance on the speed achievable for your display.
    • See whether your selected speed is smooth and exact, along with nearby smooth-speed alternatives when available. Guidance reflects the panel’s refresh rate when suitable measurement is available.
  • Documentation
    • Updated the web interface guide to explain the scroll-speed guidance and smooth-speed suggestions.

The Vegas Scroll Speed slider now says what the panel will do with the
chosen speed and offers the nearest smooth ones to click. Backed by
scroll_config.speed_advice() and GET /api/v3/config/scroll-speed-advice,
which uses the refresh the display measured rather than the cap.

Also stops the default 50 px/s snapping to a stepped 48 px/s (2px every 5
refreshes, 24fps) on a 120Hz panel: the low-fps penalty in solve_crisp()
now loses to 60 or 40 px/s. 100Hz panels are unchanged.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 20 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 8ef5291a-cd09-4441-9256-092dd8e0a3b6

📥 Commits

Reviewing files that changed from the base of the PR and between 3e53c51 and 657667b.

📒 Files selected for processing (4)
  • src/common/scroll_config.py
  • test/fixtures/api_v3_url_map.json
  • test/test_scroll_config.py
  • web_interface/templates/v3/partials/display.html

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e46b1f1f-bfa8-426c-87c5-7571ab3e431e

📥 Commits

Reviewing files that changed from the base of the PR and between 4ddc3a3 and 3e53c51.

📒 Files selected for processing (7)
  • CHANGELOG.md
  • docs/SCROLL_PERFORMANCE.md
  • src/common/scroll_config.py
  • test/test_scroll_config.py
  • test/web_interface/test_api_v3_scroll_speed_advice.py
  • web_interface/blueprints/api_v3/config.py
  • web_interface/templates/v3/partials/display.html

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The changes update crisp-speed ranking, add refresh-aware speed advice to the configuration API, and display that advice in the Vegas scroll-speed slider.

Changes

Scroll speed advice

Layer / File(s) Summary
Crisp speed advice
src/common/scroll_config.py, test/test_scroll_config.py, CHANGELOG.md
The below-25-fps penalty changes, and speed_advice reports an applied speed and nearby smooth alternatives. Unit tests cover speed selection and alternative choices. The changelog records the 120 Hz and 100 Hz behavior.
Refresh selection and advice API
web_interface/blueprints/api_v3/config.py, test/web_interface/test_api_v3_scroll_speed_advice.py
The API uses a measured refresh rate when it falls within 50%–150% of the configured cap; otherwise, it uses the cap. It returns speed advice and the selected refresh source. API tests cover refresh selection and invalid speed input.
Vegas slider advice
web_interface/templates/v3/partials/display.html, docs/SCROLL_PERFORMANCE.md
The slider uses 1 px/s steps and displays debounced speed advice, including smooth alternatives. The documentation describes the panel-dependent advice.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant VegasSlider
  participant ConfigAPI
  participant ConfigManager
  participant FrameStatsFile
  participant ScrollConfig
  VegasSlider->>ConfigAPI: Request speed advice with speed and slider bounds
  ConfigAPI->>ConfigManager: Read configured refresh-rate cap
  ConfigAPI->>FrameStatsFile: Read measured refresh rate
  ConfigAPI->>ScrollConfig: Call speed_advice with selected refresh rate
  ScrollConfig-->>ConfigAPI: Return applied speed and smooth alternatives
  ConfigAPI-->>VegasSlider: Return advice and refresh source
Loading

Merge Risk: ⚪ Minimal · up to 3e53c

This change adds panel-specific scroll speed advice to the Vegas slider. No merge-blocking issues were identified. The hint has not been checked visually on hardware.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3e53c

The new endpoint returns display-timing advice without changing settings, credentials, or privileges. It inherits existing access controls. Deployment-specific access and file permissions remain unconfirmed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — Caller-controlled inputs influence derived scroll advice for the application’s configured panel. They do not select another asset, file, credential, or service. Ladder iteration bounds are not exposed as query parameters, and the response does not serialize loaded configuration.

Trust Boundaries and Controls

  • observed — The new route inherits the existing session, bearer-token, and local-request authentication rules rather than adding an unauthenticated exception. Returned advice is rendered using textContent and created DOM elements, not interpreted as HTML.

Resilience and Maintainability Implications

  • observed — The initial hint call precedes initialization of its let-bound timer and sequence, causing an initialization failure. The intended asynchronous path also leaves local stale-failure and debounce-window races. These defects affect hint state and subsequent inline initialization; the inspected paths establish no authentication bypass or persisted-state mutation.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 4 files. (3 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: showing panel-specific smooth scroll-speed guidance in the Vegas slider.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 4 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 17 complexity

Metric Results
Complexity 17

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

ChuckBuilds and others added 2 commits October 1, 2026 13:55
…30fps as stepped

The Vegas speed hint called refreshScrollSpeedHint() before the let
declarations it uses, so it never rendered (found on ledpi). And the
solver's low-fps penalty stopped at 25fps, which let a measured 125.7Hz
panel keep a 25.1fps 2px-every-5-refreshes scroll.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@ChuckBuilds
ChuckBuilds merged commit 16b566e into main Oct 1, 2026
13 checks passed
@ChuckBuilds
ChuckBuilds deleted the claude/ledpi-scrolling-performance-4c4c05 branch October 1, 2026 18:15
ChuckBuilds added a commit that referenced this pull request Oct 5, 2026
* chore: prepare the 3.8.1 release

Bumps src.__version__ to 3.8.1 and moves the Unreleased CHANGELOG entries
under a 3.8.1 heading, with an empty Unreleased above it.

The reason for the release is smooth scrolling at held-frame speeds. On
3.8.0 the default 50 px/s snapped to a stepped 48 px/s on a 120 Hz panel,
and any scroll slower than one pixel per refresh showed a half-pixel step
across the middle of the panel. Both are fixed on main (#710, #711) but
were in no release, so every stable-channel device still had them.

- #710's CHANGELOG entry had been filed under 3.8.0 although it merged
  after the v3.8.0 tag; it moves to 3.8.1.
- #711 had no CHANGELOG entry; it gets one.

scripts/check_release_version.py v3.8.1 passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: sports_game_over and draw_text_outlined ship in 3.8.1

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant