Pull request overview

This PR refactors the PAN/WF FPS meter rendering in SpectrumWidget to avoid stuttering caused by once-per-second static overlay texture repaints/uploads when FPS text updates.

Changes:

• Replaces FPS meter drawing on the static overlay with two lightweight child QLabel overlays updated once per second.
• Adds label creation, text updates, and geometry positioning hooks for resize and split-ratio/divider changes.
• Keeps a fallback QPainter path for FPS meters only when labels are not available.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Thanks @rfoust — the analysis is convincing and the fix is well-scoped. Moving FPS meter readouts off the static overlay texture is exactly the right call here: those values were the only once-per-second invalidator of an otherwise stable QImage that's uploaded to a GPU texture, so the upload pause maps cleanly to the symptom.

A couple of notes on the code itself and the Copilot comments:

Copilot z-order concern (false positive) — Copilot flagged repeated label->raise() calls, but the diff doesn't actually call raise() on the FPS labels. stackUnder(m_overlayMenu) is set once at creation, and positionFpsMeterLabels() only re-raises m_overlayMenu via raiseAll() — which is the same pattern used at SpectrumWidget.cpp:468, :486, and :4432 to keep the overlay menu above VFO widgets. Show/move on a previously-shown sibling doesn't reorder Qt's stack, so the labels stay under the overlay/TNF popup and don't need any additional management.

Copilot geometry-duplication concern (valid but pre-existing) — The chromeH / contentH / specH / wfY pattern is already duplicated in ~10 spots across this file (e.g. lines 1900, 1961, 2430, 2922, 3060, 3145, 4285). Adding one more occurrence isn't great, but extracting a shared helper is a broader refactor that's out of scope for a bug fix. Worth keeping in mind for a follow-up cleanup, not blocking here.

Minor observation: createFpsMeterLabels() runs in the constructor after m_overlayMenu is created at line 283, so the stackUnder(m_overlayMenu) guarded by if (m_overlayMenu) is always taken — that's fine and defensive, just noting it works as intended.

RAII/lifetime, AppSettings, null guards (m_panFpsMeterLabel && m_wfFpsMeterLabel, m_overlayMenu) all look correct. The painted fallback in drawFpsMeters() early-returns when the labels exist, so there's no double-rendering. Toggling visibility correctly dirties the overlay once to clear the old painted path.

LGTM as a focused fix. Thanks for the clear before/after description in the PR body.

Claude here, reviewing on Jeremy's behalf.

Verdict

Clean fix for a real per-second stutter. Built locally, no warnings introduced. Merge after Jeremy eyeballs the placement.

The bug, plainly

The FPS meters timer fires every second to update m_panadapterFps / m_waterfallFps. Pre-PR, that update called markOverlayDirty(), which forces a full QImage overlay repaint AND a GPU texture re-upload. So every second, the rendering pipeline took a small synchronous hit for what amounts to a four-character text change. Operators with the meters enabled saw a visible pause exactly when the digit changed.

Why the fix works

Promoting the two FPS readouts from painter-text-in-overlay-QImage to QLabel children of SpectrumWidget:

• setText() on a QLabel only invalidates the label's tiny rectangle (~80×20 px), not the whole spectrum overlay
• No GPU texture re-upload (the QLabel renders into its own widget backing store, composited by Qt's normal child-widget paint path)
• The per-second cost drops to "two QLabel repaints" instead of "full spectrum overlay repaint + texture upload"

Verified

• WA_TransparentForMouseEvents set on both labels — spectrum click/drag/scroll/divider-drag still work through the label area
• stackUnder(m_overlayMenu) with m_overlayMenu->raiseAll() at the end of positionFpsMeterLabels() — the overlay menu drawer still covers the meters when open. Good UX preservation
• positionFpsMeterLabels() called from all the right places: resizeEvent, setSpectrumFrac, mouseMoveEvent (during divider drag), and on every updateFpsMeterLabels(). Labels track the split correctly
• Hide-on-too-small logic at lines area.width() < 56 || area.height() < 18 mirrors the painter version's behavior
• Band plan inset preserved — panBottomInset = m_bandPlanFontSize + 12 keeps the PAN meter above the band plan text, same as before
• applyFpsMeterVisibility still calls markOverlayDirty() at the end — needed to erase the legacy painter text once on toggle. Good defensive code
• Threading: updateFpsMeterValues() runs on the GUI thread (driven by m_fpsMeterTimer which is a QTimer parented to this), so the label updates are GUI-thread safe by construction
• CI all green: build, analyze (cpp), check-paths, CodeQL, check-windows (skipped — header-only changes shouldn't affect Windows-specific paths)
• Built locally clean on Linux, only pre-existing unrelated warning

One observation worth noting

drawFpsMeters() now has an early-return when both labels exist (line 3393), so the legacy painter path is effectively dead code under normal operation. Two reasonable options for the future:

• (a) Keep as-is — useful as a defensive fallback if label construction ever fails (e.g., OOM in the ctor). Tiny code size cost.
• (b) Delete drawFpsMeters() entirely in a follow-up — labels are always constructed in the ctor, so the function is unreachable in practice.

I'd lean (a) for this PR (don't expand scope) and leave the cleanup to a future small PR if it ever feels worth doing. Right now the early-return makes the dead path harmless.

Minor styling

The CSS in createFpsMeterLabels():

background: rgba(15, 15, 26, 210);
border: 1px solid rgba(255, 255, 255, 170);
color: #9ceeff;
font-size: 9pt;
font-weight: bold;
padding: 3px 6px;

Matches the visual idiom of the existing painter version. Worth a visual side-by-side comparison just to confirm the label-rendered version doesn't look noticeably different — Qt's font hinting in widget mode can differ subtly from QPainter::drawText, especially at small sizes and across HiDPI scales. Not a blocker; flag for the maintainer's eyes during testing.

Recommendation

Merge after Jeremy enables View → FPS Meters on a live panadapter and confirms:

1. The per-second pause is gone
2. The labels render at the expected position (bottom-right of spectrum and waterfall, above the band plan in spectrum)
3. The labels disappear correctly when the panel is shrunk below the size thresholds
4. The overlay menu still covers them when opened

Thanks @rfoust — moving from painter-into-shared-overlay to per-widget child labels is the right Qt idiom for this kind of "small, frequent update with bounded geometry" UI element. The positioning helper preserves every special-case of the original placement.

73, Jeremy KK7GWY & Claude (AI dev partner)

Approved — clean fix for the per-second overlay re-upload stutter.