Skip to content

feat(spectrum): GPU-composite slice flags instead of raster siblings#3695

Merged
ten9876 merged 1 commit into
aethersdr:mainfrom
svabi79:fix/panadapter-gpu-flags
Jun 22, 2026
Merged

feat(spectrum): GPU-composite slice flags instead of raster siblings#3695
ten9876 merged 1 commit into
aethersdr:mainfrom
svabi79:fix/panadapter-gpu-flags

Conversation

@svabi79

@svabi79 svabi79 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Context: I picked this up out of interest in the panadapter-performance
topic — it's your own flagship issue, so this PR is as much "here's what I
found and tried" as a finished change. The GPU-sprite path works and is
measured below; there's one visual trade-off I've left explicitly for your
call (see "What was tried"). Take it whichever direction you prefer.

Fixes #3617. GPU-composite the per-slice VFO flags instead of letting them
composite as translucent raster QWidget siblings over the panadapter
QRhiWidget — the cause of the ~75–95% main-thread CPU this issue identifies
(the GPU render itself is ~0.4 ms; the cost is Qt re-blending the translucent
siblings on every waterfall frame).

Each flag is now rasterized into its own GPU texture and drawn as a textured
quad positioned at the flag's current on-screen rect. A flag the operator is
interacting with (cursor over it, a child focused, or a popup open) swaps back
to a real live widget so every control stays fully functional; at rest it is a
cheap GPU sprite.

Measured — test rig: Windows 11, Intel Core i9-13900K / NVIDIA RTX 4090,
Qt D3D11 backend, FLEX-6600. 4 slices, steady state, cursor parked, both at
~30 fps and renderP95 ~0.2 ms:

flags main-thread CPU
legacy live widgets (AETHER_NO_GPU_FLAGS=1) 0.82 cores
GPU sprites (this PR) 0.45 cores
delta −0.37 cores ≈ −45%

Render cost and fps are identical in both modes → the win is purely the
eliminated raster-sibling composite, confirming the issue's diagnosis.
AETHER_NO_GPU_FLAGS=1 keeps the legacy path for A/B comparison.

Note the HW context: this is a fast CPU, and the composite still costs ~0.37
cores for 4 slices — so on slower hardware (or at higher slice counts / 4K–8K
panadapters, where the issue reports the worst pain) the absolute main-thread
relief should be proportionally larger. Cross-platform numbers (esp. the
Linux/Wayland setups in the issue) welcome — aether.perf + the A/B hatch make
re-measurement easy.

Constitution principle honored

Principle XI (Fixes Are Demonstrated) — ships with before/after main-thread
CPU measurements, an AETHER_NO_GPU_FLAGS A/B escape hatch, and an aether.perf
diagnostic line for re-measurement on other platforms.

How it works

  • Sprite raster (off the render thread). grabFlagSprites() runs on the
    refresh timer (~12 Hz) and on live-flag changes — never inside the QRhi
    render callback — and QWidget::render()s each non-live flag into a per-flag
    premultiplied QImage. (Rasterizing a widget inside the QRhi frame re-enters
    the renderer and crashes — see "What was tried".)
  • GPU upload + draw (in-frame). renderGpuFrame() (re)creates/uploads each
    flag's texture (a memcpy; safe in-frame) and draws it as a quad sized 1:1 to the
    texture at the flag's current device-pixel position. Following a dragged flag is
    therefore free (just a quad move) — no per-move re-raster, so no audio stall.
  • Interactivity (hover-swap). The hovered/focused flag is shown as a normal
    live widget; others are sprites. Z-order (active flag on top) and the existing
    overlapping-flag button occlusion are preserved.

Touched files are default review tier: SpectrumWidget.{cpp,h},
VfoWidget.{cpp,h}. No FlexLib/RadioModel/MainWindow core. Clean-room
(Principle IV) — no derived/decompiled sources.

What was tried, and an open trade-off for your decision

