diff --git a/CLAUDE.md b/CLAUDE.md index a5926ea..04f1acf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -449,13 +449,18 @@ Two lines of the self-check output are assertions in disguise: equal `origin + first_rel` as a *tick label* plus its caption, i.e. the labels and the caption must share one anchor. `x_origin_set` is read back out of the plot; it must be 0 in the absolute form. -- `[fit-probe] window=.. x=[..] span=.. y=[..]` (stderr, `UDPSCOPE_PROBE_FIT_MS`) — - the two fits, through the same `RowSet` calls the right-click menu wires. It first puts - the y scale and the window where the data is *not* (a fixed `[-100, 100]`, a 50 ms - window), so the assertions have something to be wrong about: **`fit x` must widen the - window to the data's span and leave `y` exactly as it was**, and **`fit all` must then - move `y` to the data's own range**. A `y` that changed on the `fit x` line is the two - paths aliased. +- `[fit-probe] window=.. event_span=.. x=[..] span=.. y=[..]` (stderr, + `UDPSCOPE_PROBE_FIT_MS`) — the two fits, through the **menu handlers** rather than the bare + `RowSet` calls, because half of what a fit now does lives there (see the time-scale invariant + below). It first puts the y scale and the window where the data is *not* (a fixed + `[-100, 100]`, a 50 ms window), so the assertions have something to be wrong about: **`fit x` + must widen the window to the data's span and leave `y` exactly as it was**, and **`fit all` + must then move `y` to the data's own range**. A `y` that changed on the `fit x` line is the two + paths aliased. **`event_span` must equal `window` on every line**: a fit states a new time + scale, so the trigger's event span, the buffer partition and the control have to move with it — + a fit that moved the view alone left `event_span` on the *old* window, and the next decade step + or unit change then applied the fitted value for real, so one action changed the time scale + twice. - `[layout-probe] rows=N buttons=B done=D done_box=[x,y,WxH] rows_y=[..] close=[..] add=[..] traces=[..]` (stderr, `UDPSCOPE_PROBE_LAYOUT_MS`) — **`buttons` must be `2N+1`** while the editor is up (one `×` per row, one `+` per boundary) and **0 on the `editor off` line**: the @@ -1356,12 +1361,30 @@ far the largest file. that grew from `1 s` to `123.456 ms` would widen the bar and relayout everything under it on a mere change of window. That is also why the window is typed into a popover rather than shown in the bar. -- **`apply_time_window()` is the only place the display window changes.** It clamps to - `window_limit()` and moves `RowSet::set_time_window()` and `ScopeModel::set_event_span()` - together, because the trigger's pre-trigger share is a share of *this* window. Paths that - change the span for their own reasons (Fit, an event pin) call `sync_window_display()` - instead, which only restates the control and never moves the plot — calling `apply_` there - would clamp a legitimately short capture up to the requested window. +- **A plot interaction may not quietly redefine the time scale.** The *time scale* is more than + the axis: `RowSet::window_s_` also feeds `ScopeModel::set_event_span()` (the trigger's + pre-trigger share is a share of *this* window), `ScopeModel::set_target_span()` (which may + re-plan the partition, move the decimation and drop history), the window control's readout and + the session file. A **zoom or a pan therefore does not touch it at all** — the controller + writes the *member* axis and `clamp_to_data()` caps the span at `window_s_`, so a gesture is + view-only by construction and needs no code to keep it that way. The **two fits do** state a + new scale, and they must state the whole of it: `on_fit_clicked`/`on_fit_x_clicked` take the + span `RowSet::fit_x()`/`fit_all()` report and hand it to **`apply_window_scale()`**. A fit that + moved `window_s_` alone — which is what both of them used to do — left the event span on the + *old* window, and the next decade step or unit change then fed the fitted value in for real: + one action, two changes of time scale, and `window == event_span` true at neither point. +- **`apply_window_scale()` is the only place the time scale changes, and `apply_time_window()` + is the only place the *view* changes with it.** The first clamps to `window_limit()` and moves + `set_target_span()`, `set_event_span()`, the control and the session save together; the second + calls it and then `RowSet::set_time_window()`. The split is not cosmetic: + `set_time_window()` unpins and re-arms the live follow, which is right for a typed window (a + statement about what to watch from now on) and wrong for a fit of a held event or a paused + view, which has nothing bound to a live edge — so the fits take the scale half only, and + `fit_x()` writes the link group directly while `pinned_`. The partition is moved **before** the + axis, so the axis is framed over the rings that will exist. Paths that change the span for + their own reasons (an event pin) still call `sync_window_display()` alone, which only restates + the control and never moves the plot — calling either `apply_` there would clamp a legitimately + short capture up to the requested window. - **The header bar's buttons are icon-only**, so headless probes must reach them through the `App` pointers (`trigger_toggle`, `pause_button`, …), not via `find_labelled_button()`. The **pane switches are toolbar buttons** too (`cursors_toggle`, diff --git a/README.md b/README.md index 83cea19..46a7d99 100644 --- a/README.md +++ b/README.md @@ -1281,6 +1281,14 @@ propagated to every row. **Fit X axis** is the same re-framing without the y sca the common case where the window should be the data's but the levels were placed by hand. The status line acknowledges either (`x fitted to the buffered data (1.25 s)`). +Either fit **states a new time window**, and states the whole of it: the span it framed goes +through the same door a typed window does, so the trigger's event span, the buffer partition and +the window control all land on the fitted value and the control reads it back. A **zoom or a pan +never does** — a gesture moves the view inside the window and leaves the window alone, which is +what keeps a wheel from silently re-planning the buffer and dropping the history. The one thing a +fit does *not* borrow from the typed path is the live re-follow: fitting a held event or a paused +view keeps it pinned rather than throwing it back onto the stream. + **Shared x axis.** The rows are the rows of **one `plot::Figure`**, drawn by one `plot::gtk4::PlotArea` in figure mode, and they all join a single x link group. A gesture changes the row it landed on; `Figure::layout()` adopts that change diff --git a/deps/plotter b/deps/plotter index c960dec..03c41ad 160000 --- a/deps/plotter +++ b/deps/plotter @@ -1 +1 @@ -Subproject commit c960decaea6ec1ba00abd1d4d10c2a29c1d0a511 +Subproject commit 03c41adbec42516f133035557227d082fb2ab188 diff --git a/src/help/06-navigate.txt b/src/help/06-navigate.txt index f073948..ef66fac 100644 --- a/src/help/06-navigate.txt +++ b/src/help/06-navigate.txt @@ -19,6 +19,10 @@ The full table is in the shortcut overview (Ctrl+/). • A pinned view is navigable within its own data but cannot leave it. While an event is held, a capture is filling, or the view is paused, a zoom in works normally — "not outside the event" is not "not readable" — but a pan or a zoom out is clamped to the data. +• A gesture never changes the time window, only the view inside it. The window is what the + header control states, and it also sets the trigger's event span and how the buffer is stored — + so a wheel that redefined it would re-plan the buffer and drop your history. The two fits + do set it, deliberately, and the control then reads the fitted value back. • A live view is clamped to the window you asked for, not to what the ring happens to hold. That is why a buffer that has only just started filling leaves its left-hand side empty instead of squeezing the window down to the first few samples. diff --git a/src/plot_host.cpp b/src/plot_host.cpp index b5040bf..9f2f707 100644 --- a/src/plot_host.cpp +++ b/src/plot_host.cpp @@ -657,21 +657,24 @@ void RowSet::unpin_pause_window() { set_time_window(window_s_); } -void RowSet::fit_x() { +bool RowSet::fit_x(double& span) { Row& first = row(0); - if (!first.plot || !figure_) return; + if (!first.plot || !figure_) return false; first.plot->auto_fit_x(); const plot::Range fitted = first.plot->x_range(first.axis_x); - const double span = fitted.span(); - if (!std::isfinite(span) || !(span > 0.0)) return; - // The same two paths `fit_all()` takes: a held event owns its window and states it - // through the link group, a live view re-arms the follow and frames the newest span. + const double fitted_span = fitted.span(); + if (!std::isfinite(fitted_span) || !(fitted_span > 0.0)) return false; if (pinned_) { - window_s_ = span; + // Fitting a captured or paused view: the window is the snapshot's own span, and + // `set_time_window()` would re-arm the follow and re-frame on a live edge that a frozen + // view has nothing bound to. + window_s_ = fitted_span; figure_->set_x_link_range(x_group_, fitted); } else { - set_time_window(span); + set_time_window(fitted_span); } + span = window_s_; + return true; } void RowSet::sync_cursors(int drag_row) { @@ -785,28 +788,16 @@ void RowSet::pin_event_window(double pre_s, double post_s) { figure_->set_x_link_range(x_group_, {-pre_s, post_s}); } -void RowSet::fit_all() { +bool RowSet::fit_all(double& span) { for (int i = 0; i < rows_; ++i) { Row& r = row(i); if (r.plot) r.plot->auto_fit_y(r.axis_y); } - Row& first = row(0); - if (!first.plot || !figure_) return; - // Row 0 fits its own x to the data it holds; that span becomes the shared window, and - // stating it through the group is what puts every row on it. - first.plot->auto_fit_x(); - const plot::Range fitted = first.plot->x_range(first.axis_x); - const double span = fitted.span(); - if (!std::isfinite(span) || !(span > 0.0)) return; - if (pinned_) { - // Fitting a captured event: the window is the snapshot's own `[-pre, +post]` span, - // and `set_time_window()` would re-arm the follow and re-frame on a live edge that - // trigger mode has nothing bound to. - window_s_ = span; - figure_->set_x_link_range(x_group_, fitted); - } else { - set_time_window(span); - } + // The x half is `fit_x()`'s exactly -- row 0's data span, stated through the link group so + // every row lands on it -- and sharing it is what keeps the two fits from drifting apart. + // The y fit above has already happened either way: a row 0 that holds nothing still gets + // every row's y scaled, it just has no span to report. + return fit_x(span); } void RowSet::note_y_gutter(double need_px) { diff --git a/src/plot_host.hpp b/src/plot_host.hpp index 16a3cc1..d8b76cd 100644 --- a/src/plot_host.hpp +++ b/src/plot_host.hpp @@ -301,7 +301,14 @@ class RowSet { /// Frames the x window on the data row 0 holds, leaving every y axis alone. The x /// counterpart of `fit_all()`, for the one thing the y axes cannot say: how wide the /// window should be. - void fit_x(); + /// + /// Reports the span it framed in `span` and `true` when it framed one at all (row 0 may hold + /// no data, and a degenerate range is refused). **The caller owes the rest of the time + /// scale**: this moves the view and `window_s_`, and the trigger's event span, the buffer + /// partition and the window control are the caller's to move with it -- see + /// `apply_window_scale()` in `ui.cpp`. Leaving them behind is what made a fit and the control + /// disagree about what the window was. + bool fit_x(double& span); /// Adopts the instruments a gesture is moving, on `drag_row` (the row whose controller holds /// a cursor), and nothing otherwise. @@ -337,10 +344,12 @@ class RowSet { void pin_event_window(double pre_s, double post_s); /// Fits every row's y axis to the data it holds, and re-frames the shared x window - /// on what row 0 retains (the same thing the plot's own `f` key does). The window - /// the toolbar reports becomes the fitted span, so the dropdown can say what is - /// actually on screen. - void fit_all(); + /// on what row 0 retains (the same thing the plot's own `f` key does). + /// + /// Reports the framed x span exactly as `fit_x()` does, and leaves the caller the same debt: + /// the y fit is this object's business alone, but the x span is the *time scale*, and the + /// trigger's event span, the buffer partition and the window control have to move with it. + bool fit_all(double& span); /// Reports the left gutter the rows currently need, and reserves at least the width /// of the widest *plausible* y label. diff --git a/src/ui.cpp b/src/ui.cpp index 039001e..8159343 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -1865,11 +1865,19 @@ void sync_window_display(App& app, double seconds) { app.updating_panels = false; } -/// The one place the display window changes. Clamps to what the buffer can hold, moves the -/// plot and the trigger's event span together -- its pre-trigger share is a share of *this* -/// window -- and then restates the result in the control, so a clamped request reads back as -/// the window that was actually taken rather than the one that was typed. -void apply_time_window(App& app, double seconds) { +/// Everything the time scale consists of *except* the view: the buffer partition, the trigger's +/// event span, the control's readout and the session. Split out of `apply_time_window()` because +/// a **fit** frames the axis itself -- and when the view is pinned it must keep the pin, which +/// `RowSet::set_time_window()` would clear, throwing a frozen picture onto the live stream -- but +/// a fit still has to move all of this. +/// +/// That is the rule: a plot interaction may not quietly redefine the window. A zoom and a pan +/// never touch it (they move the axis, and `clamp_to_data()` caps them at `window_s_`); the two +/// fits do set it, and so must come through here. A fit that moved `window_s_` alone -- which is +/// what both of them used to do -- left the trigger's event span on the *old* window, and the +/// next decade step or unit change then fed the fitted value in for real, so the time scale +/// changed twice for one action and the `window == event_span` invariant held at neither point. +void apply_window_scale(App& app, double seconds) { if (!(seconds > 0.0)) return; const double clamped = std::min(std::max(seconds, kWindowMinS), window_limit(app)); // The buffer has to cover the window, so this may re-plan the partition and drop the @@ -1882,12 +1890,25 @@ void apply_time_window(App& app, double seconds) { format_rate(app.model->partition().stored_rate()) + ", history cleared"; app.notice_until_us = g_get_monotonic_time() + 4 * G_TIME_SPAN_SECOND; } - app.rows->set_time_window(clamped); app.model->set_event_span(clamped); sync_window_display(app, clamped); schedule_layout_save(app); } +/// The one place the display window changes *on the control's behalf*. Clamps to what the buffer +/// can hold, moves the plot and the trigger's event span together -- its pre-trigger share is a +/// share of *this* window -- and then restates the result in the control, so a clamped request +/// reads back as the window that was actually taken rather than the one that was typed. +void apply_time_window(App& app, double seconds) { + if (!(seconds > 0.0)) return; + const double clamped = std::min(std::max(seconds, kWindowMinS), window_limit(app)); + // The partition first, so the axis below is framed over the rings that will exist. + apply_window_scale(app, clamped); + // And the view, which unpins and re-arms the live follow -- right for a typed window, which + // is a statement about what to watch from now on, and the reason a fit does not come here. + app.rows->set_time_window(clamped); +} + /// Enter in the entry. A number that will not parse restores the current window instead of /// reporting an error: the entry is also the readout, so leaving junk in it would leave the /// control disagreeing with the plot. @@ -4047,12 +4068,15 @@ void on_signals_toggled(GtkToggleButton* button, gpointer user) { } /// Fit: auto-scale every row's y axis and re-frame the x window on the data the -/// ring holds (the plot's own `f` semantics), then tell the toolbar what span that -/// is. +/// ring holds (the plot's own `f` semantics), then apply that span as the time scale. +/// +/// `apply_window_scale()` and not `sync_window_display()`: a fitted x span *is* a new window, so +/// the trigger's event span and the buffer partition move with it. Restating the control alone +/// left those on the previous window. void on_fit_clicked(GtkButton*, gpointer user) { App* app = static_cast(user); - app->rows->fit_all(); - sync_window_display(*app, app->rows->time_window()); + double span = 0.0; + if (app->rows->fit_all(span)) apply_window_scale(*app, span); app->notice = "fitted to the buffered data (" + format_window(app->rows->time_window()) + ")"; app->notice_until_us = g_get_monotonic_time() + 3 * G_TIME_SPAN_SECOND; } @@ -4062,8 +4086,8 @@ void on_fit_clicked(GtkButton*, gpointer user) { /// were placed by hand. void on_fit_x_clicked(GtkButton*, gpointer user) { App* app = static_cast(user); - app->rows->fit_x(); - sync_window_display(*app, app->rows->time_window()); + double span = 0.0; + if (app->rows->fit_x(span)) apply_window_scale(*app, span); app->notice = "x fitted to the buffered data (" + format_window(app->rows->time_window()) + ")"; app->notice_until_us = g_get_monotonic_time() + 3 * G_TIME_SPAN_SECOND; } @@ -7182,9 +7206,14 @@ void fit_probe_report(App& a, const char* what) { const RowSet::Row& r0 = a.rows->row(0); const plot::Range x = r0.plot != nullptr ? r0.plot->x_range(r0.axis_x) : plot::Range{}; const plot::Range y = r0.plot != nullptr ? r0.plot->y_range(r0.axis_y) : plot::Range{}; + // `event_span` is the assertion that a fit moved the whole time scale and not just the view: + // it is the trigger's, written only by `apply_window_scale()`, so a fit that moved `window` + // alone shows up here as the two disagreeing. std::fprintf(stderr, - "[fit-probe] %-10s window=%.6f x=[%.6f..%.6f] span=%.6f y=[%.6f..%.6f]\n", - what, a.rows->time_window(), x.min, x.max, x.span(), y.min, y.max); + "[fit-probe] %-10s window=%.6f event_span=%.6f x=[%.6f..%.6f] span=%.6f " + "y=[%.6f..%.6f]\n", + what, a.rows->time_window(), a.model->event_span(), x.min, x.max, x.span(), y.min, + y.max); std::fflush(stderr); } @@ -7275,15 +7304,16 @@ gboolean fit_probe_step(gpointer user) { a->dbg_fit_step = 1; break; case 1: - a->rows->fit_x(); - sync_window_display(*a, a->rows->time_window()); + // The handlers themselves, not `RowSet::fit_x()` alone: the whole point of the + // `event_span=` reading below is that a fit moves the *time scale* and not just the view, + // and that half lives in the handler. + on_fit_x_clicked(nullptr, a); g_timeout_add(400, fit_probe_step, a); a->dbg_fit_step = 2; break; case 2: fit_probe_report(*a, "fit x"); - a->rows->fit_all(); - sync_window_display(*a, a->rows->time_window()); + on_fit_clicked(nullptr, a); g_timeout_add(400, fit_probe_step, a); a->dbg_fit_step = 3; break;