[codex] Fix zoomed FFT spectrum rendering#3836
Conversation
f411b15 to
8a89018
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a panadapter FFT rendering regression when zoomed (trace collapsing into a flat “floor” with only peaks visible), and improves visual smoothness of steep FFT peaks by making both the FFT decoding and the trace rendering more robust to stale/tiny y_pixels and display scaling.
Changes:
- Prevent stale/tiny radio
y_pixelsvalues from corrupting FFT dBm decoding; request panadapter dimensions in render-device pixels and re-push dimensions after radio resets. - Preserve/reproject the current FFT trace during zoom/range changes to reduce flicker and improve continuity.
- Add display-only spatial smoothing + interpolation and GPU line edge antialiasing (feather/core line passes + shader coverage) for smoother trace rendering; reset now restores FFT line width to
2.0.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/models/RadioModel.cpp | Ignores tiny y_pixels echoes for decoding and triggers dimension re-push when radio resets to defaults. |
| src/core/PanadapterStream.cpp | Makes FFT decode resilient to mismatched/stale y_pixels by deriving an effective Y scale from the frame content. |
| src/gui/MainWindowHelpers.cpp | Sends panadapter dimension requests in device pixels (DPR-aware) and maintains a Y floor for better zoom precision. |
| src/gui/MainWindow_Wiring.cpp | Ensures display reset restores the existing DisplayFftLineWidth setting to the default 2.0. |
| src/gui/SpectrumWidget.h | Adds helper API for generating a display trace at the desired point count. |
| src/gui/SpectrumWidget.cpp | Implements trace reprojection continuity, display-trace interpolation/smoothing, and GPU line feathering/AA pipeline changes. |
| resources/shaders/spectrum.vert | Adds inEdge attribute to pass edge distance for AA coverage. |
| resources/shaders/spectrum.frag | Applies derivative-based edge coverage to antialias FFT line edges. |
| batch->updateDynamicBuffer(m_fftLineVbo, 0, | ||
| n * 2 * kFftVertStride * sizeof(float), lineVerts.constData()); | ||
| n * 4 * kFftVertStride * sizeof(float), lineVerts.constData()); |
| // Display-only spatial smoothing: m_smoothed is temporal, so it reduces | ||
| // frame shimmer but leaves adjacent-bin stair steps intact. | ||
| QVector<float> displayBins(srcCount); | ||
| displayBins[0] = bins[0]; | ||
| displayBins[srcCount - 1] = bins[srcCount - 1]; |
There was a problem hiding this comment.
Thanks @rfoust — this is a careful, well-reasoned fix. The decode-recovery heuristic, render-device-pixel request, smoothing-reset-on-zoom flag, and the two-pass feather/core line with shader-side edge AA all hang together coherently, and CI is green across build/macOS/Windows/CodeQL/a11y. The VBO sizing checks out: n is capped at kMaxFftBins (8192) which matches the trace point cap, so the n*4 line / n*2 fill writes stay within the buffers. The Catmull-Rom upsampler clamping to [min(p1,p2), max(p1,p2)] is the right call — it prevents overshoot artifacts on a dBm trace.
A couple of notes, both confirming the Copilot findings — neither is a correctness bug, just per-frame waste worth tightening:
1. Line VBO is uploaded every frame even when the line is off (Copilot, SpectrumWidget.cpp:7817). When m_fftLineWidth == 0, writeLineStrip() is skipped (line 7788) but m_fftLineScratch is still resized (7678) and updateDynamicBuffer(m_fftLineVbo, …, n*4*stride) still runs (7817). The draw is correctly gated at 7882, so this is harmless visually — the uploaded data is stale/zero scratch, not uninitialized-memory UB as the bot phrased it — but at full width/25fps that's ~900 KB/frame of pointless CPU→GPU traffic for a trace the user has turned off. Gating the upload on m_fftLineWidth > 0.0f (mirroring the draw gate) closes it.
2. buildFftDisplayTrace() heap-allocates two QVectors per frame from both hot paths (Copilot, ~4694). It's called from renderGpuFrame (7633) and drawSpectrum (8437), and each call allocates a fresh displayBins plus trace (up to 8192 floats each). This is the same per-frame churn the PR deliberately eliminated for the vertex scratch by moving to m_fftLineScratch/m_fftFillScratch members — worth applying the same treatment here (mutable member scratch buffers) so steady-state rendering stays alloc-free. Consistency win as much as a perf one.
Optional: clampedCatmullRom() and the decodeFFT over-range recovery in PanadapterStream.cpp are both pure, deterministic functions and prime unit-test candidates (e.g. a stale-y_pixels frame recovers the floor; upsampling never overshoots). The PR validated end-to-end on hardware, but a focused regression test would lock the heuristic's threshold (overRangeCount > max(8, count/8)) against future drift. Not a blocker.
Nice work — the structure is clean and the fix is well-targeted.
🤖 aethersdr-agent · cost: $8.3405 · model: claude-opus-4-8
ten9876
left a comment
There was a problem hiding this comment.
Solid rendering fix. Verified the substantive parts:
- y_pixels guard (the flatline root cause): the radio encodes bins as Y-pixel positions, so a stale/tiny y_pixels under-scales every bin → flatline. The guard detects it structurally (bins exceeding the declared range can't happen if y_pixels is correct, so >max(8, count/8) over-range means stale) and rescales effectiveYPixels to the actual bin max. Threshold avoids outlier false-positives. Correct recovery.
- Shader edge-AA is clean standard distance-field coverage (1.0 - smoothstep(1-aa, 1, edge) with fwidth), and edge==0 (fill) keeps full coverage — non-edge case unchanged.
- Vertex layout exactly matches the shader (locations 0/1/2, offsets 0/2/6, stride 6→7) — the one runtime-GPU mismatch risk is ruled out.
- Device-pixel dims, trace preserve/reproject on zoom, display-only interpolation, reset restoring DisplayFftLineWidth=2.0 — visual, no data/safety surface, live-verified on real hardware for all three symptoms.
No new settings key, clean-room, Principle XI. Thanks @rfoust.
…u-gated smoothing — Principle XI. (#3975) ## Summary Fixes #3967. Also fixes #3932. Root-caused with per-commit builds (pre-#3836, post-#3836, 26.7.1-main, this fix) captured minutes apart against the same live AM broadcast band on a FLEX-8400M:  ## The regression chain (two releases, two causes) **26.6.5 — #3836's unconditional spatial smoothing ("out of focus", #3932).** `buildFftDisplayTrace`'s 5-tap binomial smooth at a fixed 0.75 blend exists to melt the radio's RBW stair-steps when zoomed in. Applied to every frame at every span, it rounds narrow carriers (visible height loss on AM carriers, panel 2) and defocuses the noise floor. **Fix:** gate the blend on the measured plateau fraction (adjacent equal-pixel bins). Zoomed-in plateau runs (frac ≳0.65) keep the full blend — #3836's stair-step fix is preserved; busy wide spans (frac ≲0.35) get zero — the pre-#3836 crispness returns; the ramp between prevents a visible mode flip while zooming. **26.7.1 — #3958's per-pixel stroke ("spiky above and below the line", the rest of #3967).** The stroke distance was approximated as vertical distance scaled by a `dFdx`-derived slope — i.e. distance to the segment's **infinite line**. On steep segments that line passes near the entire pixel column, so the stroke drew full-height needles above and below the trace (panel 3), and the fixed ±0.75 px smoothstep band read as a soft fat stroke at 1× DPI. **Fix:** true endpoint-clamped distance to the three adjacent polyline segments (4 column samples — matches the coverage the old geometry quads had), plus falloff parity with the old `spectrum.frag`: solid to `halfWidth−1`, 1 px fade, 1 px feather annulus at α 0.22. Also folds in #3968's R16F-unconditional column format as hardening (float32 linear filtering is optional on GLES and `isTextureFormatSupported()` can't detect filterability). Note it could **not** have caused #3967: the reporter's own About screenshot shows `D3D11; Intel(R) HD Graphics 520` — feature level 12.1 hardware where R32F linear filtering is mandatory. With this folded in, #3968 can be closed. Not implicated (left untouched, documented for the record): #3836's `decodeFFT` over-range renormalization only fires on radio/client `y_pixels` mismatch (transient), and the DPR-scaled `x_pixels`/`y_pixels` requests are orthogonal to trace sharpness. ## Why our platforms missed it The #3958 parity checks ran on 2× Retina, where the extra AA softness is half the logical size and the spike needles read as band activity in static grabs. The 4-way same-band comparison above is exactly the test that should have been run: panel 4 (this fix) restores panel 1 (pre-#3836). ## Verification - 4-way live comparison above (same band, same radio pan, same display settings; three grabs per build). - `get panstats 0`: frame prep unchanged at ~121 µs/frame, 58 presents/s, `fftBuildMsPerSec` 0.28 (plateau gate skips the smoothing copy on wide spans). - 3D stacked-trace mode, lean mode, line-width 0, heat/solid fill: unaffected paths re-checked by inspection; the software (non-QRhi-build) QPainter renderer shares `buildFftDisplayTrace` and picks up the plateau gate automatically. ## Also on the #3967 thread (separate follow-ups, not in this PR) - "3D option no longer exists": the 2D/3D combo is the **last row of the Display panel**, below Background — on short screens (768p laptops) it clips off the bottom of the panadapter. Needs a scrollable/compacted Display panel; the option itself ships fine in 26.7.1. - The 60 fps FFT slider ceiling from #3958 is unrelated to this report but worth an explicit release-notes mention. 💻 Generated with Claude Code (claude-fable-5 7/2/26) with architecture by @jensenpat 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Fixes a zoomed panadapter FFT rendering regression where the spectrum trace could collapse into a mostly flat floor with only peaks visible, and improves the visible smoothness of steep FFT peaks. The fix keeps stale/tiny radio
y_pixelsvalues from corrupting FFT dBm decoding, requests panadapter dimensions in render-device pixels, preserves/reprojects the current FFT trace during zoom changes to avoid flicker, and adds display-only trace interpolation plus shader edge antialiasing for smoother line rendering. Display reset now also restores the existing FFT line-width setting to the default2.0.Constitution principle honored
Principle XI — Fixes Are Demonstrated. The patch was validated with a local app build, focused regression tests, and live user verification of the original flatline, stepping, and zoom flicker symptoms.
Test plan
cmake --build build --target AetherSDR profile_load_command_test panadapter_model_rx_antenna_test vita_tile_frequency_test -j8)Checklist
docs/COMMIT-SIGNING.md)AppSettingscalls — no new setting key was added; reset now restores the existingDisplayFftLineWidthkeyMeterSmoother— not applicable; no meter UI changed