feat(gui): stacked-trace spectrum stream panadapter render mode#3899
Conversation
3998eb8 to
822f4d3
Compare
822f4d3 to
7fff14e
Compare
There was a problem hiding this comment.
Nice work, @jensenpat — this is a well-structured feature. The static-grid-plus-ring-texture design (no geometry rebuild on pan/zoom), the three-tier graceful degradation (GPU mesh → cached-image quad → CPU QPainter), the R16F capability guard, the thorough qWarning+fallback on every GPU init step, and the careful "3D replaces only the spectrum trace, everything else stays 2D" framing all read cleanly. AppSettings is used correctly (the Display* keys follow the existing per-panadapter pattern), releaseResources() tears down every new GPU resource, and CI is green on Windows/macOS/CodeQL. A few things worth a look before merge:
1. GPU mesh uploads only the single newest row per frame — skipped rows are still drawn (likely bug).
In renderGpuFrame() the height texture is updated only when head != m_dssMeshHeadUploaded, and only that one head row is uploaded:
if (m_dss.rowCount() > 0 && head != m_dssMeshHeadUploaded) {
... upload one row at DestinationTopLeft(0, head) ...
m_dssMeshHeadUploaded = head;
}But pushRow() runs per incoming FFT frame in updateSpectrum(), while this upload runs per repaint. If the FFT push rate ever outruns the repaint rate (coalesced paints under load), the head advances by more than one ring slot between uploads. The intermediate slots are within drawnRows and are drawn, but their texels were never uploaded → stale/uninitialized data recedes across the surface. Consider uploading every row between the previously-uploaded head and the current head (the slots are contiguous mod kRows), rather than just the latest. Worth confirming on a fast FFT rate with a throttled repaint.
2. zCurve floor-expansion is applied only on the GPU path, not the CPU fallback (render-path parity gap).
dss_mesh.vert does s = pow(s, max(zCurve, 0.05)) to lift the floor→signal band, but DssRenderer::rebuild() computes strength linearly with no equivalent curve. So the GPU mesh surfaces the noise-floor carpet (the headline feature) while the cached-image/CPU fallback flattens it — the two paths render visibly different surfaces. (Slope-shading and the outline styling also differ, but those are more cosmetic.) This is a recurring bug class here; matching the curve in rebuild() would keep the fallback faithful.
3. dss_mesh.vert: the half-texel offset the comment describes isn't actually applied.
// Sample the ring-buffered height row (+half texel to hit the row centre).
float texY = fract(rowOffset + v);rowOffset = head/rows and v = rr/rows, so texY lands on the row boundary (head+rr)/rows, not the center. With Nearest filtering, fp rounding at an exact boundary can floor to the neighbouring row (off-by-one row sampling). Either add the + 0.5/rows the comment promises or drop the comment.
4. Background "Clear → disable" looks out of scope.
The right-click-Clear-disables-background change (backgroundImageDisabled signal, the "none" persistence in MainWindow_Wiring.cpp, and the bgPath != "none" guard in MainWindow_Session.cpp) is unrelated to the stacked-trace render mode. It's reasonable on its own, but it's a distinct user-facing behaviour that isn't mentioned in the PR description — consider splitting it out or at least documenting it so it's reviewed on its own merits.
None of these block the 2D path (unchanged) or compilation. (1) and (2) are the ones I'd most want resolved since they affect the actual 3D output. Thanks for the detailed write-up and the cross-platform care.
🤖 aethersdr-agent · cost: $6.0567 · model: claude-opus-4-8
ten9876
left a comment
There was a problem hiding this comment.
Automated review (8 finder angles + verify). Nice feature — a GPU height-map mesh with a CPU fallback. Requesting changes for two user-visible correctness issues and a persistence bug; the rest are consistency/perf. Most fixes here are design-level rather than one-liners, so the inline notes are prescriptive fixes (one has an applicable suggestion).
Blockers:
- #1 Overlays + click-to-tune are wrong in 3D. Markers, squelch/auto-SQL lines, and band-plan labels are drawn over the receding-trapezoid surface using the 2D linear-x / dynamic-range-y mappings, and click-to-tune maps screen-x→freq linearly. In 3D they all point at the wrong place. Either project them through the same trapezoid the shader uses, or suppress freq/dB-anchored overlays in 3D.
- #2 GPU height texture only uploads the current ring head, so when the FFT rate exceeds the frame rate the rows pushed between frames render stale. Upload every row from the last-uploaded head to the current head.
- #4 3D mode/floor don't survive a session reload (
wirePanLifecycleomits them) and thesyncDisplaySettings()default-arg trap desyncs the controls.
Also worth addressing: the GPU palette LUT ignores the color-gain/black-level/min-dBm controls (#3); the CPU fallback renders a different surface than the GPU path — it omits the zCurve lift and uses a different depth parametrization (#5); switching into 3D starts blank (history only fed in 3D) and KiwiSDR uses a different floor anchor (#6); the software path rebuilds an uncapped full-res QImage every frame (#7, suggestion inline); and the mesh samples texels on their boundaries rather than centers (#8).
Minor (not inlined): GPU mode resources stay resident for the session after the first 3D visit; the perspective constants are triplicated (DssRenderer constexpr / SpectrumWidget magic floats / shader uniforms) and the floor-anchor math is copy-pasted CPU↔GPU (will drift); the hand-maintained 12 * sizeof(float) UBO size and kCols-vs-QVarLengthArray<…,1024> coupling are silent footguns; the full 2D FFT-trace vertex pipeline still runs (and is discarded) every 3D frame; and braces-on-control-flow (AGENTS.md C++ style) is omitted throughout the new DssRenderer.cpp / SpectrumWidget.cpp code.
Add a perspective stacked-trace spectrum stream as a panadapter render mode, selectable from the Display pane (Spectrum: 2D / 3D). In 3D the spectrum region becomes a rolling history of FFT traces receding into the distance, drawn over the existing waterfall; the 2D path is unchanged. Built on the existing Qt + QRhi infrastructure and designed for macOS, Windows, and Linux. Features: - Perspective stacked-trace surface: 96-row x 768-col rolling history, newest trace full-width at the front, older traces receding into a narrower/higher trapezoid, painter's-algorithm occlusion. - GPU height-map mesh: a static vertex grid samples a ring-buffered height texture, so pan/zoom/resize rebuild no geometry (fluid pan); one ~1.5 KB texture row uploaded per frame instead of re-rasterizing the surface. - Noise-floor-anchored amplitude with a floor->peak colour gradient and a non-linear floor-expansion curve, so the floor is visible and coloured up through the peaks. Palette follows the waterfall colour scheme. - Dim depth-faded per-row trace outline. - Broadband impulse rejection (temporal median) so a 1-frame interference burst never enters the height history as a full-height receding artifact. - Overlays preserved: spots, memories, slice/VFO markers, nav hints, band plan, and the scales render as in 2D (the surface replaces only the spectrum trace). - Display-pane controls (persisted per-panadapter): the 2D/3D selector and a "3D Floor" depth slider. Cross-platform: - No platform-specific shader code. The new GLSL 440 shaders bake via qsb to SPIR-V / GLSL / HLSL / MSL (same variant set as the existing spectrum shaders). - Backend selection unchanged: Metal on macOS (#ifdef Q_OS_MAC), platform default elsewhere (D3D11 / OpenGL-Vulkan); the surface rides the same QRhiWidget. - Portable choices: no geometry/tessellation shaders, occlusion by draw order (no depth-attachment dependency), 1-px lines, R16F guarded by a capability check. - Graceful degradation: GPU mesh -> GPU cached-image quad (RGBA8) -> CPU QPainter surface (also covers builds where the GPU spectrum is compiled out). Verified live RX on macOS/Metal (dense/smooth surface, floor visible with gradient, dim outline, fluid pan, impulse rejection; full FFT rate, ~1 ms UI lag, 0% frame loss; 2D round-trips unchanged) and the impulse rejection with a standalone render harness. Windows/Linux runtime not yet eyeballed; CI covers the build + shader bake. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
7fff14e to
b803a41
Compare
|
Thanks both for the thorough review — all points addressed in Blockers
Consistency / perf
Minors
Background "Clear → disable" scope. Documented in the PR description (it's a small, self-contained Display-pane affordance) rather than split out, since it's already squashed into this one commit — happy to break it into its own PR if you'd prefer. |
|
Hi @jensenpat — thanks for this, the stacked-trace surface is a genuinely cool addition and the cross-platform care in the PR description shows. 👏 Quick hand from CI triage on the red build. What failedOn commit
So this is a Linux-only compile error, identical across the two Linux jobs (they share Why Linux only (and what it's not)I want to head off a tempting-but-wrong theory: this is not the usual "GPU spectrum compiled out on old Linux Qt" divergence. That leaves the most common reason this exact repo goes "green on my Mac, red on the Linux container": GCC + libstdc++ is stricter than Clang + libc++. libc++ transitively pulls in a lot of standard headers that libstdc++ does not, and GCC rejects a few constructs Clang accepts. The only new translation units in this PR are Most likely culprits, in rough order:
Getting the exact lineThe fastest path is the failed-job log directly — the compiler prints
To reproduce locally, build against GCC + Qt ≥ 6.7 (the macOS/Clang build hides it). If you only have a Mac handy, a quick container repro: (You'll want the Qt 6.8.3 install the workflow does, or point On the Copilot review commentsWorth flagging so you don't chase them as the build break: the Copilot notes (overlay projection through the 2D axes in 3D, the stale-ring catch-up upload, the palette LUT ignoring color-gain/black-level, per-pan render-mode persistence, the Once Linux compiles, please re-push and I'll keep an eye on 🤖 aethersdr-agent · cost: $9.1184 · model: claude-opus-4-8 |
The 3D surface rendered near-monochrome: the palette LUT was baked through dbmToRgb(), whose waterfall black-level window forces everything below ~(min + (125-black)*0.4) dBm to black. Since the surface spans down to the noise floor, the bottom ~30% of every curtain was clipped to black and only the strongest signals showed any colour — white trace lines over black. - Map the surface's strength axis (noise floor -> ref) across the FULL colormap gradient instead, via a shared dssStrengthToRgb() helper used by both the GPU LUT bake and the CPU fallback (buildDssImage), so both paths colour identically. - Add a user-adjustable "3D Gain" slider (0-100, default 70) to the Display overlay menu: gamma-shapes the strength->colour lookup so colour can be pulled down toward the noise floor or kept on strong signals only. Persisted per-panadapter (Display3DGain), with reset wiring. Also addresses review nits in the same code: - null-guard the CPU fallback against a partial GPU init (m_dssGpuTex/SRB/ sampler) so it can't deref null on OOM/device-loss; - reuse a member qfloat16 row buffer for the mesh height upload (COW-safe; no per-frame allocation in the common single-new-row case); - route the 3DSS-path warnings through qCWarning(lcGui) instead of bare qWarning(); - hoist the duplicated kDssMaxW/kDssMaxH caps to one class constant; - document the GUI-thread-only access invariant on m_dss. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The CI build job compiles with AETHER_GPU_SPECTRUM off (Qt < 6.7), where the CPU paint path still builds the 3DSS surface but the GPU-guarded members it referenced were compiled out: - m_dssZCurve was inside the #ifdef block yet used by buildDssImage() (CPU) — a pre-existing break for GPU-off builds; moved it out alongside m_dss. - kDssMaxW/kDssMaxH (this branch hoisted them into the guard) are used by the CPU paintEvent fallback; moved them out too. - setDssGain() reset m_dssLutToken (a GPU-only member) unconditionally; guarded that line — the CPU surface re-colours via m_dss.invalidate() regardless. Verified SpectrumWidget.cpp now compiles cleanly both with and without AETHER_GPU_SPECTRUM. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ten9876
left a comment
There was a problem hiding this comment.
Approved by the maintainer (Jeremy / KK7GWY). 3D stacked-trace render mode: colour mapping fixed (full colormap across the strength axis), user-adjustable 3D Gain control added, review nits addressed, and the GPU-off build (pre-existing break) fixed. CI fully green.
#3961) ## Release documentation prep for **v26.7.1** (2026-07-02) 29 commits since v26.6.5. Docs-only + version bump — no code behavior change. ### Version bump 26.6.5 → 26.7.1 The three canonical locations (per `AGENTS.md`): - `CMakeLists.txt:2` — `project(AetherSDR VERSION 26.7.1)` → `AETHERSDR_VERSION` (the app's version string) - `README.md` — Current version line - `AGENTS.md` — Current version line ### `CHANGELOG.md` Promoted `[Unreleased]` → **`## [v26.7.1] — 2026-07-02`** in house style (v-prefix, em-dash, "N commits since…" opener + headline), authored from all merged PRs and categorized: - **Added:** 3D stacked-trace spectrum (#3899), 3D dBm scale (#3937), in-process NVIDIA BNR + AFX download-on-demand (#3902), TX meter mouse-over readouts (#3936), SWR manual sweep range (#3885), CW sidetone capture (#3895), KiwiSDR metadata scaffolding (#3898), bridge verbs (#3920) - **Changed:** BNR container/NIM backend removed, 60 fps + per-pixel GPU FFT trace (#3958), FlexLib-sourced model capabilities (#3954/#2177), RX speaker-latency cut (#3897), Display pane sections (#3935) - **Fixed:** #3922, #3926, #3941, #3940, #3942, #3921, #3903, #3892, #3924, #3891, #3890, #3900, #3947 - Fresh empty `[Unreleased]` restored above the release block. Internal/CI/docs/self-fix PRs (#3931/#3916/#3934/#3929) omitted per house style. ### `ROADMAP.md` - Cycle header → "post-v26.7.1" - NVIDIA BNR removed from **In flight** (shipped this cycle) - Five v26.7.1 highlights prepended to **Recently shipped** ### `README.md` - Highlights: GPU spectrum bullet now notes the **per-pixel FFT trace at up to 60 fps** and the **3D stacked-trace** spectrum mode ### Reviewer notes - Touches protected paths (`*.md` + `AGENTS.md` Tier-1 + `CMakeLists.txt`) — needs `@aethersdr/infrastructure` sign-off. - Tagging `v26.7.1` and cutting the release build are **not** part of this PR (docs prep only). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Adds a stacked-trace spectrum stream as a panadapter render mode, selectable from the Display pane (Spectrum: 2D / 3D). In 3D the spectrum region becomes a perspective stacked-trace surface — a rolling history of FFT traces receding into the distance — while the waterfall, frequency scale, and all overlays keep working underneath. The 2D path is unchanged. Built on the project's existing Qt + QRhi infrastructure and designed to run on macOS, Windows, and Linux.
Features
Cross-platform
qsb) to the full variant set — SPIR-V (Vulkan), GLSL (OpenGL/GLES), HLSL (Direct3D 11), and MSL (Metal) — the same set the existing spectrum shaders produce.#ifdef Q_OS_MAC), the platform default elsewhere (Direct3D 11 on Windows, OpenGL/Vulkan on Linux). The new surface rides the sameQRhiWidget.QPaintersurface, so it still renders where the height texture or QRhi itself isn't available (including older-Qt builds where the GPU spectrum is compiled out).Verification
💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat