Skip to content

Add FPS meters and performance telemetry#2480

Merged
ten9876 merged 1 commit into
aethersdr:mainfrom
jensenpat:aether/fps-telemetry
May 9, 2026
Merged

Add FPS meters and performance telemetry#2480
ten9876 merged 1 commit into
aethersdr:mainfrom
jensenpat:aether/fps-telemetry

Conversation

@jensenpat

Copy link
Copy Markdown
Collaborator

Summary

Adds the FPS meter UI and low-overhead performance telemetry work on a clean branch based on origin/main.

  • Adds Ctrl+F / View menu toggle for persisted panadapter and waterfall FPS meters.
  • Draws PAN/WF FPS badges inside their respective panes, positioned left of the scale bars with a subtle white border.
  • Adds PerfTelemetry behind the existing aether.perf category, emitting compact PerfSummary lines and thresholded PerfStall events only when Performance logging is enabled.
  • Instruments pan/waterfall frame age, update/render timing, UDP drain health, packet loss/restarts, GPU upload pressure, interaction timing, and S-history processing cost.
  • Extends pan/waterfall frame signals with network emit timestamps for UI backlog/age correlation.

Scope Notes

This PR intentionally excludes the already-submitted display-rate reconciliation and async logging work. It is based on current origin/main, which already contains PR #2465, and does not include the async logging PR #2478 files or LogManager changes.

Validation

  • Configured clean worktree with:
    • cmake -S . -B build-ninja -G Ninja
  • Built exactly with:
    • env -u CPLUS_INCLUDE_PATH cmake --build build-ninja --parallel 22
  • Ran registered tests:
    • ctest --test-dir build-ninja --output-on-failure
  • Confirmed PR diff is limited to FPS meter/telemetry files and does not include async logging files.

👨🏼‍💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat

Co-authored-by: Codex <noreply@openai.com>
@jensenpat jensenpat marked this pull request as ready for review May 8, 2026 20:05
@jensenpat jensenpat requested a review from ten9876 as a code owner May 8, 2026 20:05

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the careful work on this — the diff is well-organized, the lcPerf category is reused (rather than introducing a new one), aether.perf gating keeps the cost near zero when disabled, AppSettings is used (not QSettings), and the RAII PerfInputScope / PerfUpdateScope / ScopeExit helpers are the right shape. The signal-signature change for spectrumReady / waterfallRowReady looks complete — all three connection points in MainWindow.cpp accept the new qint64 emittedNs.

A few small things worth a look:

1. Ctrl+F collides with Find conventions

MainWindow.cpp:6577 binds the FPS Meters toggle to Ctrl+F, but Ctrl+F (QKeySequence::Find) is already used as window-scoped Find in MemoryDialog.cpp:331 and HelpDialog.cpp, and it's the cross-platform OS convention. While dialog-scoped shortcuts will win when those dialogs are focused, users will likely hit Ctrl+F in the main window expecting search behavior and toggle FPS instead. Suggest Ctrl+Shift+F, Ctrl+Alt+F, or a dedicated F-key — purely advisory, not a bug.

2. Redundant FPS-meter broadcast / settings write

MainWindow.cpp:6580 already iterates all panadapter applets and calls sw->setShowFpsMeters(on) for each. SpectrumWidget::setShowFpsMeters (line ~531) then also writes DisplayFpsMeters to AppSettings and re-broadcasts to findChildren<SpectrumWidget*>() siblings on the top-level window. With N panadapters that's N redundant AppSettings saves and O(N²) applyFpsMeterVisibility calls. Not buggy, but I'd suggest having MainWindow call applyFpsMeterVisibility directly (or letting setShowFpsMeters do the broadcast and just calling it once on any one widget). The single-write path is cleaner and avoids the AppSettings save thrash.

3. setShowFpsMeters AppSettings key is not pan-scoped

Most other display keys in this widget use settingsKey(...) to namespace per pan (e.g. DisplayShowGrid, DisplayFftHeatMap), but DisplayFpsMeters is global. That's actually consistent with the "global FPS meter" intent and the menu wiring in MainWindow, but it's worth a code comment explaining why this one bypasses settingsKey() so future readers don't try to "fix" the inconsistency.

Otherwise the implementation looks solid — beginRecord correctly gates the cold path with m_wasEnabled.exchange() and the per-window reset, percentile/stall thresholds are reasonable, and the frame-restart detection in decodeFFT/decodeWaterfallTile correctly checks frame.totalBins > 0 && !frame.isComplete() to avoid false positives on first frame. Nice work.

@ten9876 ten9876 merged commit 59c1c37 into aethersdr:main May 9, 2026
5 checks passed
@ten9876

ten9876 commented May 9, 2026

Copy link
Copy Markdown
Collaborator

Claude here — really substantial, well-engineered diagnostic foundation, jensenpat. The frame-age primitive is the genuinely missing piece that turns #2471's stutter investigation from guesswork into measurement, and the drag-aware threshold split (50ms idle / 33ms during drag) maps directly to operator-perceived UX. Hot-path overhead is provably zero when lcPerf is disabled — atomic-bool fast-path means production builds can ship this without paying for it. Pairs cleanly with the async-logging foundation in #2478 you also landed. Follow-up filed at #2500 for unit tests once the threshold values stabilize. Merged.

73, Jeremy KK7GWY & Claude (AI dev partner)

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.

2 participants