perf(spectrum): skip overlay re-bake on cursor move when no readout shown#3780
Conversation
…ersdr#3779) updateTrackedCursorState() re-baked the ENTIRE static overlay (band plan, freq/dBm/time scales, spot/slice/TNF markers, SWR sweep) and re-uploaded the static + background framebuffer-sized overlay textures on every cursor-position change over the panadapter -- even when neither the cursor frequency readout (aethersdr#726) nor a tune guide is shown, in which case the re-baked image is byte-identical. This silently defeated the GPU overlay-upload cache added for macOS in aethersdr#719. Gate the bare cursor-position term on (m_showCursorFreq || m_tuneGuideVisible); TNF-hover and tune-guide-visibility changes stay unconditional invalidations. Measured (i9-13900K/RTX4090, GPU path, 2 panes, dummy load, readouts OFF = default), cursor moving over the panadapter: before: static overlay re-baked ~55x/s (every frame), renderP95 6.2 ms, overlay uploads ~150/s after: 0 re-bakes, renderP95 0.3 ms (idle level), uploads 44/s (flags only) ~20x render-time reduction during mouse motion; the win grows with overlay content (spots / SWR sweep). With a readout active (tune guide on) the re-bake correctly returns so the guide tracks the cursor -- verified. Also routes setShowCursorFreq() through markOverlayDirty() (mirroring setShowTuneGuides) so toggling the readout off re-bakes immediately on the GPU path, hardening aethersdr#952 now that the cursor-move re-bake no longer masks it. Principle XI -- Fixes Are Demonstrated: before/after measured one variable at a time behind the gated aether.perf category, including the readout-active regression check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed against the full SpectrumWidget source — this is a clean, well-reasoned fix. Thanks for the thorough write-up and the before/after measurement, @svabi79.
Correctness of the gate — confirmed the only static-overlay content that reads m_cursorPos is the cursor-freq label (SpectrumWidget.cpp:7755, guarded by m_showCursorFreq) and the tune guide (:7784, guarded by m_showTuneGuides && m_tuneGuideVisible). The new cursorReadoutShown = m_showCursorFreq || m_tuneGuideVisible covers exactly those. And because m_tuneGuideVisible is only ever set true while m_showTuneGuides is true (:5111), the guide still re-bakes on every cursor move and tracks correctly — matching your readout-active regression check.
TNF hover still tracks — moving within a single notch (id unchanged, both readouts off) no longer dirties the static overlay, but the TNF popup is a separate QLabel repositioned in updateTnfHoverPopup(), which is still called unconditionally at :5129. So that path is unaffected. m_hoveredTnfId and m_tuneGuideVisible transitions also still invalidate unconditionally, as intended.
The setShowCursorFreq → markOverlayDirty() change is the right companion fix: on the software path markOverlayDirty() is just update() (SpectrumWidget.h:1315), so it's a strict superset of the old behavior, and on the GPU path it sets m_overlayStaticDirty so toggling the readout off re-bakes immediately — correctly preventing the #952 regression that the old code only cleared via the next cursor-move bake.
One harmless note (no change needed): when m_showCursorFreq is on but a TNF is hovered, the label is suppressed (m_hoveredTnfId < 0 guard) yet cursor moves still re-bake. That's safe over-invalidation and not worth special-casing.
No AppSettings/QSettings, RAII, null-pointer, or scope concerns. LGTM pending CI.
🤖 aethersdr-agent · cost: $1.5531 · model: claude-opus-4-8
|
Two files, 12 lines changed, ~20× render-time improvement at the default cursor position. Tight and correct. Root cause is right and the gate is exactly bounded. The only static-overlay content that reads TNF hover is unaffected. The TNF popup is a separate
One harmless over-invalidation (no change needed): When CI: All six checks green including Constitution: Principles VIII (root cause confirmed by instrumented measurement — early noise-floor hypothesis disproven by data), IX (eliminating wasted re-bakes that produced no visible change), and XI (rigorous before/after table with temporary rebake counter, regression check included) all honored. Author self-cited XI in the commit body — appreciated.
|
NF0T
left a comment
There was a problem hiding this comment.
Gate verified against source: m_cursorPos read sites at SpectrumWidget.cpp:7755 (m_showCursorFreq) and :7784 (m_tuneGuideVisible) — cursorReadoutShown OR covers exactly those. TNF popup path unaffected (separate QLabel, unconditional). setShowCursorFreq companion correctly prevents #952 regression via markOverlayDirty() on GPU path. All six CI checks green. Tier 3 only.
Squash subject: perf(spectrum): skip overlay re-bake on cursor-move when no readout shown — Principle XI. (#3780)
Summary
Fixes #3779.
updateTrackedCursorState()calledmarkOverlayDirty()on every cursor-position change over the panadapter, which re-bakes the entire static overlay (band plan, freq/dBm/time scales, spot/slice/TNF markers, SWR sweep) and re-uploads the static + background framebuffer-sized overlay textures. But the only static-overlay content that depends onm_cursorPosis the cursor-frequency readout (#726) and the tune-guide line — both off by default. With them off the re-baked overlay is byte-identical, so moving the mouse re-rastered and re-uploaded the whole overlay every frame for nothing, defeating the GPU overlay-upload cache added for macOS in #719.Fix: gate the bare cursor-position term on
(m_showCursorFreq || m_tuneGuideVisible). TNF-hover and tune-guide-visibility changes are genuine content changes and still invalidate unconditionally — so the readouts still track the cursor when enabled.Also routes
setShowCursorFreq()throughmarkOverlayDirty()instead ofupdate()(mirroring the existingsetShowTuneGuides()), so toggling the readout off re-bakes immediately on the GPU path. Without this, gating the cursor-move dirty would reintroduce #952 (the cursor-frequency label persisting after it's switched off), which previously relied on the next cursor-move re-bake to clear it.Measurements
i9-13900K / RTX 4090, GPU path, 2 panadapters, dummy load, cursor-freq + tune-guides OFF (default); gated
aether.perfplus a temporary static-rebake counter (added to measure, reverted before commit). Stimulus: real mouse moving continuously over the panadapter.~20× render-time reduction during mouse motion; the win grows with overlay content (spots / SWR sweep). Regression check (readout active): with tune guides enabled, the re-bake correctly returns (~55/s while moving) so the guide tracks the cursor — verified visually ("works flawlessly") and in the counter.
Constitution principle honored
Principle XI — Fixes Are Demonstrated. Before/after measured one variable at a time behind the gated
aether.perfcategory (table above), including the readout-active regression check. Principle VIII — Evidence Over Assertion (the root cause was confirmed by instrumented measurement, not static reasoning alone — an early "noise-floor churn" hypothesis was disproven by the data).Test plan
QRhiWidgetoverlay-cache path; this restores Fix macOS GPU rendering: CPU optimization + build fix #719's intended caching — cross-platform numbers welcomeChecklist
AppSettingscalls (N/A)MeterSmoother(N/A — no meter changes)