From 96ee35da252c3b578c9e35399992f2b53d4515fc Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Tue, 6 Oct 2026 22:26:31 +0200 Subject: [PATCH] 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 --- .../2026-10-06-math-node-editor-design.md | 452 ++++++++++++++++++ 1 file changed, 452 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-06-math-node-editor-design.md diff --git a/docs/superpowers/specs/2026-10-06-math-node-editor-design.md b/docs/superpowers/specs/2026-10-06-math-node-editor-design.md new file mode 100644 index 0000000..95b0984 --- /dev/null +++ b/docs/superpowers/specs/2026-10-06-math-node-editor-design.md @@ -0,0 +1,452 @@ + + + +# 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 = `, + `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.