From 620473a5bedc2a925ec8fb5e20459bbcff124ed3 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Wed, 14 Oct 2026 16:18:24 +0200 Subject: [PATCH] feat: the locked trigger pane carries Arm/Disarm, and one function states both arm buttons The compact line -- the pane's locked, "what is it set to" mode -- showed the arm state in words and gave no way to change it, so arming from there meant unfolding the panel first, which is the thing the lock exists to avoid. The control is built into the pane's *heading*, the row that stays on screen when the body goes, and it is wired to the same `on_arm_clicked` the body's button uses, so both do one thing. Its label, sensitivity and visibility are stated by `sync_arm_buttons()`, called from the four places any of its inputs can move: the click, the mode toggle, the curator's per-tick re-read (a single-shot latch disarms by itself) and `apply_pane_lock()`. The label comes from the producer's `trigger_armed()` atomic, the other two from the toolbar toggle that owns `enabled`, so no caller needs the model lock. Visibility is `enabled && locked`: with trigger mode off the line beside it already reads "stream (trigger off)" and there is no engine to arm, and with the pane open the body's own button is the one on screen. Never both -- two arm buttons would be two controls for one state. Verified with the arm probe, which locks the pane the way a user does (the [edit] click): compact arm with the trigger off: visible=0; with the trigger on and the pane locked: visible=1 sensitive=1 width=51 label=Disarm [udpscope] compact arm probe: locked=1 enabled=1 own=1 effective=1 width=51 panel_width=-1 label=Disarm `panel_width=-1` is the "not both" assertion: the body's button is not on screen while the compact one is. The width cost a defect to read. The control is created hidden and shown for the first time inside the handler, so a same-tick `gtk_widget_get_width()` reads 0 while `gtk_widget_is_visible()` already says 1 -- and the body button's *stale* allocation (270 px, from when the body was still up) reads as a live one, because `gtk_widget_get_visible()` is the widget's own flag and stays true under a hidden parent. The reading is therefore taken one frame later, by a step of its own, and the control is compared with `is_visible()`. Both defects are in the invariants, because the next control read this way will meet them. 0 Gtk-CRITICAL; clean build with the documented four warnings (the help resource re-measured); test-ui 14/14. Docs: the rule and both readings in `invariants-trigger-and-pinning.md`, the probe's entry in `probe-readings.md`, and the user-facing sentence in `src/help/08-trigger.txt`. --- docs/claude/invariants-trigger-and-pinning.md | 17 +++++ docs/claude/probe-readings.md | 10 +++ docs/claude/verification.md | 2 +- src/help/08-trigger.txt | 6 +- src/ui.cpp | 18 ++++-- src/ui_app.hpp | 17 +++++ src/ui_probe.cpp | 62 +++++++++++++++++-- src/ui_trigger.cpp | 45 ++++++++++++-- 8 files changed, 160 insertions(+), 17 deletions(-) diff --git a/docs/claude/invariants-trigger-and-pinning.md b/docs/claude/invariants-trigger-and-pinning.md index f43d389..b9fdd31 100644 --- a/docs/claude/invariants-trigger-and-pinning.md +++ b/docs/claude/invariants-trigger-and-pinning.md @@ -18,6 +18,23 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear reports it as `zoom ... span=.. window=.. limited=1`. Its pan clamping is likewise gated on the data being wider than the view, or the empty left-hand side drags the live edge off the right-hand edge. +- **The compact pane's arm control is built in the heading, and *one* function states both arm + buttons.** Arming is a runtime decision and the locked pane is exactly where it is made, so the + control cannot live in the body — that is the part which goes away. `sync_arm_buttons()` is what + states the label, the sensitivity and the visibility of both buttons, and it is called from the four + places any of its three inputs can change (the click, the toolbar toggle, the curator's re-read, + `apply_pane_lock()`). Visibility is `enabled && locked`: with trigger mode off the line beside it + already says "stream (trigger off)" and there is no engine to arm, and with the pane open the body's + button is the one on screen — **two arm buttons at once would be two controls for one state**, which + is why the probe reads the body's allocation as `-1` while the compact one is up. +- **A widget's allocation is not readable in the tick it is first shown.** The compact arm control is + created hidden and shown for the first time inside a signal handler, so `gtk_widget_get_width()` in + that same tick reads `0` while `gtk_widget_is_visible()` already says `1` — and its sibling's stale + allocation (270 px, from when the *body* was still on screen) reads as a live one, because + `gtk_widget_get_visible()` is the widget's *own* flag and stays true under a hidden parent. The width + only becomes the proof that a control is on the surface **one frame later**, which is why the probe + has a `compact_arm_step()` of its own; and the control to compare against must be measured with + `is_visible()` or not at all. - **Signals start disabled** — enabling is what starts acquisition. A trigger on a signal nobody acquires can never fire, so selecting/arming enables it. - **The trigger's event window is separate from the display window**, and diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index 480694a..83b8103 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -1207,6 +1207,16 @@ its pane. a trigger is configured before it is switched on — so this reads `1` always, and a `0` means the settings have been made unreachable again. The *cursors'* body is disabled with the cursors, and that is the pane whose readout has nothing to show. +- **`compact arm with the trigger off: visible=0` / `with the trigger on and the pane locked: + visible=1 sensitive=1 width=51 label=Disarm`** is the compact pane's arm control, in the two states + that define it. The first says it is *not* on the line when there is no engine to arm; the second says + it is, and that it is live (`sensitive=1`) and laid out (`width` > 0 — a width can only be read the + frame *after* it is first shown, see the invariants). `label` must agree with `model->trigger_armed()` + rather than with a fixed word, because a single-shot latch may have disarmed it by the time the probe + looks. `[udpscope] compact arm probe: locked=1 enabled=1 own=1 effective=1 width=51 panel_width=-1 + label=Disarm` prints the inputs beside the outcome, and **`panel_width=-1` is the assertion that the + body's own Arm button is not on screen at the same time** — two arm controls for one state is the + defect it exists to catch. - **`arm sensitive with the trigger off=0`** is the one exception inside that body: `Arm` is a *runtime* control, so with trigger mode off there is no engine to arm and it is disabled (and labelled `Arm` again). `1` here is an Arm button that does nothing when clicked. diff --git a/docs/claude/verification.md b/docs/claude/verification.md index 51110a9..f0aa18b 100644 --- a/docs/claude/verification.md +++ b/docs/claude/verification.md @@ -15,7 +15,7 @@ xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=8000 UDPSCOPE_ENABLE_ALL_MS=1000 \ Two baselines every run is read against. A clean build emits **four** warnings and no more: two `-Wmissing-field-initializers` on `MathNode::source` and `-Woverlength-strings` on **both** -generated GResource `.c` files — the help pages' (**515700** bytes) and the icons', whose blob is +generated GResource `.c` files — the help pages' (**516044** bytes) and the icons', whose blob is **12352** against C99's 4095-byte string-literal limit with the seven action icons and the two app icons that remain (a note about a generated file, which GCC itself supports) -- re-measured when the tab work changed the set (that icon left, the two tab glyphs arrived, and this file's diff --git a/src/help/08-trigger.txt b/src/help/08-trigger.txt index 8c4a654..a05247f 100644 --- a/src/help/08-trigger.txt +++ b/src/help/08-trigger.txt @@ -9,7 +9,11 @@ The captured window is frozen on screen until you re-arm. The trigger button in the toolbar (t) reveals the trigger panel: the signal, the level, the shape, the edge (rising, falling, either), the mode (Normal or Single) and the pre-trigger -share. Arm starts watching; a arms and disarms from the keyboard. +share. Arm starts watching; a arms and disarms from the keyboard. When the pane is +locked with its [edit] button — so that it states what the trigger is set to instead of showing +the controls — Arm/Disarm is in its heading, and the trigger can be armed or disarmed without +unfolding the panel. It is offered only while trigger mode is on, because a trigger that is off has no +engine to arm. Search the buffer is at the foot of the same panel. Worth knowing diff --git a/src/ui.cpp b/src/ui.cpp index 4941deb..26b194b 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -3659,7 +3659,7 @@ gboolean tick(gpointer user) { gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(app->trigger_toggle), trigger_on); } const bool armed = model.trigger_armed().load(std::memory_order_relaxed); - gtk_button_set_label(GTK_BUTTON(app->arm_button), armed ? "Disarm" : "Arm"); + sync_arm_buttons(*app); if (new_capture) { // The edge has been accepted, which means the pre window was already full, so the // whole event window can be framed now and the post side filled in view. @@ -4442,6 +4442,9 @@ void apply_pane_lock(App& app) { }; edit_button(app.trigger_edit, app.trigger_locked, "the trigger"); edit_button(app.signals_edit, app.signals_locked, "the signals"); + // The lock decides whether the compact line's arm control is the one on screen: with the body shown the + // body's own button is there, and two arm buttons at once would be two controls for one state. + sync_arm_buttons(app); sync_trigger_summary(app); // The list, because the lock is what decides whether a row carries a tick, a menu and the `+`. if (app.signal_list != nullptr) rebuild_signal_panels(app); @@ -4898,8 +4901,9 @@ void on_activate(GtkApplication* application, gpointer user) { // switch that turns it back on. The trigger's body is deliberately *not* disabled: a trigger is // configured before it is switched on, so its settings have to stay editable while it is off. gtk_widget_set_sensitive(app->cursors_body, FALSE); - // The trigger's Arm button is the one control in its pane that needs trigger mode running. - if (app->arm_button != nullptr) gtk_widget_set_sensitive(app->arm_button, FALSE); + // The arm controls are the one part of the trigger pane that needs trigger mode running: both of them, + // and the compact line's visibility, are stated from the toggle by `sync_arm_buttons()`. + sync_arm_buttons(*app); gtk_widget_set_sensitive(app->cursors_reset, FALSE); if (app->side_stack != nullptr) { @@ -6014,10 +6018,14 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit app.dbg_badge_frames[3]); std::printf("[udpscope] trigger panel sensitive before the probe toggled it: %d; " "signal list items=%d selected=%d; column visible after sidebar toggle off=%d; " - "pane switches live while their panes are off=%d; arm sensitive with the trigger off=%d\n", + "pane switches live while their panes are off=%d; arm sensitive with the trigger off=%d; " + "compact arm with the trigger off: visible=%d; with the trigger on and the pane " + "locked: visible=%d sensitive=%d width=%d label=%s\n", app.dbg_trigger_sensitive_default, app.dbg_trigger_items, app.dbg_trigger_selected, app.dbg_side_column_after_hide, - app.dbg_panel_switch_live, app.dbg_arm_sensitive); + app.dbg_panel_switch_live, app.dbg_arm_sensitive, app.dbg_summary_arm_off, + app.dbg_summary_arm_visible, app.dbg_summary_arm_sensitive, app.dbg_summary_arm_width, + app.dbg_summary_arm_label.c_str()); { // The panel's *frame* read back: the row boxes the `.ui` holds, and that every control the code built // ended up inside the panel rather than in a row that is not there. A control is created in code and diff --git a/src/ui_app.hpp b/src/ui_app.hpp index a794b57..fb24e85 100644 --- a/src/ui_app.hpp +++ b/src/ui_app.hpp @@ -281,9 +281,18 @@ struct App { static constexpr int kTriggerSummaryRows = 8; GtkWidget* trigger_summary_keys[kTriggerSummaryRows] = {}; GtkWidget* trigger_summary_values[kTriggerSummaryRows] = {}; + /// The compact line's arm control. It lives in the pane's *heading*, the part that stays on screen when + /// the body is hidden, and `sync_arm_buttons()` states it: present only while trigger mode is on and the + /// pane is locked, which is the one combination where the body's own Arm button has gone with the body. + GtkWidget* trigger_summary_arm = nullptr; GtkWidget* trigger_edit = nullptr; GtkWidget* signals_edit = nullptr; int dbg_trigger_locked = -1; + int dbg_summary_arm_off = -1; ///< the compact line's arm control with the trigger *off* + int dbg_summary_arm_visible = -1; ///< the compact line's arm control, locked and with the trigger on + int dbg_summary_arm_sensitive = -1; + std::string dbg_summary_arm_label; ///< its text, which is the armed state in the user's words + int dbg_summary_arm_width = -1; ///< its allocation a frame later: 0 means "not laid out" int dbg_signals_locked = -1; int dbg_trigger_body_visible = -1; /// The compact line the locked trigger pane shows. It is the pane's whole content in that mode, so it @@ -1476,6 +1485,14 @@ void row_drag_sidebar(App& app, bool dragging); /// bodies, their [edit] buttons and the signal list's rows all follow from `trigger_locked` and /// `signals_locked`. void sync_trigger_summary(App& app); + +/// Both arm buttons -- the panel's and the compact line's -- from the one armed state the producer keeps in +/// `model->trigger_armed()`. The label was written from four places (the click, the toolbar toggle, the +/// curator's re-read, the start-up default) and a second button would have needed a fifth copy of the same +/// two lines; the drift that costs is a button reading "Arm" while the engine is armed. Sensitivity, and +/// the compact button's visibility, are stated here too and read from the toolbar toggle -- the control +/// that owns `enabled`, per the restore rules -- so no caller needs the model lock to state any of it. +void sync_arm_buttons(App& app); void apply_pane_lock(App& app); void on_pane_edit_clicked(GtkButton* button, gpointer user); void apply_trigger_layout(App& app, const LayoutState::Trigger& t); diff --git a/src/ui_probe.cpp b/src/ui_probe.cpp index 61f437f..96d97ff 100644 --- a/src/ui_probe.cpp +++ b/src/ui_probe.cpp @@ -206,6 +206,44 @@ gboolean exit_probe_step3(gpointer user) { return G_SOURCE_REMOVE; } +/// The frame after the pane was locked: read the compact arm control's *allocation*, then leave the +/// configuration mode and trigger mode as step 2 used to, and hand the chain back its 400 ms. The button +/// is created hidden and this is the first tick it is shown, so its width in the same tick would always +/// read 0 -- a number that says "not yet laid out" rather than "not on the surface". +gboolean compact_arm_step(gpointer user) { + App* a = static_cast(user); + GtkWidget* b = a->trigger_summary_arm; + a->dbg_summary_arm_width = b != nullptr ? gtk_widget_get_width(b) : -1; + std::fprintf(stderr, + "[udpscope] compact arm probe: locked=%d enabled=%d own=%d effective=%d " + "width=%d panel_width=%d label=%s\n", + a->trigger_locked ? 1 : 0, + a->trigger_toggle != nullptr && + gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(a->trigger_toggle)) != FALSE + ? 1 : 0, + b != nullptr && gtk_widget_get_visible(b) ? 1 : 0, + b != nullptr && gtk_widget_is_visible(b) ? 1 : 0, a->dbg_summary_arm_width, + // The body's own Arm button, read only if it is itself on screen: with the pane locked it + // is not, and its stale allocation would read as if it were. -1 is "not the one on screen". + // `is_visible`, not `get_visible`: the flag is the widget's own and stays true while its + // *body* is hidden, which reported the body button's stale allocation as a live one. + a->arm_button != nullptr && gtk_widget_is_visible(a->arm_button) + ? gtk_widget_get_width(a->arm_button) + : -1, + a->dbg_summary_arm_label.c_str()); + std::fflush(stderr); + if (a->trigger_edit != nullptr) { + g_signal_emit_by_name(a->trigger_edit, "clicked"); // and back to the view only mode + } + if (a->trigger_toggle != nullptr) { + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(a->trigger_toggle), FALSE); + } + // The tick keeps running for these 400 ms, so by step 3 the status line reads + // the state the app is actually in after leaving trigger mode. + g_timeout_add(400, exit_probe_step3, a); + return G_SOURCE_REMOVE; +} + gboolean exit_probe_step2(gpointer user) { App* a = static_cast(user); a->dbg_mode_triggered = a->status_detail; @@ -215,12 +253,21 @@ gboolean exit_probe_step2(gpointer user) { a->dbg_trigger_time = a->model->trigger_time().load(); a->dbg_trigger_time_valid = a->model->trigger_time_valid().load() ? 1 : 0; } - if (a->trigger_toggle != nullptr) { - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(a->trigger_toggle), FALSE); + // The combination the compact control exists for: the trigger *on*, the pane *locked*, so the body's + // own Arm button has gone with the body. Locking is the [edit] button's click, because that is what a + // user does, and the two properties read are the two the user sees. The label is checked against the + // engine's armed state rather than against a word: a single-shot latch can have disarmed it by now. + if (a->trigger_edit != nullptr) { + g_signal_emit_by_name(a->trigger_edit, "clicked"); // into configuration mode + GtkWidget* b = a->trigger_summary_arm; + a->dbg_summary_arm_visible = b != nullptr && gtk_widget_get_visible(b) ? 1 : 0; + a->dbg_summary_arm_sensitive = b != nullptr && gtk_widget_get_sensitive(b) ? 1 : 0; + const char* text = b != nullptr && GTK_IS_BUTTON(b) ? gtk_button_get_label(GTK_BUTTON(b)) : nullptr; + a->dbg_summary_arm_label = text != nullptr ? text : "-"; } - // The tick keeps running for these 400 ms, so by step 3 the status line reads - // the state the app is actually in after leaving trigger mode. - g_timeout_add(400, exit_probe_step3, a); + // The pane stays locked for one more frame: the allocation the next step reads is only meaningful once + // a frame has run with the control shown. Leaving trigger mode is that step's job now. + g_timeout_add(400, compact_arm_step, a); return G_SOURCE_REMOVE; } @@ -2400,6 +2447,11 @@ gboolean exit_probe_step1(gpointer user) { // and asking it for a label's text was itself a critical. a->dbg_trigger_summary_early = a->dbg_trigger_summary; // as found, before this probe touches it a->dbg_arm_sensitive = a->arm_button != nullptr && gtk_widget_get_sensitive(a->arm_button) ? 1 : 0; + // The compact line's arm control must be *absent* here: the trigger is off, the line beside it reads + // "stream (trigger off)", and a trigger that is off has no engine to arm. Read before the toggle, which + // is where that absence is the assertion -- and it is the pair to the reading in step 2. + a->dbg_summary_arm_off = + a->trigger_summary_arm != nullptr && gtk_widget_get_visible(a->trigger_summary_arm) ? 1 : 0; 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) diff --git a/src/ui_trigger.cpp b/src/ui_trigger.cpp index 6ed5e92..f5a9511 100644 --- a/src/ui_trigger.cpp +++ b/src/ui_trigger.cpp @@ -14,6 +14,31 @@ namespace udpscope { namespace ui_impl { +/// Both arm buttons from the one state, so the panel's and the compact line's cannot disagree about what +/// the engine is doing. Three properties, three sources, and they are deliberately not the same source: +/// the *label* is the producer's atomic (no lock), the *sensitivity* and the compact button's +/// *visibility* are the toolbar toggle's state -- the toggle is what owns `enabled`, and reading it +/// instead of the model keeps this callable from a signal handler that does not hold, and must not take, +/// the model lock. Visibility is `enabled && locked`: with trigger mode off the line's own text says +/// "stream (trigger off)" and there is no engine to arm, and with the pane open the body's button is the +/// one on screen -- never both at once. +void sync_arm_buttons(App& app) { + const bool armed = app.model->trigger_armed().load(std::memory_order_relaxed); + const bool enabled = + app.trigger_toggle != nullptr && + gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(app.trigger_toggle)) != FALSE; + const char* label = armed ? "Disarm" : "Arm"; + if (app.arm_button != nullptr) { + gtk_button_set_label(GTK_BUTTON(app.arm_button), label); + gtk_widget_set_sensitive(app.arm_button, enabled); + } + if (app.trigger_summary_arm != nullptr) { + gtk_button_set_label(GTK_BUTTON(app.trigger_summary_arm), label); + gtk_widget_set_sensitive(app.trigger_summary_arm, enabled); + gtk_widget_set_visible(app.trigger_summary_arm, enabled && app.trigger_locked); + } +} + void on_arm_clicked(GtkButton* button, gpointer user) { App* app = static_cast(user); (void)button; @@ -37,7 +62,7 @@ void on_arm_clicked(GtkButton* button, gpointer user) { // is switched on with it -- otherwise the trigger could never see a sample. // Outside the lock: set_signal_enabled() takes it. if (armed && signal >= 0) app->model->set_signal_enabled(static_cast(signal), true); - gtk_button_set_label(GTK_BUTTON(app->arm_button), armed ? "Disarm" : "Arm"); + sync_arm_buttons(*app); } /// The trigger type dropdown's order, as a table rather than a cast: the dropdown lists @@ -409,10 +434,10 @@ void on_trigger_toggled(GtkToggleButton* button, gpointer user) { // column of stale events beside the plot. Recording is the other half of that answer, so this is // `sync_events_availability()`'s call and not the trigger's alone. sync_events_availability(*app); - gtk_button_set_label(GTK_BUTTON(app->arm_button), on ? "Disarm" : "Arm"); - // Arming is a *runtime* control: with trigger mode off there is no engine to arm, and the button is the - // one part of this pane that is not a setting. (The body itself stays live -- see above.) - if (app->arm_button != nullptr) gtk_widget_set_sensitive(app->arm_button, on); + // Arming is a *runtime* control: with trigger mode off there is no engine to arm, and the arm buttons + // are the one part of this pane that is not a setting. (The body itself stays live -- see above.) Both + // of them are stated from this switch's new state in one place, so they cannot disagree with each other. + sync_arm_buttons(*app); // The pane's own switch shows the same state, whichever of the two was clicked, and the compact line // changes shape with it: with trigger mode off it says "stream" and nothing else. sync_panel_switches(*app); @@ -664,6 +689,16 @@ GtkWidget* build_trigger_panel(App& app) { gtk_widget_set_tooltip_text(app.trigger_edit, "configure the trigger"); g_signal_connect(app.trigger_edit, "clicked", G_CALLBACK(on_pane_edit_clicked), &app); gtk_box_append(GTK_BOX(head), app.trigger_edit); + // The compact line's own arm control, built here beside the [edit] and the switch for the same reason + // the switch is: this row stays on screen when the body is hidden. A locked pane is exactly where + // arming is decided -- the body's button left with the body -- and the heading is where it has to be. + app.trigger_summary_arm = gtk_button_new_with_label("Arm"); + gtk_widget_set_valign(app.trigger_summary_arm, GTK_ALIGN_CENTER); + gtk_widget_set_tooltip_text(app.trigger_summary_arm, + "arm or disarm the trigger; disarming freezes the view where it stands"); + gtk_widget_set_visible(app.trigger_summary_arm, FALSE); + g_signal_connect(app.trigger_summary_arm, "clicked", G_CALLBACK(on_arm_clicked), &app); + gtk_box_append(GTK_BOX(head), app.trigger_summary_arm); app.trigger_switch = gtk_switch_new(); gtk_widget_set_valign(app.trigger_switch, GTK_ALIGN_CENTER); gtk_widget_set_tooltip_text(app.trigger_switch,