diff --git a/docs/claude/invariants-math-channels.md b/docs/claude/invariants-math-channels.md index 3f0d4a9..58c8d1b 100644 --- a/docs/claude/invariants-math-channels.md +++ b/docs/claude/invariants-math-channels.md @@ -42,7 +42,13 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear 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` + derived channels over it: the load's own pushes run the math once the set is in place. It fires only when the + buffer was *really* dropped -- `stored_slots() > 0` means the change found nothing to do -- and it says so + on stderr (`reloaded after a structural change`), because a second read of a file is worth + accounting for. `set_math_channels()` in turn treats an identical list as no change at all, so a no-op + application (the session apply, a re-open) neither rebuilds nor drops nor re-reads; the comparison leaves the + nodes' `x`/`y` out on purpose, since those are the canvas's own and no op reads them -- a graph that only + *moved* is not a change, and rebuilding for it would drop the buffer to store two numbers. 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.** diff --git a/src/math_channel.hpp b/src/math_channel.hpp index 2f97e3e..4c9a569 100644 --- a/src/math_channel.hpp +++ b/src/math_channel.hpp @@ -58,6 +58,17 @@ struct MathNode { double hi = 1.0; double x = 0.0; ///< the editor's box position, logical pixels. double y = 0.0; + + /// Whether two nodes are the same node for the *engine's* purposes. + /// + /// `x`/`y` are the canvas's own and no op reads them, so a graph that only moved is not a change: + /// rebuilding for it replaces the buffer and drops its history -- and in a file view that means + /// re-reading the file -- to store two numbers nothing but the editor draws with. + bool operator==(const MathNode& other) const { + return op == other.op && source == other.source && in_a == other.in_a && in_b == other.in_b && + gain == other.gain && offset == other.offset && fc == other.fc && q == other.q && + lo == other.lo && hi == other.hi; + } }; /// A derived channel: a graph of nodes and which node is the answer. @@ -67,6 +78,11 @@ struct MathChannel { bool enabled = false; std::vector nodes; int output = -1; ///< the node whose value is the channel's. + + bool operator==(const MathChannel& other) const { + return name == other.name && unit == other.unit && enabled == other.enabled && + output == other.output && nodes == other.nodes; + } }; /// The per-channel runtime: the filter memory and the per-node scratch. Kept apart from the diff --git a/src/model.cpp b/src/model.cpp index e4c2c03..6b1eb87 100644 --- a/src/model.cpp +++ b/src/model.cpp @@ -257,6 +257,11 @@ void ScopeModel::append_math_signals_locked() { void ScopeModel::set_math_channels(std::vector channels) { std::lock_guard lock(mutex_); + // An identical list is not a change. The rebuild below replaces the buffer and drops its history -- + // which a file view then has to re-read from disk -- and `apply_math_channels()` runs this on every + // session start, where a session with no graphs hands over an empty list against a model that has + // none either. Comparing the graphs is what makes that call free. + if (channels == math_) return; math_ = std::move(channels); // Lift the previous synthetic entries out of the signal list; `append_math_signals_locked()` // puts the current ones back, so an edit is the same kind of structural rebuild as a CONFIG. @@ -314,6 +319,11 @@ void ScopeModel::push_loaded_slot(const std::vector& values, f64 t, f64 abs push_math_sample(slot); } +std::uint64_t ScopeModel::stored_slots() { + std::lock_guard lock(mutex_); + return store_ ? store_->time().size() : 0; +} + void ScopeModel::push_math_sample(const std::vector& sample) { // The common case is no derived channels at all, and this runs once per *stored slot* -- so the // early-out is what keeps a stream with no math from paying for the feature. diff --git a/src/model.hpp b/src/model.hpp index cd6950c..2437880 100644 --- a/src/model.hpp +++ b/src/model.hpp @@ -281,6 +281,10 @@ class ScopeModel { /// evaluated from those, in order, so a math channel may use another one. void push_math_sample(const std::vector& sample); + /// How many slots the store currently holds. A file view reads it to tell a structural change that + /// dropped the buffer (0) from one that could not have (see `reload_file_view()` in ui.cpp). + std::uint64_t stored_slots(); + /// Pushes one stored slot of a **file** load -- the values, the shared clock and the absolute /// clock -- and then the derived channels, in the producer's own order and under the model's own /// lock. The loader cannot simply hold the lock and push into `live()` itself: every setter it diff --git a/src/ui.cpp b/src/ui.cpp index a50d989..654212c 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -4190,6 +4190,10 @@ gboolean on_app_key_pressed(GtkEventControllerKey*, guint keyval, guint, GdkModi /// math once the set is in place. bool reload_file_view(App& app) { if (!app.replaying && app.model->config().open_waveform.empty()) return false; + // Nothing to restore: a change that really happened leaves the store empty in a file view -- the + // rebuild replaces it and there is no producer to refill it -- while one that found nothing to do + // leaves it full. Reading the file again for the second kind is the waste this avoids. + if (app.model->stored_slots() > 0) return false; const std::string path = !app.waveform_name.empty() ? app.waveform_name : app.model->config().open_waveform; if (path.empty()) return false; @@ -4199,6 +4203,10 @@ bool reload_file_view(App& app) { set_notice(app, "cannot reload " + path + " after the change: " + error, 6); return false; } + // Said out loud because it is the one place a file is read twice: a structural change really did + // drop the buffer, and the file is the only thing that can put it back. + std::fprintf(stderr, "[udpscope] reloaded %s after a structural change: %llu samples back\n", + path.c_str(), static_cast(report.samples)); return true; }