Add clock-aligned waterfall time markers

[rfoust]

Summary

Closes #5537.

Right-click the panadapter or waterfall and choose Waterfall Time Markers to display thin horizontal lines with small UTC timestamps at the left edge. Intervals are Off, 15/30 seconds, 1/5/10/15/30 minutes, and 1 hour. Off remains the default, and the selection persists independently per panadapter slot in the existing Display document.

Markers align to clock boundaries (for example, HH:mm:00 for one-minute markers) and follow their captured signal rows through live scrolling, paused history, and resize. Changing the waterfall rate affects future row cadence; existing history retains its captured row spacing. GPU and software rendering share timestamp geometry and a cached label atlas. Dedicated theme tokens provide subdued pale gray-blue lines/text with a dark label backing.

This is a client display feature with no radio protocol, TX, dependency, or unrelated behavior changes. Final visual/UX acceptance is requested from the maintainer. The minimum interval is intentionally 15 seconds to limit clutter.

Constitution principle honored

• Principle V: preference lives in the existing nested Display settings document, without a new flat settings key.
• Principle IV: original Qt implementation; WSJT-X is a behavioral reference only, with no proprietary binary-derived code.
• Principle XI: demonstrated via focused tests and native Demo automation-bridge checks.

Test plan

• Native macOS ARM64 RelWithDebInfo build with the local toolchain and RADE enabled; ARM64 executable verified and RNNoise x86 sources absent from the build graph.
• Four focused CTests passed: waterfall_time_markers_test, waterfall_time_marker_settings_test, waterfall_history_buffer_test, and spectrum_preview_logic_test.
• Geometry tests cover all intervals, wrap, fractional scrolling, paused rows, resize mapping, duplicate timestamps, missing timestamps, gaps, backward clock movement, Off, and invalid values. Settings test checks independent slots, sibling preservation, and reload.
• Mutation checks: reversing fractional-scroll direction fails the geometry checks; reverting clock-boundary labeling fails delayed-row assertions for every interval.
• Native Cocoa/Metal automation bridge using isolated settings, DEMO-0001, and TX disabled: menu selections, live motion, paused history, resize, themes, Off, and restart persistence. A retained marker stayed at row 226 when the paused waterfall grew from 410 to 470 rows. Corrected timestamps were asserted divisible by the selected 15000 ms interval and rendered pixels inspected.
• Theme seed, touchpoint manifest, test registration, engine-boundary strict check, and whitespace checks passed. No increase in the hardcoded-color ratchet.
• Windows/Linux GPU runtime verification and actual Flex/Kiwi source-switching validation have not been performed. Software renderer was syntax-checked but not exercised as a full runtime build.
• CI results pending the PR run.

New CTests are socket-free. Bridge proof is a separate manual Demo session. No live-radio validation is claimed; reopening the earlier testing profile briefly auto-connected to a saved FLEX, so that instance was closed without transmit and the final handoff used the clean Demo profile.

Checklist

• Commit is SSH-signed and locally verified.
• No new flat-key AppSettings calls.
• Clean-room code.
• Theme token documentation and generated inventories updated; CHANGELOG unchanged.
• Local code review completed; identified packet-arrival timestamp labeling corrected before this PR.

Prepared with Codex assistance. Meter smoothing and GHSA references are not applicable to this display-only feature.

[Copilot]

🔵 Needs a closer look

The unresolved font-dependent atlas-cache issue and rendering breadth require human review before approval.

Pull request overview

Adds optional clock-aligned UTC markers to the waterfall with persistent per-panadapter intervals, shared rendering, and theme integration.

Changes:

• Adds marker geometry, timestamp tracking, scrolling/history integration, and menu selection.
• Persists marker intervals in Display settings.
• Adds focused geometry/settings tests and theme-token updates.

File summaries

