fix: a recompute that renumbered its ring, and readouts frozen under a zoom

Three defects, all of them a window describing data that is not there.

A math edit refilled the derived ring with `clear()`, which renumbers it
from 0, leaving `produced()` short by the inputs' retained base. Since
`live_window_locked()` clamps against every channel's `produced()`, that
one lagging channel answered `n = 0` for the live view, every pause view
and every trigger capture until a whole ring of fresh data had gone by --
the reported freeze, invisible until the ring first wraps, hence its
intermittence. It now restarts with `clear_to(base)`, and
`ChannelBuffer::republish_from(base)` puts the published window back:
`acquire()` is incremental, so a rewrite in place published nothing and
the pyramid went on describing the samples the edit replaced.

The trigger's channel is an index, re-resolved by name after a renumbering
-- but only in `rebuild_enabled_locked()`, so a math add or delete went
through `set_math_channels()` with no restore, the index was clamped to
-1, and `process()` returned at once while the panel still named the
channel. The restore moves into `rebuild_channels()`, which *is* the
renumbering, so no caller can forget it.

And `update_measures()` suppressed its "the window moved" test while an
event was held -- the one mode where a gesture is the only thing that can
move it. Zooming a latched event left the pane describing the whole
captured event. A pinned window needs no exemption: it lands on the range
it already measured and fails the 2 % test by itself.

The checks that missed all three: `UDPSCOPE_PROBE_MATHWRAP_MS` edits a
graph whose ring has already wrapped, which `math_probe` structurally
cannot reach; `[measure-trace]` now prints the mode, the window on screen
and the window the numbers describe; and the dump's `marks_in_window`
counts against the window on screen instead of against the one the marks
were computed over, where it was a tautology.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Martino FerrariandClaude Opus 4.6 committed 2026-10-15 16:30:48 +02:00
1 parent 074cb611bc
commit d91957a2fe
15 files changed
+374 -45

No files matched your search

