feat(spectrum): pan-follow-VFO triggers off flag outer edge (#2761)#2784
Merged
Conversation
When a slice tunes near a pan edge, the existing Pan Follows VFO machinery slides the pan center so the slice stays visible. But the VFO flag panel (callsign / freq / mode badge hanging off the slice marker) is fixed-width and renders OUTSIDE the slice marker — so the slice can be well within the trigger boundary while the flag panel is already clipping the pan edge. Split pairs (LockLeft + LockRight on the same marker, #2663) are particularly affected: PR #2744 intentionally accepted that the outward-facing flag clips and documented "the user pans toward center to read it" as the workaround. This change removes that workaround by treating the flag's outer edge as the trigger reference. revealFrequencyIfNeeded gains two defaulted offset parameters, leftFlagEdgeOffsetMhz and rightFlagEdgeOffsetMhz. When non-zero (IncrementalTune only — other intents ignore them), the trigger compares (mhz - leftOffset) against the left pan boundary and (mhz + rightOffset) against the right pan boundary, instead of mhz on both sides. Default 0.0 preserves the original slice- frequency comparison for non-flag callers (CommandedTargetCenter, RevealOffscreen). Offsets are clamped to 95% of triggerDistance- FromCenter so a tiny panadapter where flag-width > trigger-margin doesn't oscillate the pan around the slice. panFollowVfo computes the offsets by: 1. Reading the spectrum's bandwidthMhz and pixel width to derive mhzPerPixel. 2. Multiplying VfoWidget::width() by mhzPerPixel for flag-width- in-MHz. 3. For split pairs (SpectrumWidget::sliceHasSplitPartner returns true — flags on both sides via LockLeft/LockRight), passing the same offset for both sides. For non-split slices, passing the offset only on the side where the flag currently renders (VfoWidget::onLeft()). For collapsed/compact-mode flags, both offsets stay 0.0 (compact mode doesn't render a wide flag). Existing PR #2744 still locks split-pair flags to their outward sides; this change ensures the pan slides early enough that the lock doesn't cause clipping in the first place. Files: src/gui/MainWindow.h — revealFrequencyIfNeeded signature + explanatory comment block src/gui/MainWindow.cpp — implementation: clamp + apply offsets in the trigger comparison; panFollowVfo: compute offsets from VfoWidget width + onLeft + split state src/gui/SpectrumWidget.h — sliceHasSplitPartner(int) accessor src/gui/SpectrumWidget.cpp — sliceHasSplitPartner implementation (scans m_sliceOverlays for the slice + checks splitPartnerId) src/gui/VfoWidget.h — onLeft() accessor exposing the existing m_lastOnLeft state set by updatePosition Local build clean (401/401 link). Manual verification deferred to post-merge smoke test: tune toward a pan edge with a wide VFO flag visible and confirm the pan slides before the flag clips, both for single-flag and split-pair scenarios. Closes #2761. Closes-as-replacement for #2778 (incomplete recovery). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Thanks @ten9876 — clean recovery of #2778 with the wiring actually plumbed through this time. The implementation reads well; defaulted offsets preserve every existing caller, and the incremental ? … : mhz collapse means only the IncrementalTune path consumes them, exactly as the PR description claims.
Things I checked and like
- Null safety: both
spectrumForSlice(s)andsw->vfoWidget(s->sliceId())are guarded withif (auto*), andspecW > 0/bw > 0.0cover the early-construction window where width hasn't been laid out yet. - Compact-mode handling:
!vfo->isCollapsed()keeps both offsets at 0.0, which is the right call — the collapsed flag isn't wide enough to need the extension. - Split-pair detection:
sliceHasSplitPartner()correctly scansm_sliceOverlaysforsplitPartnerId >= 0, which is the same source-of-truth the renderer uses for LockLeft/LockRight layout. - Anti-oscillation clamp:
triggerDistanceFromCenter * 0.95is a sensible safety margin so flag-wider-than-trigger doesn't ping-pong the pan center around the slice. Worth noting that on a very narrow pan this means the flag will clip a bit rather than oscillate — that's the right trade. onLeft()accessor: just exposes the existingm_lastOnLeftthatupdatePosition()already maintains, so the read is always consistent with whatever the flag last rendered as.
Minor observations (non-blocking)
clampedRightOffset = std::min(rightFlagEdgeOffsetMhz, safeOffsetCap)only clamps the upper bound; a hypothetical negative offset would pass through and shrink the trigger zone. Callers in this PR always pass>= 0.0, so it's fine — but if you ever add a caller that computes the offset by subtraction, astd::clamp(x, 0.0, safeOffsetCap)would be more defensive. Not worth changing for this PR.vfo->width()is the laid-out widget width. If the flag is mid-animation or the layout hasn't settled, this could briefly be slightly off, but the 95% clamp absorbs it and the trigger re-evaluates on the next tune step. Reasonable.
No conventions issues (no settings touched, no resource lifecycle, no thread-boundary concerns — all main-thread GUI state). The 5 touched files are all in scope. LGTM as a COMMENT review — leaving the merge decision to @jensenpat.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the plan from #2761. Replaces incomplete agent attempt #2778 (which committed the API surface without the implementation wiring).
When a slice tunes near a pan edge, the existing Pan Follows VFO machinery slides the pan center so the slice stays visible. But the VFO flag panel hanging off the slice marker is fixed-width — the slice can be well inside the trigger boundary while the flag panel is already clipping the pan edge. Split pairs (LockLeft + LockRight on the same marker, #2663 / #2744) are particularly affected: PR #2744 intentionally accepted that the outward-facing flag clips and documented "the user pans toward center to read it" as the workaround.
This change removes that workaround by treating the flag's outer edge as the trigger reference, not the slice frequency.
What changes
Files
Net: 85 insertions / 9 deletions across 5 files.
Local verification
Test plan (manual, post-merge)
Closes
🤖 Generated with Claude Code