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.
This commit is contained in:
1 parent
5ca762031d
commit
bafa391ad8
1 file changed
+117
-38
+117
-38
@@ -3351,6 +3351,10 @@ struct MathDialog {
|
|||||||
GtkWidget* hi = nullptr;
|
GtkWidget* hi = nullptr;
|
||||||
GtkWidget* rows[6] = {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr};
|
GtkWidget* rows[6] = {nullptr, nullptr, nullptr, nullptr, nullptr, nullptr};
|
||||||
GtkWidget* inputs = 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
|
/// The channels a math channel may read: the ones the model actually acquires. Offering the whole
|
||||||
@@ -3365,6 +3369,25 @@ std::vector<std::string> math_source_names(App& app) {
|
|||||||
|
|
||||||
void math_dialog_sync(MathDialog* d);
|
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<std::string> 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.
|
/// The box a node is drawn in, in canvas pixels.
|
||||||
GdkRectangle math_node_rect(const MathNode& n) {
|
GdkRectangle math_node_rect(const MathNode& n) {
|
||||||
return GdkRectangle{static_cast<int>(n.x), static_cast<int>(n.y), static_cast<int>(kNodeW),
|
return GdkRectangle{static_cast<int>(n.x), static_cast<int>(n.y), static_cast<int>(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));
|
n.hi = gtk_spin_button_get_value(GTK_SPIN_BUTTON(d->hi));
|
||||||
if (d->source_drop != nullptr) {
|
if (d->source_drop != nullptr) {
|
||||||
const guint sel = gtk_drop_down_get_selected(GTK_DROP_DOWN(d->source_drop));
|
const guint sel = gtk_drop_down_get_selected(GTK_DROP_DOWN(d->source_drop));
|
||||||
std::vector<std::string> names;
|
const std::vector<std::string> names = math_source_names(*d->app);
|
||||||
{
|
|
||||||
std::lock_guard<std::mutex> lock(d->app->model->mutex());
|
|
||||||
for (const SignalInfo& sig : d->app->model->signals()) names.push_back(sig.name);
|
|
||||||
}
|
|
||||||
if (sel < names.size()) n.source = names[sel];
|
if (sel < names.size()) n.source = names[sel];
|
||||||
}
|
}
|
||||||
gtk_widget_queue_draw(d->canvas);
|
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
|
/// 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.
|
/// nodes its inputs are wired to.
|
||||||
void math_dialog_sync(MathDialog* d) {
|
void math_dialog_sync(MathDialog* d) {
|
||||||
if (d->selected < 0 || d->selected >= static_cast<int>(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<int>(d->nodes.size())) {
|
||||||
|
if (d->canvas != nullptr) gtk_widget_queue_draw(d->canvas);
|
||||||
|
return;
|
||||||
|
}
|
||||||
MathNode& n = d->nodes[static_cast<std::size_t>(d->selected)];
|
MathNode& n = d->nodes[static_cast<std::size_t>(d->selected)];
|
||||||
const MathOp op = n.op;
|
const MathOp op = n.op;
|
||||||
const bool gain = math_op_uses_gain(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->lo), n.lo);
|
||||||
gtk_spin_button_set_value(GTK_SPIN_BUTTON(d->hi), n.hi);
|
gtk_spin_button_set_value(GTK_SPIN_BUTTON(d->hi), n.hi);
|
||||||
if (d->source_drop != nullptr) {
|
if (d->source_drop != nullptr) {
|
||||||
std::vector<std::string> names;
|
// 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,
|
||||||
std::lock_guard<std::mutex> lock(d->app->model->mutex());
|
// which has no samples at all, at the top of the list.
|
||||||
for (const SignalInfo& sig : d->app->model->signals()) names.push_back(sig.name);
|
const std::vector<std::string> names = math_source_names(*d->app);
|
||||||
}
|
|
||||||
GtkStringList* list = gtk_string_list_new(nullptr);
|
GtkStringList* list = gtk_string_list_new(nullptr);
|
||||||
guint pick = 0;
|
guint pick = 0;
|
||||||
bool found = false;
|
bool found = false;
|
||||||
@@ -4142,37 +4172,19 @@ void math_dialog_sync(MathDialog* d) {
|
|||||||
void on_math_apply(GtkButton*, gpointer user) {
|
void on_math_apply(GtkButton*, gpointer user) {
|
||||||
MathDialog* d = static_cast<MathDialog*>(user);
|
MathDialog* d = static_cast<MathDialog*>(user);
|
||||||
on_math_param_changed(nullptr, d);
|
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;
|
MathChannel m;
|
||||||
m.name = gtk_editable_get_text(GTK_EDITABLE(d->name));
|
m.name = gtk_editable_get_text(GTK_EDITABLE(d->name));
|
||||||
m.unit = gtk_editable_get_text(GTK_EDITABLE(d->unit));
|
m.unit = gtk_editable_get_text(GTK_EDITABLE(d->unit));
|
||||||
m.nodes = d->nodes;
|
m.nodes = d->nodes;
|
||||||
m.output = d->output;
|
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<std::string> 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 (m.name.empty()) m.name = "math";
|
||||||
if (d->edit >= 0 && d->edit < static_cast<int>(d->app->math.size())) {
|
if (d->edit >= 0 && d->edit < static_cast<int>(d->app->math.size())) {
|
||||||
m.enabled = d->app->math[static_cast<std::size_t>(d->edit)].enabled;
|
m.enabled = d->app->math[static_cast<std::size_t>(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_widget_set_halign(d->inputs, GTK_ALIGN_START);
|
||||||
gtk_box_append(GTK_BOX(form), math_row("wired", d->inputs, &d->rows[5]));
|
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* add = gtk_button_new_with_label("add node");
|
||||||
GtkWidget* del = gtk_button_new_with_label("delete node");
|
GtkWidget* del = gtk_button_new_with_label("delete node");
|
||||||
GtkWidget* out = gtk_button_new_with_label("set as output");
|
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);
|
gtk_box_append(GTK_BOX(outer), actions);
|
||||||
|
|
||||||
app.math_dialog = window;
|
app.math_dialog = window;
|
||||||
|
d->apply = apply;
|
||||||
math_dialog_sync(d);
|
math_dialog_sync(d);
|
||||||
gtk_window_present(GTK_WINDOW(window));
|
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);
|
if (ed->offset != nullptr) gtk_spin_button_set_value(GTK_SPIN_BUTTON(ed->offset), 0.0);
|
||||||
on_math_param_changed(nullptr, ed);
|
on_math_param_changed(nullptr, ed);
|
||||||
const std::size_t built = ed->nodes.size();
|
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 gain_read = ed->nodes.empty() ? 0.0 : ed->nodes.back().gain;
|
||||||
const double offset_read = ed->nodes.empty() ? 0.0 : ed->nodes.back().offset;
|
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`
|
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());
|
cyc_ok ? 0 : 1, cyc_why.c_str());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (a->math_dialog != nullptr) {
|
||||||
|
auto* v2 = static_cast<MathDialog*>(
|
||||||
|
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<std::string> acquired = math_source_names(*a);
|
||||||
|
std::size_t signals_n = 0;
|
||||||
|
{
|
||||||
|
std::lock_guard<std::mutex> 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);
|
std::fflush(stderr);
|
||||||
return G_SOURCE_REMOVE;
|
return G_SOURCE_REMOVE;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user