Keep spectrum visible during panadapter zoom#2246
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the panadapter rendering path to keep the FFT spectrum trace visible during zoom operations by reprojection of existing spectrum bin data across overlapping frequency ranges (mirroring the existing waterfall reprojection behavior).
Changes:
- Added
reprojectSpectrum()to resample existing spectrum bins into the new frequency span when the old/new ranges overlap. - Updated zoom/frequency-range change paths to call spectrum reprojection to avoid transient blanking during zoom.
- Preserved “clear on non-overlap” behavior for spectrum buffers in
setFrequencyRange()when ranges do not overlap.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/gui/SpectrumWidget.h |
Declares the new reprojectSpectrum() helper used to preserve spectrum visibility across zoom changes. |
src/gui/SpectrumWidget.cpp |
Implements spectrum reprojection and wires it into zoom and frequency-range update paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| reprojectWaterfall(m_centerMhz, m_bandwidthMhz, newCenter, newBw); | ||
| reprojectSpectrum(m_centerMhz, m_bandwidthMhz, newCenter, newBw); |
There was a problem hiding this comment.
Addressed in e85273d: direct zoom paths now check reprojectSpectrum() and clear m_bins/m_smoothed when the old and new ranges do not overlap, matching the intended clear-on-non-overlap behavior.
| return; | ||
| } | ||
|
|
||
| const QVector<float> oldBins = bins; |
There was a problem hiding this comment.
Addressed in e85273d: reprojectSpectrum() now moves the source QVector into the temporary source buffer before assigning the reprojected result back, avoiding the extra full copy in the zoom path.
CI failure analysisHey @rfoust, thanks for the contribution! The CI failure here is not caused by your code — it's a CI infrastructure issue. What failedThe "Cleanup artifacts" job failed at the "Delete artifacts" step (job log). This is a post-build housekeeping step that deletes temporary build artifacts and has nothing to do with compilation or tests. The other jobs in the pipeline look fine:
What to doNothing on your end. This is likely a transient GitHub Actions permissions or API rate-limit issue with the artifact cleanup step. A maintainer can re-run the failed job, or it may resolve on the next push. Copilot review notes (unrelated to CI, but worth noting)Copilot left two suggestions on
These are optional improvements and don't need to block this PR. Thanks for volunteering your time on this — the fix looks good! 🎙️ |
rfoust
force-pushed
the
codex/panadapter-zoom-no-flash
branch
from
208b29c to
e85273d
Compare
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nice work, @rfoust — the approach is clean and well-scoped. Mirroring the existing reprojectWaterfall pattern for spectrum data is the right call, and the linear interpolation in reprojectBins should keep things smooth during continuous zoom gestures. CI is green.
A few observations:
m_wfWriteRow = 0 removal in setFrequencyRange (large-shift path)
The diff removes m_wfWriteRow = 0; alongside the m_bins/m_smoothed clearing at line 1056. This isn't mentioned in the PR description and is a waterfall behavioral change (not spectrum). The zoom-button and drag paths never reset m_wfWriteRow after reprojectWaterfall, so removing it here makes the large-shift path consistent — but worth confirming this was intentional rather than collateral from editing adjacent lines. If the ring buffer position was being reset to avoid visual glitches on non-overlapping band jumps, it might still be needed in the !keptSpectrum fallback path (i.e., alongside the buffer clears).
Copilot findings — both valid, both addressed
- The stale-spectrum-on-no-overlap concern was real:
reprojectSpectrumreturnsfalsewithout touchingm_bins/m_smoothed, and all four call sites now clear them onfalse. Confirmed fixed in the diff. - The
std::moveoptimization for the source vector is correct.const QVector<float> oldBins = std::move(bins)moves the buffer out, avoids the full copy, andbins = std::move(reprojected)puts the result back. No false positives from Copilot here.
Minor: reprojectSpectrum return value semantics
The function returns !m_bins.isEmpty() || !m_smoothed.isEmpty() — so it returns false when both buffers were already empty before the call (no FFT data received yet), even if the frequency ranges overlap. This is harmless (callers would clear already-empty vectors), but it means the return value conflates "no overlap" with "nothing to reproject." A comment clarifying this would help future readers, but not a blocker.
Everything else looks correct: changes are limited to the two stated files, no QSettings usage, no resource leaks, and the [&] capture on the reprojectBins lambda is fine since it doesn't outlive the enclosing scope. Thanks for the contribution!
rfoust
force-pushed
the
codex/panadapter-zoom-no-flash
branch
from
e85273d to
8462fa1
Compare
|
Addressed the waterfall write-row observation in 8462fa1 after local testing. The large-shift path now preserves the old reset behavior only when spectrum reprojection cannot keep data because the old/new ranges do not overlap:
Verified with: |
Reliability sweep release. Headline is jensenpat's DAX2 coexistence policy refactor (#2271): platform-aware decision table, lazy dax_tx stream creation, anti-stomp on incoming foreign DAX TX status, LAN VITA UDP rebind on Flex's "Port/IP pair already in use" error, dead- orphan DAX RX detection, and 24+ test assertions. Substantial reliability fixes around disconnect teardown (#2247 — sequenced stream remove with response wait, dropped self-disconnect, dying-gasp byte), worker-thread shutdown (#2248 — no more killTimer warnings), and panadapter zoom (#2246 — no spectrum flash). Closes 12 issues via two paired triage-cleanup sweep PRs (#2270 + #2272) covering small UI bugs, contrast, indicator routing, click-to- tune behaviour, and visible regressions from recent feature work. Also includes the #2273 follow-up to #2271: setDax() and DAX TX stream creation are now both gated on platforms that can actually feed DAX audio, so Linux non-PipeWire builds keep digital TX on the physical mic input rather than going silent. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Summary
Fixes a panadapter display issue where the spectrum could flash or briefly blank while zooming in or out.
What changed
Testing