The path here, in case it's useful, and one point I'd like your call on:

  1. Shared full-screen flag texture, rasterized in the render callback — worked
    with one slice but crashed with several (QWidget::render inside the QRhi
    frame → beginOffscreenFrame within a still active frame re-entrancy), and
    caused audio crackle when dragging (per-move full re-raster on the main thread).
    Abandoned for the per-flag, off-frame-rasterized "movable quad" above.

  2. Open visual trade-off (needs a maintainer call). A non-hovered flag is a
    GPU sprite rasterized via QWidget::rendergrayscale AA; a hovered flag
    is a live widget → ClearType subpixel AA. So non-hovered flag text reads
    subtly heavier/softer than the crisp hovered one. This is fundamental
    ClearType subpixel rendering cannot be reproduced in an ARGB texture. I tried a
    2× supersample + linear downscale (made it worse — downscale blur) and
    settled on 1:1 texel-per-pixel + NEAREST sampling + pixel-snapped quads
    (removes the blur; crispest a grayscale texture can be), but a subtle
    grayscale-vs-ClearType difference remains. Options, your call:

    • (a) accept the subtle difference (current);
    • (b) render the live/hovered flag grayscale too (QFont::NoSubpixelAntialias)
      → no hover difference, but all flags a touch heavier than today's ClearType;
    • (c) keep flags fully live but opaque (the issue's short-term lever) —
      ~13% only, no GPU-compositing rework;
    • (d) a retained-mode / Qt Quick overlay scene (the issue's flagship
      option) — eliminates the gap entirely, but a much larger change.

    This PR implements the GPU-sprite path (a); happy to take it whichever way you
    prefer.

Test plan

  • Local build passes
  • Behavior verified on a real radio (FLEX-6600): flags render correctly,
    drag-follow is smooth, no audio crackle, interaction works via hover-swap
  • Before/after CPU measured (table above); A/B via AETHER_NO_GPU_FLAGS
  • CI

Checklist

  • Commits signed
  • No new flat-key AppSettings calls (Principle V) — none added
  • Code is clean-room (Principle IV)
  • Meter UI unchanged (still MeterSmoother)
  • Docs — n/a (no user-visible setting; behavior is visually equivalent modulo the AA note above)

… Principle XI.

Fixes aethersdr#3617.

The per-slice VFO flags composited as translucent raster QWidget siblings
over the panadapter QRhiWidget, so Qt re-blended them on the main thread on
every waterfall frame — the ~75-95% main-thread CPU the issue identifies
(the GPU render itself is ~0.4 ms). Each flag is now rasterized into its own
GPU texture off the render thread (refresh timer / on change, never inside
the QRhi frame) and drawn as a textured quad at its current device-pixel
position; following a dragged flag is just a quad move (no re-raster, no
audio stall). A flag the operator is interacting with (cursor over it, a
child focused, or a popup open) swaps to a live widget so every control stays
functional; at rest it is a cheap GPU sprite. AETHER_NO_GPU_FLAGS=1 keeps the
legacy live-widget path for A/B.

Measured (i9-13900K / RTX 4090, D3D11, FLEX-6600, 4 slices, cursor parked,
both ~30 fps and renderP95 ~0.2 ms): legacy 0.82 cores main-thread CPU ->
GPU sprites 0.45 = -45%. Render cost and fps identical in both modes, so the
win is purely the eliminated raster-sibling composite. One open visual
trade-off (sprite grayscale AA vs live ClearType) is documented in the PR for
maintainer decision.

Touched files are default review tier (SpectrumWidget, VfoWidget); no FlexLib
/ RadioModel / MainWindow core. Clean-room (Principle IV).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@svabi79 svabi79 force-pushed the fix/panadapter-gpu-flags branch from a502d5f to ea6d53e Compare June 20, 2026 17:36
@aethersdr-agent

Copy link
Copy Markdown
Contributor

Hi @svabi79 — thanks for digging into #3617; this is a genuinely nice piece of work (measured before/after, an A/B escape hatch, and a thoughtful write-up of the AA trade-off). The CI failure here is not a problem with your GPU approach — it's a build-configuration edge case. Here's the breakdown.

What failed

Only the two Linux jobs failed, both at the Build step:

check-macos, check-windows, and the a11y checks are all green. ✅

Why Linux only

The Linux jobs run in the ghcr.io/ten9876/aethersdr-ci container, which is Ubuntu 24.04 → Qt 6.4.2 (see .github/docker/Dockerfile, qt6-base-dev). macOS/Windows CI use Qt 6.8.3.

That matters because QRhiWidget is Qt 6.7+, so the GPU spectrum path auto-disables below 6.7. From CMakeLists.txt:

# Requires: Qt 6.7+ (QRhiWidget) ...
# Defaults to ON but auto-disables on Qt < 6.7 (e.g. Ubuntu 24.04 ships Qt 6.4).

So on Linux CI, AETHER_GPU_SPECTRUM is OFF and the project compiles the software (QWidget / paintEvent) path instead. <rhi/qrhi.h>, QRhiWidget, QRhiTexture, etc. are all #ifdef AETHER_GPU_SPECTRUM-only (see SpectrumWidget.h:19-25), and your new flag plumbing (FlagSprite, m_flagSprites, the GPU buffers/samplers) is necessarily GPU-only too. On macOS/Windows everything is in scope, so they build fine.

The actual cause

Most of the new code is correctly behind #ifdef AETHER_GPU_SPECTRUM (the constructor timer block is), but a couple of touch points in the diff are not guarded, so they reference GPU-only symbols in the software build:

  1. SpectrumWidget::mouseMoveEvent() calls updateLiveFlag(); unconditionally.
  2. SpectrumWidget::removeVfoWidget() references m_liveFlagSliceId, m_flagSprites, and releaseFlagSprite() unconditionally.

In the non-GPU configuration those names don't exist, which is what breaks the Linux compile. (I'm inferring the exact symbols from the diff + the #ifdef layout — the CI-log tool I have only exposes step status, not the raw compiler line, so grab the precise error: from the Build log to confirm which name it trips on first.)

How to fix

Wrap every new flag reference in #ifdef AETHER_GPU_SPECTRUM so the software build sees none of it — at minimum the two spots above, and make sure the new methods/members (updateLiveFlag, setLiveFlag, grabFlagSprites, releaseFlagSprite, repositionVfoFlags, FlagSprite, m_flag*, m_liveFlag*) are all declared inside the same guard in SpectrumWidget.h. For example:

void SpectrumWidget::mouseMoveEvent(QMouseEvent* ev)
{
    PerfInputScope perfScope("mouseMove");
#ifdef AETHER_GPU_SPECTRUM
    updateLiveFlag();   // hover-swap promote-to-live
#endif
    ...

Reproduce locally (no Qt downgrade needed)

You can hit the exact failure on any Qt by forcing the software path:

cmake -B build-sw -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DAETHER_GPU_SPECTRUM=OFF
cmake --build build-sw -j$(nproc)

That's the cheapest way to keep both configs green going forward — worth doing before each push since CI builds the GPU path (mac/win) and the software path (Linux) separately.

No Copilot/reviewer comments were posted on the PR, so nothing else to fold in. Really solid contribution — this is a one-config-guard fix, not a rethink. 🚀


🤖 aethersdr-agent · cost: $6.0597 · model: claude-opus-4-8

@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 picking up #3617 and for an unusually well-documented PR, @svabi79 — the measured before/after, the AETHER_NO_GPU_FLAGS A/B hatch, the aether.perf trace line, and the honest write-up of what was tried (incl. the in-frame QWidget::render re-entrancy crash) make this easy to reason about. The architecture is sound: rasterize off the render callback at ~12 Hz, upload + draw as positioned quads in-frame, swap to a live widget on hover/focus/popup. CI is green on all six checks (build, macOS, Windows, CodeQL, accessibility, analyze).

A few observations, all non-blocking — your call:

1. Brief blank window when a flag is first created (low). addVfoWidget() does w->show() but doesn't grab a sprite. On the next frame repositionVfoFlags() hides the panel (it isn't the live flag), yet m_flagSprites[id] has no image until the refresh timer fires — so a newly-created slice flag can render as neither a live widget nor a sprite for up to ~80 ms (a flash/blank on slice creation). A grabFlagSprites() at the end of addVfoWidget() (it skips the live flag and is safe off-frame) would close the gap.

2. setButtonsOccluded restore can go stale (low). m_occludeRestore snapshots which external buttons were visible at occlusion time and restores exactly those. If a button's visibility legitimately changes while occluded (e.g. record/play toggling with recording state), un-occluding will restore the old set rather than the current state. Narrow edge case given the buttons involved, but worth a comment noting the assumption.

3. On-by-default AA trade-off (your flagged open question). Since AETHER_GPU_SPECTRUM is ON by default (CMakeLists.txt:27), the grayscale-vs-ClearType difference ships to everyone by default, not just opt-in testers. That raises the stakes on your option (a)/(b)/(d) question — this is squarely a maintainer call, just flagging that "default on" is the reason it matters. My own lean would be to keep (a) but let a maintainer weigh in given it's visible on every install.

On conventions: no new AppSettings/QSettings usage, env-var gating and RAII look correct, and GPU resources are freed in both removeVfoWidget() (per-slice) and releaseResources() (teardown) — consistent with the existing m_ovGpuTex lifecycle. Nice work overall.


🤖 aethersdr-agent · cost: $4.0724 · model: claude-opus-4-8

@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 this, @svabi79 — this is an unusually well-executed contribution. The diagnosis (raster-sibling re-blend, not GPU render, is the cost), the off-frame QWidget::render to dodge the QRhi re-entrancy crash, the per-flag movable-quad design so drag-follow is free, the AETHER_NO_GPU_FLAGS A/B hatch, and the aether.perf trace line all show real care. CI is green across build/macOS/Windows/CodeQL/accessibility.

I traced the pieces that could realistically break and they hold up:

  • Resource lifecycleFlagSprite tex/SRB are freed in both removeVfoWidget and releaseResources, and releaseFlagSprite resets texSize so the next frame recreates lazily. No leak.
  • VBO/draw indexing — the vertex pass packs 16 floats per m_flagDrawOrder entry unconditionally, and draw(4, 1, i*4, 0) indexes by the same i, so the continue-on-null-srb in the draw loop stays consistent. Good.
  • Button occlusion — confirmed the external buttons are parented to the SpectrumWidget (btnParent = parentWidget()), not the flag panel, so hiding the panel for compositing keeps them live; setButtonsOccluded correctly fills the gap that natural widget stacking used to provide. Coherent.

A few things for the maintainer/you to weigh — none are blockers:

  1. Default-ON ships the AA trade-off to everyone. Because AETHER_GPU_SPECTRUM defaults ON and m_gpuFlagMode is opt-out, every user gets grayscale-AA flags at rest (your option (a)) the moment this merges. You already flagged this as a maintainer call, so just making it explicit: the decision between (a) ship-as-is, (b) NoSubpixelAntialias everywhere for consistency, vs gating GPU-flags behind its own default-off env until a call is made, is the gating decision for this PR — the code itself is ready either way.

  2. Residual raster siblings. The floating per-slice buttons (record/play/lock/close/freq label) stay live widgets for all flags, so they still composite as translucent siblings each frame. That's fine and expected (20×20 each ≪ full panel), but worth noting the −45% is the panel win specifically; at high slice counts the button composite is the remaining floor. Not asking you to change it — just so the number isn't read as "all sibling cost eliminated."

  3. Minor: unconditional 12.5 Hz re-raster. The refresh timer QWidget::renders every non-live flag every 80 ms regardless of whether geometry/content changed. Justified while the S-meter is live, but if a flag is steady you're paying a raster you could skip with a dirty flag on the VfoWidget. Pure optimization, leave it if it's not showing up in profiles.

Nice work — I'd be happy to see this land once the default-on question is settled.


🤖 aethersdr-agent · cost: $4.3492 · model: claude-opus-4-8

@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 picking up #3617 — this is a careful, well-instrumented piece of work, and the writeup (measured A/B with AETHER_NO_GPU_FLAGS, the "what was tried" section, and the explicit open trade-off left for a maintainer) is exactly the kind of contribution that's easy to evaluate. The diagnosis (cost is Qt re-blending translucent raster siblings, not the GPU render) matches the issue, and the per-flag off-frame raster → movable GPU quad design avoids both the re-entrancy crash and the per-move re-raster audio stall. Nice.

I went through the whole diff; no blocking issues. A few notes, mostly for your/the maintainer's awareness rather than changes I'd require:

Scope / conventions — clean. All four touched files are in the stated tier, everything new is gated behind #ifdef AETHER_GPU_SPECTRUM (the VfoWidget::setButtonsOccluded members are unconditional but trivial and only exercised from the GPU path). No QSettings/flat-key AppSettings introduced; the A/B hatch is a runtime env var, appropriate for a dev lever. RAII is sound — FlagSprite GPU resources are freed in releaseFlagSprite() on slice removal and in releaseResources(), and the resize path mirrors the existing m_ovGpuTex deferred-release pattern.

Indexing is consistent — the vertex-build loop writes a quad for every m_flagDrawOrder[i] at i*4, and the draw loop uses firstVertex = i*4; the !sit->srb skip in the draw loop leaves an unused slot but never misaligns. verts[kMaxFlagSprites*16] is bounded by nDraw = qMin(..., kMaxFlagSprites). Good.

Worth considering (efficiency): m_flagRefreshTimer fires unconditionally at 12.5 Hz while m_gpuFlagMode is set, and each tick grabFlagSprites() does a full QWidget::render() of every non-live flag plus an update(), regardless of whether any flag content (S-meter/freq) actually changed or whether the panadapter is even visible. On a genuinely idle radio (no FFT frames, parked cursor, or the panadapter on a hidden tab) this is a new steady wakeup + raster that the legacy path didn't have. Your net-CPU win still holds, but it might be worth pausing the timer when the widget !isVisible(), and/or skipping the re-raster when nothing dirtied — a cheap further reduction on the slow-hardware/idle case the issue cares about.

DPI corner: grabFlagSprites() sizes the sprite image from devicePixelRatioF(), while the quad placement uses fdpr = renderTarget pixelSize / width(). These agree on normal setups; under fractional/mismatched scaling they could diverge by a pixel of position (size stays 1:1 texel). Minor, and you've clearly thought about the snapping — just flagging.

On the open AA trade-off (your option a/b/c/d): that's genuinely a maintainer call and you've framed it well; I won't try to decide it in review. One framing note for whoever picks it up: option (a) ships the grayscale-vs-ClearType difference on by default the moment AETHER_GPU_SPECTRUM is built, so the decision is effectively "ship (a) now, revisit later" unless the default is flipped — worth being explicit about that when merging.

CI is green across the board, including macOS/Windows on Qt 6.8 where this path is actually compiled and CodeQL-analyzed (the Linux build job is on the Qt 6.4 floor and skips it, as expected). Solid work — thank you.


🤖 aethersdr-agent · cost: $3.9969 · model: claude-opus-4-8

@svabi79

svabi79 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up: the triage sweep that closed #3617 ("slice flags are now GPU-composited … verified at HEAD") looks like it verified against this PR's description rather than main. At origin/main the per-slice flags are still raster VfoWidget children (new VfoWidget(this) in addVfoWidget; zero FlagSprite/grabFlagSprites symbols) — the exact translucent-sibling re-blend #3617 describes. Could you reopen #3617 and take a look here when you get a chance? CI's green on all six and it's mergeable, and I'm happy to settle the open default-on AA question whichever way you prefer.

@NF0T NF0T left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Human-directed independent review (Claude / @NF0T) | Comment — no blocking issues found; one unresolved maintainer decision gates merge


This review was conducted by Claude under direction from @NF0T, independently of the three bot passes already on record. Those reviews are treated as unverified starting points. The full diff was read, every structural claim was checked against the codebase, and each resource path was traced individually before forming a conclusion. The AA visual trade-off question @svabi79 explicitly left for a maintainer has also been surfaced with a clear framing below, since no prior review has resolved it.


Preliminary note — issue #3617 was closed incorrectly

Before the code: ten9876 closed #3617 with "verified against the codebase at HEAD" — but the fix is in this branch only. I independently confirmed by grepping every new symbol introduced by this PR (FlagSprite, m_flagSprites, grabFlagSprites, m_liveFlagSliceId, kMaxFlagSprites, etc.) against the current main: zero results. The codebase at HEAD still uses new VfoWidget(this) in addVfoWidget with no GPU-sprite path. The issue was closed against the PR description, not the actual tree. The "Fixes #3617" auto-close will be a no-op when this lands. Worth the maintainer noting before merge.


What I verified and how

#ifdef AETHER_GPU_SPECTRUM guard coverage — verified exhaustively against the full diff.

The bot's first comment warned that mouseMoveEvent and removeVfoWidget were missing guards and breaking Linux CI. I verified the final commit's guard layout in full:

Code section Guarded?
Constructor timer / m_gpuFlagMode init
removeVfoWidget() GPU cleanup (m_liveFlagSliceId, m_flagSprites, releaseFlagSprite)
updateLiveFlag(), grabFlagSprites(), releaseFlagSprite() implementations
repositionVfoFlags() implementation
renderGpuFrame() VBO build and flag draw loop
releaseResources() additions
mouseMoveEvent() updateLiveFlag() call
SpectrumWidget.h declarations and member variables

VfoWidget::setButtonsOccluded is unconditional in VfoWidget.{h,cpp} — but it is only ever called from inside #ifdef AETHER_GPU_SPECTRUM blocks in SpectrumWidget. The software path never reaches it. This is correct. The original CI failure was fixed before the commit landed; all six CI checks (build, macOS, Windows, CodeQL, accessibility, analyze) are now green.

Resource lifecycle — traced individually, no leaks found.

The bots asserted RAII is sound without showing the trace. I followed each new QRhi resource through creation and deletion:

  • m_flagQuadVbo: created in GPU init; delete m_flagQuadVbo; m_flagQuadVbo = nullptr; in releaseResources(). ✓
  • m_flagSampler: same pattern in releaseResources(). ✓
  • Per-flag FlagSprite.tex / .srb: freed via releaseFlagSprite(), called in removeVfoWidget() on per-slice removal and in the releaseResources() loop over m_flagSprites. ✓

No orphaned QRhi resources.

VBO/draw indexing — verified consistent.

Vertex buffer: verts[kMaxFlagSprites * 16] — 4 verts × 4 floats × 16 max sprites. Written at verts + i*16 per quad. Draw call: draw(4, 1, i*4, 0)firstVertex = i*4. Byte offsets agree: i × 4 verts × 4 floats × 4 bytes = i × 64 bytes matches verts[i*16] × 4 bytes per float. ✓

The continue-on-null-srb in the draw loop skips drawing a slot but leaves a zero-size quad in the VBO for that position. Because firstVertex is absolute (i*4), not relative to drawn slots, no misalignment occurs. ✓

Button occlusion — verified safe.

setButtonsOccluded(true) snapshots visible buttons into QList<QPointer<QWidget>> and hides them; false restores from the snapshot. QPointer is the correct choice — if a button is deleted while occluded, the pointer goes null and the restore loop's null-check skips it cleanly. The idempotency guard prevents double-hide or double-restore. ✓

Known limitation (already noted by the bots): if a button's visibility legitimately changes while occluded — e.g. a recording-state toggle — un-occluding restores the pre-occlusion snapshot rather than current state. Narrow edge case; worth a comment in the code noting the assumption. Not a blocker.


Non-blocking observations

80ms blank on first slice creation. addVfoWidget() shows the widget but doesn't immediately rasterize a sprite. If the new flag is not the live flag, repositionVfoFlags() hides the panel and the sprite isn't available until the timer fires up to 80ms later — a brief blank. In practice, newly-created slices are typically the active slice and show as a live widget, so this may not manifest in normal usage. A grabFlagSprites() call at the end of addVfoWidget() would close the gap if it becomes visible in testing.

Unconditional 12.5 Hz re-raster. The timer fires every 80ms and re-rasterizes every non-live flag regardless of whether content or geometry changed, and regardless of panadapter visibility. The net CPU saving still dominates over the legacy path, but stopping the timer when !isVisible() would reduce the idle floor further — an easy follow-up optimization.


The merge gate — AA visual trade-off requires a maintainer decision

This is the one unresolved item, and @svabi79 has flagged it honestly and completely. Summarising for the record:

At rest, a flag is a GPU sprite rasterized via QWidget::render → grayscale AA. On hover/focus/popup it is a live widget → ClearType subpixel AA. The two paths produce visibly different text rendering. Because AETHER_GPU_SPECTRUM defaults ON and m_gpuFlagMode is opt-out, this difference ships to every GPU user the moment this merges. The author tried 2× supersample + downscale, found it worse, and settled on 1:1 NEAREST sampling as the crispest a grayscale texture can be — but the gap is fundamental and cannot be closed at the texture level.

The four options the author laid out are clear and complete: (a) ship as-is and accept the difference; (b) force QFont::NoSubpixelAntialias on live flags too for consistency; (c) keep flags fully live but opaque for a smaller (~13%) CPU win without the GPU-compositing rework; (d) retained-mode / Qt Quick overlay — eliminates the gap but a much larger change.

Per project governance, visual design decisions belong to the maintainer. The three bot reviews each flagged this without making the call — none of them have the standing to. This review doesn't either. What I can say from the code side is that the implementation is ready in any of the four directions: (a) needs nothing, (b) is a one-line font-hint change, (c) would largely revert the GPU-flag path, (d) is a separate project. The decision itself is the only thing between this PR and a merge.

Pinging @ten9876 directly: the code here is sound, the performance win is real and measured, and the author has done the work of laying out the trade-offs clearly. The AA question is yours to call.


Summary

Dimension Status
#ifdef guard coverage ✅ All new GPU symbols correctly guarded in final commit
Resource lifecycle ✅ No leaks — flagQuadVbo, flagSampler, FlagSprite.tex/.srb all properly freed
VBO/draw indexing ✅ Consistent — absolute firstVertex indexing, no misalignment on null-srb skip
Button occlusion ✅ QPointer used correctly; snapshot-stale limitation is narrow and safe
CI ✅ All six checks green
Issue #3617 closure ⚠️ Incorrectly closed against PR description, not main
Slice-creation blank ⚠️ Up to 80ms, low-impact, easy fix available
Idle re-raster ⚠️ Optimization opportunity for a follow-up
AA visual trade-off ⏳ Unresolved — maintainer decision required before merge

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving — the one merge gate (the AA visual trade-off) is resolved: maintainer decision is option (a), ship as-is. The grayscale-at-rest / ClearType-on-hover difference is accepted for now to get the #3617 performance win to users immediately; the AA-consistency refinement (option b) plus the two idle-re-raster optimizations are tracked as a follow-up in #3746.

The code is sound — concurring with @NF0T's independent trace and the three bot passes, and I spot-confirmed:

  • All new GPU symbols correctly behind #ifdef AETHER_GPU_SPECTRUM (the earlier Linux-CI miss on mouseMoveEvent/removeVfoWidget was fixed before the final commit).
  • No QRhi leaks — m_flagQuadVbo, m_flagSampler, and per-flag FlagSprite.tex/.srb all freed in releaseFlagSprite()/removeVfoWidget() and releaseResources().
  • VBO/draw indexing consistent (absolute firstVertex = i*4 vs the i*16-float packing; null-srb continue never misaligns).
  • Button occlusion QPointer snapshot/restore correct.

CI green on all six. This is the #3617 fix — ~−45% main-thread CPU from eliminating the translucent raster-sibling re-blend, directly relevant to the slow-hardware/laptop case the issue cares about.

(Process note: #3617 was reopened — it had been closed against the PR description rather than main, where the fix doesn't yet exist. This PR's Fixes #3617 will close it correctly on merge. Thanks @NF0T and @svabi79 for catching that.)

Excellent, well-instrumented work, @svabi79.

@ten9876 ten9876 merged commit b27dd9b into aethersdr:main Jun 22, 2026
6 checks passed
ten9876 added a commit that referenced this pull request Jun 23, 2026
…ender blank (#3760)

## Summary

Disables the identity `QGraphicsOpacityEffect` on fully-enabled SmartMTR
option rows so they don't render blank when multiple VFO flags are
GPU-composited.

A fully-enabled option row keeps its `QGraphicsOpacityEffect` at opacity
`1.0` (identity). With the GPU-composited slice flags from #3695, an
identity opacity effect can render the row's contents blank. The fix
disables the effect entirely while it is at full strength (a disabled
`QGraphicsOpacityEffect` is a no-op, so the row paints directly) and
keeps it installed only while it is actually dimming the row.

## Provenance

This is the net-new fix extracted from #3757. The remainder of that PR
re-implemented SmartMTR `#3750` hardening that already landed via #3751
(H1), #3752 (M1–M4), and #3753 (L1–L3); #3757 is being closed in favor
of this focused change to avoid re-churning already-merged code. Part of
#3750.

## Test plan

- [x] Local build passes (`cmake --build build --target AetherSDR`)
- [ ] Behavior verified on a real radio if applicable — visual (open the
SmartMTR option panel with multiple flags up; rows render, not blank)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
ten9876 pushed a commit that referenced this pull request Jun 23, 2026
…prite on new slice (#3746) (#3764)

## Summary

Two of the idle re-raster follow-ups tracked in #3746 (itself a
follow-up to the GPU slice-flag work in #3695). **Refs #3746 — this does
not close it:** option (b) AA-consistency stays deferred per the issue's
own rationale (ship (a) first, revisit after real-world exposure), and
the dirty-skip was measured and deferred (see below).

**③a — pause the flag re-raster timer when the panadapter isn't
presented.** The 12.5 Hz refresh timer re-rasterized every non-live
slice flag every 80 ms regardless of whether the panadapter was on
screen. It now runs only while the panadapter is visible and not
minimized. A minimized top-level keeps child `isVisible() == true`, so a
`WindowStateChange` watcher on the top-level handles minimize/restore;
`Show`/`Hide` and `ParentChange` (float/dock reparent) handle the
hidden-tab and reparent cases — all funnelled through
`updateFlagRefreshTimer()`. The watcher reattaches on reparent and
detaches in `prepareForTopLevelChange()`.

**② — rasterize a new slice's flag sprite immediately.** A newly created
non-live flag had no GPU sprite until the next timer tick:
`addVfoWidget()` showed the widget but didn't grab it, and
`repositionVfoFlags()` hides the non-live panel — so for the gap the
flag was neither a live widget nor a sprite (a brief blank).
`grabFlagSprites()` at the end of `addVfoWidget()` closes it.

Plus a gated `aether.perf` trace (`FlagGrab` / `FlagCreated` /
`FlagFirstGrab`, zero cost when the category is off) used to measure all
of this — committed so the numbers are reproducible on any platform.

## Measured (i9-13900K / RTX 4090, 4 slices, `aether.perf`)

| change | before | after |
|---|---|---|
| ③a — **minimized** window | ~2.1 ms/tick × 12.5 Hz = **~27 ms/s**
main-thread, ongoing | **0** (timer stopped) |
| ③a — **visible** (regression check) | 2.12 ms/tick (p95 2.46) | 2.23
ms/tick (p95 2.54) — unchanged |
| ② — new non-live slice blank gap | **~41 ms** (max 49) | **~3 ms**
(synchronous grab) |

Each change made and measured one at a time.

## Measured and deferred — dirty-skip (③b)

Also evaluated skipping the re-raster of a flag whose content is
unchanged. Measured the skip *ceiling* by hashing each render: each
slice's S-meter animates on its own noise floor, so even on a dead /
dummy-load input only ~2.7 of 4 flags were ever byte-identical
tick-to-tick (fewer on an active band). The realizable idle saving is
**negligible** (≤ ~0.02 cores, idle-only), and a correct *pre-render*
skip would need fragile per-widget dirty-tracking. Not worth it versus
③a, which already removes the larger hidden/minimized waste. Left
deferred in #3746.

## Constitution principle honored

**Principle XI — Fixes Are Demonstrated.** Every change has a
before/after `aether.perf` measurement (tables above), taken one change
at a time.

## Test plan

- [x] Local build passes (Windows, Qt 6.8.3 msvc2022_64, GPU path on)
- [x] ③a — timer STOP/START verified on minimize/restore and hidden tab;
0 grabs while not presented; visible-period grab cost unchanged (no
regression)
- [x] ② — new-slice blank gap 41 ms → ~3 ms, measured
- [ ] Existing tests pass (CI)
- [ ] macOS / Linux GPU paths — same `QRhiWidget` / `QWidget` code;
cross-platform numbers welcome

## Checklist

- [x] Commits are signed (SSH)
- [x] No new flat-key `AppSettings` calls (N/A)
- [x] Code is clean-room (Principle IV)
- [x] All meter UI uses `MeterSmoother` (N/A — no meter changes)
- [x] Documentation updated if user-visible behavior changed (N/A — no
visible behavior change; ② removes a transient blank)
- [x] Security-sensitive changes reference a GHSA if applicable (N/A)

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
ten9876 added a commit that referenced this pull request Jun 26, 2026
…ve (#3777) (#3806)

## 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}` and
`VfoWidget.{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)
- 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 #3771/#3773/#3775.
- `event()`/`eventFilter()` non-flag logic (`setMouseTracking`, Mac QRhi
teardown in `prepareForTopLevelChange`) 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
- [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 (#3695's
original concern)
- [ ] 4-panadapter main-thread CPU re-profile (the decision gate)
- [ ] a11y linter clean; SmartMTR meter renders correctly

Refs #3777, #3617. Supersedes #3803. Plan: see the #3777 comment.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

perf(panadapter): main-thread CPU ~75–95% is Qt CPU-side compositing of translucent child widgets over the QRhiWidget (render is 0.4 ms)

3 participants