From 7aa9cf466444c40c23c11c30559c33ab79607298 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Thu, 8 Oct 2026 11:18:51 +0200 Subject: [PATCH] fix: a rebuild, a spurious observation, and a width limit protect the invariants they were breaking - A buffer rebuild mid-scan (CONFIG or window change) now calls `search.clear()` beside `reset_measure_stats()` at both sites; without it the scan kept stale at_/end_ and could report Done over fabricated hits (fix 1). - `search_marks_label` now carries `PANGO_ELLIPSIZE_END` so the side pane's minimum width is bounded; the longest text (~58 chars) was driving the pane width (fix 2). - `update_measures()` gains a third `accumulate` parameter (no default) so every call states its intent; the Statistics toggle now passes `false`, stopping the same window being counted twice and deflating sd on repeated toggles (fix 5). - `search_jump()` records whether `publish_pause_view()` returned an empty snapshot and `update_search_panel()` appends "(no longer in the buffer)" to the position label when it did (fix 9). Co-Authored-By: Claude Opus 4.6 --- src/ui.cpp | 62 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 20 deletions(-) diff --git a/src/ui.cpp b/src/ui.cpp index c084b2f..2b94cc6 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -171,6 +171,9 @@ struct App { GtkWidget* search_marks_label = nullptr; GtkWidget* search_prev_button = nullptr; GtkWidget* search_next_button = nullptr; + /// Set true when the last `search_jump()` produced an empty cut — the hit was so close to the + /// oldest retained sample that `publish_pause_view()` held nothing. Cleared at every jump. + bool search_jump_empty = false; GtkWidget* menu_button = nullptr; GtkWidget* cursors_toggle = nullptr; /// Where the panel controls ended up, decided once at build time: the dump runs after the window @@ -1731,10 +1734,10 @@ void sync_trigger_rows(App& app) { pulse && (cmp == WidthCmp::Within || cmp == WidthCmp::Outside)); gtk_widget_set_visible(app.trigger_level2_row, window); gtk_widget_set_visible(app.trigger_timeout_row, type == TriggerType::Dropout); - // A band of zero width is not a degenerate edge trigger -- a window event is a change of - // *side*, so equal levels silently never fire, which looks exactly like a broken trigger. - // The panel is where that is refused: the second level is pushed at least one increment - // clear of the first, in whichever direction it already lay. + // A band of zero width reduces the condition to a float-equality test on a real-valued + // signal, which is unusable: it fires on the exact value and nothing else, which in practice + // is never. The panel is where that is refused: the second level is pushed at least one + // increment clear of the first, in whichever direction it already lay. if (window) { GtkSpinButton* l2 = GTK_SPIN_BUTTON(app.trigger_level2_spin); const double level = gtk_spin_button_get_value(GTK_SPIN_BUTTON(app.trigger_level_spin)); @@ -2257,7 +2260,9 @@ void apply_time_window(App& app, double seconds) { sync_window_display(app, clamped); // A measurement over a different window is a different figure -- a mean over 20 ms and a // mean over 2 s are not observations of one quantity -- so the history behind it goes. + // The search shares the same premise: the ring was rebuilt, so its at_/end_ describe nothing. reset_measure_stats(app); + app.search.clear(); schedule_layout_save(app); } @@ -2361,6 +2366,7 @@ void on_pause_toggled(GtkToggleButton* button, gpointer user) { /// next frame's `clamp_to_data()` would follow the live edge and walk the frozen window off /// the hit entirely. void search_jump(App& app, double t) { + app.search_jump_empty = false; const double w = app.rows->time_window(); if (app.pause_button != nullptr && !gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(app.pause_button))) { @@ -2376,7 +2382,11 @@ void search_jump(App& app, double t) { if (!snap->stamps.empty()) { app.rows->pin_pause_window(static_cast(snap->stamps.front()), static_cast(snap->stamps.back())); + } else { + app.search_jump_empty = true; } + } else { + app.search_jump_empty = true; } } @@ -2409,9 +2419,11 @@ void update_search_panel(App& app) { } gtk_label_set_text(GTK_LABEL(app.search_status_label), text); - char pos[64]; + char pos[96]; if (s.cursor() < 0 || n == 0) { std::snprintf(pos, sizeof(pos), "--"); + } else if (app.search_jump_empty) { + std::snprintf(pos, sizeof(pos), "%d of %zu (no longer in the buffer)", s.cursor() + 1, n); } else { std::snprintf(pos, sizeof(pos), "%d of %zu", s.cursor() + 1, n); } @@ -2702,7 +2714,7 @@ constexpr gint64 kMeasureRefreshUs = 100 * 1000; constexpr gint64 kMeasureHeavyUs = 5 * 1000; void rebuild_measures_pane(App& app); -void update_measures(App& app, bool force); +void update_measures(App& app, bool force, bool accumulate); void open_measure_dialog(App& app, int edit_index); /// One pane row: the measurement's badge, its type, its value and an `edit` button. The value @@ -2813,7 +2825,7 @@ void rebuild_measures_pane(App& app) { gtk_box_append(GTK_BOX(app.measures_list), measures_row(app, i, app.measures[i])); app.measure_text.emplace_back(); } - update_measures(app, true); + update_measures(app, /*force=*/true, /*accumulate=*/true); } // --- the cursors pane ------------------------------------------------------- @@ -3423,7 +3435,7 @@ void place_search_marks(App& app) { /// threshold is a small fraction of the span, so the marks stay within a few pixels of where /// the current window would put them, and the cost is bounded by the window's own motion /// rather than by the frame rate. -void update_measures(App& app, bool force) { +void update_measures(App& app, bool force, bool accumulate) { if (app.measure_labels.size() != app.measures.size()) return; // mid-rebuild const RowSet::Row& r0 = app.rows->row(0); if (r0.plot == nullptr) return; @@ -3654,10 +3666,11 @@ void update_measures(App& app, bool force) { app.measure_text[i] = text; gtk_label_set_text(GTK_LABEL(label), text.c_str()); } - // One observation per recompute, and only when the pass produced one. An invalid pass - // contributes nothing -- the marks' `measure_last_valid` fallback below is a *drawing* - // convenience, and accumulating it would count one answer as many observations. - if (r.valid && i < app.measure_stats.size()) app.measure_stats[i].add(r.primary); + // One observation per recompute, and only when the pass produced one and the caller + // asked for it. An invalid pass contributes nothing -- the marks' `measure_last_valid` + // fallback is a *drawing* convenience. `accumulate=false` is the toggle handler, which + // forces a repaint of the newly-visible labels without counting the window again. + if (accumulate && r.valid && i < app.measure_stats.size()) app.measure_stats[i].add(r.primary); if (i < app.measure_stats_labels.size() && app.measure_stats_labels[i] != nullptr) { GtkWidget* sl = app.measure_stats_labels[i]; gtk_widget_set_visible(sl, app.measure_stats_on); @@ -5312,7 +5325,9 @@ GtkWidget* build_measures_panel(App& app) { } if (a->measures_stats_reset != nullptr) gtk_widget_set_sensitive(a->measures_stats_reset, a->measure_stats_on); - update_measures(*a, true); + // Refresh the now-visible labels without counting the window again: the + // toggle does not reset or disturb the history (README, Statistics). + update_measures(*a, /*force=*/true, /*accumulate=*/false); schedule_layout_save(*a); }), &app); @@ -5327,7 +5342,7 @@ GtkWidget* build_measures_panel(App& app) { G_CALLBACK(+[](GtkButton*, gpointer user) { App* a = static_cast(user); reset_measure_stats(*a); - update_measures(*a, true); + update_measures(*a, /*force=*/true, /*accumulate=*/true); }), &app); gtk_box_append(GTK_BOX(stats_row), app.measures_stats_reset); @@ -6177,8 +6192,10 @@ gboolean tick(gpointer user) { } if (model.generation() != app->signal_generation) { // A CONFIG re-describes the stream and drops the buffer, so nothing measured before it - // belongs in the same series as what comes after. + // belongs in the same series as what comes after. The search shares the same premise: + // the ring it was scanning no longer exists. reset_measure_stats(*app); + app->search.clear(); rebuild_signal_panels(*app); } // A gesture moves the *plot's* cursor; adopting it here is what makes the two @@ -6199,7 +6216,7 @@ gboolean tick(gpointer user) { } app->rows->sync_cursors(cursor_drag_row); update_cursors_pane(*app); - update_measures(*app, false); + update_measures(*app, /*force=*/false, /*accumulate=*/true); // Every frame, whether or not a scan is running: the marks are in stream seconds and the axis // is newest-relative, so a live window that has rolled since the last frame needs them moved. place_search_marks(*app); @@ -6966,8 +6983,9 @@ GtkWidget* build_trigger_panel(App& app) { app.trigger_level2_spin = spin(-1.0e12, 1.0e12, 0.01, 4); gtk_widget_set_tooltip_text(app.trigger_level2_spin, "the band's other edge; it is kept at least one increment away " - "from the level, because a band of zero width can never be " - "crossed"); + "from the level, because a band of zero width reduces the " + "condition to a float-equality test, which on a real-valued " + "signal never fires"); app.trigger_level2_row = labeled_box("to level", app.trigger_level2_spin); gtk_box_append(GTK_BOX(box), app.trigger_level2_row); @@ -7103,6 +7121,10 @@ GtkWidget* build_trigger_panel(App& app) { app.search_marks_label = gtk_label_new(" "); gtk_widget_set_halign(app.search_marks_label, GTK_ALIGN_START); gtk_widget_add_css_class(app.search_marks_label, "dim-label"); + // Ellipsize so the label's minimum width is bounded: without this, the longest text + // (~58 chars) would widen the side pane just as `search_status_label` would without its own + // wrap — the same fix the statistics label carries at line 2780. + gtk_label_set_ellipsize(GTK_LABEL(app.search_marks_label), PANGO_ELLIPSIZE_END); gtk_box_append(GTK_BOX(sbox), app.search_marks_label); gtk_expander_set_child(GTK_EXPANDER(search_exp), sbox); @@ -8647,7 +8669,7 @@ gboolean measure_probe(gpointer user) { a->measures.push_back({name, t, options}); } rebuild_measures_pane(*a); - update_measures(*a, true); + update_measures(*a, /*force=*/true, /*accumulate=*/true); std::fprintf(stderr, "[measure-probe] %zu measurements on '%s'\n", a->measures.size(), name.c_str()); std::fflush(stderr); @@ -8731,7 +8753,7 @@ gboolean stats_probe_report(gpointer user) { } g_stats_phase = 2; reset_measure_stats(*a); - update_measures(*a, true); + update_measures(*a, /*force=*/true, /*accumulate=*/true); g_timeout_add(50, stats_probe_report, a); } return G_SOURCE_REMOVE;