+12
View File
@@ -78,6 +78,18 @@ void ChannelBuffer::invalidate() noexcept {
pending_amends_.clear();
}
void ChannelBuffer::republish_from(u64 absolute) noexcept {
published_ = absolute;
window_x0_ = 0.0;
pending_amends_.clear();
// `clear()` leaves the pyramid empty at absolute 0, and `drop_before()` past the end
// restarts an empty window at the index it is given -- which is the only way to say
// "nothing is published, and the next append is sample `absolute`". The no-op guard is
// for `absolute == 0`, where `clear()` has already said exactly that.
pyramid_.clear();
if (absolute > 0) pyramid_.drop_before(absolute);
}
f64 ChannelBuffer::time_of(u64 absolute_index) const noexcept {
// Channel sample `j` is produced on the tick that also pushes time slot
// `j*stride + stride - 1` (see `Acquisition::on_data`): the producer stamps the
+10
View File
@@ -48,6 +48,16 @@ class ChannelBuffer final : public plot::data::Series {
void acquire() noexcept;
/// Consumer thread: forgets the published window (after a rebind or a reset).
void invalidate() noexcept;
/// Consumer thread: drops the published window and re-publishes it from absolute sample
/// `absolute` on the next `acquire()`.
///
/// For a ring whose contents were *rewritten in place* rather than appended to -- a math
/// channel recomputed over its inputs' retained window. `acquire()` is incremental, so
/// a rewrite that leaves `produced()` where it was publishes nothing and the pyramid goes
/// on describing the samples the edit replaced. `invalidate()` would do it too, but it
/// restarts at 0 and `acquire()` would then walk every absolute index ever produced; this
/// restarts at the oldest index the ring still holds, so the rebuild is one window.
void republish_from(u64 absolute) noexcept;
/// Producer thread, under `ScopeModel::mutex()`: records that absolute sample `absolute`
/// has been backfilled, for `acquire()` to fold into the pyramid.
+37 -18
View File
@@ -449,6 +449,14 @@ void ScopeModel::recompute_math_locked(const std::vector<bool>& affected) {
const std::size_t slots = static_cast<std::size_t>(store.time().size());
if (slots == 0) return;
// The refill replays the inputs' *retained* window, which starts at this absolute index and not at
// zero, so an emptied derived ring has to resume its count there: `clear()` would renumber it from 0
// and leave its `produced()` short by `base` for a whole ring's worth of fresh data. `live_window_locked()`
// clamps the window against every channel's `produced()`, so that one lagging channel collapses the
// live view, every pause view and every trigger capture to nothing -- the "acquisition freezes after a
// math edit" defect, invisible until the ring has wrapped once (`base == 0` before that).
const std::size_t base = static_cast<std::size_t>(store.time().base());
std::vector<bool> derived(channels_.size(), false);
for (std::size_t m = 0; m < math_.size(); ++m) {
const int channel = m < math_channel_.size() ? math_channel_[m] : -1;
@@ -456,14 +464,18 @@ void ScopeModel::recompute_math_locked(const std::vector<bool>& affected) {
derived[static_cast<std::size_t>(channel)] = true;
// Only the affected channels start again. The rest keep their samples *and* their runtimes: their graphs
// are untouched, so the state they hold is the answer to the inputs they have always had.
if (affected[m]) store.values(static_cast<std::size_t>(channel)).clear();
if (!affected[m]) continue;
store.values(static_cast<std::size_t>(channel)).clear_to(base);
// And the published window with them: this is a rewrite in place, so `produced()` lands back where it
// started and the incremental `acquire()` would publish nothing at all -- leaving the pyramid, which is
// what the renderer and every measurement read, describing the samples the edit just replaced.
store.channel(static_cast<std::size_t>(channel)).republish_from(base);
}
// One bulk copy per channel per chunk, through the ring's own reader. A derived channel is fed a zero:
// `math_apply_locked()` overwrites its value as it walks the list in order, and reading its half-rebuilt
// ring here would read out of range.
constexpr std::size_t kChunk = 1u << 14;
const std::size_t base = static_cast<std::size_t>(store.time().base());
std::vector<std::vector<f32>> inputs(channels_.size());
for (std::size_t c = 0; c < channels_.size(); ++c) {
if (!derived[c]) inputs[c].resize(kChunk);
@@ -482,10 +494,22 @@ void ScopeModel::recompute_math_locked(const std::vector<bool>& affected) {
math_apply_locked(slot, &affected, /*timed=*/false);
}
}
// The pyramid is the consumer's and the tick publishes it; the samples are written here.
// The pyramid is the consumer's and the tick publishes it (`republish_from()` above put its window
// back at `base` so that publish is one window's worth of work); the samples are written here.
}
void ScopeModel::rebuild_channels() {
// The trigger names a channel by its display name, which is stable across the renumbering this
// function *is*; the index is not. Captured and restored here rather than at the call sites
// because every caller renumbers and every caller needs it: adding a math channel appends a
// signal and shifts nothing, but removing one, or a CONFIG that reorders the stream's own
// signals, moves every index after it. `rebuild_locked()` then only clamps an out-of-range
// index to -1, and `TriggerEngine::process()` returns immediately on a channel of -1 -- so the
// panel went on naming the channel while nothing could ever fire again.
const std::string trigger_name =
(trigger_.channel >= 0 && trigger_.channel < static_cast<int>(channels_.size()))
? channels_[static_cast<std::size_t>(trigger_.channel)].name
: std::string();
channels_.clear();
// Which channel each math channel ended up with, by name: a disabled one has none, and the
// producer must not push a derived sample into another channel's ring.
@@ -524,6 +548,15 @@ void ScopeModel::rebuild_channels() {
}
channels_.push_back(std::move(ch));
}
// A name that is no longer a channel -- its signal was disabled, or the stream stopped
// declaring it -- leaves the trigger unassigned, which is a wait rather than a silent
// mis-aim; re-enabling the signal brings it back here.
trigger_.channel = -1;
if (trigger_name.empty()) return;
for (std::size_t i = 0; i < channels_.size(); ++i) {
if (channels_[i].name == trigger_name) trigger_.channel = static_cast<int>(i);
}
}
void ScopeModel::rebuild_locked() {
@@ -639,22 +672,8 @@ bool ScopeModel::enable_locked(std::size_t signal, bool on) {
}
void ScopeModel::rebuild_enabled_locked() {
// The trigger names a channel by its display name, which is stable across
// renumbering: when its own signal is disabled the channel vanishes (the trigger
// then waits, and comes back when the signal is re-enabled). Read before
// `rebuild_channels()`, which is what renumbers.
const int old_trigger = trigger_.channel;
const std::string trigger_name =
(old_trigger >= 0 && old_trigger < static_cast<int>(channels_.size()))
? channels_[static_cast<std::size_t>(old_trigger)].name
: std::string();
// `rebuild_channels()` carries the trigger's channel across the renumbering by name.
rebuild_channels();
trigger_.channel = -1;
if (!trigger_name.empty()) {
for (std::size_t i = 0; i < channels_.size(); ++i) {
if (channels_[i].name == trigger_name) trigger_.channel = static_cast<int>(i);
}
}
rebuild_locked();
generation_.fetch_add(1, std::memory_order_release);
}
+14 -5
View File
@@ -5809,6 +5809,9 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
if (const char* env = g_getenv("UDPSCOPE_PROBE_MATH_MS")) {
g_timeout_add(static_cast<guint>(std::atoi(env)), math_probe, &app);
}
if (const char* env = g_getenv("UDPSCOPE_PROBE_MATHWRAP_MS")) {
g_timeout_add(static_cast<guint>(std::atoi(env)), mathwrap_probe, &app);
}
if (const char* env = g_getenv("UDPSCOPE_PROBE_MEASURE_MS")) {
g_timeout_add(static_cast<guint>(std::atoi(env)), measure_probe, &app);
}
@@ -6476,17 +6479,23 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
// 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`).
const double s0 = model.stream_at_x0();
const RowSet::Row& r0 = rows.row(0);
const plot::Range view = r0.plot != nullptr ? r0.plot->x_range(r0.axis_x) : plot::Range{};
const plot::Range win{view.min + s0, view.max + s0};
//
// `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.
const plot::Range win = app.measure_view_window;
const double span = app.measure_window.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.
for (const RowSet::MeasureMark& mk : app.measure_marks) {
if (mk.level) continue;
++marks;
if (mk.value >= app.measure_window.min && mk.value <= app.measure_window.max) ++in_window;
if (mk.value >= win.min && mk.value <= win.max) ++in_window;
}
// Whether the marks the app placed are *unselectable* -- the property the pointer's hit
// test respects. Read back out of the plot, so this is the app's own setting and not
+10
View File
@@ -487,6 +487,13 @@ struct App {
plot::Range measure_window{};
bool measure_window_valid = false;
bool measure_was_held = false;
/// The window **on screen**, in the same stream seconds, as of the last *tick* -- which is the
/// only thing `measure_window` is comparable with. Captured here rather than read back in the
/// dump for the reason every other dump field is (see verification.md): the dump runs from the
/// destroy handler, after the exit probe has switched the trigger on, and `stream_at_x0()` by
/// then answers for a mode no frame was ever drawn in -- which read as a 45000 % drift on a run
/// whose every frame was exact.
plot::Range measure_view_window{};
/// What the two walks over one channel's visible window produced, shared by every measurement
/// on that signal: the window's statistics, and one accumulator per *distinct level* any of
/// them asked for. Several measurements on one signal must cost two walks, not two each -- and
@@ -1699,6 +1706,9 @@ gboolean trig_trace_probe(gpointer user);
gboolean eventzoom_wait(gpointer user);
gboolean eventzoom_probe(gpointer user);
gboolean math_probe(gpointer user);
gboolean mathwrap_probe(gpointer user);
gboolean mathwrap_probe_wait(gpointer user);
gboolean mathwrap_probe_report(gpointer user);
gboolean measure_probe(gpointer user);
gboolean measure_trace(gpointer user);
gboolean stats_probe_report(gpointer user);
+25 -3
View File
@@ -284,8 +284,19 @@ void update_measures(App& app, bool force, bool accumulate) {
const double s0 = app.model->stream_at_x0();
const plot::Range view = r0.plot->x_range(r0.axis_x);
const plot::Range swin{view.min + s0, view.max + s0};
app.measure_view_window = swin;
const bool held = app.model->display_mode() == DisplayMode::Held;
const bool moved = app.measure_window_valid && !held &&
// "It moved" is asked in **every** mode, a held event included. This test used to carry a
// `&& !held`, and a held event is precisely the mode where a *gesture* is the only thing that
// can move the window: with it, zooming a latched event forty times over left the pane
// describing the whole 2 s event (`for=[0.47..2.47]` against a `swin=[1.417..1.467]`, `passes`
// frozen), which is what "the measurements move when zooming or panning" was -- the numbers and
// the time marks belonged to a window the plot had left. The term was aimed at a *pinned*
// window recomputing every frame, and nothing is needed for that: a settled window satisfies
// `swin == measure_window` exactly and so reads `moved == false` on its own. A pause is the
// standing proof, since `display_mode()` reports `Paused` rather than `Held` for one and the
// statistics probe already requires `n` to advance by at most one across a two-second pause.
const bool moved = app.measure_window_valid &&
std::fabs(swin.min - app.measure_window.min) +
std::fabs(swin.max - app.measure_window.max) >
0.02 * std::max(1.0e-12, swin.span());
@@ -638,8 +649,19 @@ gboolean measure_trace(gpointer user) {
RowSet::Row& r0 = a->rows->row(0);
if (r0.plot == nullptr) return G_SOURCE_REMOVE;
const plot::Range view = r0.plot->x_range(r0.axis_x);
std::string line = "[measure-trace] view=[" + std::to_string(view.min) + ".." +
std::to_string(view.max) + "]";
// The window on screen *and* the window the numbers describe, both on the stream's clock, with
// the offset that converts between them: a reading that moved without the view moving is a
// change of `s0` (a new snapshot under the same mode), and only these three fields tell the two
// apart. `passes` beside them says whether a recompute answered it.
const double s0 = a->model->stream_at_x0();
std::string line = "[measure-trace] mode=" +
std::string(display_mode_name(a->model->display_mode())) + " view=[" +
std::to_string(view.min) + ".." + std::to_string(view.max) + "] s0=" +
std::to_string(s0) + " swin=[" + std::to_string(view.min + s0) + ".." +
std::to_string(view.max + s0) + "] for=[" +
std::to_string(a->measure_window.min) + ".." +
std::to_string(a->measure_window.max) + "] passes=" +
std::to_string(a->measure_passes);
for (std::size_t i = 0; i < a->measures.size() && i < a->measure_results.size(); ++i) {
const MeasureResult& r = a->measure_results[i];
line += " " + std::string(measure_type_name(a->measures[i].type)) + "=" +
+122
View File
@@ -1170,6 +1170,122 @@ gboolean math_probe(gpointer user) {
return G_SOURCE_REMOVE;
}
// The one state `math_probe` structurally cannot reach: an edit of a graph whose ring has already
// *wrapped*. Every edit that probe makes follows a structural change, which replans the buffer and
// restarts every ring at absolute 0 -- and at `base == 0` the recompute's two renumbering bugs are
// both identities. Run it with a small `--buffer-mb` and give it time to saturate; `base=0` in its
// second line means the run was too short or the buffer too large to have read anything.
int g_mathwrap_index = -1;
double g_mathwrap_y_before = 0.0;
int g_mathwrap_polls = 0;
gboolean mathwrap_probe_report(gpointer user) {
App* a = static_cast<App*>(user);
if (a->window == nullptr || g_mathwrap_index < 0) return G_SOURCE_REMOVE;
u64 produced = 0;
u64 slots = 0;
plot::Range y{0.0, 0.0};
{
std::lock_guard<std::mutex> lock(a->model->mutex());
const std::size_t idx = static_cast<std::size_t>(g_mathwrap_index);
if (idx >= a->model->channel_count()) return G_SOURCE_REMOVE;
produced = a->model->live().values(idx).produced();
slots = a->model->live().time().produced();
y = a->model->live().channel(idx).y_bounds();
}
// Two readings, one per half of the recompute. `produced` against `slots` is the ring's absolute
// count: short by `base` is a channel the live window clamps everything else down to, which is the
// reported freeze. `y` is the *pyramid's*, so it reads the published window rather than the samples,
// and the edit **shrinks** the gain (2 -> 0.5) for that reading to exist at all: a pyramid that was
// not republished still describes the replaced +-2 samples, and a window holding both would quote
// the larger -- so a growing gain reads correct either way.
std::fprintf(stderr,
"[mathwrap-probe] after edit: produced=%llu slots=%llu (want equal) y=[%.4f,%.4f] "
"(want +-0.5, was +-%.4f)\n",
static_cast<unsigned long long>(produced), static_cast<unsigned long long>(slots),
y.min, y.max, g_mathwrap_y_before);
std::fflush(stderr);
return G_SOURCE_REMOVE;
}
gboolean mathwrap_probe_wait(gpointer user) {
App* a = static_cast<App*>(user);
if (a->window == nullptr || a->math.empty()) return G_SOURCE_REMOVE;
u64 base = 0;
u64 slots = 0;
{
std::lock_guard<std::mutex> lock(a->model->mutex());
base = a->model->live().time().base();
slots = a->model->live().time().produced();
const std::size_t idx = static_cast<std::size_t>(g_mathwrap_index);
if (g_mathwrap_index >= 0 && idx < a->model->channel_count()) {
g_mathwrap_y_before = a->model->live().channel(idx).y_bounds().max;
}
}
// Polled rather than timed: the wrap is a function of the rate and of `--buffer-mb`, and a fixed
// delay that fell short would print a passing line having tested nothing.
if (base == 0 && ++g_mathwrap_polls < 200) return G_SOURCE_CONTINUE;
std::fprintf(stderr, "[mathwrap-probe] wrapped: base=%llu slots=%llu (want base > 0)\n",
static_cast<unsigned long long>(base), static_cast<unsigned long long>(slots));
std::fflush(stderr);
// The edit itself: the *same* channel list with one node's gain changed, which is the in-place
// path -- `set_math_channels()` replans only when the names or the enables moved.
for (MathNode& n : a->math.front().nodes) {
if (n.op == MathOp::Gain) n.gain = 0.5;
}
apply_math_channels(*a);
// After a tick, because the pyramid half of the reading is published by one.
g_timeout_add(300, mathwrap_probe_report, a);
return G_SOURCE_REMOVE;
}
gboolean mathwrap_probe(gpointer user) {
App* a = static_cast<App*>(user);
if (a->window == nullptr) return G_SOURCE_REMOVE;
// Its own signal and its own graph: a probe must not depend on another's side effects, and this one
// needs a source whose extremes are known (a unit sine doubled reads +-2) for the pyramid reading.
{
const std::vector<SignalInfo>& signals = a->model->signals();
for (std::size_t i = 0; i < signals.size(); ++i) {
if (signals[i].name == "sine") a->model->set_signal_enabled(i, true);
}
}
std::string name;
{
std::lock_guard<std::mutex> lock(a->model->mutex());
if (a->model->channels().empty()) return G_SOURCE_REMOVE;
name = a->model->channels().front().name;
}
MathChannel gain;
gain.name = "math wrap";
gain.enabled = true;
// Field by field: a partial brace-initialiser would add a fourth build warning, and the baseline
// is exactly three.
MathNode src;
src.op = MathOp::Source;
src.source = name;
MathNode op;
op.op = MathOp::Gain;
op.in_a = 0;
op.gain = 2.0;
gain.nodes = {src, op};
gain.output = 1;
a->math.assign(1, gain);
apply_math_channels(*a);
{
std::lock_guard<std::mutex> lock(a->model->mutex());
for (std::size_t c = 0; c < a->model->channels().size(); ++c) {
if (a->model->channels()[c].name == gain.name) g_mathwrap_index = static_cast<int>(c);
}
}
std::fprintf(stderr, "[mathwrap-probe] installed 'math wrap' on '%s': channel=%d\n", name.c_str(),
g_mathwrap_index);
std::fflush(stderr);
if (g_mathwrap_index < 0) return G_SOURCE_REMOVE;
g_timeout_add(100, mathwrap_probe_wait, a);
return G_SOURCE_REMOVE;
}
gboolean measure_probe(gpointer user) {
App* a = static_cast<App*>(user);
if (a->window == nullptr) return G_SOURCE_REMOVE;
@@ -2768,6 +2884,12 @@ gboolean measures_probe_step(gpointer user) {
rebuild_measures_pane(*a);
std::fprintf(stderr, "[measures-probe] added %zu measurements on '%s'\n", a->measures.size(),
signal.c_str());
// This is the only probe set that asks for every type, and so the only one that places a
// *time* mark: `pulse duration`, `period` and `frequency` mark the crossings they were read
// from, and those are the marks `marks_in_window` counts. Without the trace the run reports
// the count with nothing to attribute it to -- which is how a window that lagged the screen
// read `0/6` with no way to tell a stale recompute from a mark placed correctly.
g_timeout_add(250, measure_trace, a);
measures_probe_next(*a, 1, 400);
break;
}