diff --git a/CLAUDE.md b/CLAUDE.md index cb97c64..df31fde 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -435,20 +435,31 @@ Two lines of the self-check output are assertions in disguise: *not* every channel, and never a stream's timestamp signal, which is what an acquired-but- unplotted list put first and auto-selected), and `drop_h` must be a combo box's height (tens of pixels — a scrolled-window wrapper asked it to grow to 160). -- `[timelabels-probe] relative=D window=.. origin=.. note=[..] ticks=[..] - first_value=.. first_rel=.. range=[..]` + the dump's `time labels: relative=D origin=.. - note=[..] x_origin_set=.. ticks=[..] first_rel=..` (stderr, +- `[timelabels-probe] relative=D distinct=D window=.. origin=.. note=[..] ticks=[..] + first_value=.. first_rel=.. range=[..]` + the dump's `time labels: relative=D distinct=D + origin=.. note=[..] x_origin_set=.. ticks=[..] first_rel=..` (stderr, `UDPSCOPE_PROBE_TIMELABELS_MS`) — the x label form across a zoom, driven through `apply_time_window()`: **under the figure's x link group the group owns the window, so a row-level `set_time_window()` does nothing** (a probe that changes the window must use the - app's path). Absolute at 2 s and at 500 ms, `relative=1` below a second, and the caption - must be non-empty exactly then. The probe *races the paint* — it reads the axis on a timer, + app's path). **`distinct` must be 1 on every line, and that is the assertion** — it says no two + major labels on the axis are the same string, which is the one thing the relative form exists to + buy and the only reading that holds whatever the stream's uptime is. `relative=D` deliberately + is *not* fixed per window: the form is decided by the tick step against the label resolution at + the axis' **magnitude** (see the invariant below), so the same 50 ms window reads absolute at + 2 s of uptime (`2.13s, 2.14s, 2.15s`) and relative at 17 s (`100µs, 10.1ms, 20.1ms`, with the + caption) — both correct, and a probe pinned to one of them would assert the stream's clock + rather than the app. The **caption must be non-empty exactly when `relative=1`**. Run it after + `UDPSCOPE_ARM_TRIGGER_MS` as well, which is the case the span rule got wrong: a triggered 50 ms + window must read `relative=0 note=[] ticks=[-20ms,-10ms,0,10ms]` — a `t0` caption on an axis + that is already edge-relative re-anchors the ticks away from the trigger, which is the one + position the axis exists to show. The probe *races the paint* — it reads the axis on a timer, and the ticks it sees are the previous layout's — so its `first_rel` (the first major minus the axis' origin) is a fraction of a step; the dump runs after the last paint and is the race-free reading. Either way the assertion that matters is the *sum*: `first_value` must 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. + the absolute form. Note the last two steps both report `window=0.05`: `RowSet::set_time_window()` + floors the window there, so the 20 ms step is clamped. - `[fit-probe] pinned=D 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 the notice and the control are the handler's half. A fit is the **undo @@ -1406,6 +1417,26 @@ far the largest file. reasons (an event pin) call `sync_window_display()` alone, which only restates the control and never moves the plot — calling `apply_time_window()` there would clamp a legitimately short capture up to the requested window. +- **The x labels go relative on the tick *step against the label resolution at the axis' + magnitude*, never on the span.** `RowSet::update_time_labels()`'s question is only ever "can + this step be written at this magnitude": `TickFormat::Time` carries three significant figures + below a minute, **whole seconds** through `m:ss`/`h:mm:ss` up to a day, and three figures of + days above — so the resolution is a step function of the magnitude, which `time_label_resolution()` + states and nothing else may re-derive. The old test was `span < 1.0` and it was wrong in both + directions: a 2 s window at twenty minutes of uptime reads `20:00` on every tick and stayed + absolute, and a **triggered** sub-second window — whose axis is already `[-25ms, +25ms]` about + the edge — went relative and so re-anchored its ticks *away from the trigger*, with a `t0` + caption that had nothing to explain. Three details hold it up. The step is read back from the + majors the **last layout** produced, which is not a feedback loop (a tick's value depends on the + range and the axis' pixel width, never on the label form) but *is* absent before the first + layout — and with no ticks the form must be **left alone**, because a step guessed from the span + would be latched by the hysteresis below. The hysteresis is a 1.05 band on *leaving* the relative + form. And *entering* it needs a ULP of slack (`1 - 1e-9`): a step differenced from two tick + values and a resolution from `pow(10, n)` differ in their last bits when they are the same + number, and `step == res` is a readable axis (`1.73s`, `1.74s`, `1.75s` — one in the last digit + is exactly enough), so the bare comparison picked a different form from one frame to the next. + The guard is `[timelabels-probe]`'s `distinct=1`, which asserts what the form is *for* rather + than which form a given window gets. - **`fit_x()` restores the *whole of what the view may show*, which is not always `window_s_`.** Live, that is the newest `window_s_` at the live edge (`set_time_window(window_s_)`, the same path a resume takes). Pinned — a held event, a filling capture or a paused view — it is the diff --git a/README.md b/README.md index 669071b..8d34333 100644 --- a/README.md +++ b/README.md @@ -74,13 +74,28 @@ so the times on the axis do not change when the pause begins. Resuming drops the re-frames on the newest data, so the seconds that passed while paused are *there*, not a hole — which is what pausing used to leave, because it stopped the producer outright. -**Sub-second windows read relative.** The x ticks are absolute until the window's span goes -under a second (with a band to 1.05 s, so the form cannot flicker while a zoom is nudged), and -relative below it: the ticks are then generated on a grid anchored at the window's left edge — -so they read `0`, `5ms`, `10ms` instead of `12.345671s`, `12.345672s`, which three significant -digits cannot even tell apart — and a caption names that anchor in the axes' top-left corner: -`t0 = 12.3456 s`. The labels scale through the prefixed units the SI formatter uses — `500ms`, -`500us`, `5us`, `5ns` — so a microsecond window reads in microseconds instead of `0.005ms`. Adding the two is the tick's absolute time. The caption is an annotation +**The x ticks read relative when, and only when, they could not be written absolutely.** A +`TickFormat::Time` label carries three significant digits, so the test is the tick **step** +against the label's own **resolution at the axis' magnitude**: `12.345671s` and `12.345672s` are +the same string, and there the ticks are re-anchored on a grid at the window's left edge — so +they read `0`, `5ms`, `10ms` — with a caption naming that anchor, `t0 = 12.3456 s`. Adding the +two is the tick's absolute time. The labels scale through the prefixed units the SI formatter +uses — `500ms`, `500us`, `5us`, `5ns` — so a microsecond window reads in microseconds instead of +`0.005ms`. + +The rule used to be the *span* alone (relative under a second, with a band to 1.05 s), and a +span says nothing about whether a label can be written: it was wrong in **both** directions. A +2 s window at twenty minutes of uptime is labelled `20:00` on every tick — `m:ss` carries whole +seconds — and was left absolute and unreadable. And a **triggered** sub-second window, whose +axis is already `[-25ms, +25ms]` about the edge, reads `-20ms`, `-10ms`, `0`, `10ms` perfectly +well, yet was re-anchored anyway: the one position a triggered axis exists to show was displaced +by a `t0` caption that had nothing to explain. The step is read back from the majors the last +layout produced (a function of the range and the axis width, not of the label form, so no +feedback loop), the magnitude is the larger of the two endpoints, and the comparison carries a +1.05 band leaving the relative form plus a ULP of slack entering it — a step *equal* to the +resolution is readable (`1.73s`, `1.74s`, `1.75s`: one in the last digit is exactly enough), so a +decision taken on the last bits of two differently computed doubles picked a different form from +one frame to the next. The caption is an annotation inside the axes rather than the axis title on purpose: its text changes with the window, and a title of a changing width would relayout the plot every frame. Everything a user copies out — the cursors' `T0`/`T1`/`dT`, a measurement, an export — stays in absolute stream seconds. diff --git a/src/plot_host.cpp b/src/plot_host.cpp index ae48429..0a481c9 100644 --- a/src/plot_host.cpp +++ b/src/plot_host.cpp @@ -34,6 +34,21 @@ constexpr plot::u32 kMeasureColor = 0x9BE8A0u; constexpr plot::u32 kTimeNoteColor = 0xC8C8C8u; constexpr plot::u32 kTriggerColor = 0xFF5B5Bu; +/// The smallest difference a `TickFormat::Time` label can *write* at the magnitude `t` -- what +/// the last digit of a label at that magnitude is worth. `format_time` writes three significant +/// figures below a minute (of the SI-prefixed mantissa under a second, of the seconds above it), +/// whole seconds through `m:ss`/`h:mm:ss` up to a day, and three figures of days above. So the +/// resolution is a step function of the *magnitude* and not of the span -- which is the whole +/// reason a relative axis is ever needed, and the reason a span threshold alone gets it wrong in +/// both directions. +double time_label_resolution(double t) { + t = std::fabs(t); + if (!std::isfinite(t) || !(t > 0.0)) return 0.0; + if (t >= 86400.0) return 86400.0 * std::pow(10.0, std::floor(std::log10(t / 86400.0)) - 2.0); + if (t >= 60.0) return 1.0; + return std::pow(10.0, std::floor(std::log10(t)) - 2.0); +} + constexpr double kParked = -1.0e12; ///< hides an instrument without removing it /// The measurement marks' stroke: thinner than the chrome that frames the plot (a trace is 1.5, /// the axis lines and the trigger marker `Style::line_width_px`), and broken, so a readout over @@ -509,12 +524,48 @@ bool RowSet::update_time_labels() { const double span = view.span(); if (!(span > 0.0) || !std::isfinite(span)) return false; - // Sub-second windows read relative: `TickFormat::Time` writes three significant digits, so - // `12.345671s` and `12.345672s` are the same string and a 1 ms step around a multi-second - // time cannot be labelled at all. The band is the switch's hysteresis -- entering below a - // second, leaving only above 1.05 -- so a span sitting on the boundary cannot flicker - // between two forms while a zoom is nudged. - const bool want_relative = relative_x_ ? span < 1.05 : span < 1.0; + // A relative axis is the answer to one problem only: a tick step the label form cannot write. + // `TickFormat::Time` carries three significant digits, so a 1 ms step around a 12 s stamp + // writes `12.3s` on every tick -- there the ticks have to be re-anchored and the anchor named. + // The test is therefore the *step against the resolution at the axis' own magnitude*, never the + // span: the two part company in both directions, and both cases are real here. A 2 s window at + // twenty minutes of uptime is labelled `20:00` throughout and needs the anchor although its + // span is wide; a 50 ms trigger window spanning [-25ms, +25ms] is labelled `-20ms .. +20ms` + // perfectly well and needs nothing, although its span is narrow. That second case is what made + // a triggered sub-second view carry a `t0` caption *and* re-anchor its ticks away from the + // edge, which is the one position a triggered axis exists to show. + // The 1.05 band is the switch's hysteresis, so a step sitting on the boundary cannot flicker + // between two forms while a zoom is nudged, and the `1 - 1e-9` is a slack the *entry* needs for + // its own reason: a step read back from two tick values and a resolution from `pow(10, n)` + // differ in their last bits even when they are the same number, and `step == res` is a readable + // axis (`1.73s, 1.74s, 1.75s` -- one in the last digit is exactly enough), so a decision taken + // on those bits picked a different form from one frame to the next. + const double mag = std::max(std::fabs(view.min), std::fabs(view.max)); + const double res = time_label_resolution(mag); + // The step the labels are actually written at: the majors the *last* layout produced. They are + // a function of the range and the axis' pixel width, not of the label form, so reading them + // back here is no feedback loop. + double step = 0.0; + { + const plot::Axis& ax = first.plot->x_axis(first.axis_x); + double prev = 0.0; + bool have = false; + for (const plot::Tick& t : ax.ticks) { + if (!t.major) continue; + if (have) { + step = t.value - prev; + break; + } + prev = t.value; + have = true; + } + } + // No ticks yet -- the frames before the first layout. The form is a property of the labels, so + // with no labels there is nothing to decide, and keeping the current one is right where + // guessing a step from the span is not: a guess that entered the relative form would be + // *latched* there by the hysteresis above. + const bool want_relative = + step > 0.0 ? step < res * (relative_x_ ? 1.05 : 1.0 - 1.0e-9) : relative_x_; const bool changed = want_relative != relative_x_; relative_x_ = want_relative; diff --git a/src/ui.cpp b/src/ui.cpp index 6f10f6c..e83329a 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -7246,13 +7246,23 @@ gboolean timelabels_probe_step(gpointer user) { const RowSet::Row& last = a->rows->row(a->rows->row_count() - 1); std::string ticks; std::string note; + // Whether *every* major label on the axis is a different string. That, and not a span + // threshold, is what the relative form exists to buy: two adjacent ticks reading `12.3s` are + // not labels. It is therefore the one assertion that holds whatever the stream's uptime is -- + // which decides where the switch actually falls, since the label resolution is a function of + // the axis' magnitude. + bool distinct = true; if (last.plot != nullptr) { int n = 0; + std::vector seen; for (const plot::Tick& t : last.plot->x_axis(last.axis_x).ticks) { if (!t.major) continue; - if (n++ != 0) ticks += ","; - ticks += t.label; - if (n >= 4) break; + if (std::find(seen.begin(), seen.end(), t.label) != seen.end()) distinct = false; + seen.emplace_back(t.label); + if (n++ < 4) { + if (n != 1) ticks += ","; + ticks += t.label; + } } if (last.time_origin_note < last.plot->annotation_count()) { const plot::Annotation& an = last.plot->annotation(last.time_origin_note); @@ -7271,9 +7281,10 @@ gboolean timelabels_probe_step(gpointer user) { shown = last.plot->x_range(last.axis_x); } std::fprintf(stderr, - "[timelabels-probe] %-12s relative=%d window=%.6g origin=%.6f note=[%s] " - "ticks=[%s] first_value=%.6f first_rel=%.6f range=[%.6f..%.6f]\n", - what, a->rows->time_labels_relative() ? 1 : 0, a->rows->time_window(), + "[timelabels-probe] %-12s relative=%d distinct=%d window=%.6g origin=%.6f " + "note=[%s] ticks=[%s] first_value=%.6f first_rel=%.6f range=[%.6f..%.6f]\n", + what, a->rows->time_labels_relative() ? 1 : 0, distinct ? 1 : 0, + a->rows->time_window(), a->rows->time_labels_origin(), note.c_str(), ticks.c_str(), first_value, first_value - (last.plot != nullptr ? last.plot->x_axis_origin(last.axis_x) : 0.0), shown.min, shown.max); @@ -8317,13 +8328,18 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit // The labels themselves, from the axis the reader looks at: the assertion is what the // ticks *say*, not only which mode is on. std::string ticks; + bool distinct = true; if (rlt.plot != nullptr) { int shown = 0; + std::vector seen; for (const plot::Tick& t : rlt.plot->x_axis(rlt.axis_x).ticks) { if (!t.major) continue; - if (shown++ != 0) ticks += ","; - ticks += t.label; - if (shown >= 4) break; + if (std::find(seen.begin(), seen.end(), t.label) != seen.end()) distinct = false; + seen.emplace_back(t.label); + if (shown++ < 4) { + if (shown != 1) ticks += ","; + ticks += t.label; + } } } // The labels against the anchor they were built with: the dump runs after the last @@ -8340,10 +8356,10 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit } } } - std::printf("[udpscope] time labels: relative=%d origin=%.6f note=[%s] x_origin_set=%.6f " - "ticks=[%s] first_rel=%.9f\n", - rows.time_labels_relative() ? 1 : 0, rows.time_labels_origin(), note.c_str(), - origin_set, ticks.c_str(), first_value - origin_set); + std::printf("[udpscope] time labels: relative=%d distinct=%d origin=%.6f note=[%s] " + "x_origin_set=%.6f ticks=[%s] first_rel=%.9f\n", + rows.time_labels_relative() ? 1 : 0, distinct ? 1 : 0, rows.time_labels_origin(), + note.c_str(), origin_set, ticks.c_str(), first_value - origin_set); } { std::lock_guard lock(model.mutex());