File	Summary
tests/waterfall_time_markers_test.cpp	Tests marker geometry and edge cases.
tests/waterfall_time_marker_settings_test.cpp	Tests persistence, validation, and slot isolation.
tests/tests.cmake	Registers the focused tests.
src/gui/WaterfallTimeMarkers.h	Defines marker intervals and geometry.
src/gui/SpectrumWidget.h	Declares marker state and rendering APIs.
src/gui/SpectrumWidget.cpp	Integrates timestamps, menus, history, and rendering.
src/gui/SpectrumWidget_TimeMarkers.cpp	Implements atlas, software, and GPU rendering. Moderate: the atlas cache omits the widget font; also requires direct Qt includes.
src/gui/DisplaySettings.h	Persists per-slot marker settings.
src/core/ThemeSeedGenerated.cpp	Adds fallback marker theme values.
resources/themes/default-light.json	Adds light-theme marker colors.
resources/themes/default-dark.json	Adds dark-theme marker colors.
docs/theming/canonical-tokens.md	Documents marker theme tokens.
docs/architecture/aetherd-touchpoints.md	Updates touchpoint metadata.
CMakeLists.txt	Registers the marker implementation source.

Review details

Suppressed comments (1)

src/gui/SpectrumWidget_TimeMarkers.cpp:60

