3 Commits
Author SHA1 Message Date
Martino FerrariandClaude Opus 4.6 96ee35da25 docs: a design for a node editor whose wires can be drawn, deleted and navigated
The reported "cannot add edges" has a specific cause: the output port's hit
radius reaches past the box it belongs to, and the press handler re-derives the
owning node with a box test, so clicking the visible dot's outer half sets
pending_out to -1 and silently disarms. Edge deletion has no code path at all.

The design replaces the two-click protocol with a drag gesture, makes the list
order the editor's own invariant via a topological re-sort rather than a rule the
user must obey, adds a view transform with every hit test routed through its
inverse, and trades the modal alert on Apply for a live validation line. It also
records that the Cancel button still calls gtk_window_destroy -- the dangling
dialog pointer CLAUDE.md documents as fixed was only fixed on the Apply path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-10-06 22:26:31 +02:00
Martino FerrariandClaude Opus 4.6 f307d27e41 docs: a pause publish costs no packets, and the look is checked with frame path
An A/B of the same fast-mock run with and without a pause reads gaps=0 both
ways, the paused one having published in 86 ms -- so the gaps=73 recorded
yesterday as "a pause costs packet gaps, unexplained" was incidental load on
one run. It never scaled with the publish length and has not reproduced.

Record the publish figures as the range they actually span across runs
(event 69-115 ms, pause 22-86 ms) rather than as single readings, and name
`frame path:` across a pause as the check that the paused trace is drawn like
a live one -- raw=1 lod=555 against raw=0 lod=373, where the defect read
raw=21 lod=118.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-10-06 20:45:13 +02:00
Martino FerrariandClaude Opus 4.6 d2c51f2448 fix: a paused view carries its own LOD pyramid, so it is drawn like a held event
publish_pause_view() filled stamps, values and y and left pyramids empty, so
level_for() descended to raw samples and a frozen pause was re-gathered from
every sample it held on every frame. It cost draw time (12.34 -> 18.18 ms mean)
and, because stage_trace_density() normalises each column against its own peak,
it also changed how the trace looked -- which is how it was noticed.

Both publishes are now timed and reported as `publish cost:`, counted apart
because they stall opposite threads: the event publish runs on the producer
under mutex() and blocks the UI, the pause publish runs on the UI thread and
blocks the producer. That is the number a second live ring has to be judged
against.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-10-06 20:36:27 +02:00
6 changed files with 641 additions and 4 deletions

No files matched your search

