fix: a zoomed trace is drawn on the x its readout quotes
gather() staged x as f32 relative to the series' own x0() -- the start of the whole retained window -- so the quantisation was a fraction of the ring rather than of the view: 1.9 us at 20 s of retained data, which at a 10 us view across a 716 px axis is over 100 px between where a sample is drawn and the x a readout quotes for it (crosshair_point() reads x_at() in exact f64). It halved on every zoom out, which is what made it read as "zooming in makes the crosshair select a point beside the data". The origin is now the caller's (plotter c2c8c59): the renderer passes the visible range's minimum, so the error is a fixed 2^-24 of what is on screen -- about 4e-5 px over a 700 px axis -- at any zoom. The parameter sits before out_xy so every call site is a compile error rather than a silent change of behaviour. The renderer no longer reads series.x0() at paint time at all, which removes two of the three symptoms of the torn window_x0_ read by construction; the ink probe's now-vacuous kept_fixed_x0 walk goes with them, replaced by a drawn-vs-quoted pixel figure. neighbours() still tests the visible range against x_at(0), so the cache stays load-bearing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
1 parent
39f30d913f
commit
2f4530c128
7 files changed
+176
-90
No files matched your search
+5
-5
@@ -189,7 +189,7 @@ void ChannelBuffer::raw(u64 begin, u64 end, f32* out) const {
|
||||
for (u64 i = count; i < end - begin; ++i) out[i] = 0.0f;
|
||||
}
|
||||
|
||||
void ChannelBuffer::gather(u32 level, u64 begin, u64 end, std::vector<f32>& out_xy,
|
||||
void ChannelBuffer::gather(u32 level, u64 begin, u64 end, f64 x_origin, std::vector<f32>& out_xy,
|
||||
plot::data::GatherMode mode) const {
|
||||
const u64 n = pyramid_.size();
|
||||
const u64 b = std::min(begin, n);
|
||||
@@ -197,7 +197,7 @@ void ChannelBuffer::gather(u32 level, u64 begin, u64 end, std::vector<f32>& out_
|
||||
if (b >= e) return;
|
||||
|
||||
const u64 origin = pyramid_.window_base();
|
||||
const f64 base = x0(); // x is emitted relative to this (see the header)
|
||||
const f64 base = x_origin; // x is emitted relative to this (see the header)
|
||||
|
||||
const auto emit_raw = [&](u64 from, u64 to) {
|
||||
for (u64 i = from; i < to; ++i) {
|
||||
@@ -302,13 +302,13 @@ u64 SlotSeries::snap_index(f64 x) const {
|
||||
return buffer_ != nullptr ? buffer_->snap_index(x) : 0;
|
||||
}
|
||||
|
||||
void SlotSeries::gather(u32 level, u64 begin, u64 end, std::vector<f32>& out_xy,
|
||||
void SlotSeries::gather(u32 level, u64 begin, u64 end, f64 x_origin, std::vector<f32>& out_xy,
|
||||
plot::data::GatherMode mode) const {
|
||||
if (event_ != nullptr) {
|
||||
event_->gather(level, begin, end, out_xy, mode);
|
||||
event_->gather(level, begin, end, x_origin, out_xy, mode);
|
||||
return;
|
||||
}
|
||||
if (buffer_ != nullptr) buffer_->gather(level, begin, end, out_xy, mode);
|
||||
if (buffer_ != nullptr) buffer_->gather(level, begin, end, x_origin, out_xy, mode);
|
||||
}
|
||||
|
||||
void SlotSeries::copy_raw(u64 begin, u64 end, f32* out) const {
|
||||
|
||||
+9
-6
@@ -80,9 +80,11 @@ class ChannelBuffer final : public plot::data::Series {
|
||||
plot::Range x_bounds() const override;
|
||||
plot::Range y_bounds() const override;
|
||||
const plot::data::MinMaxPyramid& pyramid() const override { return pyramid_; }
|
||||
/// x is emitted **relative to `x0()`** (the library's contract): staging an
|
||||
/// absolute epoch time as f32 would quantise it far coarser than a pixel.
|
||||
void gather(u32 level, u64 begin, u64 end, std::vector<f32>& out_xy,
|
||||
/// x is emitted **relative to `x_origin`** (the library's contract): staging an
|
||||
/// absolute epoch time as f32 would quantise it far coarser than a pixel, and
|
||||
/// staging it relative to the whole *retained window* quantises it coarser than a
|
||||
/// pixel of a zoomed view -- see `plot::data::Series::gather()`.
|
||||
void gather(u32 level, u64 begin, u64 end, f64 x_origin, std::vector<f32>& out_xy,
|
||||
plot::data::GatherMode mode = plot::data::GatherMode::Envelope) const override;
|
||||
|
||||
/// `copy_raw`, published so the retargetable trace series can forward the
|
||||
@@ -109,8 +111,9 @@ class ChannelBuffer final : public plot::data::Series {
|
||||
/// slot cannot be overwritten under the read. Everything else on this object is read at
|
||||
/// *paint* time with no lock, and this one slot is the ring's oldest: the one the producer is
|
||||
/// about to overwrite. Reading it there returned a stamp a whole ring-span too new whenever
|
||||
/// the producer lapped it between the bounds check and the copy -- and `x0()` is the origin
|
||||
/// the renderer reconstructs every trace point from (`gather[i] + series.x0()`), so one such
|
||||
/// the producer lapped it between the bounds check and the copy -- and `x0()` is what
|
||||
/// `neighbours()` tests the visible range against, as well as the origin the renderer
|
||||
/// reconstructed every trace point from before the caller named one, so one such
|
||||
/// read moved the entire polyline a ring-span off its axis (an invisible trace) or, caught
|
||||
/// inside the renderer's increasing-suffix filter, made it drop most of the points. That is
|
||||
/// the "live plot flickers as if there were no data" defect. The cached value is also the
|
||||
@@ -163,7 +166,7 @@ class SlotSeries final : public plot::data::Series {
|
||||
plot::Range x_bounds() const override;
|
||||
plot::Range y_bounds() const override;
|
||||
const plot::data::MinMaxPyramid& pyramid() const override;
|
||||
void gather(u32 level, u64 begin, u64 end, std::vector<f32>& out_xy,
|
||||
void gather(u32 level, u64 begin, u64 end, f64 x_origin, std::vector<f32>& out_xy,
|
||||
plot::data::GatherMode mode = plot::data::GatherMode::Envelope) const override;
|
||||
bool streaming() const noexcept override { return event_ == nullptr && buffer_ != nullptr; }
|
||||
|
||||
|
||||
+12
-12
@@ -108,7 +108,7 @@ class EventSeries final : public plot::data::Series {
|
||||
/// Indices need no rebasing: the pyramid is built from sample 0 and nothing is ever
|
||||
/// dropped from it, so `window_base()` is 0 and a window index *is* the absolute one
|
||||
/// (unlike the ring, whose window slides).
|
||||
void gather(u32 level, u64 begin, u64 end, std::vector<f32>& out_xy,
|
||||
void gather(u32 level, u64 begin, u64 end, f64 x_origin, std::vector<f32>& out_xy,
|
||||
plot::data::GatherMode mode =
|
||||
plot::data::GatherMode::Envelope) const override {
|
||||
if (channel_ >= snap_->values.size()) return;
|
||||
@@ -118,17 +118,17 @@ class EventSeries final : public plot::data::Series {
|
||||
const u64 b = std::min(begin, n);
|
||||
const u64 e = std::min(std::max(end, b), n);
|
||||
if (b >= e) return;
|
||||
// x **relative to x0()**, which is the library's gather contract: the renderer
|
||||
// adds `x0()` back before mapping to the screen. Emitting the absolute stamp made
|
||||
// it add x0 twice, so the whole capture was drawn one `x0()` -- a full
|
||||
// pre-trigger window -- to the left of the axis it had just been measured
|
||||
// against. The event's own stamps are small (edge-relative, so [-pre, +post]),
|
||||
// which is why that shift looked like a plausible picture: with a 50/50 split the
|
||||
// right half of the plot was simply empty, zooming out showed the trace ending at
|
||||
// the x = 0 marker, and zooming in past the shift showed nothing at all while the
|
||||
// crosshair -- which reads `x_at()`, never `gather()` -- went on reporting the
|
||||
// right values.
|
||||
const f64 base = x0();
|
||||
// x **relative to `x_origin`**, which is the library's gather contract: the
|
||||
// renderer adds that same origin back before mapping to the screen. Emitting the
|
||||
// absolute stamp made it add an origin twice, so the whole capture was drawn one
|
||||
// `x0()` -- a full pre-trigger window -- to the left of the axis it had just been
|
||||
// measured against. The event's own stamps are small (edge-relative, so
|
||||
// [-pre, +post]), which is why that shift looked like a plausible picture: with a
|
||||
// 50/50 split the right half of the plot was simply empty, zooming out showed the
|
||||
// trace ending at the x = 0 marker, and zooming in past the shift showed nothing
|
||||
// at all while the crosshair -- which reads `x_at()`, never `gather()` -- went on
|
||||
// reporting the right values.
|
||||
const f64 base = x_origin;
|
||||
const auto rel = [&](u64 i) {
|
||||
return static_cast<f32>(stamps[static_cast<std::size_t>(i)] - base);
|
||||
};
|
||||
|
||||
+68
-45
@@ -418,6 +418,14 @@ struct App {
|
||||
int dbg_ink_staircase = 0; ///< the filter dropped over three quarters of the points
|
||||
int dbg_ink_short = 0; ///< the ink reached under a quarter of the axis' width
|
||||
double dbg_ink_worst_cover = 1.0;
|
||||
/// The worst distance, in pixels, between where a sample is *drawn* and the x a readout
|
||||
/// quotes for the same sample. `gather()` stages x as an f32 relative to an origin the
|
||||
/// renderer names, while `Plot::crosshair_point()` reads `x_at()` in exact f64, so this is
|
||||
/// the quantisation of that f32 expressed in the unit the user sees it in. Measured on the
|
||||
/// raw levels only, where a gathered point *is* a sample; above them a point is a bucket's
|
||||
/// envelope and has no single sample to be off from.
|
||||
double dbg_ink_worst_offset_px = 0.0;
|
||||
int dbg_ink_raw_checked = 0; ///< frames the figure above was measured over
|
||||
int dbg_ink_reports = 0;
|
||||
/// Frames the badge spent in each `BadgePhase`. Both `lit` and `dim` have to be non-zero
|
||||
/// over a run that acquired anything -- that is what "it blinks" means, and a badge
|
||||
@@ -1093,21 +1101,19 @@ void check_frame_ink(App& app) {
|
||||
const plot::u32 level = spp >= 64.0 ? s->pyramid().level_for(n, spp) : 0u;
|
||||
std::vector<plot::f32>& xy = app.measure_ink_scratch;
|
||||
xy.clear();
|
||||
s->gather(level, first, last, xy);
|
||||
// The same origin the renderer names (`x_range.min`), so the reconstruction below is its
|
||||
// reconstruction: see `plot::data::Series::gather()` for why the caller owns it.
|
||||
s->gather(level, first, last, axis.min, xy);
|
||||
const std::size_t pts = xy.size() / 2;
|
||||
|
||||
std::size_t kept = 0;
|
||||
double lo = 0.0;
|
||||
double hi = 0.0;
|
||||
// Where the filter's drops fall, and how far back each one steps. The renderer reconstructs
|
||||
// every point's absolute x as `gather[i] + series.x0()` **inside** this loop, so an `x0()`
|
||||
// that is not constant for the loop's duration shifts the comparison under itself -- and on a
|
||||
// ring-backed series `x0()` is `time_of(window_base)`, which *clamps to the oldest readable
|
||||
// slot* and therefore advances with the producer. The two readings that separate that from a
|
||||
// genuinely out-of-order gather are the drops' *positions* (spread through the points, or
|
||||
// bunched at the front where the clamp lives) and `back` (one sample's spacing, or a packet's
|
||||
// worth). So the loop is run twice: once the renderer's way, and once with `x0()` read a
|
||||
// single time.
|
||||
// Where the filter's drops fall, and how far back each one steps. The origin is a constant
|
||||
// for the walk's duration *by construction* now -- the renderer reconstructs
|
||||
// `gather[i] + x_range.min`, a local of its own, where it used to re-read `series.x0()` on
|
||||
// every point and a ring-backed `x0()` advances with the producer. So a drop here is a
|
||||
// genuinely out-of-order gather, and `back` says by how much.
|
||||
std::size_t drop_first = 0;
|
||||
std::size_t drop_last = 0;
|
||||
double worst_back = 0.0;
|
||||
@@ -1115,7 +1121,7 @@ void check_frame_ink(App& app) {
|
||||
{
|
||||
double next = std::numeric_limits<double>::infinity();
|
||||
for (std::size_t k = pts; k > 0; --k) {
|
||||
const double x = double(xy[2 * (k - 1)]) + s->x0();
|
||||
const double x = double(xy[2 * (k - 1)]) + axis.min;
|
||||
if (!(x < next)) {
|
||||
worst_back = std::max(worst_back, x - next);
|
||||
if (!any_drop) drop_last = k - 1;
|
||||
@@ -1129,18 +1135,24 @@ void check_frame_ink(App& app) {
|
||||
++kept;
|
||||
}
|
||||
}
|
||||
std::size_t kept_fixed = 0;
|
||||
{
|
||||
const double base = s->x0(); // one read, held for the whole walk
|
||||
double next = std::numeric_limits<double>::infinity();
|
||||
for (std::size_t k = pts; k > 0; --k) {
|
||||
const double x = double(xy[2 * (k - 1)]) + base;
|
||||
if (!(x < next)) continue;
|
||||
next = x;
|
||||
++kept_fixed;
|
||||
}
|
||||
}
|
||||
++app.dbg_ink_checked;
|
||||
// Drawn against quoted, in pixels. At level 0 a gathered pair *is* sample `first + k`, so
|
||||
// this is the one place the two numbers the user compares by eye -- the polyline's position
|
||||
// and the crosshair's readout -- can be differenced. The origin is what decides it: the f32
|
||||
// resolves 2^-24 of whatever span it is relative to, so a view narrower than that fraction
|
||||
// of the *retained window* drew a sample visibly away from the x quoted for it, which is
|
||||
// what "when zooming there is a visible offset between the real data point and the crosshair
|
||||
// selected one" was.
|
||||
if (level == 0 && pts > 0) {
|
||||
const double px_per_x = axes_w / axis.span();
|
||||
double worst = 0.0;
|
||||
for (std::size_t k = 0; k < pts; ++k) {
|
||||
const double drawn = axis.min + double(xy[2 * k]);
|
||||
worst = std::max(worst, std::abs(drawn - s->x_at(first + k)) * px_per_x);
|
||||
}
|
||||
app.dbg_ink_worst_offset_px = std::max(app.dbg_ink_worst_offset_px, worst);
|
||||
++app.dbg_ink_raw_checked;
|
||||
}
|
||||
const double cover = kept >= 2 ? (hi - lo) / axis.span() : 0.0;
|
||||
app.dbg_ink_worst_cover = std::min(app.dbg_ink_worst_cover, cover);
|
||||
const bool collapse = kept < 2;
|
||||
@@ -1155,22 +1167,23 @@ void check_frame_ink(App& app) {
|
||||
if ((collapse || staircase || short_ink) && app.dbg_ink_reports < 40) {
|
||||
++app.dbg_ink_reports;
|
||||
// The series' own answers beside the reconstruction, because the two disagreeing is itself
|
||||
// a reading: `rel` is what `gather()` emitted (relative to the `x0()` *it* read) and `x0`
|
||||
// is the `x0()` read here one call later, so a trace whose ink sits a whole ring-span off
|
||||
// its axis separates into "the stamps moved under us" and "the window is the wrong one".
|
||||
// a reading: `rel` is what `gather()` emitted (relative to `axis.min`, the origin it was
|
||||
// given) while `x0`/`x_at`/`bounds` are the window the series says it holds, so a trace
|
||||
// whose ink sits a whole ring-span off its axis separates into "the stamps moved under us"
|
||||
// and "the window is the wrong one".
|
||||
const plot::Range xb = s->x_bounds();
|
||||
std::fprintf(stderr,
|
||||
"[ink] paint %d %s%s%s mode=%s axis=[%.6f..%.6f] n=%llu idx=[%llu..%llu) "
|
||||
"spp=%.1f level=%u pts=%zu kept=%zu ink=[%.6f..%.6f] cover=%.1f%% "
|
||||
"x0=%.6f x_at=[%.6f..%.6f] bounds=[%.6f..%.6f] rel=[%.6f..%.6f] dx=%.3g "
|
||||
"kept_fixed_x0=%zu drops=[%zu..%zu] back=%.3g\n",
|
||||
"drops=[%zu..%zu] back=%.3g\n",
|
||||
app.dbg_paint_n, collapse ? "collapse " : "", staircase ? "staircase " : "",
|
||||
short_ink ? "short " : "", display_mode_name(app.dbg_display), axis.min, axis.max,
|
||||
(unsigned long long)n, (unsigned long long)first, (unsigned long long)last, spp,
|
||||
level, pts, kept, lo, hi, cover * 100.0, s->x0(), s->x_at(0), s->x_at(n - 1),
|
||||
xb.min, xb.max, pts > 0 ? double(xy[0]) : 0.0,
|
||||
pts > 0 ? double(xy[2 * (pts - 1)]) : 0.0, s->dx(), kept_fixed, drop_first,
|
||||
drop_last, worst_back);
|
||||
pts > 0 ? double(xy[2 * (pts - 1)]) : 0.0, s->dx(), drop_first, drop_last,
|
||||
worst_back);
|
||||
std::fflush(stderr);
|
||||
}
|
||||
}
|
||||
@@ -4681,24 +4694,29 @@ void report_event_zoom(App& a, const char* what) {
|
||||
const double spp = double(last - first) / axes_w;
|
||||
const plot::u32 level = spp >= 64.0 ? s->pyramid().level_for(n, spp) : 0u;
|
||||
std::vector<plot::f32> xy;
|
||||
s->gather(level, first, last, xy);
|
||||
// `axis.min` is the origin the renderer names, so every reconstruction below is its own --
|
||||
// and at the deep end of this sweep that choice is the whole point: relative to the retained
|
||||
// window instead, the f32's ULP would be a large fraction of these spans.
|
||||
s->gather(level, first, last, axis.min, xy);
|
||||
double ink_lo = 0.0, ink_hi = 0.0;
|
||||
if (xy.size() >= 2) {
|
||||
ink_lo = s->x0() + double(xy[0]);
|
||||
ink_hi = s->x0() + double(xy[xy.size() - 2]);
|
||||
ink_lo = axis.min + double(xy[0]);
|
||||
ink_hi = axis.min + double(xy[xy.size() - 2]);
|
||||
}
|
||||
// The renderer's own increasing-suffix filter, counted rather than applied: it drops every
|
||||
// point that is not strictly older than the one kept after it, which for a *run* of equal x
|
||||
// keeps the last of the run. The stamps here are f32, so once the sample spacing falls below
|
||||
// the f32 step at the stream's magnitude the gathered x has such runs -- and `kept` is how
|
||||
// many distinct positions the polyline is left with. A `kept` far below `pts` is a staircase,
|
||||
// and a `kept` of a handful is a trace with nothing to draw.
|
||||
// the f32 step at the *gather origin's* magnitude the gathered x has such runs -- and `kept`
|
||||
// is how many distinct positions the polyline is left with. A `kept` far below `pts` is a
|
||||
// staircase, and a `kept` of a handful is a trace with nothing to draw. This is the other
|
||||
// reading the origin change buys: the step is now 2^-24 of the span on screen, so a sweep
|
||||
// that used to grind `kept` down as it zoomed keeps every point.
|
||||
std::size_t kept = 0;
|
||||
double worst_back = 0.0; // the largest backwards step, in data units: non-monotonic gather
|
||||
{
|
||||
double next = std::numeric_limits<double>::infinity();
|
||||
for (std::size_t k = xy.size() / 2; k > 0; --k) {
|
||||
const double x = double(xy[2 * (k - 1)]) + s->x0();
|
||||
const double x = double(xy[2 * (k - 1)]) + axis.min;
|
||||
if (!(x < next)) {
|
||||
worst_back = std::max(worst_back, x - next);
|
||||
continue;
|
||||
@@ -6247,23 +6265,27 @@ gboolean trig_trace_probe(gpointer user) {
|
||||
if (found) break;
|
||||
}
|
||||
|
||||
// Where that slot's samples are *drawn*, reconstructed exactly as the renderer
|
||||
// does it: `Series::gather()` emits x relative to `x0()`, and the renderer adds
|
||||
// `x0()` back before mapping to the screen. So `x0() + gathered` must equal
|
||||
// `x_at()`, and `drawn=` must equal `data=` to the f32's precision.
|
||||
// Where that slot's samples are *drawn*, reconstructed the way the renderer does it:
|
||||
// `Series::gather()` emits x relative to an origin **the caller names**, and the
|
||||
// consumer adds that same origin back before mapping to the screen. So
|
||||
// `origin + gathered` must equal `x_at()`, and `drawn=` must equal `data=` to the
|
||||
// f32's precision. The origin named here is `x0()`, deliberately -- the renderer
|
||||
// passes the visible range's minimum, but this line reads the series' *whole* span,
|
||||
// which is the widest relative value a gather can be asked to stage and so the worst
|
||||
// case for the f32.
|
||||
//
|
||||
// This is the one defect in this area the picture alone shows and no other probe
|
||||
// line does: a series that emits absolute x instead makes the renderer add `x0()`
|
||||
// twice, so the trace lands a whole `x0()` away from the axis it was measured
|
||||
// line does: a series that emits absolute x instead makes the consumer add the origin
|
||||
// twice, so the trace lands a whole origin away from the axis it was measured
|
||||
// against -- off screen entirely once the view is narrower than that shift, while
|
||||
// the crosshair (which reads `x_at()`) goes on reporting correct values.
|
||||
double drawn_lo = 0.0, drawn_hi = 0.0, data_lo = 0.0, data_hi = 0.0;
|
||||
if (shown != nullptr && shown->size() > 0) {
|
||||
const u64 n = shown->size();
|
||||
std::vector<f32> xy;
|
||||
shown->gather(0u, 0u, 1u, xy);
|
||||
shown->gather(0u, 0u, 1u, shown->x0(), xy);
|
||||
const std::size_t head = xy.size();
|
||||
shown->gather(0u, n - 1, n, xy);
|
||||
shown->gather(0u, n - 1, n, shown->x0(), xy);
|
||||
if (head >= 2 && xy.size() >= head + 2) {
|
||||
drawn_lo = shown->x0() + double(xy[0]);
|
||||
drawn_hi = shown->x0() + double(xy[head]);
|
||||
@@ -8065,9 +8087,10 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
|
||||
app.dbg_notrace_frames, app.dbg_notrace_after_bound, app.dbg_notrace_last);
|
||||
std::printf("[udpscope] worst side/pane min width: %d px\n", app.dbg_min_w_worst);
|
||||
std::printf("[udpscope] trace ink: %d paints checked, %d collapsed, %d staircase, %d short, "
|
||||
"worst cover %.1f%%\n",
|
||||
"worst cover %.1f%%, worst drawn-vs-quoted %.4f px over %d raw paints\n",
|
||||
app.dbg_ink_checked, app.dbg_ink_collapse, app.dbg_ink_staircase,
|
||||
app.dbg_ink_short, app.dbg_ink_worst_cover * 100.0);
|
||||
app.dbg_ink_short, app.dbg_ink_worst_cover * 100.0, app.dbg_ink_worst_offset_px,
|
||||
app.dbg_ink_raw_checked);
|
||||
std::printf("[udpscope] blank view frames: %d of %d\n", app.dbg_blank_frames,
|
||||
app.dbg_axes_frames);
|
||||
std::printf("[udpscope] trigger events: %llu total, %llu at the disarm (no event after it "
|
||||
|
||||
Reference in new issue
Block a user