• The atlas cache key omits the widget font even though labelFont is rebuilt from font() on every call. If the application/widget font or style changes while the labels, pixel size, and DPR stay the same, this early return keeps the old glyphs and m_wfTimeMarkerLabelHeight, so marker labels can be clipped or misaligned; include the relevant font in the cache key or invalidate the atlas on font changes.

    if (!m_wfTimeMarkerAtlasDirty && labels == m_wfTimeMarkerLabels
        && m_wfTimeMarkerAtlas.size() == pixels
        && m_wfTimeMarkerAtlas.devicePixelRatio() == dpr) {

• Files reviewed: 14/14 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[aethersdr-agent]

1. Issue fit

Yes. Issue #5537 lists eight behaviours and the diff covers all eight: the submenu's nine intervals map exactly onto kWaterfallMarkerIntervals with the labels the issue names; default Off with per-pan-slot persistence; thin line plus left-edge UTC stamp; clock-boundary alignment with the boundary time as the label (boundaryMs = (t / intervalMs) * intervalMs, not arrival time); rows followed through live scroll, pause, history scrub and resize; two theme tokens; both GPU and software render paths; and the preference stored in the nested Display document rather than a flat key.

I also checked the three "review checkpoints" the triage comment on #5537 asked for, and all three hold:

1. Ring re-laid from origin 0 on rebuild — rebuildWaterfallViewportForFrame resets m_wfVisibleTimeRows alongside m_wfWriteRow = 0 (SpectrumWidget.cpp:6129), and paintWaterfallRowsFromHistory re-lays it with the same waterfallVisibleRowForAge(writeRowOrigin, age, height) mapping it uses for pixels, so markers cannot shear away from their signal rows.
2. History scrub refills from m_wfHistoryTimestamps — done at SpectrumWidget.cpp:5988-5991 using m_wfHistoryOffsetRows + age, matching the rowIndex lookup on the line above it.
3. GPU markers get their own quad — drawWaterfallTimeMarkersGpu draws from a dedicated dynamic VBO and does not touch m_overlayStatic / m_overlayStaticDirty, so smooth scroll does not force a full-overlay repaint plus upload.

2. Scope

File(s)	Change	Claimed?	Verdict
src/gui/WaterfallTimeMarkers.h	New header-only interval table + pure geometry function	Yes	In scope
src/gui/SpectrumWidget_TimeMarkers.cpp	New TU: setter, marker query, text atlas, CPU + GPU draw	Yes	In scope
src/gui/SpectrumWidget.{h,cpp}	Row-timestamp ring, submenu, both draw hooks, save/restore, automation snapshot	Yes	In scope
CMakeLists.txt	Registers the new TU in GUI_SOURCES	Yes	In scope (verified it is actually referenced)
src/gui/DisplaySettings.h	Per-pan-slot accessor pair	Yes	In scope; mirrors panMenuExpanded exactly
resources/themes/*.json, src/core/ThemeSeedGenerated.cpp, docs/theming/canonical-tokens.md	Two new tokens + doc	Yes	In scope; seed insertion is correctly alphabetical and Static checks passed
docs/architecture/aetherd-touchpoints.md	ThemeManager.h includer count 145 → 146	Not in the title, but mechanically forced	In scope — the new TU includes core/ThemeManager.h, so the manifest count is exactly right
tests/tests.cmake, two new tests	Registration + coverage	Yes	In scope; settings test correctly joins AETHER_SETTINGS_CONSUMERS

Everything in the diff is explained by #5537. No unrelated files, no formatting churn, no deleted guards — I read the - lines and the only removals are the context lines the hunks re-emit. CHANGELOG.md correctly untouched.

New public surface, flagged for a maintainer ruling rather than waved through: a Q_PROPERTY(int waterfallTimeMarkerSeconds) on SpectrumWidget; two automation-snapshot keys (waterfallTimeMarkerSeconds, waterfallTimeMarkers); a waterfallTimeMarkers key inside the Display settings document; two theme tokens; and the waterfallTimeMarkersMenu / waterfallTimeMarkers<N> object names. All of it is what the feature needs, and #5537 is labelled maintainer-review with "Final visual/UX direction is subject to maintainer review", so this is a decision to make, not a violation.

Preference check (§6): this adds a new overlay defaulting to Off; it does not restyle, reorder or re-default any existing UI. Not smuggled preference.

Socket tests: neither new test opens, binds, listens or connects — waterfall_time_markers_test links Qt6::Core only and drives the pure geometry function; waterfall_time_marker_settings_test links aethercore + Qt6::Core and touches only the settings store. No fake radio/amp/tuner peer added, and none removed. Nothing to record beyond that.

3. Blockers

None. I tried hard to find one — see §5 for the specific attacks and why each failed.

4. Nits (all non-blocking)

• Test settings isolation hand-rolls the sandbox. tests/waterfall_time_marker_settings_test.cpp:10 sets AETHER_SETTINGS_DIR directly instead of using TestSettingsProfile.h, which the CMake contract names as the isolation mechanism. Inline comment has the detail; it is genuinely lower-risk than it sounds and there is precedent, so a nit.
• Two assertions in that test are tautological (lines 31-34) — the getter validates independently of the setter, so they pass even if the setter wrote garbage. Inline comment.
• m_wfVisibleTimeRows is the only restored vector without a post-restore size re-init (SpectrumWidget.cpp:6821) — currently unreachable, but the parity gap is worth a comment for the next editor. Inline.
• The atlas-caching comment slightly overstates the code (SpectrumWidget_TimeMarkers.cpp:85) — the label dedup depends on marker.y, so row motion can invalidate the atlas. Rare at ≥15 s intervals. Inline.
• QAccessibleValueChangeEvent on the SpectrumWidget (SpectrumWidget_TimeMarkers.cpp:22) is an odd fit — the widget is not a value-type accessible, and the checkable menu actions already expose the selection. Not wrong, just probably not doing anything for a screen reader.
• The 30-minute and 1-hour entries will rarely place a visible line. Already raised in the #5537 triage: kWaterfallHistoryMs caps retention and the visible ring is only height rows, so at typical rates a screenful is well under a minute and an hour boundary lands inside it only occasionally. This matches WSJT-X and is harmless — flagging only because it is a maintainer call on menu space, not a defect.
• Neither new test is on the frozen per-PR ctest gate. ci.yml's -R steps cover the four DV tests, cross_needle_meter_test, mac_nr_filter_test and asr_gpu_probe_test. The five green checks here compiled the new code but did not execute either new test; they run in full-suite.yml at merge. Correct per canon — noting it so "CI is green" is not read as "the new tests passed in CI".

5. What I tried to break (and failed)

Everything below is reasoned from reading the code in /tmp/aetherclaude/pr-5538. I have no build and no way to drive the GUI, so nothing here was reproduced at runtime — where I say a path is safe, I mean I traced every writer, not that I watched it run.

• Out-of-bounds read on the new ring. appendVisibleRow reads m_wfVisibleTimeRows[m_wfWriteRow] before the pre-existing m_wfWriteRow = (m_wfWriteRow - 1 + h) % h normalizes it — and that modulo is itself the tell that the old code did not assume m_wfWriteRow < h. I traced every reallocation of m_waterfall: resetCurrentWaterfallRowsForSize ends in clearCurrentWaterfallRows() which sets m_wfWriteRow = 0 (:6556); the resize commit sets it to 0 (:11808); rebuildWaterfallViewportForFrame sets it to 0 (:6129); and restoreCurrentWaterfallStreamState bails to the reset branch on stateHasWrongWaterfall before ever assigning restored.wfWriteRow. The invariant holds at entry, so the unnormalized index is in range. This was my best blocker candidate and it does not survive.
• Stale timestamp latch. m_wfIncomingTimestampMs is a latch, so I checked all three appendVisibleRow call sites — the Flex native-tile loop (:8751), the fallback path (:12350) and the Kiwi path (:12407). Each is immediately preceded by appendHistoryRow, and the latch assignment is the first statement in appendHistoryRow, ahead of the hidden-source early returns. No path reaches appendVisibleRow with a stale stamp.
• Markers invisible when the time-scale strip is hidden. The width is std::min(wfContentRect.width(), waterfallTimeScaleRect(wfRect).left() - wfRect.left()), which goes negative if that rect is ever null. waterfallTimeScaleRect is unconditional and anchored to wfRect.right(), so left() - wfRect.left() is always wfRect.width() - stripWidth. No negative-width clip.
• GPU SRB layout incompatibility with the borrowed m_ovPipeline. The new SRB has one binding; m_ovSrb, which the pipeline was created against, also has exactly one (sampledTexture(1, FragmentStage, …)) — layout-compatible, so setShaderResources is legal.
• The #3294 premultiply-squared class of bug. The atlas is Format_RGBA8888_Premultiplied and m_ovPipeline's blend is srcColor = One / dstColor = OneMinusSrcAlpha — the correct pairing for a premultiplied source. Had it been SrcAlpha, the translucent label backing would have been crushed exactly as the passband fill was.
• Scroll-offset sign, i.e. markers sliding against their rows during animation. m_waterfallScrollDistanceRows - waterfallScrollProgressRows() is byte-identical to the pre-existing sampleOffsetRows at SpectrumWidget.cpp:15699, which is what drawWaterfall's sourceStart uses. Same convention, not a re-derivation.
• Duplicate/wrong labels from the m_wfTimeMarkerLabels.indexOf(...) lookup. For a monotonic run, two visible markers cannot share a boundary bucket (the younger row's predecessor would already be inside it, so it would not register a crossing). The one non-monotonic case — an NTP step back that re-crosses the same boundary — yields two markers with the same correct label, which is what indexOf renders.
• Degenerate geometry. The line quad's zero-height source rect, an empty labels list, and zero-width clips: the clipped.isEmpty() early return precedes every division, atlasHeight is ≥ 2 even with no labels, and labels is non-empty whenever markers is (the first marker always clears the dedup threshold).
• Token hex byte order. #dc0f0f1a is 9 chars, so Qt reads #AARRGGBB — alpha 220 over #0f0f1a. colorToTokenString emits HexArgb for alpha < 255, and #5a0f0f1a already ships in default-dark.json with the same RGB. The value follows the house convention rather than inventing one.
• Whether the new tests would pass against a broken implementation. waterfall_time_markers_test is real: I hand-evaluated the wrapped-ring case (head 3, size 4, offset 0.25, height 40) and the asserted y values 7.5 and 27.5 and the count of 2 all follow from the guards. Deleting the timestampMs <= previousMs check breaks "a batched tile must not repeat a timestamp"; deleting the bucket comparison breaks "before boundary"; returning arrival time instead of the boundary breaks the per-interval delayed-row case. It does not assert the implementation back to itself.
• Could not check: any runtime/visual behaviour, the actual GPU draw on a real RHI backend, and whether the 1 px line reads well against a live waterfall. The PR body's macOS ARM64 automation-bridge verification is not something I can reproduce headless — a maintainer eyeballing the overlay in both render paths (and at DPR 2) would settle the visual half.

6. Recommendation

Needs maintainer decision — on the feature itself, not on the code. #5537 carries maintainer-review and explicitly reserves visual/UX direction, and this PR adds new public surface (a Q_PROPERTY, two automation keys, a settings key, two theme tokens). On engineering grounds I would merge it: it sits on structures that already existed rather than adding a retention layer, the geometry is a pure function with a genuine test behind it, both render paths are covered, the per-slot scope correctly avoids the global propagation the triage warned about, and it survived every attack in §5. The two open calls are whether the 30-minute and 1-hour entries earn their menu space, and a visual check of the overlay in both paths. Nits are all cosmetic or test-hygiene and none need to block.

Nice work — this is unusually careful about the ring/scroll invariants, which is where this feature had the most room to go quietly wrong.

---

_{🤖 aethersdr-agent · cost: $14.2168 · model: claude-opus-5}