Add Ctrl-drag dBm scale zoom#2717
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new interaction mode to the SpectrumWidget dBm scale: Ctrl-drag on the right-side dBm strip adjusts vertical dB span smoothly while anchoring the bottom level, and updates draw ordering so the right-side scale strips render above other overlays.
Changes:
- Added Ctrl-drag “range zoom” mode on the dBm scale strip while preserving existing drag-to-pan behavior.
- Extended drag state tracking to include the new range-drag mode.
- Moved dBm scale and waterfall time scale drawing later in the overlay pipeline so those strips stay on top.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/gui/SpectrumWidget.h | Adds new drag state for dBm range dragging and updates isDraggingDbmScale() to reflect both drag modes. |
| src/gui/SpectrumWidget.cpp | Implements Ctrl-drag range zoom behavior, updates drag lifecycle logic, and adjusts overlay draw ordering for scale strips. |
Comments suppressed due to low confidence (1)
src/gui/SpectrumWidget.cpp:3045
m_draggingDbmRangeis now a second dBm-scale drag mode, but some internal logic still gates onm_draggingDbmonly. For example, the noise-floor auto-adjust path (updateNoiseFloorBaseline()) early-returns whenm_draggingDbmis true, but does not checkm_draggingDbmRange, so auto-adjust can still modifym_refLevelwhile the user is Ctrl-drag zooming. Consider updating those checks to includem_draggingDbmRange(or consistently useisDraggingDbmScale()).
if (m_draggingDbmRange) {
const int dragHeight = std::max(1, specH);
const int dy = m_dbmDragStartY - y;
const float deltaDb = (static_cast<float>(dy) / dragHeight) * m_dbmDragStartRange;
m_dynamicRange = std::max(10.0f, m_dbmDragStartRange + deltaDb);
m_refLevel = m_dbmDragStartBottom + m_dynamicRange;
markOverlayDirty();
ev->accept();
return;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifdef Q_OS_MAC | ||
| const bool rangeDrag = modifiers.testFlag(Qt::ControlModifier) | ||
| || modifiers.testFlag(Qt::MetaModifier); | ||
| #else | ||
| const bool rangeDrag = modifiers.testFlag(Qt::ControlModifier); | ||
| #endif | ||
| const bool primaryClick = ev->button() == Qt::LeftButton; | ||
| const bool controlClick = rangeDrag | ||
| && (primaryClick || ev->button() == Qt::RightButton); |
There was a problem hiding this comment.
Addressed in 6acd484: on non-macOS the range-drag gesture now requires Ctrl+LeftButton, while the RightButton path is only accepted on macOS for Control-click behavior. That leaves Ctrl+RightClick available for the normal context menu on Windows/Linux.
f7287dd to
6acd484
Compare
|
Also addressed the suppressed Copilot note in 6acd484: updateNoiseFloorBaseline() now uses isDraggingDbmScale(), so noise-floor auto-adjust is paused during both normal dBm scale drags and Ctrl-drag range zoom. The branch was rebased onto current upstream/main, rebuilt, and GitHub now reports the PR as mergeable. |
There was a problem hiding this comment.
Nice addition — the Ctrl-drag interaction reads cleanly and the math is sound (bottom anchored, relative-scale deltaDb proportional to start range, 10 dB floor enforced, release path reuses the existing echo/finalize plumbing). Thanks for contributing this, @rfoust!
A few observations:
Copilot finding (line 2727 / 3037): Verified resolved in 6acd484. The non-mac path now requires Ctrl+LeftButton for range drag, and Ctrl+RightButton falls through to the context-menu handler on Windows/Linux — that matches the expected platform conventions. Behavior on macOS still routes Ctrl/Meta + Left or Right through the range-drag path (correct for Qt's Ctrl-click → synthesized RightButton delivery).
Minor behavior change worth noting: Previously if (mx >= stripX) { … return; } was an unconditional consume, so a plain right-click on the dBm strip never reached the context-menu code below. With the new structure, an unmodified right-click on the dBm strip now falls through and can trigger the spectrum context menu (because the spectrum painter now draws behind the strip and the strip overlays on top, xToMhz(mx) will land at the far-right edge of the spectrum). Probably harmless — and arguably an improvement — but worth a manual check that nothing surprising happens (e.g., spot/TNF hit-testing under the strip area).
Small nice-to-have: m_dynamicRange has a lower clamp (10 dB) but no upper bound. Dragging hard upward can grow it arbitrarily large. The existing arrow-button path has the same property, so this is consistent, but you might consider a sensible cap (e.g., 200 dB) to keep the noise floor / grid math well-behaved in degenerate cases. Not blocking.
Style nit (skip if you prefer): in mousePressEvent the rangeDrag local on macOS is only used to compute controlClick, so it could be inlined — but the current form makes the intent readable, so leaving it is fine.
The draw-order change (moving drawDbmScale/drawTimeScale to render after overlays in all three paint paths — GPU overlay, paintEvent main, paintEvent end) is consistent across the three sites.
Looks good overall.
…nciple III. The Ctrl-drag span-zoom shortcut shipped in #2717 is invisible: only the arrow buttons advertise the strip is interactive. This wires a hover tooltip on the dBm strip listing the three operations (drag pans, Ctrl/Cmd-drag zooms, arrows step by 10 dB) and adds the same three bullets to the panadapter-actions section of resources/help/configuring-aethersdr-controls.md. The tooltip is built next to the existing strip-cursor branch in SpectrumWidget::mouseMoveEvent and uses the same per-region QToolTip::showText pattern as band-plan-spot hovers — Qt auto-hides it when the cursor leaves the strip rect. On macOS the modifier line mentions both Ctrl and Cmd to mirror the existing mousePressEvent modifier acceptance. Principle III applies: user-facing prose uses the on-screen labels (arrow glyphs, "dBm scale") and the modifier name the user actually types on their platform. Blast radius: PreToolUse hook flagged risk_score=0.321 with 9 high-risk callers (MainWindow::MainWindow, buildUI, wirePanadapter, buildMenuBar, registerShortcutActions, ...). The edit is purely additive inside an existing branch of mouseMoveEvent — no signature, signal, slot, or control-flow change. Those callers wire SpectrumWidget signals up; none observe the dBm strip's hover-tooltip text, so the file-level coupling does not translate into a behavioural ripple. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The CWX F1-F12 commit (781358a) was generated against a SpectrumWidget snapshot from before PR #2717 landed and inadvertently reverted those ~107 lines: removed the m_draggingDbmRange / m_dbmDragStartRange / m_dbmDragStartBottom members and undid the matching press/move/release handling, the isDraggingDbmScale() update, the noise-floor guard swap, and the paint-order fix. Restore src/gui/SpectrumWidget.{cpp,h} to the current main state so the CWX F1-F12 additions in MainWindow.cpp and RadioSetupDialog.cpp can land without reverting #2717. Same pattern as the restoration commit on PR #2714 earlier today — worth investigating the agent's snapshot/cache behaviour upstream. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2869) ## Summary - Ctrl+wheel now zooms the panadapter bandwidth in/out by ×1.5 per scroll step, anchoring on the frequency under the mouse cursor. - Plain wheel scroll is unchanged — it continues to tune the VFO. ## Implementation Single change in `SpectrumWidget::wheelEvent`: after the debounce/step calculation, an early branch catches `Qt::ControlModifier` and runs the cursor-anchored zoom, then returns before the existing VFO tune path. The zoom math mirrors the existing NativeGesture pinch-to-zoom exactly: ```cpp const double mouseXFrac = ev->position().x() / width() - 0.5; const double anchorMhz = m_centerMhz + mouseXFrac * m_bandwidthMhz; const double newCenter = std::max(anchorMhz - mouseXFrac * newBw, newBw / 2.0); ``` The `std::max(..., newBw / 2.0)` clamp prevents the left edge from going below 0 Hz, consistent with PR #2867. ## UX consistency note The existing `+`/`−` zoom buttons use a different anchor policy: zoom-in re-centers on the active VFO frequency (added in #1932 to prevent repeated clicks pushing the slice off-screen), zoom-out keeps the pan center fixed. That behavior makes sense for a discrete click where the cursor position carries no intent. Ctrl+wheel is a continuous, cursor-positioned interaction — the user is looking at a specific part of the spectrum when they scroll. Cursor-anchored zoom is therefore the correct policy here, matching the NativeGesture pinch-to-zoom and every major spectrum/map application that implements scroll-to-zoom. The button inconsistency is deliberate and acceptable. Ctrl is already the modifier used for dBm-scale drag (PR #2717) and waterfall time-scale drag (PR #2783), so Ctrl+wheel is consistent with the existing convention of Ctrl = “modify the view, not the VFO.” ## Testing - Scroll wheel without Ctrl: VFO tunes as before, no regression. - Ctrl+scroll up: bandwidth narrows, cursor frequency stays fixed. - Ctrl+scroll down: bandwidth widens, cursor frequency stays fixed. - Ctrl+scroll at band edge: 0 Hz clamp prevents left edge going negative. - Ctrl+scroll at BW limits: clamped to `m_minBwMhz`/`m_maxBwMhz`, no crash. - Pinch-to-zoom: unaffected. Fixes #1518 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…dr#1518) (aethersdr#2869) ## Summary - Ctrl+wheel now zooms the panadapter bandwidth in/out by ×1.5 per scroll step, anchoring on the frequency under the mouse cursor. - Plain wheel scroll is unchanged — it continues to tune the VFO. ## Implementation Single change in `SpectrumWidget::wheelEvent`: after the debounce/step calculation, an early branch catches `Qt::ControlModifier` and runs the cursor-anchored zoom, then returns before the existing VFO tune path. The zoom math mirrors the existing NativeGesture pinch-to-zoom exactly: ```cpp const double mouseXFrac = ev->position().x() / width() - 0.5; const double anchorMhz = m_centerMhz + mouseXFrac * m_bandwidthMhz; const double newCenter = std::max(anchorMhz - mouseXFrac * newBw, newBw / 2.0); ``` The `std::max(..., newBw / 2.0)` clamp prevents the left edge from going below 0 Hz, consistent with PR aethersdr#2867. ## UX consistency note The existing `+`/`−` zoom buttons use a different anchor policy: zoom-in re-centers on the active VFO frequency (added in aethersdr#1932 to prevent repeated clicks pushing the slice off-screen), zoom-out keeps the pan center fixed. That behavior makes sense for a discrete click where the cursor position carries no intent. Ctrl+wheel is a continuous, cursor-positioned interaction — the user is looking at a specific part of the spectrum when they scroll. Cursor-anchored zoom is therefore the correct policy here, matching the NativeGesture pinch-to-zoom and every major spectrum/map application that implements scroll-to-zoom. The button inconsistency is deliberate and acceptable. Ctrl is already the modifier used for dBm-scale drag (PR aethersdr#2717) and waterfall time-scale drag (PR aethersdr#2783), so Ctrl+wheel is consistent with the existing convention of Ctrl = “modify the view, not the VFO.” ## Testing - Scroll wheel without Ctrl: VFO tunes as before, no regression. - Ctrl+scroll up: bandwidth narrows, cursor frequency stays fixed. - Ctrl+scroll down: bandwidth widens, cursor frequency stays fixed. - Ctrl+scroll at band edge: 0 Hz clamp prevents left edge going negative. - Ctrl+scroll at BW limits: clamped to `m_minBwMhz`/`m_maxBwMhz`, no crash. - Pinch-to-zoom: unaffected. Fixes aethersdr#1518 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Verification
cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfocmake --build buildManual check: