From 8bc1fe8b5ce0ac9bb080070b0569fce1aee6bc94 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Wed, 7 Oct 2026 07:17:46 +0200 Subject: [PATCH] fix: a repaired source is reconciled above the validation line, not below it `math_dialog_sync_form()` wrote the label and then moved a node whose source names an unacquired channel -- a mutation that changes what `math_dialog_problem()` reports. With the sync re-entry now guarded, the nested call that used to repair the label is suppressed, so the line kept describing a source that had just been fixed and left `Apply` insensitive until the next mutation. The reconcile moves above the write and the dropdown reuses its list. --- docs/claude/invariants-math-channels.md | 3 ++- src/ui.cpp | 35 +++++++++++++++---------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/docs/claude/invariants-math-channels.md b/docs/claude/invariants-math-channels.md index 92906e5..11064e5 100644 --- a/docs/claude/invariants-math-channels.md +++ b/docs/claude/invariants-math-channels.md @@ -25,7 +25,8 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear two-click protocol is gone), drag a box to move it; **wheel zooms about the pointer, background drag pans, `f` fits, `Ctrl+0` resets**. Its widgets are created once and only shown, hidden and set by `math_dialog_sync()`, so no rebuild - can invalidate a live callback handle -- the same shape the measurements dialog uses. `UDPSCOPE_PROBE_MATH_MS` defines two graphs and opens the editor (a construction crash test + can invalidate a live callback handle -- the same shape the measurements dialog uses. + `UDPSCOPE_PROBE_MATH_MS` defines two graphs and opens the editor (a construction crash test as much as a state check), and the dump prints each derived channel's extremes beside the source's. Read it on the fast mock, where the two assertions that matter are **`gaps=0`** (the math is on the producer thread, so what it can break is packet capture, never the frame rate) diff --git a/src/ui.cpp b/src/ui.cpp index b334b7b..3cd4b9e 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -4085,9 +4085,9 @@ gboolean math_canvas_key(GtkEventControllerKey*, guint keyval, guint, GdkModifie return FALSE; } -/// Adds a node of `op` and selects it. The grid placement is the same one the auto-place uses, so a -/// node added here lands clear of the others -- 160 is narrower than `kNodeW` plus a wire's tread, -/// which is why the tidy layout uses a wider pitch; this is only a first guess at a free cell. +/// Adds a node of `op` and selects it, at the first grid cell whose box overlaps nothing, so a node +/// added here lands clear of the others -- 160 is narrower than `kNodeW` plus a wire's tread, which +/// is why the tidy layout uses a wider pitch. void math_add_node(MathDialog* d, MathOp op) { // A node added to a graph that was *linear* -- its output was the last node -- becomes the output, // which is what "add a gain and it applies" means. A graph whose output is an earlier node is a @@ -4233,6 +4233,20 @@ void on_math_op_changed(GObject* object, GParamSpec*, gpointer user) { /// nodes its inputs are wired to. Called only through `math_dialog_sync()`, which guards it against /// the re-entry the dropdowns' `notify::selected` would otherwise cause. void math_dialog_sync_form(MathDialog* d) { + // A source naming a channel nobody acquires is moved to the first acquired one -- see the dropdown + // below for why. It happens *here*, above the validation line, because the move is a mutation that + // changes what `math_dialog_problem()` reports: reconciled after the label is written, a repaired + // source would be described as broken, with `Apply` insensitive, until some later sync. + std::vector names; + const bool have_form = + d->selected >= 0 && d->selected < static_cast(d->nodes.size()); + if (d->source_drop != nullptr && have_form) { + names = math_source_names(*d->app); + MathNode& sel = d->nodes[static_cast(d->selected)]; + if (!names.empty() && std::find(names.begin(), names.end(), sel.source) == names.end()) { + sel.source = names.front(); + } + } // Before the early return below: an out-of-range selection is exactly the state in which a stale // validation line is most misleading -- a graph with no node selected can still be broken, and the // user has no form to read instead. @@ -4275,22 +4289,15 @@ void math_dialog_sync_form(MathDialog* d) { if (d->source_drop != nullptr) { // The *acquired* channels, which is what Apply validates against. Listing every signal let a // user pick a source that could never be made valid -- and put the stream's timestamp signal, - // which has no samples at all, at the top of the list. - const std::vector names = math_source_names(*d->app); + // which has no samples at all, at the top of the list. A source that is not in this list would + // read NaN for ever while the dropdown showed the first entry as if it were selected, so the + // node was already moved to the first entry at the top of this function. GtkStringList* list = gtk_string_list_new(nullptr); guint pick = 0; - bool found = false; for (std::size_t i = 0; i < names.size(); ++i) { gtk_string_list_append(list, names[i].c_str()); - if (names[i] == n.source) { - pick = static_cast(i); - found = true; - } + if (names[i] == n.source) pick = static_cast(i); } - // A source that is not in the list is a graph naming a channel nobody acquires -- it would read - // NaN for ever while the dropdown showed the first entry as if it were selected. The node is - // moved to what the form is actually showing, so what is on screen is what the graph says. - if (!found && !names.empty()) n.source = names.front(); gtk_drop_down_set_model(GTK_DROP_DOWN(d->source_drop), G_LIST_MODEL(list)); gtk_drop_down_set_selected(GTK_DROP_DOWN(d->source_drop), pick); g_object_unref(list);