+44
View File
@@ -719,6 +719,29 @@ Two lines of the self-check output are assertions in disguise:
whole window fits in the raw path and the publish is too short to lose a race. (Do not
raise `--batch` to chase the rate: a batch of 20000 exceeds the datagram limit and the mock
dies with `OSError: [Errno 90] Message too long`.)
- `publish cost: N events, A/B ms (mean/worst, blocks the UI); M pauses, C/D ms (mean/worst,
blocks the producer)` — what *freezing a view* costs, and the line that attributes the stall
`frame cost:`'s `tick` max can only hint at. The two publishes are counted apart because they
stall **opposite threads**, and nothing else in the dump distinguishes them:
`publish_event()` runs on the producer with `mutex()` already held, so its cost lands on the UI
as a long tick; `publish_pause_view()` runs on the UI thread and takes the lock, so its cost
lands on the producer and can only surface as `gaps`. Read the event figure against `tick`'s max
and the pause figure against `gaps`. Both are **wide**: on the fast mock the event publish has
read **69, 89, 92 and 115 ms** and the pause publish **22 and 86 ms** across runs, so treat the
line as an order of magnitude and not a figure to compare a change against at the millisecond.
The event reading matters anyway because it is far above the ~40 ms the `frame cost:` entry above
quotes — treat that 40 as the old reading and this line as the authority.
It is also **the number to judge a second live ring against**, which is the one design question
it exists to answer: freezing a whole `BufferStore` copies nothing and inherits the pyramid a
`ChannelBuffer` already maintains incrementally, so both `worst` figures would go to **0** — at
the price of a second budget's worth of memory (README records that one live ring is why
`--buffer-mb 200` really is 200 MB) and a second per-sample write on the producer. What the line
says about that trade is that the stall is real but **costs no packets**: an A/B of the same run
with and without a pause read `gaps=0` both ways, the paused one with an 86 ms publish, so a
second ring would buy a smoother frame at a latch rather than recovered data. (An earlier run
reported `gaps=73` with a pause against 0 without, which is what made the stall worth measuring;
it has not reproduced and did not scale with the publish length — 79 gaps with the pyramid build
removed against 73 with it — so it was incidental load on one run. Do not quote it as a cost.)
- `trace ink: N paints checked, C collapsed, S staircase, H short, worst cover P%, worst
drawn-vs-quoted D px over R raw paints` plus a
`[ink] paint N <flags> mode=.. axis=[..] n=.. idx=[..) spp=.. level=.. pts=.. kept=..
@@ -1138,6 +1161,27 @@ far the largest file.
is built once in `publish_event()` inside the `O(n)` pass that already computes the y
bounds, and the same channel on two rows shares it. It costs ≈1 byte per sample per
channel. `frame path:`'s `segments` is the guard.
**"Every" includes the pause**, and that is the half this invariant was written without and
which then went wrong for real: `publish_pause_view()` filled `stamps`, `values` and `y` and
left `pyramids` empty, so a *paused* view was re-gathered raw on every frame. Both halves of
the cost are readable, and the second is the one a performance line cannot show.
`frame path:` read `raw=21 lod=118` over a 1.6 s pause against `raw=1 lod=170` for the same run
live, with the draw mean 12.34 → 18.18 ms — and the **trace changed appearance**, which is what
gets reported. `stage_trace_density()` grades a column against *its own peak* from a `1/height`
deposit per segment, so a column fed ~660 raw inter-sample segments accumulates a far spikier
distribution, and a far larger peak to normalise against, than one fed the one or two bucket
envelopes the pyramid gives: the paused trace read as a hard bright core with its dwell tail
dimmed out. **A pause is not a cheaper case than a capture — it freezes the same rings**, so
anything `publish_event()` does to a snapshot, `publish_pause_view()` owes too. Building it is
not free (see `publish cost:` below): it took the pause publish from 6.15 ms to 22.31 ms, a
one-off producer stall against ~5 ms a frame of draw for the pause's whole length, which at
~60 fps repays about thirty times over on a 1.6 s pause — and the producer absorbs it without
losing packets (`gaps=0` with and without a pause, the paused run at an 86 ms publish).
**The check that the look is restored is `frame path:` across the pause, not the cost line**:
a paused run must read the same path as a live one — measured `raw=1 lod=555 columns=555
graded=555` against `raw=0 lod=373 columns=373 graded=373`, same 24711-segment worst frame,
segment mean within 5 % — where the defect read `raw=21 lod=118`. A `raw` in the tens over a
pause is this returning, and it is the appearance as much as the cost.
- **`EventSeries::gather()` emits a bucket's envelope, not every `2^level`-th sample.**
Point decimation hides exactly the spike the capture was triggered on; a scope has to
draw a bucket's extremes. It needs no index rebasing, unlike the ring: the snapshot's
+44 -4
View File
@@ -459,7 +459,8 @@ ring per channel (N enabled signals -- see the limitation on multi-element signa
below). The spec's second buffer is the latched `EventSnapshot`, which is already
an immutable copy of the captured window (see "Trigger" below); a second live ring
bought nothing and cost half the memory budget, so `--buffer-mb 200` really is
200 MB.
200 MB. What it *would* buy is priced in the dump's `publish cost:` line — see
"There is one live buffer, not two" under the review findings.
`plan_partition()` sizes the rings to cover `--max-window` seconds and then grows
them with the rest of the budget. When a channel's declared sample rate is lower
than the fastest channel's, it is decimated by the integer ratio
@@ -973,8 +974,38 @@ which hides exactly the spike a capture was triggered on, and since the level wa
it never even ran. On the same repro: **138,078 segments** (43× fewer) and a 66 ms worst
frame, with the steady-state draw around 2 ms. It costs about 1 byte per sample per
channel (≈25 % on top of the snapshot's `f32` values) and one stall per latch, since the
build runs under `ScopeModel::mutex()` — at 1.3 MS/s that is a ~40 ms tick, once, with no
packet gaps.
build runs under `ScopeModel::mutex()`. That stall is now measured rather than estimated —
the dump's `publish cost:` line read **115 ms** for one event publish at 1.3 MS/s, against
the ~40 ms this paragraph used to quote — and it produced no packet gaps.
**A paused view carries one too**, and that half was missing until it was found by its
*appearance*. `publish_pause_view()` filled `stamps`, `values` and `y` and left `pyramids`
empty, so a frozen pause was drawn from every raw sample it held, on every frame — the same
defect as above with the same cause, in the other snapshot. `frame path:` read
`raw=21 lod=118` over a 1.6 s pause against `raw=1 lod=170` for the same run live, with the
draw mean going 12.34 → 18.18 ms and its worst frame 30.35 → 86.92 ms. But the reason it was
noticed is that the trace *looked* different: `Renderer::stage_trace_density()` grades each
pixel column against that column's own peak, from a `1/height` dwell deposit per segment, so
a column fed several hundred raw inter-sample segments accumulates a far spikier
distribution — and a far larger peak to normalise against — than one fed the one or two
bucket envelopes the pyramid gives. The paused trace read as a hard bright core with its
dwell tail dimmed away. The rule the invariant was written without: **a pause is not a
cheaper case than a capture — it freezes the same rings.**
It is not free, and the trade is one-off against per-frame. Building the pause pyramid took
that publish from 6.15 ms to 22.31 ms, and unlike the event publish this one runs on the
**UI** thread and so blocks the *producer* — against ~5 ms a frame of draw for as long as the
pause lasts, which over a 1.6 s pause at ~60 fps repays thirty times over. The producer
absorbs it: an A/B of the same fast-mock run with and without a pause read `gaps=0` both
ways, with the pause publish at 86 ms in the paused one. (One earlier run reported `gaps=73`
with a pause against 0 without, which is why the stall was worth measuring at all — but it
has not reproduced, it did not scale with the publish length, and an 86 ms publish costing
nothing is the stronger reading. Treat that 73 as incidental load on one run.)
The verification that the *look* is restored is the same A/B read through `frame path:`:
`raw=0 lod=373 columns=373 graded=373` without a pause against `raw=1 lod=555 columns=555
graded=555` with one — the same path, the same 24711-segment worst frame and a segment mean
within 5 % — where before the fix the paused run read `raw=21 lod=118`.
The self-check's two `frame cost:` / `frame path:` lines are the guard: they split a frame
into the tick, the plot's own draw and the whole GTK paint, and report the segment counts
@@ -1738,7 +1769,16 @@ now — they are listed so they are not mistaken for anything else:
latched `EventSnapshot` — an immutable copy of the captured window, which is
exactly what "the buffer the view is frozen on" needs to be. Two live rings would
also mean acquisition had to follow the trigger state, which is what left the first
latch after arming with no pre-trigger history.
latch after arming with no pre-trigger history. What a second ring would buy is the
publish: freezing a whole `BufferStore` copies nothing and inherits the pyramid the
producer already maintains, so both figures in the dump's `publish cost:` line would
go to zero. They are worth knowing before pricing that — at 1.3 MS/s an event publish
measured **69–115 ms of blocked UI tick** and a pause publish **22–86 ms of blocked
producer** — but they do not pay for a second budget's worth of memory, a second
per-sample write on the producer, and a single frozen slot where the app today holds
an event, a pause and an export's window at once. The *observable* cost is also
smaller than the stall: an A/B with and without a pause read `gaps=0` both ways, so
what a second ring would buy is a smoother frame at a latch, not recovered packets.
- **The buffer is allocated at the first CONFIG, not at initialisation.**
The spec's "N+1 split" needs N, which only a CONFIG gives; `--buffer-mb` is the
*budget*, and the power-of-two ring rounding means the final
@@ -0,0 +1,452 @@
<!-- SPDX-License-Identifier: MIT -->
<!-- SPDX-FileCopyrightText: 2026 Martino Ferrari -->
# The math node editor: wires you can draw and delete, and a canvas you can navigate
The math channel editor works as a *form* and fails as a *graph editor*. Three things were
reported — edges cannot be added, edges cannot be deleted, and the canvas cannot be zoomed or
panned — and all three are true. The first is a defect with a specific cause, the second has no
code path at all, and the third is an absence. Beyond them the editor asks the user to obey a
rule the data structure has (a node may only read nodes above it in the list) rather than
maintaining that rule itself, and it reports an invalid graph in a modal alert at the end
instead of on the canvas as it is being built.
This redesigns the canvas interaction. Nothing below changes `MathNode`, `MathChannel`, the
evaluator in `src/math_channel.cpp`, the session format, or the producer's path.
## Scope
In: the canvas view transform, the port and wire hit tests, the gesture state machine
(node drag / wire drag / background pan / wheel zoom), edge creation and deletion, a
topological re-sort that lets a wire be drawn in either direction, an op picker on the add
button, a live validation line that replaces the modal alert, a tidy/auto-layout pass, the
`Cancel` button's close path, and the `[math-probe]` assertions for all of it.
Out: parameters drawn on the node boxes (explicitly declined — the form beside the canvas is
where a node's values live). No new ops. No change to `math_channel_valid()`'s rules, to the
evaluator, to `MathRuntime`, or to how a derived channel is acquired. No node *library* panel,
no copy/paste, no undo.
## Why edges cannot be added today
`math_canvas_pressed` (`src/ui.cpp:3455`) handles an output-port hit like this:
```cpp
const int port = math_port_at(d, x, y, which);
if (port == -2) {
const int node = math_node_at(d, x, y);
d->pending_out = (node >= 0 && d->pending_out == node) ? -1 : node;
...
}
```
`math_port_at` throws away *which* node's output was hit — it returns the sentinel `-2` — and
the handler recovers the node with `math_node_at()`, which matches only **inside the box**. The
output square is drawn at `x = n.x + kNodeW`, i.e. straddling the right edge: half of it is
outside. Its hit radius is `kPort` (±10) while it is drawn at `±kPort * 0.5` (±5), so the
clickable region extends 10 px past the edge and `math_node_at()` answers `-1` for every pixel
of it beyond the first. `pending_out` is then set to `-1` — a *silent disarm*. Clicking the
visible dot's outer half, which is most of what the eye aims at, arms nothing, and the input
squares never light up. That is the whole of "it is not possible to add edges": the mechanism
exists and the first click of the two-click sequence usually fails.
It is worth fixing the cause and not only the symptom, because the same re-derivation would
break again under the view transform below.
Edge *deletion* needs no diagnosis: no path in the file ever writes `-1` back into `in_a` or
`in_b` except `on_math_node_deleted`, which is deleting the whole node.
## The view transform
`MathDialog` gains three doubles:
```cpp
double view_scale = 1.0; ///< canvas pixels per graph unit
double view_x = 0.0; ///< canvas pixels: where graph (0,0) is drawn
double view_y = 0.0;
```
Graph coordinates are what `MathNode::x`/`y` already are and what the session file stores, so a
view change is never persisted — reopening the editor shows the graph at 1:1, which is the
right default for a window whose size the user did not choose either.
Two conversions, and **every** hit test goes through the second:
```cpp
// canvas <- graph: applied once to the cairo context, never per item.
cairo_translate(cr, d->view_x, d->view_y);
cairo_scale(cr, d->view_scale, d->view_scale);
// graph <- canvas: the inverse, for a pointer position.
void math_to_graph(const MathDialog* d, double cx, double cy, double& gx, double& gy) {
gx = (cx - d->view_x) / d->view_scale;
gy = (cy - d->view_y) / d->view_scale;
}
```
Applying the transform to the context rather than to each item is what keeps the drawing code
unchanged: `math_canvas_draw` keeps every coordinate it has today, and the line widths and font
sizes scale with the graph, which is what a zoom should do to a diagram.
**A hit tolerance is a screen distance, not a graph distance.** A port is aimed at with a
pointer, so its clickable radius must be the same number of *canvas* pixels at every zoom —
which in graph units is `kPort / view_scale`. The same for the wire tolerance below. Getting
this backwards makes the ports unhittable when zoomed out, which is exactly when a user is
looking for them.
**Scale is clamped to `[0.25, 4.0]`.** Below a quarter the op names are unreadable and the
boxes are smaller than the pointer tolerance; above four a single node fills the canvas. The
clamp lives in one place, a `math_set_scale()` helper, because three callers reach it (wheel,
Fit, reset).
### Wheel zoom about the pointer
A `GtkEventControllerScroll` with `GTK_EVENT_CONTROLLER_SCROLL_VERTICAL`. The anchor is the
graph point under the pointer, and it must not move:
```cpp
double gx = 0.0, gy = 0.0;
math_to_graph(d, px, py, gx, gy); // before
d->view_scale = clamp(d->view_scale * (dy < 0 ? 1.1 : 1.0 / 1.1), 0.25, 4.0);
d->view_x = px - gx * d->view_scale; // restore the anchor
d->view_y = py - gy * d->view_scale;
```
The pointer position comes from the controller's own `GtkEventController` current event rather
than from a tracked motion position: a scroll event carries one, and tracking motion to supply
it is a second source of truth that can be stale.
No modifier is involved. The canvas has no other use for the wheel, so a bare wheel zoom is
unambiguous here — unlike the plot, where the wheel's modifiers are already spoken for.
### Pan, Fit, and reset
Dragging the empty background pans: `view_x = pan_x0 + dx`, from a `pan_x0` recorded at
`drag-begin`, for the same total-offset reason the node drag already documents at
`src/ui.cpp:3507`.
A **Fit** button frames every node:
```
bbox over all math_node_rect() (empty graph -> identity view)
scale = clamp(min(w_canvas / (bbox.w + 2*pad), h_canvas / (bbox.h + 2*pad)), 0.25, 4.0)
view_* = centre the scaled bbox in the canvas
```
`Ctrl+0` resets to `scale = 1`, `view_x = view_y = 0`. Both are one call to a shared
`math_fit_view()` / `math_reset_view()` so the button and the accelerator cannot drift apart.
## The port and wire hit tests
`math_port_at` is replaced by one that **names the node it found**:
```cpp
/// The port under a *graph* position. Returns the node index, with `which` set to
/// `kPortOut` (-1) for its output or 0/1 for an input; `-1` when no port is under it.
int math_port_at(const MathDialog* d, double gx, double gy, double tol, int& which);
```
Two changes beyond the signature. It scans **backward**, topmost first, so it agrees with
`math_node_at()` about which of two overlapping nodes owns a shared pixel — a disagreement
between the two searches is how a click lands on one node and a drag on another. And the
caller never re-derives the owner, which is the defect above made structurally impossible.
The hit radius stays *larger* than the drawn square: ±`kPort` canvas pixels around the port's
centre, against a square drawn at ±`kPort * 0.5` — a 20 px target for a 10 px mark. That is
deliberate and, once the owner is no longer re-derived, harmless: a generous radius is the
difference between a diagram that can be wired and one that cannot. The asymmetry is written
down in a comment at the hit test, because it is what the old defect fed on.
A wire is picked by distance to the curve it is drawn as. The curve is the same cubic the
renderer uses (`src/ui.cpp:3401`), so the pick and the ink cannot disagree:
```cpp
/// The wire under a graph position, as (consumer node, which input), or {-1, 0}.
/// The bezier is sampled at 24 points and the nearest segment measured; a curve 40 units of
/// tread wide over a node pitch of 160 is smooth at that sampling, and the tolerance is 6
/// canvas pixels, so a miss by a pixel is not a wrong edge deleted.
MathWireRef math_wire_at(const MathDialog* d, double gx, double gy, double tol);
```
Sampling rather than solving: the exact distance to a cubic is a fifth-degree root-find, and 24
samples of a curve whose control points are 40 units out is well inside a 6 px tolerance. The
sample count is a named constant so a future wider tread can raise it.
**Order of precedence on a press**: port, then wire, then node body, then background. A port
sits on a node's edge and a wire ends on a port, so each later test must only see what the
earlier ones refused.
## The gesture state machine
One `GtkGestureDrag` serves four gestures, decided entirely by what the press landed on. A
`GtkGestureClick` is kept for the selection-only cases, because a click is a drag of zero
length and GTK delivers both; `drag-begin` records the mode and `pressed` only ever *selects*.
```cpp
enum class MathDrag { None, Node, Wire, Pan };
MathDrag drag_mode = MathDrag::None;
int wire_from = -1; ///< Wire: the node whose output the live wire leaves
int wire_detach = -1; ///< Wire: the consumer whose input was detached to start it, or -1
int wire_detach_which = 0;
double wire_x = 0.0; ///< Wire: the pointer, in graph units, for the preview
double wire_y = 0.0;
double pan_x0 = 0.0;
double pan_y0 = 0.0;
MathWireRef selected_wire{-1, 0};
```
`drag-begin`, in precedence order:
- **An output port** → `Wire`, `wire_from = node`, `wire_detach = -1`.
- **A wired input port** → `Wire`, `wire_from = <the node it was wired from>`,
`wire_detach = node`, `wire_detach_which = which`, and the input is cleared *immediately*.
Dragging an existing wire off its input and dropping it elsewhere is one gesture, and
clearing on begin is what makes the preview show the truth: the wire the user is holding.
Dropping it on nothing therefore *detaches*, which is the agreed detach gesture, and needs
no separate case.
- **An unwired input port** → `None` (select the node; there is nothing to drag from an empty
input). Wiring is always output→input, so this is not a connect gesture.
- **A node body** → `Node`, exactly as today.
- **Anything else** → `Pan`.
`drag-update` moves the node, the live wire's endpoint, or the view, by mode.
`drag-end` resolves `Wire`:
- Released on an **input port** of a node that is not `wire_from` → `math_connect()`.
- Released on an **input port of `wire_from` itself** → refused, with "a node cannot read
itself" on the validation line. A self-edge is the one cycle a topological sort cannot
break, and refusing it by name reads better than "the graph has a cycle".
- Released anywhere else → nothing further. If the drag began by detaching, the detachment
stands.
`Escape` during a `Wire` drag cancels and **restores** a detached input: a cancel must undo the
clear-on-begin, or Escape would be a way to delete an edge by accident.
A `GtkEventControllerKey` on the canvas handles `Delete`/`BackSpace` (clear `selected_wire`),
`Escape` (cancel a drag, else clear the wire selection) and `Ctrl+0` (reset the view). It is
added to the canvas and the canvas is given `gtk_widget_set_focusable(TRUE)` plus focus on
press, because a drawing area is not focusable by default and a key controller on an unfocused
widget never fires. **The key controller must not swallow text**: the dialog's name and unit
entries are `GtkEditable`s, and the controller is on the *canvas*, so a key only reaches it
while the canvas has focus — the same reasoning as the app-level accelerators' bubble phase,
reached by a narrower route.
## The edge model
One function owns connection:
```cpp
/// Wire `from`'s output into `to`'s input `which`. Re-sorts the node list when the wire runs
/// backwards. Returns false and sets `why` when the wire would make a cycle.
bool math_connect(MathDialog* d, int from, int to, int which, std::string* why);
```
and deletion is a one-liner at the call site (`in_a`/`in_b` back to `-1`), because there is
nothing to validate in removing an edge: a disconnected input is a state the validation line
already names.
### Backwards wires re-sort the list
`MathChannel::nodes` is a list walked in order, and the evaluator's correctness rests on a node
reading only nodes before it. Today that rule is the *user's* problem: the input ports of nodes
above the armed output are not even drawn as armed, so a wire that would run backwards cannot
be drawn at all. In a two-column auto-placed graph that is a real obstruction — the node you
want to feed is frequently the one that happens to sit earlier in the list.
The rule becomes the editor's invariant instead. `math_connect()` on a backwards wire
(`from > to`) builds the new edge set, runs a topological sort, and commits only if it is
acyclic:
1. Trial edge set = every node's existing inputs, plus the new one.
2. Kahn's algorithm over it. Ties broken by **current list index**, so a sort that changes
nothing leaves the list alone and a sort that must move something moves as little as
possible. (A stable order matters: the node positions on the canvas do not change, so a
gratuitous renumber would silently re-point `output` and `selected` for no reason the user
can see.)
3. On a cycle: nothing is written, `why` is set, and the validation line shows it. The edge is
*not* created — this is the one refusal left, and it is the honest one.
4. On success: permute `nodes`, then remap through the permutation **everything that holds an
index** — `output`, `selected`, every `in_a`/`in_b`, `drag_node`, `selected_wire.node`. A
forgotten one is a silent mis-wire, so they are remapped in a single block with a comment
naming the set.
The session format is untouched: what is written is still a list in evaluation order. A file
written by an older build still loads, because the sort is an editor operation and an
already-ordered list sorts to itself.
`math_canvas_draw` stops gating a wire on `from < i` once this holds — but keeps the gate as a
**defensive skip**, because a `MathChannel` loaded from a file the editor did not write could
hold a forward reference, and drawing it would be drawing an edge the evaluator ignores.
### The armed-port highlight goes away
`pending_out` and the "input ports of later nodes light up" highlight are both deleted. They
exist to serve the two-click protocol and the list-order rule, and neither survives: with a
drag gesture the feedback is the live wire following the pointer, and with the re-sort every
input is a legal target. What replaces the highlight is a hover accent on the port under the
pointer during a `Wire` drag, which says the one thing that still matters — whether releasing
here will connect.
## The live validation line
`on_math_apply` validates in two steps today (`math_channel_valid()`, then an acquired-source
check the validator cannot make) and reports each in a modal `GtkAlertDialog`. A modal alert at
the end of a build is the worst time and place to be told; the graph is right there and can say
so continuously.
One extraction:
```cpp
/// Why the graph as it stands could not be saved, or "" when it could. Both checks the Apply
/// path made: `math_channel_valid()`'s graph rules and the dialog's own "a Source must name an
/// acquired channel", which the validator cannot make because only the dialog sees that list.
std::string math_dialog_problem(const MathDialog* d);
```
`math_dialog_sync()` — already the one place that rewrites the form from the model, and already
called from every mutation — sets the line's text and `gtk_widget_set_sensitive(apply, problem
.empty())`. An insensitive Apply with the reason beside it is a better refusal than a dialog
that must be dismissed.
`on_math_apply` keeps the check and loses the **dialogs**: it calls `math_dialog_problem()` and
returns on a non-empty answer, with no `GtkAlertDialog`. The guard has to stay even though the
button cannot be clicked while it would fire, because the probe calls `on_math_apply()` directly
and must not be able to save a broken graph through it. That `d->apply` pointer is new on
`MathDialog`; the button is built after the canvas today, so `math_dialog_sync()`'s first call
(at `src/ui.cpp:3972`, after the whole tree) already runs late enough to set it.
The line is a `GtkLabel` under the canvas, `error` CSS class when non-empty, and
`gtk_label_set_ellipsize(END)` with a tooltip carrying the full text — **the layout-must-not-
depend-on-content rule applies here as much as to the status bar**: this label sits in a dialog
whose canvas is `hexpand`, and a wrapping error message would otherwise widen the dialog on a
mere change of graph.
## The op picker on add
`add node` always makes a `MathOp::Gain` (`src/ui.cpp:3516`), so adding a low pass is two
actions and a dropdown hunt. It becomes a `GtkMenuButton` labelled `add` whose popover holds a
flat button per op, built once from `math_op_count()` / `math_op_at(i)` — the accessor order,
not the enum order, for the reason `CLAUDE.md` already records about writing an op's index.
Each button carries its op index with `g_object_set_data`, so there is no action-name plumbing
and nothing to parse.
The two behaviours that already work are kept: the new node becomes the output when the graph
`was_linear`, and placement is the existing grid. Placement gains one thing — a node is placed
at the **first free grid cell** rather than at `nodes.size()`, so the slot left by a deletion is
reused instead of the grid growing for ever. The overlap rule stands (`CLAUDE.md`: boxes must
not overlap, or `math_node_at()`'s topmost-first search returns the wrong one).
## Tidy / auto-layout
A `tidy` button lays the graph out left to right by dependency depth:
```
depth(i) = 0 for a Source or an unwired node
= 1 + max(depth of its wired inputs) // well-defined: inputs precede i
x = 24 + depth * (kNodeW + 48)
y = 20 + row_within_depth * (kNodeH + 22)
```
It needs no recursion and no visited set: the list is in evaluation order — the invariant the
re-sort now maintains — so the depth of node `i` depends only on nodes before it and a single
forward pass computes every depth. Then `math_fit_view()`, because a tidied graph that is off
screen is not tidy.
The column pitch is `kNodeW + 48`, which is 180 against the 160 the current grid uses: 160
leaves 28 units for the wire's 40-unit tread, so the curve doubles back on itself between
adjacent columns.
## The `Cancel` button closes rather than destroys
`src/ui.cpp:3962` connects `Cancel` to `gtk_window_destroy`:
```cpp
g_signal_connect_swapped(cancel, "clicked", G_CALLBACK(gtk_window_destroy), window);
```
This is the defect `CLAUDE.md` documents under the math channel invariants — "a Cancel that
*destroyed* the window left `app.math_dialog` pointing at a dead widget: every later
`open_math_dialog()` took the 'already open' branch and presented it" — and it is still live.
The fix for it reached the Apply path (`src/ui.cpp:3731`) and the probe (`src/ui.cpp:6672`) and
never reached the button. It is `gtk_window_close`, one word, and the probe must assert it,
which it does not today: it only ever closes programmatically.
## What `[math-probe]` must assert
Every gesture above is reachable from the probe, because the handlers take the `MathDialog*`
and plain coordinates — which is already how the drag test at `src/ui.cpp:6694` works. The
probe is extended with one line per claim; each is an assertion stated as a `want` beside the
reading, in the style of the existing lines.
- `[math-probe] hit: output at edge+1 -> node N which=-1, at edge+9 -> node N which=-1
(want N N)` — the silent-disarm defect, read at two offsets inside the hit radius and outside
the box. Both must name the node. **This is the one line that would have caught the reported
bug**, and it is the reason the hit test returns the node rather than a sentinel.
- `[math-probe] wire: connect 0->1.a wired=W (want 0)` then a backwards
`[math-probe] wire: backwards connect 2->0.a order=[..] wired=W reordered=D (want 1)` — the
re-sort, with the permuted op order printed so a wrong remap is visible as a wrong list.
- `[math-probe] wire: self refused=D problem=[..] (want 1)` and
`[math-probe] wire: cycle refused=D problem=[..] (want 1)` — the two refusals, with the
cycle built by wiring a two-node pair both ways. `refused=1` **and** the edge absent.
- `[math-probe] wire: pick at (x,y) -> (node,which)=(N,K) delete -> in=-1 (want -1)` — the
pick samples the curve at its midpoint, which is the one place a wrong tolerance or a wrong
curve shows.
- `[math-probe] detach: drag from in 1.a to empty -> in=-1 (want -1)`, and
`[math-probe] detach: escape restores in=F (want F)` — the clear-on-begin and its undo, which
is the half that silently deletes an edge if Escape is forgotten.
- `[math-probe] view: zoom scale=S anchor kept dx=.. dy=.. (want 0.0 0.0)` — the graph point
under the pointer before and after a wheel step, which is the only reading that distinguishes
"zoomed about the pointer" from "zoomed about the origin". Both must be zero to a rounding.
- `[math-probe] view: fit scale=S box=[..] all_inside=D (want 1)`, and
`[math-probe] view: reset scale=S offset=(..) (want 1.000 (0,0))`.
- `[math-probe] view: pan dx=.. dy=.. offset=(..) (want ..)` plus a hit test **after** a pan and
a zoom — `port at the same screen point -> node N` — because a hit test that forgot the
inverse transform passes every test taken at the identity view.
- `[math-probe] tidy: depths=[..] columns=[..] overlaps=D (want 0)` — the overlap count is the
assertion (`CLAUDE.md`'s own rule), and the depths say the layout followed the graph.
- `[math-probe] validate: problem=[..] apply_sensitive=D` before and after wiring the missing
input: `(want non-empty 0)` then `(want empty 1)`.
- `[math-probe] cancel: dialog=cleared` — the `Cancel` button's `::clicked` emitted directly,
then `app.math_dialog` read back. `STILL SET` is the regression.
Two things the probe deliberately cannot assert, and which are therefore stated as design
constraints rather than checks: that the canvas takes **focus** on press (a synthetic press
does not run GTK's focus machinery), and that the live wire *looks* like it follows the pointer
(a draw is not readable from a probe). The second is bounded by the first assertion set — the
endpoint it is drawn from is `wire_x`/`wire_y`, which the drag test writes and can print.
The existing lines are all kept. `drag:` in particular stays as it is: the total-offset rule is
unchanged by any of this, and the gesture that exercises it is still a node-body drag.
## Documentation
Three places, following the project's existing split:
- **`CLAUDE.md`** — the `[math-probe]` bullet under Verification gains the new readings and
their `want`s; the math channel invariants gain the **hit-test-names-its-own-node** rule (a
port hit must not be re-derived with a box test), the **tolerance-is-a-screen-distance** rule,
the **list order is the editor's invariant, maintained by a topological re-sort** rule with
the index-remap set named, and a note that the `Cancel` path is now asserted.
- **`README.md`** — a subsection under the math channels describing the editor as it now
behaves, and the diagnosis of the silent disarm, since README is where "what was actually
built and why" lives.
- **`src/help/11-math.txt`** gains the four canvas gestures (drag a port to wire, drag a wired
input away to detach, click a wire and press Delete, wheel to zoom and drag the background to
pan) and the two buttons. It is an edit to an existing page, so `pages` stays 14 and
`[help] pages=14 parsed=14` must still hold — the markup is Pango and a page that fails to
parse renders a stand-in.
## Risks
- **The re-sort touching an index nothing remapped.** The mitigation is that the remap is one
block, the probe prints the permuted op order, and `selected`/`output` are both read back.
- **The key controller stealing a keystroke from the name entry.** It is on the canvas, which
must have focus; the probe cannot see this, so it is checked by hand once, by typing in the
name field with a wire selected.
- **The wheel controller fighting a future scrolled window.** There is none and this design
removes the reason for one, which is worth stating so a later "wrap the canvas in a
`GtkScrolledWindow`" is recognised as undoing the zoom rather than adding to it.
- **`GtkGestureDrag` and `GtkGestureClick` on one widget both claiming a press.** They already
coexist on this canvas; the change is that `pressed` now only selects and never mutates the
graph, which removes the ordering question rather than answering it.
+32
View File
@@ -516,6 +516,9 @@ void ScopeModel::reset_live_history_locked() {
}
void ScopeModel::publish_pause_view(double lo_s, double hi_s, double x_offset) {
// Timed from before the lock: the wait for it is part of what freezing a view costs the thread
// doing it, and this one is the UI's.
const std::uint64_t t0 = now_ns();
auto snap = std::make_shared<EventSnapshot>();
snap->x_offset = x_offset;
std::lock_guard<std::mutex> lock(mutex_);
@@ -537,6 +540,7 @@ void ScopeModel::publish_pause_view(double lo_s, double hi_s, double x_offset) {
}
snap->values.resize(channels_.size());
snap->y.resize(channels_.size(), plot::Range{0.0, 0.0});
snap->pyramids.resize(channels_.size());
for (std::size_t c = 0; c < channels_.size(); ++c) {
const u64 stride = std::max<u64>(1, part_.stride[c]);
auto& vals = snap->values[c];
@@ -545,16 +549,42 @@ void ScopeModel::publish_pause_view(double lo_s, double hi_s, double x_offset) {
if (take > 0) src.values(c).copy_from(first + n - take, take, vals.data());
f32 lo = vals.empty() ? 0.0f : vals.front();
f32 hi = lo;
// The LOD index, folded into the bounds pass exactly as `publish_event()` folds it, and for
// the same reason: a snapshot that answers `pyramid()` with an empty one has `level_for()`
// descend to level 0, so the frozen view is re-gathered from every raw sample it holds on
// every frame it is on screen. A pause is not a cheaper case than a capture -- it freezes the
// *same* rings -- and leaving it out was visible twice over. The cost: `frame path:` read
// `raw=21 lod=118` over a 1.6 s pause against `raw=1 lod=170` for the same run live, with the
// draw mean 12.34 -> 18.18 ms. And the *look* changed, which is the half a cost figure does
// not show: `stage_trace_density()` grades a column against its own peak from a `1/height`
// deposit per segment, so a column fed ~660 raw inter-sample segments accumulates a far
// spikier distribution -- and a far larger peak to normalise against -- than one fed the one
// or two bucket envelopes the pyramid gives. The paused trace read as a hard bright core with
// its dwell tail dimmed out, i.e. the trace changed appearance on pause.
//
// It is not free, and the trade is one-off against per-frame: building it took the pause
// publish from 6.15 ms to 22.31 ms (`publish cost:`), which blocks the *producer* -- against
// ~5 ms a frame of draw for as long as the pause lasts, which at ~60 fps a 1.6 s pause repays
// thirty times over. The producer absorbs the stall: `gaps=0` with and without a pause, the
// paused run having published in 86 ms.
auto& pyr = snap->pyramids[c];
pyr.reserve(take);
for (f32 v : vals) {
lo = std::min(lo, v);
hi = std::max(hi, v);
pyr.append(v);
}
snap->y[c] = plot::Range{lo, hi};
}
pause_view_.store(std::move(snap), std::memory_order_release);
note_publish(publish_pause_count_, publish_pause_us_total_, publish_pause_us_worst_,
(now_ns() - t0) / 1000u);
}
void ScopeModel::publish_event(double pre_s, double post_s, double edge) {
// The caller is the producer and already holds `mutex()`, so every microsecond counted here is a
// microsecond the UI tick is blocked for -- which is what the documented `tick` max is made of.
const std::uint64_t t0 = now_ns();
auto snap = std::make_shared<EventSnapshot>();
snap->x_offset = edge; // the stream time x = 0 means, for everything that has to cross back
BufferStore& src = live();
@@ -605,6 +635,8 @@ void ScopeModel::publish_event(double pre_s, double post_s, double edge) {
// Bumped after the snapshot is in place, so a consumer that sees the new sequence
// number sees the snapshot it counts.
event_seq_.fetch_add(1, std::memory_order_release);
note_publish(publish_event_count_, publish_event_us_total_, publish_event_us_worst_,
(now_ns() - t0) / 1000u);
}
void BufferStore::set_epoch(f64 epoch) noexcept {
+47
View File
@@ -224,6 +224,37 @@ class ScopeModel {
std::uint64_t math_cost_ns() const noexcept {
return math_cost_ns_.load(std::memory_order_relaxed);
}
/// What freezing a view costs, and *which thread it costs it to*. The two publishes are stalls on
/// opposite sides of `mutex()` and so are counted apart: `publish_event()` runs on the **producer**
/// with the lock already held, so its cost lands on the UI as a long tick (the documented `tick`
/// max); `publish_pause_view()` runs on the **UI** thread and takes the lock, so its cost lands on
/// the producer and can only surface as `gaps`. Neither is attributable in a frame mean, which is
/// why they are counted rather than inferred. Each counter is written by one thread and read by the
/// dump, so a relaxed store of a value already in hand is enough.
///
/// This is the number to judge a second live ring against: freezing a whole `BufferStore` would
/// make both figures zero, since a `ChannelBuffer` maintains its pyramid incrementally and there
/// would be nothing to copy -- but it costs a second budget's worth of memory and a second
/// per-sample write on the producer, so it is worth paying only if the worst figures here are
/// large enough to matter.
std::uint64_t publish_event_count() const noexcept {
return publish_event_count_.load(std::memory_order_relaxed);
}
std::uint64_t publish_event_us_total() const noexcept {
return publish_event_us_total_.load(std::memory_order_relaxed);
}
std::uint64_t publish_event_us_worst() const noexcept {
return publish_event_us_worst_.load(std::memory_order_relaxed);
}
std::uint64_t publish_pause_count() const noexcept {
return publish_pause_count_.load(std::memory_order_relaxed);
}
std::uint64_t publish_pause_us_total() const noexcept {
return publish_pause_us_total_.load(std::memory_order_relaxed);
}
std::uint64_t publish_pause_us_worst() const noexcept {
return publish_pause_us_worst_.load(std::memory_order_relaxed);
}
const std::vector<SignalInfo>& signals() const noexcept { return signals_; }
const std::vector<ChannelInfo>& channels() const noexcept { return channels_; }
@@ -491,6 +522,22 @@ class ScopeModel {
std::atomic<std::uint64_t> math_slots_{0};
std::atomic<std::uint64_t> math_timed_{0};
std::atomic<std::uint64_t> math_cost_ns_{0};
/// See `publish_event_count()`. Single-writer each: the first three are the producer's, the last
/// three the UI thread's.
std::atomic<std::uint64_t> publish_event_count_{0};
std::atomic<std::uint64_t> publish_event_us_total_{0};
std::atomic<std::uint64_t> publish_event_us_worst_{0};
std::atomic<std::uint64_t> publish_pause_count_{0};
std::atomic<std::uint64_t> publish_pause_us_total_{0};
std::atomic<std::uint64_t> publish_pause_us_worst_{0};
/// Fold one publish's elapsed microseconds into a counter triple. Called by the one thread that
/// owns the triple, so the load/store pairs need no read-modify-write.
static void note_publish(std::atomic<std::uint64_t>& count, std::atomic<std::uint64_t>& total,
std::atomic<std::uint64_t>& worst, std::uint64_t us) noexcept {
count.store(count.load(std::memory_order_relaxed) + 1, std::memory_order_relaxed);
total.store(total.load(std::memory_order_relaxed) + us, std::memory_order_relaxed);
if (us > worst.load(std::memory_order_relaxed)) worst.store(us, std::memory_order_relaxed);
}
std::vector<bool> enabled_;
Partition part_;
+22
View File
@@ -8383,6 +8383,28 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
per_slot * static_cast<double>(app.model->math_slots()) / 1.0e6);
}
}
// What freezing a view costs, counted apart for the two publishes because they stall opposite
// threads (see `ScopeModel::publish_event_count()`). This is the figure a second live ring would
// buy: freezing a whole `BufferStore` copies nothing and inherits the pyramid the producer
// already maintains, so both `worst` figures would be 0 -- at a second budget's worth of memory
// and a second per-sample write on the producer. Read `event worst` against `frame cost:`'s
// `tick` max (the same stall, seen from the UI) and `pause worst` against `gaps`.
{
const auto ev = app.model->publish_event_count();
const auto pa = app.model->publish_pause_count();
std::printf("[udpscope] publish cost: %llu events, %.2f/%.2f ms (mean/worst, blocks the UI);"
" %llu pauses, %.2f/%.2f ms (mean/worst, blocks the producer)\n",
static_cast<unsigned long long>(ev),
ev > 0 ? static_cast<double>(app.model->publish_event_us_total()) /
static_cast<double>(ev) / 1000.0
: 0.0,
static_cast<double>(app.model->publish_event_us_worst()) / 1000.0,
static_cast<unsigned long long>(pa),
pa > 0 ? static_cast<double>(app.model->publish_pause_us_total()) /
static_cast<double>(pa) / 1000.0
: 0.0,
static_cast<double>(app.model->publish_pause_us_worst()) / 1000.0);
}
// Unconditional, unlike the pane's own lines below: this is about the instruments the app
// *created*, which exist whether or not any measurement was added.
std::printf("[udpscope] readout instruments locked: %d (trigger marker + measurement marks)\n",