fix: a still frame's measurements are the frame's, not the viewport's
A measurement is a statistic of the window the pane is *about*, and on a pinned view -- a held event, a paused stream, an opened file -- that is the frame, not the part of it on screen. The pane followed the viewport instead, so a wheel on a latched event moved `average` and `top`: the reported "when measuring a still frame, moving the zoom makes measurements such as average or top change". Nothing about the data had changed; the user was only looking closer. The window is `RowSet::pinned_range()` while the pin frames the view and the viewport otherwise, and the choice is made in `update_measures()` -- the call site -- rather than inside `MeasureWindowReader`, because it is *booked* as well as read: that window is what the "it moved" test compares against `App::measure_window`, so a gesture inside the pin spends no recompute either, and a reader that chose its own would leave a pinned view recomputing every gesture frame for a number that cannot change. `pinned_range()` is now the one expression of the pin's extent, shared with the framing (`hold_pinned_window()`, `fit_x()`), and the reader is handed the window instead of reading the plot. Live is unchanged: there the window really moves and the statistic follows it, and the accumulator's rule about what resets the history is about neither. `UDPSCOPE_PROBE_MEASUREPIN_MS` is the guard, three legs of before/after a gesture on the row's own axis. On a latched event zoomed to a quarter (`view=[-1..1]` -> `[-0.25..0.25]`, `pin` unchanged): `same=4/4 for_same=1 pass_delta=0`, every value bit-identical -- `average=0.000000 top=0.957975 peak to peak=2.000000 amplitude=1.915950` both sides. The same on a pause taken from it. Since the key `for=` follows `pin=` now, `[measure-trace]` gained the field, so a held run reads `for==pin` with `swin` free to sit inside it. On a live view a zoom to an eighth reads `same=0/4 for_same=0 pass_delta=3 span_after=0.125000 follows=1` with the values moving -- the case the rule must not swallow. Before the fix the same probe reads `same=1/4 for_same=0 pass_delta=1 span_after=0.500000` on the held event (`top` `0.957975` -> `0.957966`) and `same=0/4 span_after=0.125000` on the pause (`top` `0.957944` -> `0.358614`). Live reads the same either way. Also fixed, found by the pause leg: pausing while an event is *held* froze an **empty** picture. `on_pause_toggled()` read `stream_at_x0()` after setting the pause state, and that accessor follows the display -- `paused` with no pause view published yet answers `-display_epoch()`, which on a held event is 0, so the cut searched the ring at edge-relative x, found nothing, and the frame bound an empty snapshot: every measurement `--`, every ruler gone (`ev=650001` on the probe's line now, `ev=0` before). The offset and the window are read first, which is the offset the axis is actually displaying. The live case is unaffected, which is why the pause probe, which pauses a live view, never saw it. Docs corrected where they asserted the old half-truth: the invariants now separate the accumulator's rule (unchanged) from the window a value is computed over (this change), the trigger/pinning file names the pin as load-bearing for both the framing and the numbers and records the pause ordering rule, `verification.md`/`probe-readings.md` describe the new probe and `pin=`, and the README and the manual say what a still frame measures. Checked: `cmake --build build --clean-first -j4` 0 errors and the documented three warnings; `make -C build test-ui` 13 ok; `make -C build test-help` green; an exit-dump run 0 Gtk-WARNING/Gtk-CRITICAL and `gtk warnings: 0`; `[measurepin-probe]`, `[pan-probe]` (`held=1`, `allowed=1`, `limited=1`), `[pause-probe]` (`event_size` non-zero exactly while paused, `axis` inside `frozen_x`, `skips` 0 live / 30 paused), `[stats-probe]` (`dn=1` across the pause), `[zoom-probe]` (`gap_px=0.0` at every step) and `[measure-probe]` + the dump's `measures window:` (all-types run `marks_in_window=4/4`, `pin=off` live) on the fast mock. `measure cost:` is unchanged in kind: `raw=37298 (0.0%)` of `covered=121704537`, 1.04 ms a pass -- the runs that read higher are the probe's own zoomed-to-a-slice windows, where the window is legitimately shorter than a bucket.
This commit is contained in:
1 parent
d91957a2fe
commit
0a209becb1
13 files changed
+719
-114
No files matched your search
+65
-31
@@ -1136,22 +1136,38 @@ void on_window_step(GtkButton* button, gpointer user) {
|
||||
void on_pause_toggled(GtkToggleButton* button, gpointer user) {
|
||||
App* app = static_cast<App*>(user);
|
||||
const bool paused = gtk_toggle_button_get_active(button);
|
||||
// Freeze the *view*: snapshot exactly the window on screen, and pin the axes there. The
|
||||
// acquisition thread keeps writing the live ring behind it, so resuming shows the data that
|
||||
// arrived while paused instead of a hole.
|
||||
//
|
||||
// The window and its offset are read **before** the pause is set, and that order is the whole
|
||||
// of what makes a pause taken while an event is *held* work. `stream_at_x0()` follows the
|
||||
// *display*: the moment `paused` is true with no pause view published yet, it answers
|
||||
// `-display_epoch()`, which on a held event is 0 -- so the cut searched the ring at
|
||||
// edge-relative x, found no samples, and froze an *empty* picture (`ev=0` on the probe's own
|
||||
// line, every measurement `--`). What it has to hand the snapshot is the offset the axis is
|
||||
// reading at the instant of the click, which is exactly what it says below.
|
||||
plot::Range view{0.0, 0.0};
|
||||
double s0 = 0.0;
|
||||
bool have_view = false;
|
||||
if (RowSet::Row& r0 = app->rows->row(0); r0.plot != nullptr) {
|
||||
view = r0.plot->x_range(r0.axis_x);
|
||||
have_view = true;
|
||||
}
|
||||
if (have_view) {
|
||||
// The axis is in display x and the ring is on the stream's clock, so the window has to be
|
||||
// put back on that clock to be cut -- `stream_at_x0()` is the one accessor that is right in
|
||||
// every mode, including a pause taken while an event is *held*, where the axis is
|
||||
// edge-relative and the old `- display_epoch()` form searched the ring with event
|
||||
// coordinates. The same offset is handed to the snapshot as its `x_offset`, so its stamps
|
||||
// come back rebased onto the x the axis is already showing and the frozen picture does not
|
||||
// move at the instant of the pause.
|
||||
s0 = app->model->stream_at_x0();
|
||||
}
|
||||
app->acq->set_paused(paused);
|
||||
app->model->set_paused(paused);
|
||||
if (paused) {
|
||||
// Freeze the *view*: snapshot exactly the window on screen, and pin the axes there. The
|
||||
// acquisition thread keeps writing the live ring behind it, so resuming shows the data that
|
||||
// arrived while paused instead of a hole.
|
||||
if (RowSet::Row& r0 = app->rows->row(0); r0.plot != nullptr) {
|
||||
const plot::Range view = r0.plot->x_range(r0.axis_x);
|
||||
// The axis is in display x and the ring is on the stream's clock, so the window has to be
|
||||
// put back on that clock to be cut -- `stream_at_x0()` is the one accessor that is right in
|
||||
// every mode, including a pause taken while an event is *held*, where the axis is
|
||||
// edge-relative and the old `- display_epoch()` form searched the ring with event
|
||||
// coordinates. The same offset is handed to the snapshot as its `x_offset`, so its stamps
|
||||
// come back rebased onto the x the axis is already showing and the frozen picture does not
|
||||
// move at the instant of the pause.
|
||||
const double s0 = app->model->stream_at_x0();
|
||||
if (have_view) {
|
||||
app->model->publish_pause_view(view.min + s0, view.max + s0, s0);
|
||||
app->pause_started_us = g_get_monotonic_time();
|
||||
// Pin on what the snapshot *got*: the cut is clamped to the ring, so it can be narrower than
|
||||
@@ -5815,6 +5831,9 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
|
||||
if (const char* env = g_getenv("UDPSCOPE_PROBE_MEASURE_MS")) {
|
||||
g_timeout_add(static_cast<guint>(std::atoi(env)), measure_probe, &app);
|
||||
}
|
||||
if (const char* env = g_getenv("UDPSCOPE_PROBE_MEASUREPIN_MS")) {
|
||||
g_timeout_add(static_cast<guint>(std::atoi(env)), measurepin_probe, &app);
|
||||
}
|
||||
if (const char* env = g_getenv("UDPSCOPE_PROBE_EVENTZOOM_MS")) {
|
||||
g_timeout_add(static_cast<guint>(std::atoi(env)), eventzoom_probe, &app);
|
||||
}
|
||||
@@ -6474,24 +6493,34 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
|
||||
{
|
||||
std::lock_guard<std::mutex> lock(model.mutex());
|
||||
// `results_for` is the window the numbers describe, `win` the one on screen now: the
|
||||
// drift between them is the assertion that a measurement is a statistic of the *current*
|
||||
// window -- a timer-decoupled recomputation is what used to leave a marker outside it.
|
||||
// Both on the **stream's** clock, which is the space `measure_window` and the time marks
|
||||
// are stored in: a live axis is a fixed `[-window, 0]`, so comparing display x would read
|
||||
// no drift whatever the data did (see `App::measure_window`).
|
||||
// drift between them is the assertion that a measurement is a statistic of the window it
|
||||
// *should* describe -- a timer-decoupled recomputation is what used to leave a marker
|
||||
// outside it. Both on the **stream's** clock, which is the space `measure_window` and
|
||||
// the time marks are stored in: a live axis is a fixed `[-window, 0]`, so comparing display x
|
||||
// would read no drift whatever the data did (see `App::measure_window`).
|
||||
//
|
||||
// `win` comes from the last **tick** (`App::measure_view_window`) and is not read back here:
|
||||
// this runs from the destroy handler, where the exit probe has already switched the trigger
|
||||
// on, and `stream_at_x0()` then answers for a mode no frame was drawn in.
|
||||
// Which window that should be depends on the view, so `drift` is measured against the
|
||||
// **target**: `win` live, and the **pin** (`pin=`, the same stream seconds) while the view
|
||||
// is pinned. On a pinned view the pane is about the frame, and a zoom inside it is allowed
|
||||
// to leave the screen showing a slice of that frame -- reading that as drift would report
|
||||
// the intended behaviour as the defect it was already reported as once.
|
||||
//
|
||||
// `win` and `pin` come from the last **tick** (`App::measure_view_window`,
|
||||
// `App::measure_pin_window`) and are not read back here: this runs from the destroy
|
||||
// handler, where the exit probe has already switched the trigger on, and `stream_at_x0()`
|
||||
// then answers for a mode no frame was drawn in.
|
||||
const plot::Range win = app.measure_view_window;
|
||||
const double span = app.measure_window.span();
|
||||
const plot::Range target = app.measure_pin_valid ? app.measure_pin_window : win;
|
||||
const double span = target.span();
|
||||
int in_window = 0;
|
||||
int marks = 0;
|
||||
// Against `win`, the window **on screen** -- not against `measure_window`, which is the
|
||||
// window the mark was computed over and so contains it by construction. Read that way the
|
||||
// count was a tautology: it printed `2/2` while the pane's numbers described a window 11 s
|
||||
// and a forty-fold zoom away from the one displayed, which is how a held event that never
|
||||
// recomputed under a gesture stayed invisible to every run of this check.
|
||||
// Against `win`, the window **on screen** -- not against `measure_window`/`target`, which
|
||||
// contain every mark by construction: read that way the count was a tautology, and it
|
||||
// printed `2/2` while the pane's numbers described a window 11 s and a forty-fold zoom
|
||||
// away from the one displayed. A *time* mark may legitimately sit outside `win` on a
|
||||
// pinned view that has been zoomed into the pin -- the mark is a position in the window the
|
||||
// result describes, and that window is the frame, not the slice -- so the strict `K/K`
|
||||
// reading belongs to a live view and to a pinned view left at its pin.
|
||||
for (const RowSet::MeasureMark& mk : app.measure_marks) {
|
||||
if (mk.level) continue;
|
||||
++marks;
|
||||
@@ -6510,11 +6539,16 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
|
||||
if (r.plot->cursor(r.measure_v[mi]).selectable) ++selectable;
|
||||
}
|
||||
}
|
||||
std::printf("[udpscope] measures window: win=[%.6f..%.6f] results_for=[%.6f..%.6f] "
|
||||
const std::string pin_text =
|
||||
app.measure_pin_valid ? std::to_string(app.measure_pin_window.min) + ".." +
|
||||
std::to_string(app.measure_pin_window.max)
|
||||
: std::string("off");
|
||||
std::printf("[udpscope] measures window: win=[%.6f..%.6f] pin=[%s] results_for=[%.6f..%.6f] "
|
||||
"drift=%.4f%% marks_in_window=%d/%d marks_selectable=%d\n",
|
||||
win.min, win.max, app.measure_window.min, app.measure_window.max,
|
||||
span > 0.0 ? 100.0 * (std::fabs(win.min - app.measure_window.min) +
|
||||
std::fabs(win.max - app.measure_window.max)) /
|
||||
win.min, win.max, pin_text.c_str(), app.measure_window.min,
|
||||
app.measure_window.max,
|
||||
span > 0.0 ? 100.0 * (std::fabs(target.min - app.measure_window.min) +
|
||||
std::fabs(target.max - app.measure_window.max)) /
|
||||
span
|
||||
: 0.0,
|
||||
in_window, marks, selectable);
|
||||
|
||||
Reference in new issue
Block a user