refactor(spectrum): excise GPU flag-sprite machinery, flags always live (#3777)#3806
Merged
Conversation
…ve (#3777) Strategy B of the #3617-lineage reversion plan (attached to #3777): the full removal that #3803 (the default-flip) was the reversible precursor to. VFO slice flags are now unconditionally ordinary live child widgets — the off-screen-sprite + live-on-hover path is gone, resolving the #3777 idle-flag text blur and the Diversity-mode RX-antenna input regression at the source. Removed (all gated on / part of the sprite path): - SpectrumWidget: m_gpuFlagMode + the AETHER_GPU_FLAGS/NO_GPU_FLAGS gate, m_flagRefreshTimer + its lambda, grabFlagSprites/releaseFlagSprite, setLiveFlag/updateLiveFlag, updateFlagRefreshTimer/attachFlagTimerWindowWatcher + m_flagTimerFilteredWindow, the FlagSprite struct + m_flagSprites/ m_flagDrawOrder/m_flagQuadVbo/m_flagSampler/kMaxFlagSprites, the renderGpuFrame sprite draw-order/texture/quad blocks, and the QRhi flag-resource init/teardown. - VfoWidget: setButtonsOccluded + m_occludeRestore/m_buttonsOccluded. Kept intact: - repositionVfoFlags() — the SHARED flag positioner (calls updatePosition() on every flag); only its trailing GPU-mode hide-block was removed. Still called each frame from renderGpuFrame, so live flags position correctly. - The entire GPU panadapter path (AETHER_GPU_SPECTRUM: FFT/waterfall/overlay). - The SmartMTR render-mode-agnostic fixes from #3771/#3773/#3775 (m_meterMenuOpen, setEnabled row dimming, ComboStyle :disabled, drawSmartMtrLabels size-gate). Net -582 lines. Trade-off unchanged from #3803: re-introduces the #3617 main-thread composite cost (~+0.37 cores) #3695 removed — separate from, and likely additive to, #3797. Unlike #3803 there is no env opt-in to restore sprites; the machinery is gone, so this is the commit-to-it option once the 4-panadapter re-profile confirms the trade is acceptable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Thanks @ten9876 — this is a clean, well-scoped excision. I verified the mechanics rather than re-litigate the strategy:
Code review: clean ✅
- No dangling references. Repo-wide grep for every removed symbol (
m_gpuFlagMode,m_flagRefreshTimer,grabFlagSprites/releaseFlagSprite,setLiveFlag/updateLiveFlag,updateFlagRefreshTimer/attachFlagTimerWindowWatcher,m_flagTimerFilteredWindow,FlagSprite/m_flagSprites/m_flagDrawOrder/m_flagQuadVbo/m_flagSampler/kMaxFlagSprites,m_liveFlag*,setButtonsOccluded/m_occludeRestore/m_buttonsOccluded,AETHER_NO_GPU_FLAGS/AETHER_GPU_FLAGS) returns zero matches. The machinery was self-contained. - Header/impl parity. SpectrumWidget.h / VfoWidget.h declaration removals match the .cpp; the now-unused
#include <QPointer>is correctly dropped from SpectrumWidget.h (no remainingQPointeruse in that header or its .cpp — VfoWidget.h keeps its ownQPointermembers and include). - The correctness invariant holds.
repositionVfoFlags()is intact, still invoked each frame fromrenderGpuFrame, and only callsupdatePosition()per flag — it never hides anything.addVfoWidget()callsw->show()/raise()unconditionally, so with the sprite/hover-swap layer gone the flags are simply always-live child widgets, which is the documented baseline. No resource leak (theQRhiTexture/SRB teardown is removed only because the sprites it freed are gone). - CI green on this SHA: build, check-windows, check-macos, CodeQL, and Qt a11y static analysis all pass.
No convention, null-safety, or scope issues — all four touched files are in scope, and a pure removal touches no AppSettings/system boundaries.
One non-code note (process / merge-gating)
This is correct as code but, per your own plan, shouldn't land yet:
- It re-introduces the #3617 main-thread composite cost (~+0.37 cores / +45% main-thread) that #3695 removed — a real, deliberate regression that you've flagged as additive to #3797 on multi-panadapter setups.
- You've marked the 4-panadapter re-profile as the decision gate, and that box (plus the Windows crisp-text, Diversity-RX, multi-pan positioning, and drag-no-crackle checks) is still unchecked. The recommended sequence in the description — validate via the reversible #3803 first, then merge this as the clean removal and close #3803 — is the right call.
So: ✅ for the code itself; the gate is the perf re-profile + the validation checklist, not anything in the diff. Nice surgical work.
🤖 aethersdr-agent · cost: $7.7482 · model: claude-opus-4-8
This was referenced Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Strategy B of the GPU-flag reversion plan (attached to #3777) — the full excision that #3803 (the one-line default-flip) was the reversible precursor to. VFO slice flags are now unconditionally live child widgets; the off-screen-sprite + live-on-hover machinery is removed entirely, fixing the #3777 idle-flag blur and the Diversity-mode RX-antenna regression at the source.
Net −582 lines across
SpectrumWidget.{cpp,h}andVfoWidget.{cpp,h}.Relationship to #3803
These are alternatives — merge one, not both. #3803 flips the default but keeps the sprite path behind
AETHER_GPU_FLAGS=1(reversible, for measurement). This PR deletes the machinery (no env opt-in remains). Recommended sequence: validate live-flag behavior + the 4-panadapter re-profile via #3803 first; if the group commits to it, merge this as the clean removal and close #3803.Removed (all sprite-path)
m_gpuFlagMode+ the env gate,m_flagRefreshTimer+ lambda,grabFlagSprites/releaseFlagSprite,setLiveFlag/updateLiveFlag,updateFlagRefreshTimer/attachFlagTimerWindowWatcher+m_flagTimerFilteredWindow,FlagSprite+m_flagSprites/m_flagDrawOrder/m_flagQuadVbo/m_flagSampler/kMaxFlagSprites, therenderGpuFramesprite draw-order/texture/quad blocks, and the QRhi flag-resource init/teardown.setButtonsOccluded+m_occludeRestore/m_buttonsOccluded.Kept intact (verified)
repositionVfoFlags()— the shared positioner that callsupdatePosition()on every flag; only its trailing GPU-mode hide-block was removed. Still called each frame fromrenderGpuFrame, so live flags position correctly. This was the key correctness invariant.AETHER_GPU_SPECTRUM: FFT/waterfall/overlay) — untouched.event()/eventFilter()non-flag logic (setMouseTracking, Mac QRhi teardown inprepareForTopLevelChange) preserved.Trade-off (unchanged from #3803)
Re-introduces the #3617 main-thread composite cost #3695 removed (~0.82 vs 0.45 cores, +0.37 ≈ +45% main-thread). Separate from and likely additive to #3797.
Test plan
cmake --build build --target AetherSDR); zero dangling references to removed symbolsRefs #3777, #3617. Supersedes #3803. Plan: see the #3777 comment.
🤖 Generated with Claude Code