Skip to content

fix(smartmtr): timer settle, NaN guard, cross-flag sync, font cache (#3750 M1–M4)#3752

Merged
ten9876 merged 1 commit into
mainfrom
smartmtr/fix-m1-m4
Jun 23, 2026
Merged

fix(smartmtr): timer settle, NaN guard, cross-flag sync, font cache (#3750 M1–M4)#3752
ten9876 merged 1 commit into
mainfrom
smartmtr/fix-m1-m4

Conversation

@ten9876

@ten9876 ten9876 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Addresses the four MEDIUM items from #3750 (SmartMTR fast-follow), all introduced with #3723. Each is independent; grouped into one PR per the follow-up plan.

M1 — TX/mic mode never lets the 120 Hz timer settle

MeterExtremes.h: in external-peak (mic) mode the MAX marker is a held radio stat that sits above the needle by design, so the standingOff keep-alive kept tick() returning true for the entire TX, pinning ~120 Hz repaints over the GPU panadapter. Each mic packet re-arms the timer via setMeterInput (setExternalPeak keeps hasData() true), so we only need to keep animating while a marker is actually slewing — drop the standing-off keep-alive in that mode. Bounded before (PTT release resets), but this restores the idle-settle the perf work intended.

M2 — NaN/Inf permanently poisons the meter

SmartMtrWidget::setMeterInput: a non-finite value survived std::clamp (both comparisons false), stuck in the bar smoother (needsAnimation() never cleared → timer never stopped), and poisoned MeterExtremes::m_sumRaw permanently (corrupting avgRaw() → value labels). Drop non-finite value/peak at entry; the last good frame stays until a finite reading arrives.

M3 — Cross-flag option-control desync

VfoWidget: the global extremesChanged/txMeterChanged signals only re-pushed each flag's rendering, not its option controls, which are seeded once at build. With the selector open on two flags, changing a setting on one left the other's checkbox/combos stale. New syncSmartMtrSettingsControls() re-seeds the four controls from the (cached, singleton) MeterViewController, QSignalBlocker-guarded to avoid a re-broadcast loop, then re-runs syncSmartMtrSettingsState(). Wired to both global signals.

M4 — Font change doesn't invalidate the cached marker layer

SmartMtrWidget: the static-layer cache key is only {size, kind, dpr}, but the cached marker glyphs are built from font(). Override changeEvent to invalidate on FontChange/ApplicationFontChange/StyleChange so a theme/font-size change doesn't leave stale glyphs until the next resize.

Scope: +75 lines across MeterExtremes.h, SmartMtrWidget.{cpp,h}, VfoWidget.{cpp,h}. No change to the S-meter path; FLEX behavior unaffected.

🤖 Generated with Claude Code

…3750 M1-M4)

Four non-blocking follow-ups to the SmartMTR meter view (#3723):

M1 — TX/mic mode never let the animation timer settle. The external-peak
MAX marker stands off above the needle by design, so the standing-off
keep-alive kept tick() returning true for the whole TX, pinning ~120 Hz
repaints over the GPU panadapter. Each mic packet re-arms the timer via
setMeterInput, so in external-peak mode keep animating only while a marker
is actually slewing.

M2 — No isfinite guard. A NaN/Inf dBm survived std::clamp, stuck in the
bar smoother (needsAnimation never cleared) and permanently poisoned the
extremes running sum. Drop non-finite values at setMeterInput.

M3 — Cross-flag option-control desync. The global extremes/tx-meter
signals only re-pushed rendering, not the per-flag checkbox/combos, so a
change on one open flag's selector left other flags' controls stale. Add
syncSmartMtrSettingsControls() to re-seed them (QSignalBlocker-guarded)
and wire it to extremesChanged/txMeterChanged.

M4 — Font change didn't invalidate the cached marker layer (key is only
{size, kind, dpr}). Override changeEvent to drop the cache on
FontChange/ApplicationFontChange/StyleChange.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ten9876 ten9876 requested a review from a team as a code owner June 23, 2026 01:38
@github-actions

Copy link
Copy Markdown

Accessibility check findings

This PR touches src/gui/ files with 2 accessibility issue(s).
Full details appear as inline annotations on the diff above.

Files scanned : 5
Files with findings: 2
Total findings : 2

See docs/a11y.md for the canonical
Qt patterns (setAccessibleName, QAccessibleValueChangeEvent,
QAccessibleInterface subclasses, etc.). Structural remediation
(keyboard navigation, custom QAccessibleInterface subclasses)
is tracked in #3288.

Findings are warnings only — the build is not blocked.

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ten9876 — reviewed all four fixes against the code and they hold up. Tightly scoped (+75 lines, no S-meter/FLEX impact), all six checks green, and each one matches a real defect in the #3723 SmartMTR path. Detailed-comment-level write-ups, which I appreciate.

M1 — timer settle (MeterExtremes.h) ✅ Correct. m_useExtPeak ? return moving is placed after the slew, and moving is in scope (declared at line 121). In ext-peak mode maxTgt is the held peak (mapToUnits(m_maxRaw)), so the marker slews to it, moving goes false, and the timer settles — exactly the idle-settle the keep-alive was defeating. The trough already targets the needle in this mode (line 112), so dropping standingOff is safe.

M2 — NaN/Inf guard (SmartMtrWidget::setMeterInput) ✅ Correct and placed before m_input = input, so the last good frame is preserved. One small note, not blocking: the guard drops the whole input if either field is non-finite, so a frame with a finite value but a non-finite peak loses its good value too. In practice value/peak go non-finite together so this is fine, but if you wanted to be surgical you could null out only the offending field rather than dropping the frame. Your call — current behavior is defensible.

M3 — cross-flag control desync (VfoWidget) ✅ Correct. syncSmartMtrSettingsControls() re-seeds from the MeterViewController singleton, and the accessors return the same DisplaySettings enum types the combos were seeded with (int(...) + findData), so the lookups match. QSignalBlocker on each widget prevents the re-seed from echoing back — no broadcast loop. Wired to both global signals alongside the existing pushSmartMtrInput.

M4 — font-change cache invalidation (changeEvent) ✅ Correct. The static-layer key is {size, kind, dpr} but the glyphs are built from font(), so invalidating on FontChange/ApplicationFontChange/StyleChange closes the stale-glyph window. Forwards to QWidget::changeEvent.

No convention or RAII issues, no out-of-scope files, nothing missing at the boundaries. Nice clean fast-follow — good to merge from my side.


🤖 aethersdr-agent · cost: $4.3243 · model: claude-opus-4-8

@ten9876 ten9876 merged commit ec8c9d4 into main Jun 23, 2026
6 checks passed
@ten9876 ten9876 deleted the smartmtr/fix-m1-m4 branch June 23, 2026 02:33
ten9876 added a commit that referenced this pull request Jun 23, 2026
…ender blank (#3760)

## Summary

Disables the identity `QGraphicsOpacityEffect` on fully-enabled SmartMTR
option rows so they don't render blank when multiple VFO flags are
GPU-composited.

A fully-enabled option row keeps its `QGraphicsOpacityEffect` at opacity
`1.0` (identity). With the GPU-composited slice flags from #3695, an
identity opacity effect can render the row's contents blank. The fix
disables the effect entirely while it is at full strength (a disabled
`QGraphicsOpacityEffect` is a no-op, so the row paints directly) and
keeps it installed only while it is actually dimming the row.

## Provenance

This is the net-new fix extracted from #3757. The remainder of that PR
re-implemented SmartMTR `#3750` hardening that already landed via #3751
(H1), #3752 (M1–M4), and #3753 (L1–L3); #3757 is being closed in favor
of this focused change to avoid re-churning already-merged code. Part of
#3750.

## Test plan

- [x] Local build passes (`cmake --build build --target AetherSDR`)
- [ ] Behavior verified on a real radio if applicable — visual (open the
SmartMTR option panel with multiple flags up; rows render, not blank)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
ten9876 added a commit that referenced this pull request Jun 23, 2026
…rites (rebased #3765) (#3771)

Closes #3762. **Rebased + conflict-resolved version of #3765** by
@dsocha (original authorship preserved on all three commits). Opened
from a maintainer branch because the original is on a fork; #3765 should
be closed in favor of this.

## What it fixes (unchanged from #3765)
Two GPU-flag-sprite rendering bugs in the SmartMTR meter view (#3723):
1. **Extremes value labels never drew (#3762)** — `drawSmartMtrLabels()`
gated on `m_smartMtrWidget->isVisible()`, false in GPU-sprite mode. Now
gates on `!size().isEmpty()` (the same condition the sprite is drawn
under), so labels track the markers in both GPU and software paths.
2. **Meter selector vanished from the sprite** — `QWidget::render()`
can't rasterize a `QGraphicsEffect`. The selector now gates its
underline on an explicit `m_meterMenuOpen` flag (not the hidden row's
`isVisible()`), and the option rows drop the `QGraphicsOpacityEffect`s
in favor of `setEnabled(false)` + `:disabled` stylesheet dimming (new
`QComboBox:disabled` rule in `ComboStyle.h`), so they stay visible
**and** dimmed in the sprite.

## Rebase / conflict resolution (the reason this is a new branch)
The original #3765 conflicted with **#3760** ("disable identity opacity
effect…"), which merged into `main` after #3765 was branched. Both
target the same blank-rows-in-sprite bug, but:
- **#3760** only neutralized the effect at *identity* (opacity 1.0) —
i.e. for *enabled* rows. A *disabled* row still carried an active 0.45
effect, so it would **still render blank** in a sprite.
- **#3765** removes the effects entirely, so both enabled and disabled
rows render. **#3765#3760.**

Resolved `syncSmartMtrSettingsState` in favor of #3765's approach and
**dropped #3760's now-dead `setRowOpacity` lambda + the `m_*Fade`
members**. Verified: no dangling `m_*Fade`/`kDisabledOpacity`
references; the `QGraphicsOpacityEffect` on the DSP filter-level row
(meant to vanish in sprites) is untouched.

## Verification
- Rebased onto `main` (`437f7469`); conflict resolved; commits signed.
- Composes with the merged #3752 (M3 `syncSmartMtrSettingsControls`
re-seeds combos, then `syncSmartMtrSettingsState` disables the rows) and
#3758 (a11y — `QAccessibleInterface` reference preserved; a11y linter
passes 0 findings).
- Original hardware test (macOS / FlexRadio 6000) per #3765.

Credit: @dsocha.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Dariusz Socha <dariusz.socha.pl@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant