From b090cc2a219a58c170e3be341e0790b1bc4b2eb4 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Mon, 12 Oct 2026 19:49:39 +0200 Subject: [PATCH] fix: math in a file view -- the buffer was emptied, and the derived ring filled twice Two defects, both reached by adding a math channel to an open waveform or a replayed event, and both measured before and after. **A file view has no producer, so a structural change left it empty.** `set_math_channels()` rebuilds the channels *and* the buffer, and a rebuild drops the history: right for a stream, whose producer refills it from the next packet, and wrong for a file, which has nothing to refill it with. The `--open` dump read `0 samples vs 0 time slots` for *every* channel, the file's own `sine`/`square`/`ramp`/`noise` included, though the load had reported 402 -- that is what "adding a math node breaks the plotter" was. `reload_file_view()` re-reads the file after such a change, which is also what computes the derived channels over it: the load's own pushes run the math once the set is in place, so `--open` with a session that defines graphs works for the first time. **The file path pushed into the derived rings, and so did the engine.** `push_loaded_slot()` did not honour the rule `ChannelInfo::math` states for the packet path ("the ingest must skip it"), so each loaded slot put a value in a derived ring and `push_math_sample()` then put the computed one: two samples per slot, a trace at double rate -- the derived channels read `804 samples vs 402 time slots`. Skipping them there is the whole fix. Readings now, all equal, which is the assertion this file's own invariant makes: --open the file: math x2 402 samples vs 402 time slots, y=[-1.9021, 0.0063] (2x the sine) math lp 402 vs 402, mean=-0.4900 (a real filter output, not zero) live: math x2 25100 vs 25100 replayed event: math x2 4000 vs 4000 I first tried filtering the file's *derived columns* out by name, since an export writes every plotted channel. It removed the double count and zeroed `math lp`, because the column it dropped was the source a session graph reads -- so it is reverted: those columns are legitimate data, and the fix belongs in the push, not the loader. --- docs/claude/invariants-math-channels.md | 17 +++++++++++++++++ src/model.cpp | 4 ++++ src/ui.cpp | 23 +++++++++++++++++++++++ src/ui_app.hpp | 4 ++++ src/ui_math.cpp | 4 ++++ 5 files changed, 52 insertions(+) diff --git a/docs/claude/invariants-math-channels.md b/docs/claude/invariants-math-channels.md index 453346e..3f0d4a9 100644 --- a/docs/claude/invariants-math-channels.md +++ b/docs/claude/invariants-math-channels.md @@ -36,6 +36,23 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear `visible_samples()` reading row 0's axis after the window was destroyed, and `UDPSCOPE_EXIT_AFTER_MS` arms the trigger first, so the dump can land in `Waiting`. The live `ch N '' ... y=[..]` lines are the race-free reading of the same thing. +- **A file view has no producer, so a structural change would leave it empty.** `set_math_channels()` + rebuilds the channels *and* the buffer, and a rebuild drops the history: right for a stream, whose producer + refills it from the next packet, and wrong for a file, which has nothing to refill it with. Adding a math + channel to an open waveform therefore emptied *every* channel -- `samples=0` on the sources too, which a user + reads as "the plot is broken" -- and the same emptiness follows any structural change in a file view. + `reload_file_view()` (in `ui.cpp`) re-reads the file after such a change, which is also what computes the + derived channels over it: the load's own pushes run the math once the set is in place. It is why `--open` + works at all with a session that defines math channels, since `main()` loads the file *before* the UI (and + so before the session's graphs) exists. +- **Every path that fills a store must skip a derived channel, not just the packet path.** + `ChannelInfo::math` states the rule for the ingest; `push_loaded_slot()` broke it, so loading a file pushed + its own value into each derived ring and `push_math_sample()` then pushed the computed one: two samples per + slot, i.e. a trace at double rate. The reading that catches it is the one already in this file -- + `math '': N samples vs M time slots` must be **equal** -- and a file view now has to be read as well + as a live one (`--open ` with `UDPSCOPE_PROBE_MATH_MS`). An export writes every *plotted* channel, + so a file carries the derived ones as ordinary columns: those are real channels and are filled normally, + which is why the fix is in the push and not in the loader. `math cost: S slots, N ns/slot (T timed), X ms total est` is the per-slot cost; see the math-cost invariant below for why it is sampled, why it is an upper bound, and why a figure anywhere in 78--101 ns/slot is one reading of the same thing. diff --git a/src/model.cpp b/src/model.cpp index 0dbd1ae..e4c2c03 100644 --- a/src/model.cpp +++ b/src/model.cpp @@ -301,6 +301,10 @@ void ScopeModel::push_loaded_slot(const std::vector& values, f64 t, f64 abs const std::size_t channels = channels_.size(); std::vector slot(channels, 0.0f); for (std::size_t i = 0; i < channels; ++i) { + // A derived channel has no column of its own: `push_math_sample()` below fills it, and pushing the + // file's value here as well would put *two* samples in its ring per slot -- a trace at double rate. + // The same rule the packet path states in `ChannelInfo::math`: the ingest must skip it. + if (channels_[i].math) continue; const f32 v = i < values.size() ? values[i] : 0.0f; store.values(i).push(v); slot[i] = v; diff --git a/src/ui.cpp b/src/ui.cpp index 2db9862..a50d989 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -4179,6 +4179,29 @@ gboolean on_app_key_pressed(GtkEventControllerKey*, guint keyval, guint, GdkModi return TRUE; } +/// Re-reads the file a *file view* is showing; true when something was reloaded. +/// +/// Any structural change -- a math channel added, a signal enabled, a plan change -- rebuilds the buffer +/// and drops its history. That is right for a stream, whose producer refills it from the next packet. A +/// file has no producer, so the same rebuild leaves *every* channel reading `0 samples` and the view is +/// simply empty: "adding a math channel breaks the plot" was this, and it is the same emptiness whichever +/// structural change caused it. The file's own path is the only thing that can refill it, so it is loaded +/// again -- which is also what computes the derived channels over it, since the load's own pushes run the +/// math once the set is in place. +bool reload_file_view(App& app) { + if (!app.replaying && app.model->config().open_waveform.empty()) return false; + const std::string path = + !app.waveform_name.empty() ? app.waveform_name : app.model->config().open_waveform; + if (path.empty()) return false; + WaveformLoadReport report; + std::string error; + if (!load_waveform_into_model(*app.model, path, &report, &error)) { + set_notice(app, "cannot reload " + path + " after the change: " + error, 6); + return false; + } + return true; +} + /// The header bar carries only the controls you reach for *while watching a signal*; /// everything you set up once lives in the overflow menu. Nine text buttons in a /// header bar is a wall of words to read past every time you want the one you need, diff --git a/src/ui_app.hpp b/src/ui_app.hpp index 09368eb..e9a89c8 100644 --- a/src/ui_app.hpp +++ b/src/ui_app.hpp @@ -1189,6 +1189,10 @@ void place_measure_marks(App& app); void place_search_marks(App& app); void update_measures(App& app, bool force, bool accumulate); void apply_math_channels(App& app); + +/// Re-reads the file a file view is showing after a structural change rebuilt the buffer and dropped +/// its history -- see the definition in ui.cpp. True when something was reloaded. +bool reload_file_view(App& app); void add_math_channel(App& app); std::vector math_source_names(App& app); std::string math_dialog_problem(const MathDialog* d); diff --git a/src/ui_math.cpp b/src/ui_math.cpp index 02e7422..50f606a 100644 --- a/src/ui_math.cpp +++ b/src/ui_math.cpp @@ -18,6 +18,10 @@ void apply_math_channels(App& app) { // `set_math_channels()` takes the model's lock itself -- holding it here is a self-deadlock on a // non-recursive mutex, and it is exactly how the first version of the probe hung the app. app.model->set_math_channels(app.math); + // The structural rebuild above replaced the buffer and dropped its history. A stream's producer + // refills that; a file's nothing does, so the view is re-read -- and with the math set now in place + // the reload computes the derived channels over the file as it goes. + reload_file_view(app); rebuild_math_pane(app); schedule_layout_save(app); }