fix: the [edit] buttons' icons, and the locked pane says four lines not one
Two things the lock shipped wrong. **The "done" icon did not exist.** `emblem-ok-symbolic` is a breeze icon; Adwaita — which is what this app runs under — does not have it, so the button that leaves configuration mode drew a missing-image placeholder, i.e. nothing. Both names are now Adwaita's own: `document-edit-symbolic` (the pencil: edit) and `object-select-symbolic` (the check: done). This is exactly the silent failure `toolbar icons:` exists for, so the two `[edit]` buttons get the same treatment: `pane edits: locked=…(ok) config=…(ok)` is read by clicking into configuration mode and back, so one line carries both names and `gtk_icon_theme_has_icon()`'s verdict for each. The invariants now carry the rule, including the trap that a name found under `breeze` is not an answer. **The locked pane was too compact.** One line could not hold what a user opens the pane for, so it is now a small definition list: `signal`, `condition`, `level`, `mode` on four lines, and after that only the figures that are *not* at their defaults — `noise band`, `HF reject`, `pre-trigger`, `hold-off` — because a line reading "band 0" is a line that says nothing. The channel is resolved from the acquired channels and the condition quotes the figures its own shape reads (a width and its comparison, two levels, or a timeout), never one it ignores. The pane is ~192 px tall locked against 580 in configuration mode. One self-inflicted critical on the way: the probe read the summary with `gtk_label_get_text()`, and the container is a grid now — `Gtk-CRITICAL: assertion 'GTK_IS_LABEL (self)' failed`. The text comes from the sync that writes it instead. Verified: `pane edits: locked=document-edit-symbolic(ok) config=object-select-symbolic(ok)`; `panes: trigger=locked signals=locked body_visible=0 summary='signal: square | condition: window, falling | level: 0.3 .. 0.6 | mode: single | noise band: 0.2 | pre-trigger: 20 % | hold-off: 1000 ms'`; `make test-ui` 14 files; zero `Gtk-CRITICAL` and no missing icon.
This commit is contained in:
1 parent
121abbefae
commit
4f4851f4e3
7 files changed
+142
-41
No files matched your search
@@ -184,6 +184,13 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear
|
||||
**tooltip and accessible name** from the page's name, then hides the empty label that would otherwise
|
||||
leave the icon off-centre. An icon-only pill is still a named control for a screen reader, and
|
||||
`switcher: … pills=2 labels=0 names=2` is the reading for all three facts at once.
|
||||
- **A stock icon name has to be one *Adwaita* has, and a blank button is the failure mode.** The two
|
||||
panes' `[edit]` buttons are `document-edit-symbolic` (the pencil: edit) and `object-select-symbolic`
|
||||
(the check: done). The done one was `emblem-ok-symbolic`, which breeze ships and **Adwaita does not** —
|
||||
so the button that leaves configuration mode was blank, which no other reading could see: the button
|
||||
worked, the layout was unchanged, and only `pane edits:` (the two names and
|
||||
`gtk_icon_theme_has_icon()`'s verdict, clicked into each mode) would say so. Check a name against
|
||||
`/usr/share/icons/Adwaita` before using it; a name found under `breeze` is not an answer.
|
||||
- **A pane's heading switch and the header bar's button are one control in two places.** Each pane's
|
||||
heading (`build_trigger_panel()` builds the trigger's, `cursors-panel.ui` has the cursors' in its
|
||||
`head`) carries a `GtkSwitch` that runs the *same action* the toolbar button runs, and
|
||||
|
||||
@@ -1142,22 +1142,32 @@ library measures and draws the rect in tests of its own.
|
||||
## `panes:` — the locked and configuration modes
|
||||
|
||||
```
|
||||
[udpscope] panes: trigger=locked signals=locked body_visible=0 summary='window · square · 0.25…0.5 · 20 % pre'
|
||||
[udpscope] pane edits: locked=document-edit-symbolic(ok) config=object-select-symbolic(ok)
|
||||
[udpscope] panes: trigger=locked signals=locked body_visible=0 summary='signal: square | condition: window, falling | level: 0.3 .. 0.6 | mode: single | noise band: 0.2 | pre-trigger: 20 % | hold-off: 1000 ms'
|
||||
[udpscope] panes: trigger=config signals=config body_visible=1 summary=''
|
||||
```
|
||||
|
||||
**`pane edits:`** is the two `[edit]` buttons in each mode, by name and verdict, and it exists because of
|
||||
a real defect: the "done" icon was `emblem-ok-symbolic`, which **Adwaita does not have** (breeze does), so
|
||||
the button that leaves configuration mode was blank — and a blank icon button is invisible in every other
|
||||
reading, exactly as `toolbar icons:` describes for the toolbar. The probe clicks into configuration mode
|
||||
and back, so one line carries both names. **Both must read `(ok)`**, and the pair is
|
||||
`document-edit-symbolic` (the pencil: edit) and `object-select-symbolic` (the check: done).
|
||||
|
||||
Both panes have two modes. **Locked** is what a returning user gets — the trigger pane shows the one line
|
||||
in `summary`, the signal list shows only the signals that are on, and neither offers a control that
|
||||
changes the configuration — and **config** is the full pane, which is what a *first run* gets, because a
|
||||
session file that loaded is what "we have been here before" means. Each heading's `[edit]` button flips
|
||||
its pane.
|
||||
|
||||
- **`summary`** is the locked trigger pane's whole content, so it is read rather than assumed: empty here
|
||||
is a locked pane showing nothing at all. It quotes the shape, the channel, the figures that shape
|
||||
actually reads (a level, two levels, a width plus its comparison, or a timeout) and the pre-trigger
|
||||
share — never a figure the shape ignores.
|
||||
- **`body_visible=0`** is the settings being off screen, which is what makes the pane 42 px tall instead
|
||||
of 580 in `panels:`.
|
||||
- **`summary`** is the locked trigger pane's whole content, as `key: value` pairs separated by ` |`. The
|
||||
pane draws it as a small definition list — `signal`, `condition`, `level`, `mode` on four lines, and
|
||||
then only the figures that are *not* at their defaults (`noise band`, `HF reject`, `pre-trigger`,
|
||||
`hold-off`), because a line reading "band 0" is a line that says nothing. One line was tried first and
|
||||
was too little: the four answers are what a user opens the pane for. `summary=''` with
|
||||
`body_visible=0` is a locked pane showing nothing at all, which is the defect this reads for.
|
||||
- **`body_visible=0`** is the settings being off screen, which is what makes the pane a couple of hundred
|
||||
pixels instead of 580 in `panels:`.
|
||||
- **The locked signal list drops the rows of the signals that are off**, so `rows` counts what is on:
|
||||
`signals panel: … rows=3 signals=2 divider=1` is two on signals and the `MATH` divider, not four
|
||||
signals. Its `+` goes with them — a `+` on a locked list opens the editor the lock exists to keep shut.
|
||||
|
||||
+12
-1
@@ -4410,8 +4410,11 @@ void apply_pane_lock(App& app) {
|
||||
}
|
||||
const auto edit_button = [](GtkWidget* button, bool locked, const char* what) {
|
||||
if (button == nullptr) return;
|
||||
// Both names must be ones Adwaita *has*: `emblem-ok-symbolic` exists only in breeze, so the "done"
|
||||
// button was blank -- and a blank icon button is invisible in every other reading, which is why
|
||||
// `pane edits:` checks these two by name and verdict.
|
||||
gtk_button_set_icon_name(GTK_BUTTON(button),
|
||||
locked ? "emblem-system-symbolic" : "emblem-ok-symbolic");
|
||||
locked ? "document-edit-symbolic" : "object-select-symbolic");
|
||||
const std::string tip = std::string(locked ? "configure " : "keep this configuration of ") + what;
|
||||
gtk_widget_set_tooltip_text(button, tip.c_str());
|
||||
};
|
||||
@@ -5775,6 +5778,14 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
|
||||
app.dbg_trigger_sensitive, app.dbg_switcher_visible,
|
||||
rows.cursors_visible() ? 1 : 0, rows.time_window(),
|
||||
app.window_limit_s);
|
||||
{
|
||||
GtkIconTheme* theme = gtk_icon_theme_get_for_display(gdk_display_get_default());
|
||||
std::printf("[udpscope] pane edits: locked=%s(%s) config=%s(%s)\n",
|
||||
app.dbg_pane_edit_icon.c_str(),
|
||||
gtk_icon_theme_has_icon(theme, app.dbg_pane_edit_icon.c_str()) ? "ok" : "MISSING",
|
||||
app.dbg_pane_view_icon.c_str(),
|
||||
gtk_icon_theme_has_icon(theme, app.dbg_pane_view_icon.c_str()) ? "ok" : "MISSING");
|
||||
}
|
||||
std::printf("[udpscope] panes: trigger=%s signals=%s body_visible=%d summary='%s'\n",
|
||||
app.dbg_trigger_locked ? "locked" : "config",
|
||||
app.dbg_signals_locked ? "locked" : "config", app.dbg_trigger_body_visible,
|
||||
|
||||
@@ -37,7 +37,7 @@
|
||||
</child>
|
||||
<child>
|
||||
<object class="GtkButton" id="signals_edit">
|
||||
<property name="icon-name">emblem-system-symbolic</property>
|
||||
<property name="icon-name">document-edit-symbolic</property>
|
||||
<property name="valign">center</property>
|
||||
<property name="css-classes">flat</property>
|
||||
</object>
|
||||
|
||||
@@ -269,7 +269,12 @@ struct App {
|
||||
/// session: a lock is a mode, not a setting, so it is not persisted.
|
||||
bool trigger_locked = false;
|
||||
bool signals_locked = false;
|
||||
/// The locked trigger pane's content: a small definition list, because one line cannot hold the
|
||||
/// signal, the condition, the level and the mode, and those are what a user reads the pane for.
|
||||
GtkWidget* trigger_summary = nullptr;
|
||||
static constexpr int kTriggerSummaryRows = 8;
|
||||
GtkWidget* trigger_summary_keys[kTriggerSummaryRows] = {};
|
||||
GtkWidget* trigger_summary_values[kTriggerSummaryRows] = {};
|
||||
GtkWidget* trigger_edit = nullptr;
|
||||
GtkWidget* signals_edit = nullptr;
|
||||
int dbg_trigger_locked = -1;
|
||||
@@ -278,6 +283,10 @@ struct App {
|
||||
/// The compact line the locked trigger pane shows. It is the pane's whole content in that mode, so it
|
||||
/// is read rather than assumed: empty here is a locked pane showing nothing at all.
|
||||
std::string dbg_trigger_summary;
|
||||
/// The two [edit] buttons' icons, read in each mode by clicking into it and back: a button whose icon
|
||||
/// does not resolve is *blank*, and that is how the missing "done" icon was found.
|
||||
std::string dbg_pane_edit_icon;
|
||||
std::string dbg_pane_view_icon;
|
||||
/// The cursors pane: listing what the instruments read -- T0/T1/dT always, per row an Y1/Y2/dY
|
||||
/// where rulers are up, and in `mix` mode the value each cursor crosses each trace at.
|
||||
GtkWidget* cursors_mode_drop = nullptr;
|
||||
|
||||
+14
-3
@@ -2322,13 +2322,24 @@ gboolean exit_probe_step1(gpointer user) {
|
||||
App* a = static_cast<App*>(user);
|
||||
// Record the panel states before the toggles are exercised.
|
||||
a->dbg_trigger_sensitive_default = gtk_widget_get_sensitive(a->trigger_body) ? 1 : 0;
|
||||
// The two [edit] buttons, in each mode: the icon is the only thing that says which mode the pane is
|
||||
// in, and an icon the theme does not have draws nothing at all.
|
||||
const auto pane_icon = [](GtkWidget* b) {
|
||||
const char* n = b != nullptr && GTK_IS_BUTTON(b) ? gtk_button_get_icon_name(GTK_BUTTON(b)) : nullptr;
|
||||
return std::string(n != nullptr ? n : "-");
|
||||
};
|
||||
a->dbg_pane_edit_icon = pane_icon(a->trigger_edit);
|
||||
if (a->trigger_edit != nullptr) {
|
||||
g_signal_emit_by_name(a->trigger_edit, "clicked"); // into configuration mode
|
||||
a->dbg_pane_view_icon = pane_icon(a->trigger_edit);
|
||||
g_signal_emit_by_name(a->trigger_edit, "clicked"); // and back to the view
|
||||
}
|
||||
a->dbg_trigger_locked = a->trigger_locked ? 1 : 0;
|
||||
a->dbg_signals_locked = a->signals_locked ? 1 : 0;
|
||||
a->dbg_trigger_body_visible =
|
||||
a->trigger_body != nullptr && gtk_widget_get_visible(a->trigger_body) ? 1 : 0;
|
||||
if (a->trigger_summary != nullptr) {
|
||||
a->dbg_trigger_summary = gtk_label_get_text(GTK_LABEL(a->trigger_summary));
|
||||
}
|
||||
// The summary's text comes from the sync that wrote it: the container is a grid of key/value rows,
|
||||
// and asking it for a label's text was itself a critical.
|
||||
a->dbg_panel_switch_live =
|
||||
a->trigger_switch != nullptr && gtk_widget_get_sensitive(a->trigger_switch) &&
|
||||
a->cursors_switch != nullptr && gtk_widget_get_sensitive(a->cursors_switch)
|
||||
|
||||
+82
-29
@@ -152,6 +152,27 @@ void write_trigger_widgets(App& app) {
|
||||
/// rebuilt when a session loads.
|
||||
void sync_trigger_summary(App& app) {
|
||||
if (app.trigger_summary == nullptr) return;
|
||||
int row = 0;
|
||||
std::string reading; // the same content as one line, for the dump
|
||||
const auto put = [&](const char* key, const std::string& value) {
|
||||
if (row >= App::kTriggerSummaryRows) return;
|
||||
gtk_label_set_text(GTK_LABEL(app.trigger_summary_keys[row]), key);
|
||||
gtk_label_set_text(GTK_LABEL(app.trigger_summary_values[row]), value.c_str());
|
||||
gtk_widget_set_visible(app.trigger_summary_keys[row], TRUE);
|
||||
gtk_widget_set_visible(app.trigger_summary_values[row], TRUE);
|
||||
reading += std::string(reading.empty() ? "" : " | ") + key + ": " + value;
|
||||
++row;
|
||||
};
|
||||
const auto ms = [](double seconds) {
|
||||
char b[64];
|
||||
std::snprintf(b, sizeof(b), "%.6g ms", seconds * 1000.0);
|
||||
return std::string(b);
|
||||
};
|
||||
const auto number = [](double v) {
|
||||
char b[64];
|
||||
std::snprintf(b, sizeof(b), "%.6g", v);
|
||||
return std::string(b);
|
||||
};
|
||||
TriggerConfig tc;
|
||||
std::string channel;
|
||||
{
|
||||
@@ -162,42 +183,52 @@ void sync_trigger_summary(App& app) {
|
||||
channel = channels[static_cast<std::size_t>(tc.channel)].name;
|
||||
}
|
||||
}
|
||||
char buf[192];
|
||||
std::string text = trigger_type_text(tc.condition.type);
|
||||
text += " · ";
|
||||
text += channel.empty() ? "no channel" : channel;
|
||||
put("signal", channel.empty() ? std::string("none acquired") : channel);
|
||||
// The condition: the shape, and whatever qualifies it. A pulse width carries its comparison and its
|
||||
// width, which is what the shape *is*; a window carries its two levels on the line below.
|
||||
std::string condition = trigger_type_text(tc.condition.type);
|
||||
switch (tc.condition.type) {
|
||||
case TriggerType::PulseWidth:
|
||||
std::snprintf(buf, sizeof(buf), " · %s %.6g ms", width_cmp_text(tc.condition.cmp),
|
||||
tc.condition.width_s * 1000.0);
|
||||
text += buf;
|
||||
break;
|
||||
case TriggerType::Edge:
|
||||
case TriggerType::Window:
|
||||
std::snprintf(buf, sizeof(buf), " · %.6g…%.6g", tc.condition.level, tc.condition.level2);
|
||||
text += buf;
|
||||
condition += std::string(", ") + trigger_edge_text(tc.condition.edge);
|
||||
break;
|
||||
case TriggerType::PulseWidth:
|
||||
condition += std::string(", ") + width_cmp_text(tc.condition.cmp) + " " + ms(tc.condition.width_s);
|
||||
if (tc.condition.cmp == WidthCmp::Within || tc.condition.cmp == WidthCmp::Outside) {
|
||||
condition += " .. " + ms(tc.condition.width2_s);
|
||||
}
|
||||
break;
|
||||
case TriggerType::Dropout:
|
||||
std::snprintf(buf, sizeof(buf), " · %.6g ms", tc.condition.timeout_s * 1000.0);
|
||||
text += buf;
|
||||
break;
|
||||
case TriggerType::Edge:
|
||||
std::snprintf(buf, sizeof(buf), " · level %.6g", tc.condition.level);
|
||||
text += buf;
|
||||
condition += std::string(", ") + ms(tc.condition.timeout_s);
|
||||
break;
|
||||
}
|
||||
if (!(tc.condition.noise_band > 0.0) || tc.condition.type == TriggerType::Dropout) {
|
||||
// A band is not offered for a dropout, so it is not quoted for one either.
|
||||
} else {
|
||||
std::snprintf(buf, sizeof(buf), " · band %.6g", tc.condition.noise_band);
|
||||
text += buf;
|
||||
put("condition", condition);
|
||||
switch (tc.condition.type) {
|
||||
case TriggerType::Window:
|
||||
put("level", number(tc.condition.level) + " .. " + number(tc.condition.level2));
|
||||
break;
|
||||
case TriggerType::Dropout:
|
||||
put("timeout", ms(tc.condition.timeout_s));
|
||||
break;
|
||||
default:
|
||||
put("level", number(tc.condition.level));
|
||||
break;
|
||||
}
|
||||
if (tc.condition.hf_fc > 0.0) {
|
||||
std::snprintf(buf, sizeof(buf), " · HF %.6g Hz", tc.condition.hf_fc);
|
||||
text += buf;
|
||||
put("mode", trigger_mode_text(tc.mode));
|
||||
// Everything else only when it is not at its default: a line that says "band 0" is a line that says
|
||||
// nothing, and the four lines above are the ones a user reads.
|
||||
if (tc.condition.noise_band > 0.0 && tc.condition.type != TriggerType::Dropout) {
|
||||
put("noise band", number(tc.condition.noise_band));
|
||||
}
|
||||
std::snprintf(buf, sizeof(buf), " · %.6g %% pre", tc.pretrigger_pct);
|
||||
text += buf;
|
||||
gtk_label_set_text(GTK_LABEL(app.trigger_summary), text.c_str());
|
||||
if (tc.condition.hf_fc > 0.0) put("HF reject", number(tc.condition.hf_fc) + " Hz");
|
||||
if (std::fabs(tc.pretrigger_pct - 50.0) > 1.0e-9) put("pre-trigger", number(tc.pretrigger_pct) + " %");
|
||||
if (std::fabs(tc.holdoff_s - 0.010) > 1.0e-12) put("hold-off", ms(tc.holdoff_s));
|
||||
for (int i = row; i < App::kTriggerSummaryRows; ++i) {
|
||||
gtk_widget_set_visible(app.trigger_summary_keys[i], FALSE);
|
||||
gtk_widget_set_visible(app.trigger_summary_values[i], FALSE);
|
||||
}
|
||||
app.dbg_trigger_summary = reading;
|
||||
gtk_widget_set_visible(app.trigger_summary, app.trigger_locked);
|
||||
}
|
||||
|
||||
TriggerType trigger_type_at(guint i) {
|
||||
@@ -616,7 +647,7 @@ GtkWidget* build_trigger_panel(App& app) {
|
||||
gtk_widget_set_hexpand(app.trigger_summary, TRUE);
|
||||
gtk_widget_set_visible(app.trigger_summary, FALSE);
|
||||
gtk_box_append(GTK_BOX(head), app.trigger_summary);
|
||||
app.trigger_edit = gtk_button_new_from_icon_name("emblem-system-symbolic");
|
||||
app.trigger_edit = gtk_button_new_from_icon_name("document-edit-symbolic");
|
||||
gtk_widget_add_css_class(app.trigger_edit, "flat");
|
||||
gtk_widget_set_valign(app.trigger_edit, GTK_ALIGN_CENTER);
|
||||
gtk_widget_set_tooltip_text(app.trigger_edit, "configure the trigger");
|
||||
@@ -629,8 +660,30 @@ GtkWidget* build_trigger_panel(App& app) {
|
||||
g_signal_connect(app.trigger_switch, "notify::active", G_CALLBACK(on_panel_switch_toggled), &app);
|
||||
gtk_box_append(GTK_BOX(head), app.trigger_switch);
|
||||
|
||||
// The locked pane's content: the settings as a definition list, so the answer to "what is this set to"
|
||||
// is four short lines rather than a row of controls.
|
||||
app.trigger_summary = gtk_grid_new();
|
||||
gtk_grid_set_row_spacing(GTK_GRID(app.trigger_summary), 2);
|
||||
gtk_grid_set_column_spacing(GTK_GRID(app.trigger_summary), 12);
|
||||
gtk_widget_set_margin_top(app.trigger_summary, 6);
|
||||
gtk_widget_set_margin_bottom(app.trigger_summary, 6);
|
||||
gtk_widget_set_margin_start(app.trigger_summary, 8);
|
||||
gtk_widget_set_margin_end(app.trigger_summary, 8);
|
||||
for (int i = 0; i < App::kTriggerSummaryRows; ++i) {
|
||||
app.trigger_summary_keys[i] = gtk_label_new("");
|
||||
gtk_widget_add_css_class(app.trigger_summary_keys[i], "dim-label");
|
||||
gtk_widget_set_halign(app.trigger_summary_keys[i], GTK_ALIGN_START);
|
||||
app.trigger_summary_values[i] = gtk_label_new("");
|
||||
gtk_widget_set_halign(app.trigger_summary_values[i], GTK_ALIGN_START);
|
||||
gtk_label_set_ellipsize(GTK_LABEL(app.trigger_summary_values[i]), PANGO_ELLIPSIZE_END);
|
||||
gtk_grid_attach(GTK_GRID(app.trigger_summary), app.trigger_summary_keys[i], 0, i, 1, 1);
|
||||
gtk_grid_attach(GTK_GRID(app.trigger_summary), app.trigger_summary_values[i], 1, i, 1, 1);
|
||||
}
|
||||
gtk_widget_set_visible(app.trigger_summary, FALSE);
|
||||
|
||||
GtkWidget* outer = gtk_box_new(GTK_ORIENTATION_VERTICAL, 0);
|
||||
gtk_box_append(GTK_BOX(outer), head);
|
||||
gtk_box_append(GTK_BOX(outer), app.trigger_summary);
|
||||
gtk_box_append(GTK_BOX(outer), box);
|
||||
return outer;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user