fix(spectrum): always draw bandwidth indicator at full brightness for non-active slices#3547
Conversation
There was a problem hiding this comment.
Reviewed the change against the code. This is a clean, well-scoped fix — thanks @mvanhorn. 👍
Correctness verified. bandCol is reconstructed exactly the way sliceColorForOverlay() computes the active colour internally (SpectrumWidget.cpp:2642):
const int colourIdx = SliceLabel::displayColorIndex(so.sliceId, so.perClientLetter);
if (so.isActive) return SliceColorManager::instance().activeColor(colourIdx);
return SliceColorManager::instance().dimColor(colourIdx);So SliceColorManager::instance().activeColor(colourIdx) reliably returns the full-brightness colour irrespective of so.isActive — the passband fill and filter-edge lines now stay visible on inactive slices, while the VFO centre line / triangle / RIT-XIT lines keep col (dimmed) to preserve the active-slice focus cue. That matches both the stated intent and #3484.
Conventions / safety: no AppSettings/QSettings involvement, no new API surface (reuses the same singleton already in scope), no null/resource concerns, and the change is confined to the single file as described. The explanatory comment is a nice touch for the next reader.
Minor (non-blocking nit): colourIdx is now computed twice for the same slice — once here and once inside sliceColorForOverlay(so). If you wanted, a tiny sliceBandColorForOverlay() helper alongside the existing one would centralize the index math, but it's genuinely optional and not worth churning the PR over.
Looks good to merge once CI is green.
🤖 aethersdr-agent · cost: $4.5458 · model: claude-opus-4-8
|
@jensenpat — tagging you because we turned up prior discussion that's directly relevant to this PR before it moves forward. While reviewing #3547 I went back through the issue history and found two things worth surfacing: 1. This exact request was raised before and you weighed in on it Issue #2389 ("Bandwidth barely visible on not active panadapter", filed 2026-05-05 by @kn7k) is the same user-facing complaint as #3484 and makes the same request. You responded there with an explicit design rationale:
That context doesn't appear anywhere in this PR — mvanhorn likely wasn't aware of it. #2389 is still open and unresolved, and no design decision was ever formally recorded there. 2. The premultiplied-alpha bug (#3294 / #3297) complicates the picture Some of the "barely visible" reports in the wild — including the macOS regression in #3419 — were actually caused by a separate GPU compositor bug that double-multiplied alpha, crushing effective passband brightness from ~13.7% to ~1.9%. That was fixed in So the current behaviour post-#3297 is: the design intent (dim inactive slices) is working as intended, but the resulting ~5.5% brightness is arguably below the threshold where "dim" becomes "invisible." The question is whether that's an acceptable expression of the design intent or whether it's an unintended severity. 3. Governance note Per GOVERNANCE.md, visual design changes and default UX behaviour changes require an Happy to defer to your judgment here. If the design intent has evolved since #2389 and full-brightness bandwidth is now acceptable, that's great — but it would be helpful to capture that decision in #2389 (or a new RFC) so the rationale is on record. If the concern still stands, the author should be directed there before this merges. |
… non-active slices Split the colour used for bandwidth affordances (passband fill and filter-edge lines) from the colour used for focus affordances in the per-slice overlay draw path. Previously both used sliceColorForOverlay(), which applies dimColor() on non-active slices, making the passband shading and filter edges nearly invisible at ~5.5% effective brightness. Introduce bandCol = SliceColorManager::activeColor(colourIdx) so passband fill and filter-edge lines always render at full slice colour regardless of active state, while the VFO centre line, triangle marker, and RIT/XIT lines retain the dimmed col to preserve the active-slice focus cue. Fixes aethersdr#3484
…le + clearly inactive (aethersdr#3484/aethersdr#2389) Refines the aethersdr#3547 approach per maintainer direction. The original fix drew the inactive-slice passband/filter-edge affordances in the slice's full-brightness colour, which fixed visibility (aethersdr#3484) but erased the active/inactive distinction — reintroducing the TX-into-wrong-slice confusion @jensenpat designed the dim against (aethersdr#2389). Now colour, not brightness, signals inactive: an inactive slice's bandwidth renders at full brightness in the neutral secondary colour (`color.text.secondary`), while the active slice keeps its own colour. So the passband is clearly visible AND obviously not the active/TX slice. Focus affordances (VFO centre line, triangle, RIT/XIT) keep the dimmed `col`. Uses the existing theme token (re-themes; no hardcoded hex, per the aethersdr#3102 token migration) — the same `color.text.secondary` already used for the other neutral markers in this widget. Co-authored-by: mvanhorn <mvanhorn@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fd6d046 to
36d15f7
Compare
|
@mvanhorn — thanks for this, and for the clean tracing to #3484. I pushed a refinement on top ( The refinement — colour, not brightness, signals inactive. @NF0T surfaced that the dim on inactive-slice bandwidth is intentional (#2389): @jensenpat designed it so an inactive slice doesn't look TX-selectable. Drawing it in the slice's full-brightness colour (the original fix) fixed visibility but erased that cue. So instead: an inactive slice's bandwidth now renders at full brightness in the neutral secondary colour ( Implementation note: used the existing theme token (re-themes, no hardcoded hex per the #3102 migration) — the same Verified: clean build against current |
|
Thanks @ten9876 for taking this in. The bandwidth affordances staying full brightness should make the passband easier to read. |
…ur, not dimming (aethersdr#3484/aethersdr#2389) (aethersdr#3547) Inactive-slice bandwidth affordances (passband fill + filter edges) now render at full brightness in the neutral secondary colour instead of dimmed slice-colour. Restores visibility (aethersdr#3484) while colour — not brightness — signals inactive, preserving the TX-into-wrong-slice guard (aethersdr#2389). Theme-token based. Design decision recorded in aethersdr#2389. Authored by @mvanhorn with a maintainer refinement (colour instead of brightness) + rebase. Co-authored-by: mvanhorn <mvanhorn@users.noreply.github.com>
Summary
Fixes #3484. Bandwidth affordances (passband fill and filter-edge lines) in
SpectrumWidget::drawSliceMarkersnow always use the full-brightness active colour, regardless of which slice is currently active.Previously both used
sliceColorForOverlay(), which returnsdimColor()on non-active slices - combining ~40% RGB with the low-alpha draw produced approximately 5.5% effective brightness, making the bandwidth indicator nearly invisible on inactive slices.The fix introduces
bandCol = SliceColorManager::activeColor(colourIdx)alongside the existingcol = sliceColorForOverlay(so)and usesbandColonly for the two passband draw calls. Focus affordances (VFO centre line, triangle marker, RIT/XIT lines) retain the dimmedcolto preserve the active-slice visual cue. Change is confined tosrc/gui/SpectrumWidget.cpp.Constitution principle honored
Principle VIII - Evidence Over Assertion. The change traces directly to a user-reported regression (#3484). The color API used (
SliceColorManager::activeColor(colourIdx)) matches the existing pattern insidesliceColorForOverlay()itself - no new API surface, no assertion-without-evidence.Test plan
cmake --build build)Checklist
docs/COMMIT-SIGNING.md)AppSettingscalls - use nested-JSON-under-one-key (Principle V)MeterSmoother(Principle II)