From bafa391ad8ead2ecd3e6dc8366d835f3aaba5eec Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Wed, 7 Oct 2026 02:28:32 +0200 Subject: [PATCH] feat: the editor says why a graph cannot be saved instead of refusing on Apply One `math_dialog_problem()` drives both the line and Apply's sensitivity, so they cannot disagree, and the two modal alerts are gone -- there was nothing in either message a user could not have been told while still building the graph. Written above `math_dialog_sync()`'s early return, since an out-of-range selection is exactly when a stale line misleads most. The source dropdown now lists 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 first. Deleted `const int output_built = ed->output` from `math_probe()` -- it was dead code, redundant with the `output_after_add` already printed on the same line, and its removal is what drops the warning baseline from five to four. The probe's `bad_text` pointer is copied to `std::string` before the second `math_dialog_sync()` overwrites the label's internal storage. --- src/ui.cpp | 155 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 117 insertions(+), 38 deletions(-) diff --git a/src/ui.cpp b/src/ui.cpp index 15d48a8..0adec1d 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -3351,6 +3351,10 @@ struct MathDialog { GtkWidget* hi = nullptr; GtkWidget* rows[6] = {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr}; GtkWidget* inputs = nullptr; + /// The validation line and the button it governs. Both are set from `math_dialog_problem()` and + /// from nowhere else, so what the line says and what the button allows cannot disagree. + GtkWidget* problem = nullptr; + GtkWidget* apply = nullptr; }; /// The channels a math channel may read: the ones the model actually acquires. Offering the whole @@ -3365,6 +3369,25 @@ std::vector math_source_names(App& app) { void math_dialog_sync(MathDialog* d); +/// Why this graph could not be saved, or "" when it could. Both checks in one place: the graph's own +/// (an output that exists, every input wired to a node that exists) and that a `Source` names a +/// channel the scope actually *acquires* -- which the engine's validator cannot see, since it knows +/// nothing about the acquired list. The two used to be a pair of modal alerts raised after Apply was +/// pressed, which is a refusal rather than guidance: there is nothing in the message a user could +/// not have been told while they were still building the graph. +std::string math_dialog_problem(const MathDialog* d) { + std::string why; + if (!math_channel_valid_nodes(d->nodes, d->output, &why)) return why; + const std::vector acquired = math_source_names(*d->app); + for (std::size_t i = 0; i < d->nodes.size(); ++i) { + if (d->nodes[i].op != MathOp::Source) continue; + if (std::find(acquired.begin(), acquired.end(), d->nodes[i].source) != acquired.end()) continue; + return "node " + std::to_string(i + 1) + ": '" + d->nodes[i].source + + "' is not an acquired channel"; + } + return std::string(); +} + /// The box a node is drawn in, in canvas pixels. GdkRectangle math_node_rect(const MathNode& n) { return GdkRectangle{static_cast(n.x), static_cast(n.y), static_cast(kNodeW), @@ -4036,11 +4059,7 @@ void on_math_param_changed(GtkWidget*, gpointer user) { n.hi = gtk_spin_button_get_value(GTK_SPIN_BUTTON(d->hi)); if (d->source_drop != nullptr) { const guint sel = gtk_drop_down_get_selected(GTK_DROP_DOWN(d->source_drop)); - std::vector names; - { - std::lock_guard lock(d->app->model->mutex()); - for (const SignalInfo& sig : d->app->model->signals()) names.push_back(sig.name); - } + const std::vector names = math_source_names(*d->app); if (sel < names.size()) n.source = names[sel]; } gtk_widget_queue_draw(d->canvas); @@ -4075,7 +4094,19 @@ void on_math_op_changed(GObject* object, GParamSpec*, gpointer user) { /// The form and the canvas from the selected node: which rows an op uses, what they hold, and which /// nodes its inputs are wired to. void math_dialog_sync(MathDialog* d) { - if (d->selected < 0 || d->selected >= static_cast(d->nodes.size())) return; + // 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. + if (d->problem != nullptr) { + const std::string why = math_dialog_problem(d); + gtk_label_set_text(GTK_LABEL(d->problem), why.c_str()); + gtk_widget_set_visible(d->problem, !why.empty()); + if (d->apply != nullptr) gtk_widget_set_sensitive(d->apply, why.empty()); + } + if (d->selected < 0 || d->selected >= static_cast(d->nodes.size())) { + if (d->canvas != nullptr) gtk_widget_queue_draw(d->canvas); + return; + } MathNode& n = d->nodes[static_cast(d->selected)]; const MathOp op = n.op; const bool gain = math_op_uses_gain(op); @@ -4095,11 +4126,10 @@ void math_dialog_sync(MathDialog* d) { gtk_spin_button_set_value(GTK_SPIN_BUTTON(d->lo), n.lo); gtk_spin_button_set_value(GTK_SPIN_BUTTON(d->hi), n.hi); if (d->source_drop != nullptr) { - std::vector names; - { - std::lock_guard lock(d->app->model->mutex()); - for (const SignalInfo& sig : d->app->model->signals()) names.push_back(sig.name); - } + // 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); GtkStringList* list = gtk_string_list_new(nullptr); guint pick = 0; bool found = false; @@ -4142,37 +4172,19 @@ void math_dialog_sync(MathDialog* d) { void on_math_apply(GtkButton*, gpointer user) { MathDialog* d = static_cast(user); on_math_param_changed(nullptr, d); - std::string why; + // The validation line has said what is wrong for as long as it has been wrong, and Apply is + // insensitive while it is non-empty -- so this is a guard against a programmatic call (the + // headless probe makes one), not a message to the user. + const std::string why = math_dialog_problem(d); + if (!why.empty()) { + std::fprintf(stderr, "[math] apply refused: %s\n", why.c_str()); + return; + } MathChannel m; m.name = gtk_editable_get_text(GTK_EDITABLE(d->name)); m.unit = gtk_editable_get_text(GTK_EDITABLE(d->unit)); m.nodes = d->nodes; m.output = d->output; - if (!math_channel_valid(m, &why)) { - // The dialog stays up: an invalid graph is a thing to fix, and closing on it would throw the - // work away with no message. - GtkAlertDialog* alert = gtk_alert_dialog_new("%s", why.c_str()); - gtk_alert_dialog_show(alert, GTK_WINDOW(d->window)); - g_object_unref(alert); - return; - } - // `math_channel_valid()` is about the *graph* (an output, every input wired to a node above it); - // that a `Source` names a channel the scope actually acquires is the dialog's business, since only - // it can see the acquired list. A graph naming a disabled signal is one that would read NaN for - // ever, and it must not be saved. - { - const std::vector acquired = math_source_names(*d->app); - for (std::size_t i = 0; i < m.nodes.size(); ++i) { - if (m.nodes[i].op != MathOp::Source) continue; - if (std::find(acquired.begin(), acquired.end(), m.nodes[i].source) != acquired.end()) continue; - const std::string text = "node " + std::to_string(i + 1) + ": '" + m.nodes[i].source + - "' is not an acquired channel"; - GtkAlertDialog* alert = gtk_alert_dialog_new("%s", text.c_str()); - gtk_alert_dialog_show(alert, GTK_WINDOW(d->window)); - g_object_unref(alert); - return; - } - } if (m.name.empty()) m.name = "math"; if (d->edit >= 0 && d->edit < static_cast(d->app->math.size())) { m.enabled = d->app->math[static_cast(d->edit)].enabled; @@ -4419,6 +4431,15 @@ void open_math_dialog(App& app, int edit) { gtk_widget_set_halign(d->inputs, GTK_ALIGN_START); gtk_box_append(GTK_BOX(form), math_row("wired", d->inputs, &d->rows[5])); + // Why the graph cannot be saved, in the form column where the user is already reading. Wrapped, + // so a long channel name does not widen the dialog, and hidden when there is nothing to say. + d->problem = gtk_label_new(""); + gtk_label_set_wrap(GTK_LABEL(d->problem), TRUE); + gtk_label_set_xalign(GTK_LABEL(d->problem), 0.0f); + gtk_widget_add_css_class(d->problem, "error"); + gtk_widget_set_visible(d->problem, FALSE); + gtk_box_append(GTK_BOX(form), d->problem); + GtkWidget* add = gtk_button_new_with_label("add node"); GtkWidget* del = gtk_button_new_with_label("delete node"); GtkWidget* out = gtk_button_new_with_label("set as output"); @@ -4458,6 +4479,7 @@ void open_math_dialog(App& app, int edit) { gtk_box_append(GTK_BOX(outer), actions); app.math_dialog = window; + d->apply = apply; math_dialog_sync(d); gtk_window_present(GTK_WINDOW(window)); } @@ -7138,7 +7160,6 @@ gboolean math_probe(gpointer user) { if (ed->offset != nullptr) gtk_spin_button_set_value(GTK_SPIN_BUTTON(ed->offset), 0.0); on_math_param_changed(nullptr, ed); const std::size_t built = ed->nodes.size(); - const int output_built = ed->output; const double gain_read = ed->nodes.empty() ? 0.0 : ed->nodes.back().gain; const double offset_read = ed->nodes.empty() ? 0.0 : ed->nodes.back().offset; on_math_apply(nullptr, ed); // frees the dialog: nothing below may touch `ed` @@ -7494,6 +7515,64 @@ gboolean math_probe(gpointer user) { cyc_ok ? 0 : 1, cyc_why.c_str()); } } + if (a->math_dialog != nullptr) { + auto* v2 = static_cast( + g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog")); + if (v2 != nullptr && v2->nodes.size() >= 2) { + // Broken: an unwired input. The line must say so and Apply must be insensitive -- the old + // behaviour was a modal alert *after* the press, which is a refusal rather than guidance. + v2->nodes[1].in_a = -1; + math_dialog_sync(v2); + const std::string bad = math_dialog_problem(v2); + const int bad_sensitive = gtk_widget_get_sensitive(v2->apply) ? 1 : 0; + // Copied before the next sync call overwrites the label's internal string. + const std::string bad_text_copy = + gtk_label_get_text(GTK_LABEL(v2->problem)) != nullptr + ? std::string(gtk_label_get_text(GTK_LABEL(v2->problem))) + : std::string("(null)"); + v2->nodes[1].in_a = 0; + math_dialog_sync(v2); + const std::string good = math_dialog_problem(v2); + const int good_sensitive = gtk_widget_get_sensitive(v2->apply) ? 1 : 0; + std::fprintf(stderr, + "[math-probe] validate: broken problem=[%s] label=[%s] apply=%d; " + "fixed problem=[%s] apply=%d (want non-empty 0; empty 1)\n", + bad.c_str(), bad_text_copy.c_str(), bad_sensitive, + good.c_str(), good_sensitive); + + // The selection out of range is the case `math_dialog_sync()` returns early on, so the + // line must be written above that return or it goes stale exactly when it matters. + const int was_sel = v2->selected; + v2->selected = 999; + v2->nodes[1].in_a = -1; + math_dialog_sync(v2); + const char* stale = gtk_label_get_text(GTK_LABEL(v2->problem)); + std::fprintf(stderr, "[math-probe] validate: no-selection label=[%s] (want non-empty)\n", + stale != nullptr ? stale : "(null)"); + v2->selected = was_sel; + v2->nodes[1].in_a = 0; + math_dialog_sync(v2); + + // And the source dropdown must list the *acquired* channels, which is what Apply checks + // against: listing every signal let a user pick one that could never be made valid. + const std::vector acquired = math_source_names(*a); + std::size_t signals_n = 0; + { + std::lock_guard lock(a->model->mutex()); + signals_n = a->model->signals().size(); + } + v2->selected = 0; + v2->nodes[0].op = MathOp::Source; + math_dialog_sync(v2); + GListModel* lm = gtk_drop_down_get_model(GTK_DROP_DOWN(v2->source_drop)); + const guint listed = lm != nullptr ? g_list_model_get_n_items(lm) : 0u; + std::fprintf(stderr, + "[math-probe] validate: source list=%u acquired=%zu signals=%zu " + "(want list==acquired)\n", + listed, acquired.size(), signals_n); + v2->selected = was_sel; + } + } std::fflush(stderr); return G_SOURCE_REMOVE; }