feat(display): close the 3D stacked-trace wedges with offscreen spectrum - #4779
Conversation
Every 3DSS row covers the SAME frequency span while the far rows narrow to kBackWidthFrac, so the surface leaves two empty black triangles flanking it. Widen the span each row covers instead of the geometry that places it. The near rows then run off both edges of the plot and the existing perspective narrowing walks them back in with depth, closing the wedge from the front. At 1/kBackWidthFrac (1.667x, i.e. +33% per side) the deepest row lands exactly on the plot edge and no wedge remains at any depth. The projection is deliberately untouched -- a frequency still lands at 0.5 + (freq - 0.5) * depthScale -- so the converging slant of a signal through depth, the frequency ruler, and every marker stay exactly where they were. Only what a row COVERS changed, never where a frequency lands. The data path already existed: buildDssSupplementalCoverage() calibrates the native waterfall tile (wider than the panadapter) into per-row supplemental channels, and dss_mesh.vert already sampled them outside an FFT row's captured frame. Only the geometry was clipping them away. A "3D Span" slider under 3D Gain scales how much of the AVAILABLE overhang to spend, so 100 always means "everything the source gives" and 0 restores the classic narrowing trapezoid. Scaling the available span rather than the absolute maximum matters: against a ~1.15x tile an absolute reading clamps everything above ~22% to the same picture, leaving most of the travel dead. No overhang gives 1.0 at every setting -- a strict identity -- so Kiwi, the fallback producer, and the CPU image fallback render exactly as before. effectiveDbmAt() now returns coverage alongside dBm, folded into vBoundaryFade so a widened column with nothing behind it fades toward bgFill and discards instead of extruding a flat floor plateau to the edge. Verified offscreen at a matched 0.2 MHz span against a 1.153x overhang. Surface left edge, front to back, driving the slider over the bridge: span 0 gives 540 -> 460 -> 380 -> 299 -> 219 -> 144 px (the classic wedge), span 50 gives 475 -> 386 -> 297 -> 216 -> 130 -> 44, span 100 gives 410 -> 319 -> 226 -> 137 -> 41 -> 0 -- monotone, closing from the front, with the residual at the back exactly where wedgeFreeDepth(1.153) = 0.33 predicts. Confirmed on a live FLEX-8600. 237/237 tests pass. Known limits, tracked in #4778 rather than fixed here: - The retained history ring stores no supplemental and both reprojection paths wipe it, so the overhang collapses after a pan/zoom and throughout scrollback until fresh rows arrive. - Only rows carrying supplemental extend, so the deepest rows can show a ragged silhouette where coverage runs out. - DssRenderer::rebuild() (CPU fallback) ignores supplemental entirely and still draws the clipped trapezoid. AETHER_DSS_ROW_SPAN=1.0..1.667 overrides the slider for A/B, and unlike the slider it can demand more span than the source can fill, which is what exercises the coverage feather. Closes #4778
|
Note for testers: 3D Span is per-pan, not global. It follows The |
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nice piece of work, and the framing is right: widening what a row covers rather than where a frequency lands is the change that keeps the ruler, the markers and the converging slant honest, and the span-1.0 identity test is the guard that matters. I checked the std140 layout by hand — 21 scalars + 3 explicit pad floats + bgFill = 28, kDssMeshUboFloats 92→96, shadow offsets moved in step, and the writer order matches both shader blocks. AETHER_DSS_ROW_SPAN=0-equivalent (slider 0) really is bit-exact identity: rowSpanFactor lands on exactly 1.0f, freqUnit == u, insideViewport == 1 at both endpoints, and vSideDistance reduces to the old expression. Settings go through AppSettings with the same save-per-edit shape as setDssGain, and the reset path's new syncDisplaySettings arguments line up with the declaration order.
Two things I'd like resolved before merge, and both are narrow. The rest is polish.
Would like fixed before merge
- The eased
rowSpanFactorstalls partway whenever repaints stop (idle/disconnected stream) — the ease has no self-drive.src/gui/SpectrumWidget.cpp:13915 - Widening spreads the fixed mesh column count over a wider frequency range, so the visible surface is sampled at
cols / spanagainst a Nearest-filtered height texture — at minimum this wants acknowledging, since it contradicts the sampler's stated invariant.resources/shaders/dss_mesh.vert:102
Polish
AETHER_DSS_ROW_SPANis re-read and re-parsed every frame in the render path; the file already has the cached-static idiom for this.src/gui/SpectrumWidget.cpp:12513applySliceShadow's newfreqUnitparameter is always handedvFrequencyverbatim — the parameterization plus the local alias is churn with no behaviour behind it.resources/shaders/dss_mesh.frag:102
Non-blocking notes
dssRowSpanTarget()holds the branchy half of the feature (slider scaling, env clamp, non-finite bandwidth) and nothing covers it; the new tests only reach theDssRendererstatics.src/gui/SpectrumWidget.cpp:12550
The known rough edges you listed (overhang collapse on pan/zoom, ragged deep rows, CPU-fallback divergence) read as correctly scoped to #4778 to me — I'm not counting any of them against this PR.
🤖 aethersdr-agent · cost: $5.3766 · model: claude-opus-5
| { | ||
| const float target = | ||
| dssRowSpanTarget(dssTargetBandwidthMhz); | ||
| constexpr float kRowSpanAlpha = 0.12f; | ||
| m_dssRowSpanFactor += kRowSpanAlpha | ||
| * (target - m_dssRowSpanFactor); | ||
| if (std::abs(target - m_dssRowSpanFactor) < 0.002f) { | ||
| m_dssRowSpanFactor = target; | ||
| } | ||
| } |
There was a problem hiding this comment.
The ease has nothing driving it once it starts. setDssRowSpan() issues a single update(), and the only other repaint source in 3D is m_waterfallScrollTimer, which stopWaterfallScrollAnimation() kills as soon as !m_wfLive (SpectrumWidget.cpp:1968). So with a paused or disconnected stream, dragging the slider paints exactly one frame, advances m_dssRowSpanFactor by 12% of the delta, and then freezes there — the surface sits at an arbitrary intermediate span until data resumes. Same for a zoom that changes the available overhang while idle.
Cheapest fix is to keep the ease alive itself:
| { | |
| const float target = | |
| dssRowSpanTarget(dssTargetBandwidthMhz); | |
| constexpr float kRowSpanAlpha = 0.12f; | |
| m_dssRowSpanFactor += kRowSpanAlpha | |
| * (target - m_dssRowSpanFactor); | |
| if (std::abs(target - m_dssRowSpanFactor) < 0.002f) { | |
| m_dssRowSpanFactor = target; | |
| } | |
| } | |
| { | |
| const float target = | |
| dssRowSpanTarget(dssTargetBandwidthMhz); | |
| constexpr float kRowSpanAlpha = 0.12f; | |
| m_dssRowSpanFactor += kRowSpanAlpha | |
| * (target - m_dssRowSpanFactor); | |
| if (std::abs(target - m_dssRowSpanFactor) < 0.002f) { | |
| m_dssRowSpanFactor = target; | |
| } else { | |
| // Nothing else drives repaints while the stream is idle, | |
| // so the ease has to keep itself alive or it freezes | |
| // partway through a slider drag. | |
| update(); | |
| } | |
| } |
| float dssFreqUnit(float meshU) | ||
| { | ||
| return 0.5 + (meshU - 0.5) * rowSpanFactor; | ||
| } |
There was a problem hiding this comment.
The mesh grid is built once with exactly m_dss.cols() columns across u ∈ [0,1] (SpectrumWidget.cpp initialize()), and cols is the FFT bin count — which is why the height sampler is Nearest, with the comment "the grid is as dense as the texture". dssFreqUnit spreads those same columns over rowSpanFactor × the frequency range, so the portion landing inside the viewport now carries cols / rowSpanFactor columns against an unchanged cols-texel texture. At the ~1.153x tile you measured that's ~13% of bins never sampled by any vertex; at the 1.667 cap it's 40%.
With Nearest filtering that isn't a smooth softening — individual bins drop out, and which ones drop shifts as the eased factor moves, so a narrow CW carrier can flicker or vanish at the very setting the feature ships at by default. Nothing in the PR body or the tooltip mentions it.
Not asking you to resample the mesh in this PR. But it's the one invariant this change quietly breaks, so it should either be stated in #4778 alongside the other rough edges, or (cheaper) the mesh column count could be scaled by kMaxRowSpanFactor at build time so the visible density never drops below today's.
There was a problem hiding this comment.
Nice piece of work — widening what a row covers while leaving plotX = 0.5 + (freq - 0.5) * depthScale alone is the right decomposition, and pulling depthScale/rowFrequencyUnit into DssRenderer.h so the shader and the CPU-side geometry share one formula (plus the span-1.0 identity test and the "widening never moves an in-band signal" test) is exactly the guard this kind of change needs. The std140 padding is handled correctly and consistently across .vert/.frag/kDssMeshUboFloats/kShadowBandsOffset, the shadow-decal clamp widening to [-margin, 1+margin] matches dssFreqUnit's range exactly, and all three syncDisplaySettings() call sites were updated rather than leaning on the new default argument. Verification is unusually concrete.
Two things I'd like looked at before merge, both narrow.
Would like fixed before merge
AETHER_DSS_ROW_SPANis applied unconditionally, but the vertex shader's out-of-row guard is gated onrowFrequencyFrames, so on a non-FLEX source the override samples past the height texture and ClampToEdge extrudes the edge bin instead of feathering.
Polish
- The
rowSpanFactorease only advances on frames that happen to be drawn; with no data flowing it can settle mid-transition.
Non-blocking notes
vSideDistance's comment about off-screen ends doesn't match what the expression computes (the code is right, the comment isn't).- The default of 100 (and the reset-to-defaults value) means FLEX users get the new silhouette without opting in — a product call, just flagging it since the PR frames 0 as the reference rendering.
🤖 aethersdr-agent · cost: $2.2549 · model: claude-opus-5
| constexpr float kRowSpanAlpha = 0.12f; | ||
| m_dssRowSpanFactor += kRowSpanAlpha | ||
| * (target - m_dssRowSpanFactor); | ||
| if (std::abs(target - m_dssRowSpanFactor) < 0.002f) { | ||
| m_dssRowSpanFactor = target; |
There was a problem hiding this comment.
The ease only steps on frames that are actually drawn. setDssRowSpan() calls update() once, which advances the factor by 12% and schedules nothing further — so if the display isn't repainting for another reason (disconnected, paused, scrollback held still), the silhouette parks partway to the target and stays there until something else triggers a repaint.
While live data is flowing this is invisible, and dragging the slider produces enough updates to converge, but a single click on the groove or an arrow-key step on a quiet pan will visibly stall. Cheapest fix is to keep the frame pump going while it's unsettled — request another update from this block when std::abs(target - m_dssRowSpanFactor) >= 0.002f.
| float plotY = edge > 0.5 ? 1.0 : topY; | ||
| // Half the row's on-screen width, times how far this column sits from the | ||
| // nearer end. Off-screen ends give a large value, so the fade correctly | ||
| // leaves them alone. | ||
| vSideDistance = min(u, 1.0 - u) * rowSpanFactor * w; |
There was a problem hiding this comment.
The comment has it backwards: min(u, 1.0 - u) is 0 at the row ends, so an off-screen end gets the smallest value, not a large one. The behaviour is still correct — that fade band simply sits outside the plot and gets clipped — but as written the comment describes a mechanism that isn't there, and the next reader may "fix" the expression to match it.
Worth restating as: distance is measured from the row's own silhouette, so at span > 1 the faded strip falls outside the viewport and is clipped away; at span 1.0 this is bit-identical to the old min(vFrequency, 1 - vFrequency) * perspectiveWidth.
There was a problem hiding this comment.
Nice piece of work, and the framing is the right one — widening what a row covers rather than how a frequency projects is what keeps the ruler, markers and the converging slant honest, and the span-1.0 identity holds exactly under algebra, not just under test (rowFrequencyUnit is u, and vSideDistance = min(u,1-u)*w reproduces the old min(vFrequency,1-vFrequency)*perspectiveWidth term term-for-term). The UBO change checks out too: 21 scalars + 3 explicit pad floats, both the .vert and .frag blocks agree, every downstream offset is derived from kShadowBandsOffset, and std::size(uboScalars) == kDssMeshScalarFields guards the writer.
The one thing I'd want addressed is a span-collapse trigger that isn't in your known-rough-edges list, and which operators will hit far more often than pan/zoom: transmit. Everything else is polish.
Would like fixed before merge
dssRowSpanTarget()reads age-0 supplemental only, so the surface collapses to the trapezoid and re-expands on every TX and on any native-tile stall.
Polish
AETHER_DSS_ROW_SPANis re-read and re-parsed (with aQStringallocation) on every rendered frame.- The 3D Span slider is live but inert on the CPU image fallback, with nothing in the UI saying so.
Non-blocking notes
- I checked whether the new overhang could bleed onto the dBm strip: it can't. The DSS mesh viewport is
specContentWwide atspecRect.x(), so NDC clipping bounds the off-plot vertices to the plot area. - I also checked the
ClampToEdgeheight sampler as a possible smear source for the widened columns. Not reachable —rowFrequencyFramesis written as a literal1.0fin the mesh UBO, sooutsideRowalways evaluates and out-of-range columns take thecovered = 0feather. Kiwi and the fallback producer are safe even under the env override, which is a stronger guarantee than the PR description claims. - The
applySliceShadow(..., freqUnit)re-parameterisation is a no-op —freqUnitis assigned straight fromvFrequencyand nothing else is ever passed. Correct as written (thefwidthderivative is unchanged either way), just a chunk of diff that doesn't need to be there.
🤖 aethersdr-agent · cost: $9.4363 · model: claude-opus-5
…l density Two findings from review on #4779. The rowSpanFactor ease advanced one step per PRESENTED frame with nothing keeping frames coming, so it stalled wherever the last repaint left it any time nothing else was driving the pane. On an idle or disconnected stream the slider read 100 over a half-widened surface. It now requests its own frame until it converges. More seriously, widening spread a fixed kCols-wide mesh over rowSpanFactor x the viewport while the height texture still holds kCols texels ACROSS the viewport, leaving (span-1)/span of them unread. That is not shimmer: the mesh-column-to-frequency mapping is static, so the same texels were missed every frame and a narrow carrier landing on one was permanently invisible at a fixed screen position -- a real regression for CW on the primary display, and it silently falsified the height sampler's "the grid is as dense as the texture" justification for Nearest filtering. Size the mesh for the widest span instead (kMeshCols = 1280), so the on-screen columns are never sparser than the texture at any span; below the maximum it merely oversamples, which Nearest absorbs by repeating texels. A static_assert pins the invariant so a future kBackWidthFrac change cannot quietly reintroduce the blind columns, and the sampler comment now states what actually holds. Static vertex storage goes from about 20.2 to 33.7 MiB, which is the honest price of the feature. meshCols joins the UBO in one of the std140 pad slots, so the block layout, kDssMeshUboFloats and every shadow offset are unchanged. dss_mesh.vert's ribbon tangent probe now steps one mesh column rather than one texel, which are no longer the same thing. Declined from the same review: the claim that AETHER_DSS_ROW_SPAN samples past the height texture on non-FLEX sources. rowFrequencyFrames is the hardcoded literal 1.0f at UBO index 11, not derived from the source, so the out-of-row guard is active on every backend and the ClampToEdge extrusion described cannot occur. Full build green, 237/237 tests pass, shader dialects check clean, and the offscreen slider sweep is unchanged in character.
|
Thanks both — pushed Declined"
static_cast<float>(dssTargetBandwidthMhz),
0.0f, 1.0f, // targetCenterOffsetMhz, rowFrequencyFramesSo Fixed: the ease could stallCorrect, and worse than cosmetic — Fixed: the sampler invariant, properlyThis was the sharpest catch in either review, and it was worse than "wants So rather than document it, I sized the mesh for the widest span —
Not addressed in this pushThe polish and non-blocking items are still open by choice, not oversight: the The default-100 question is a product call and is with @ten9876. Full build green, 237/237 tests pass, |
rfoust
left a comment
rfoust
left a comment
There was a problem hiding this comment.
Blocking finding
[P2] Display3DSpan is introduced as a new loose AppSettings key in SpectrumWidget.cpp (load around line 2430; write around line 4540), with reset/restore handling in MainWindow_Wiring.cpp and MainWindow_Session.cpp. Constitution Principle V requires each new feature configuration to be one self-contained, owned, versioned object and explicitly says that legacy flat keys are grandfathered but nothing new may be added to them.
Please move the row-span setting into an owned 3D-display configuration object before merge. Migrating the adjacent legacy Display3DGain value into that object at the same time would give the feature one clear defaulting, migration, and atomic-persistence boundary.
Nonblocking cleanup
- Give the new 3D Span slider an accessible name/description or a proper label buddy.
- Add braces around the single-line label-update if in SpectrumOverlayMenu.cpp per project style.
The renderer geometry and data-flow changes otherwise look coherent. I reviewed the incremental update through fb3e7dd as well; it does not address the configuration blocker.
- Resolve AETHER_DSS_ROW_SPAN once instead of re-reading and re-parsing the environment on every frame from renderGpuFrame(). It cannot change under a running process. - Drop applySliceShadow()'s freqUnit parameter. After the re-anchor it was always handed vFrequency verbatim, so the parameter plus the local alias was diff noise with no behaviour behind it. - Correct the vSideDistance comment. min(u, 1-u) is ZERO at the row ends, not large, so the fade always applies there; on a widened row that strip simply falls outside the plot and is clipped. As written the comment described a mechanism that is not there, which invites "fixing" the expression to match. - Extract the taper into DssRenderer::rowSpanFactorFor() and cover it. This was the branchy half of the feature -- percentage scaling, degenerate frequency frames, saturation -- and nothing reached it, since the existing tests only exercised the geometry statics. The new tests are mutation-checked: reverting the mapping to scale the ABSOLUTE maximum rather than the AVAILABLE span fails "50% must spend half the available overhang", which is the dead-travel bug the mapping exists to avoid. Full build green, 237/237 tests pass, shader dialects clean.
Principle V: every feature's configuration is one self-contained object
under a single root key, and while the legacy flat keys are grandfathered,
nothing new may be added to them. Display3DSpan was doing exactly that.
Introduce Display3DSettings -- {"version":1,"gain":N,"span":N} -- as the 3D
view's owned object, mirroring the DisplaySourceTraceSettings pattern already
next to it. Display3DSpan was added by this branch and never shipped, so it
is gone rather than migrated. The legacy Display3DGain key is grandfathered:
it is read once to seed the object when none exists, then the object owns the
value. That gives the feature one place to default, one to migrate, and one
value to write.
Both setters now persist the whole object through a single
AppSettings::setValue + save() (Principle XIV), so a crash cannot leave the
3D view half-configured the way two independent per-key writes could. Reset
to Defaults goes through resetDisplay3DSettings(), which writes
unconditionally -- the setters early-return when a value already matches,
which would otherwise leave a stale object behind on a partial reset. Profile
recall re-applies the object as a unit instead of reading two flat keys.
Storage is AppSettings, which is sqlite-backed (app_settings table); QSettings
INI is only a one-time first-launch migration source and is never written.
Verified end to end: driving the slider persists
Display3DSettings={"gain":70,"span":0,"version":1} with no flat key and no
INI/XML file created, and seeding a legacy Display3DGain=42 with no object
present carries 42 into the object on next launch.
Also from the same review:
- Give the 3D Span slider an accessible name and description.
- Brace the single-line label update per project style.
3D Floor is deliberately not folded in: it is per-source and already owned by
DisplaySourceTraceSettings.
Full build green, 237/237 tests pass.
|
@rfoust — blocker addressed in Principle V:
|
…ntrol on the CPU fallback Two behavioural findings from the review's inline comments. The span collapsed on every transmit. dssRowSpanTarget() read age 0 alone, but pushWaterfallRow() -- the FFT-derived producer that paces rows during TX and during the RX stale-native fallback -- appends with no supplemental at all. So the moment a FLEX operator keyed up, the front row lost its overhang, the target dropped to 1.0, and the eased factor walked the whole surface back to the clipped trapezoid over ~30 frames, then back out on unkey. Every over. Take the newest VISIBLE row that actually carries a tile instead. The per-vertex coverage test in dss_mesh.vert already feathers rows that have no overhang behind them, so the host never needed the front row to be the one with data -- that split is the point of the design. Once the last covered row scrolls out of the visible ring there genuinely is no overhang on screen and relaxing to the trapezoid becomes the correct answer rather than a flicker. This covers Kiwi and rows rebuilt from retained history the same way, since they drop supplemental identically. The scan lives in DssRenderer::newestSupplementalBandwidthMhz(), which is where the ring is owned and where it can be tested. The regression test is mutation-checked: restricting the scan back to age 0 fails "overhang must survive uncovered rows at the front while covered rows are still on screen". Second, the 3D Span control was inert on the CPU image fallback, which ignores rowSpanFactor and always draws the narrowing trapezoid. The slider moved, the label updated, the setting persisted, and nothing on screen changed -- indistinguishable to the operator from "this source ships no overhang". The row is now disabled when the mesh path is unavailable, with a tooltip naming the reason (no RGBA16F support). Both tooltip wordings live in setDssRowSpanSupported() so they cannot drift apart. Full build green, 237/237 tests pass, shader dialects clean, offscreen slider sweep unchanged.
|
Both inline findings fixed in Span collapsed on every transmitConfirmed and fixed — this was a real bug and the sharpest functional catch on It now takes the newest visible row that actually carries a tile. As the The scan lives in Span control inert on the CPU fallbackAlso fixed. The row is disabled when Where the PR stands
Full build green, 237/237 tests pass, |
rfoust
left a comment
rfoust
left a comment
There was a problem hiding this comment.
Follow-up review of the fixes through add2dc2. The original Principle V flat-key blocker is substantially addressed, and the accessibility, easing, mesh-density, environment-cache, TX-collapse, runtime RGBA16F fallback, and test-coverage items all check out in source.
Issue fit remains good: the projection stays frequency-stable while supplemental coverage closes the wedges. One supported renderer configuration still exposes a nonfunctional control, so I am keeping Request changes. The legacy-gain migration note below is nonblocking.
Verification: merged this head without conflicts into current upstream/main b730345 (the GitHub merge ref was one main commit stale); full native arm64 application compile/link passed; dss_renderer_test passed; shader dialects passed; strict engine-boundary check reported 0 blockers; current GitHub Linux/macOS/Windows/static/CodeQL checks are green. Live-radio proof was not rerun.
| // Tooltip text lives in setDssRowSpanSupported() so the enabled and | ||
| // unavailable wordings cannot drift apart. | ||
| m_dssRowSpanSupported = false; | ||
| setDssRowSpanSupported(true); |
There was a problem hiding this comment.
[P2] This still enables an inert control in compile-time CPU builds. AETHER_GPU_SPECTRUM=OFF is supported (and can be selected automatically when the Qt private GPU prerequisites are unavailable), but the only later setDssRowSpanSupported(m_dssMeshReady) call is inside SpectrumWidget.cpp's AETHER_GPU_SPECTRUM block. The software branch never corrects this initial true state, while DssRenderer::rebuild() ignores rowSpanFactor. Result: the slider moves and persists but cannot change the display. Please initialize/force the row unsupported in the non-GPU path as well, ideally with a focused CPU-build configuration check.
| m_dssRowSpanPct = 100; | ||
|
|
||
| const QString raw = AppSettings::instance() | ||
| .value(display3DSettingsKey(), QString()).toString(); |
There was a problem hiding this comment.
[P3, nonblocking] The new object fixes the loose Display3DSpan key, but this does not yet perform the claimed read-once migration of legacy Display3DGain. When Display3DSettings is absent, the function seeds the members and returns without saving the object; every later launch reads the flat key again until the operator edits or resets a control. Persist the initialized object only on the absent-object legacy migration path (without overwriting malformed or future-version data), or narrow the migration claim.
…migration
Follow-up review found two gaps in the previous pass.
The CPU-only build still offered the control. rowSpanFactor is a dss_mesh.vert
uniform, so only the GPU mesh honours it -- but the single
setDssRowSpanSupported(m_dssMeshReady) call lived inside SpectrumWidget's
AETHER_GPU_SPECTRUM block. A build configured with AETHER_GPU_SPECTRUM=OFF
(supported, and selected automatically when the Qt private GPU prerequisites
are missing) never reached it, so the menu kept its enabled default while
DssRenderer::rebuild() ignores the uniform entirely: the slider moved,
labelled and persisted over a display it could not change.
There are two independent routes to the fallback -- the build flag and a
runtime without RGBA16F -- and both must disable the control, so the decision
is now one pure predicate, dssRowSpanSupported(gpuSpectrumBuild, meshReady),
in SpectrumPreviewLogic.h beside the existing host-side config predicates.
The row starts disabled and the GPU path enables it only once
initDssMeshPipeline() has fully succeeded. Covered by a focused CPU-build
configuration check in spectrum_preview_logic_test; the test is
mutation-verified, since dropping the build flag from the predicate fails
"a CPU-only build must never offer the 3D span control".
The legacy migration also did not actually migrate. loadDisplay3DSettings()
seeded the members from the grandfathered flat Display3DGain key when no
object existed but never wrote the object, so the flat key was re-read on
every launch and "the object owns the value" only became true once the
operator happened to touch a control. The absent-object path now persists the
object once, which is the migration.
Malformed and future-version objects are deliberately still left untouched --
overwriting either would destroy the evidence, or fields a newer build
stored. Verified end to end: a launch with legacy Display3DGain=42 and no
object writes {"gain":42,"span":100,"version":1} without any control being
touched, while a corrupt object and a {"version":9,...} object both survive a
launch byte-identical.
Full build green, 237/237 tests pass, shader dialects clean.
|
@rfoust — both addressed in [P2] Inert control in
|
| Starting state | After one launch, no control touched |
|---|---|
legacy Display3DGain=42, no object |
{"gain":42,"span":100,"version":1} written |
{not valid json |
byte-identical, untouched |
{"version":9,"gain":33,"span":77,"unknownField":true} |
byte-identical, untouched |
Full review ledger
Every item across all four reviews is now resolved or explicitly declined:
- Declined (1):
rowFrequencyFrames/ ClampToEdge on non-FLEX — it's a
hardcoded1.0f, so the guard is active on every backend. Your third bot pass
independently reached the same conclusion and called it "a stronger guarantee
than the PR description claims". - Fixed (13): ease stall · mesh sparser than the height texture · Principle V
flat key · TX span collapse · CPU-fallback inert control (runtime and
build) · legacy migration · per-frame env re-parse ·applySliceShadowchurn ·
vSideDistancecomment ·dssRowSpanTargetcoverage · accessible name ·
braces. - Open, and not mine to decide: default 100 vs opt-in, with @ten9876.
Three of the fixes carry mutation-verified tests — the TX collapse, the
available-vs-absolute span mapping, and now the CPU-build guard. Those are the
three places where a plausible-looking change silently produces the wrong
behaviour, so I wanted proof the tests fail on the bug rather than merely
passing on the fix.
Known and tracked in #4778, unchanged: the overhang still collapses on pan/zoom
and in scrollback because the retained history ring stores no supplemental.
Full build green, 237/237 tests pass, check_shader_dialects.py clean.
Three reviewers independently read the default of 100 as the risky choice, since 0 is the reference rendering and the PR frames it that way. It is deliberate. The control lives in the Display overlay's 3D VIEW section, and this project's discoverability is not strong enough for an opt-in default to mean anything other than "most operators never learn the feature exists". Anyone who prefers the classic narrowing trapezoid has a labelled slider; anyone who does not know to look gets the intended view. Comment only -- the default was already 100 at all six sites that express it (member initialiser, loadDisplay3DSettings, resetDisplay3DSettings, the menu row, the syncDisplaySettings parameter default, and the reset sync call). Recorded at the member so the next reviewer finds the reasoning instead of raising it a fourth time, with the condition attached: do not flip it without also solving discoverability.
Product decision: 3D Span stays defaulted to 100@ten9876 has ruled on the one item I'd left open. Keeping the default fully on, The control sits in the Display overlay's 3D VIEW section. Shipping it at 0 @rfoust and the bot reviews each raised this independently, so I've recorded the That commit is comment-only. The default was already 100 at all six sites that PR statusEvery review item across all four passes is now closed:
Three fixes carry mutation-verified tests — the TX collapse, the Still open and tracked in #4778, unchanged and by design for this PR: the Ready for the team test. Full build green, 237/237 tests pass, shader dialects |
2 participants
Closes #4778
What this does
In 3D Stacked Trace mode every row covers the same frequency span while the
far rows narrow to
kBackWidthFrac(0.60), so the surface leaves two emptyblack triangles flanking it.
This widens the span each row covers instead of the geometry that places
it. The near rows then run off both edges of the plot and the existing
perspective narrowing walks them back in with depth, so the wedge closes from
the front.
The projection is deliberately untouched — a frequency still lands at
0.5 + (freq - 0.5) * depthScale— so the converging slant of a signalthrough depth, the frequency ruler, and every marker stay exactly where they
were. Only what a row covers changed, never where a frequency lands.
The extra spectrum was already plumbed:
buildDssSupplementalCoverage()calibrates the native FLEX waterfall tile (wider than the panadapter) into
per-row supplemental channels, and
dss_mesh.vertalready sampled them outsidean FFT row's captured frame. Only the geometry was clipping them away.
What to test
There's a new 3D Span slider under 3D Gain (Display → 3D VIEW).
regression guard for anyone who doesn't want this.
always means "everything the radio gives" and every step below it visibly
narrows the surface. (An absolute mapping left most of the travel dead
against a ~1.15x tile.)
Worth a careful look at the band-edge markers, the frequency ruler, and slice
passband shading — those are what a geometry change like this breaks quietly.
A signal should sit at the same horizontal position it always did; only the
extra spectrum beyond the ruler ends is new.
Non-FLEX sources ship no overhang, so Kiwi, the fallback producer, and the CPU
image fallback render exactly as before at any slider setting.
AETHER_DSS_ROW_SPAN=1.0..1.667overrides the slider and, unlike the slider,can demand more span than the source can fill — that is what exercises the
coverage feather.
Expected rough edges
These are known and tracked in #4778, not regressions to file:
history ring stores no supplemental and both reprojection paths wipe it.
Same in scrollback.
out — only rows carrying supplemental extend.
now visibly diverges from the GPU mesh path.
Verification
Measured offscreen under Xvfb at a matched 0.2 MHz span against a 1.153x
overhang, driving the slider over the automation bridge. Surface left edge in
px, front to back:
Monotone, closing from the front, with the residual at the back exactly where
wedgeFreeDepth(1.153) = 0.33predicts. Confirmed on a live FLEX-8600.237/237tests pass, including new coverage for the span-1.0 identity, frontoverhang, monotone narrowing,
wedgeFreeDepth/coverage agreement, and thatwidening never moves an in-band signal.
tools/check_shader_dialects.pypasses— the new varying and uniform compile in all four baked GLSL slices
(130/140/150/300es).
🤖 Generated with Claude Code