fix(spectrum): default slice flags to live widgets, GPU sprites opt-in (#3777)#3803
Closed
ten9876 wants to merge 1 commit into
Closed
fix(spectrum): default slice flags to live widgets, GPU sprites opt-in (#3777)#3803ten9876 wants to merge 1 commit into
ten9876 wants to merge 1 commit into
Conversation
#3777) Strategy A of the #3617-lineage reversion plan (attached to #3777). Flips the flag-compositing default from GPU sprites to legacy live widgets, addressing the idle-flag text blur (#3777) and the Diversity-mode RX-antenna input regression — both inherent to the off-screen-sprite + live-on-hover swap. This is a one-line gate flip, not an excision: every sprite code path is already gated on m_gpuFlagMode, and the legacy live-widget path is the documented A/B baseline. The GPU-sprite machinery is left fully intact behind an opt-IN env var (AETHER_GPU_FLAGS=1) so the trade can still be measured before any code removal (Strategy B). Trade-off, stated plainly: live widgets re-introduce the #3617 main-thread composite cost #3695 removed (~0.82 vs 0.45 cores, +0.37 ≈ +45% main-thread). That is separate from #3797 and likely stacks on it on multi-panadapter setups — the required 4-panadapter re-profile is the gate for whether Strategy B (full removal) follows. Idle flag text is now crisp (native-surface ClearType) and the live↔sprite input regressions are gone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
7 tasks
Contributor
There was a problem hiding this comment.
Reviewed the change against the file — this is a clean, well-scoped default flip. Thanks @ten9876.
Verification of the "zero machinery removed" claim — holds up:
m_gpuFlagMode = qEnvironmentVariableIsSet("AETHER_GPU_FLAGS")is the only behavioral line; the GPU-sprite path stays fully intact behind the opt-in.- The 12.5 Hz re-raster timer doesn't just no-op when GPU is off — it never starts:
updateFlagRefreshTimer()(SpectrumWidget.cpp:1231) foldsm_gpuFlagModeinto itspresentpredicate, so the live default incurs no idle timer cost. Good — that's the right gate. updateLiveFlag()(:1094),grabFlagSprites()(:1248), and the GPU draw paths (:8098,:8220,:8341) all early-return on!m_gpuFlagMode, so the live path is the same untouched legacy-widget behavior the code comments already describe as the software-path baseline.- Dropping the
AETHER_NO_GPU_FLAGSread is correct: anyone who previously set it to force live flags still gets live flags (now by default), so no one's behavior silently inverts. The comment documents this.
Only affects AETHER_GPU_SPECTRUM builds; without that macro flags are already live widgets, so it's a no-op there. All six CI checks pass.
Non-blocking notes (your call / maintainer's, not review objections):
- The PR is honest that this re-introduces the #3617/#3695 main-thread composite cost (+~0.37 core) and likely stacks on #3797. The two genuinely load-bearing test-plan items — the 4-panadapter re-profile and confirming a flag drag doesn't reintroduce the #3695 re-raster crash / audio crackle — are still unchecked. Those are the actual decision gate for whether this lands as-is vs. Strategy B, so worth resolving before merge even though the code itself is sound.
- Minor: with the default now live, the comment block is the only remaining documentation of the GPU opt-in path — which is good, but
AETHER_GPU_FLAGSisn't surfaced anywhere user-facing. Fine for an A/B measurement toggle; just flagging it's comment-only.
Code LGTM as a reversible behavioral revert.
🤖 aethersdr-agent · cost: $4.9290 · model: claude-opus-4-8
NF0T
pushed a commit
to NF0T/AetherSDR
that referenced
this pull request
Jun 26, 2026
…ve (aethersdr#3777) (aethersdr#3806) ## Summary **Strategy B** of the GPU-flag reversion plan (attached to aethersdr#3777) — the **full excision** that aethersdr#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 aethersdr#3777 idle-flag blur and the Diversity-mode RX-antenna regression at the source. **Net −582 lines** across `SpectrumWidget.{cpp,h}` and `VfoWidget.{cpp,h}`. ## Relationship to aethersdr#3803 These are **alternatives — merge one, not both.** aethersdr#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 aethersdr#3803 first; if the group commits to it, merge **this** as the clean removal and close aethersdr#3803. ## Removed (all sprite-path) - SpectrumWidget: `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`, the `renderGpuFrame` sprite draw-order/texture/quad blocks, and the QRhi flag-resource init/teardown. - VfoWidget: `setButtonsOccluded` + `m_occludeRestore`/`m_buttonsOccluded`. ## Kept intact (verified) - **`repositionVfoFlags()`** — the *shared* positioner that 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. This was the key correctness invariant. - The whole **GPU panadapter** path (`AETHER_GPU_SPECTRUM`: FFT/waterfall/overlay) — untouched. - The **SmartMTR** render-mode-agnostic fixes from aethersdr#3771/aethersdr#3773/aethersdr#3775. - `event()`/`eventFilter()` non-flag logic (`setMouseTracking`, Mac QRhi teardown in `prepareForTopLevelChange`) preserved. ## Trade-off (unchanged from aethersdr#3803) Re-introduces the aethersdr#3617 main-thread composite cost aethersdr#3695 removed (**~0.82 vs 0.45 cores, +0.37 ≈ +45% main-thread**). Separate from and likely additive to aethersdr#3797. ## Test plan - [x] Builds clean (`cmake --build build --target AetherSDR`); zero dangling references to removed symbols - [ ] Idle VFO-flag text crisp; no hover-pop (Windows) - [ ] Diversity-mode RX-antenna selection works - [ ] Flags position/move correctly with 1, 2, and N panadapters; split-partner side-locking intact (repositionVfoFlags path) - [ ] Dragging a flag: no audio crackle / re-raster crash (aethersdr#3695's original concern) - [ ] 4-panadapter main-thread CPU re-profile (the decision gate) - [ ] a11y linter clean; SmartMTR meter renders correctly Refs aethersdr#3777, aethersdr#3617. Supersedes aethersdr#3803. Plan: see the aethersdr#3777 comment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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 A of the GPU-flag reversion plan attached to #3777. Flips the slice-flag compositing default from GPU sprites to legacy live widgets, so idle flag text is crisp again and the live↔sprite input regressions go away — without deleting anything.
One line, at SpectrumWidget.cpp:613:
Every sprite code path is already gated on
m_gpuFlagMode, and the live-widget path is the documented A/B baseline — so this is a genuine behavioral revert with zero machinery removed. The GPU-sprite path stays fully intact behindAETHER_GPU_FLAGS=1for measurement. (The oldAETHER_NO_GPU_FLAGStoggle is now redundant — live is the default — and pre-existing users of it still get live flags.)What this fixes
The trade-off (stated plainly)
Live widgets re-introduce the #3617 main-thread composite cost that #3695 removed — 0.82 vs 0.45 cores, +0.37 ≈ +45% main-thread (#3695's own measurement). This is separate from #3797 and likely stacks on it on multi-panadapter setups. Per the plan, the 4-panadapter re-profile is the decision gate for whether Strategy B (full excision of the sprite machinery) follows.
Why a flip, not an excision (yet)
So the −45% optimization isn't deleted on the strength of a cosmetic + interaction complaint until the cost is measured in the very area (#3797) that's currently most painful. Reversible in one line if the measurement argues against it.
Test plan
cmake --build build --target AetherSDR)AETHER_GPU_FLAGS=1still brings back GPU sprites (A/B hatch intact)Refs #3777, #3617. Plan: see the #3777 comment.
🤖 Generated with Claude Code