Nine tasks, each ending in a commit: the silent disarm in the port hit test, the view transform, the drag-based wiring, wire deletion, the topological re-sort that makes a backwards wire legal, the live validation line, the op picker, tidy, and the docs with the full verification run. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2423 lines
106 KiB
Markdown
2423 lines
106 KiB
Markdown
# Math Node Editor Implementation Plan
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||
|
||
**Goal:** Make udpscope's math node editor usable — edges can be added, deleted and detached; the canvas zooms and pans; the graph re-sorts itself so a backwards wire is legal; and the dialog says why it cannot be saved instead of refusing with a modal alert.
|
||
|
||
**Architecture:** Everything lives in `src/ui.cpp`'s math-dialog block (currently `3280`–`3990`) and is driven through the existing `MathDialog` struct. Three structural additions: a **view transform** (`view_scale`/`view_x`/`view_y` applied once to the cairo context, with `math_to_graph()` as the inverse used by *every* hit test); a **gesture state machine** (`MathDrag::{None,Node,Wire,Pan}`) replacing the two-click `pending_out` protocol; and `math_connect()`, which performs a **topological re-sort** so that a wire drawn right-to-left is accepted rather than refused. The `MathChannel` data model in `src/math_channel.{hpp,cpp}` is **unchanged** — the list-walked-in-order invariant stays, and the editor becomes the thing that maintains it.
|
||
|
||
**Tech Stack:** C++20, GTK4 (`GtkDrawingArea`, `GtkGestureClick`, `GtkGestureDrag`, `GtkEventControllerScroll`, `GtkEventControllerKey`, `GtkMenuButton`), cairo for the canvas, CMake + Ninja.
|
||
|
||
## Global Constraints
|
||
|
||
- **Every source file carries an SPDX MIT header.** Both files touched already have one; do not add or remove.
|
||
- **There is no unit test suite.** The only automated check is the app's own headless probe under `xvfb-run`. See "The TDD cycle in this project" below — it replaces the skill template's `pytest` steps everywhere.
|
||
- **A clean build emits exactly five warnings** and no more: two `-Wmissing-field-initializers` on `MathNode::source`, one unused `output_built`, and `-Woverlength-strings` on both generated GResource `.c` files. A sixth is a finding. (Note: Task 6 **removes** the `output_built` warning by using the variable; the baseline becomes **four** from Task 6 onward and `CLAUDE.md` must say so — Task 9.)
|
||
- **An exit-dump run emits no `Gtk-CRITICAL` at all.**
|
||
- **Commit to `main`. Never push.**
|
||
- `kNodeW = 132.0`, `kNodeH = 46.0`, `kPort = 10.0` — existing constants, reused throughout.
|
||
- **View scale is clamped to `[0.25, 4.0]`** in one helper, `math_set_scale()`. Three callers: the wheel, Fit, reset.
|
||
- **Hit tolerances are screen distances, divided by `view_scale` before use.** A tolerance left in graph units makes a port unhittable when zoomed out.
|
||
- **`gtk_window_destroy()` must not be used on the math dialog.** It does not emit `close-request`, so the handler that nulls `app.math_dialog` never runs.
|
||
- **Never leave an unverified figure in a comment, a doc or an assertion.** Every number quoted in `CLAUDE.md` or `README.md` must come from a run that was actually performed.
|
||
|
||
### The TDD cycle in this project
|
||
|
||
There is no `pytest`. Each task's "failing test" is a **new `[math-probe]` line in `math_probe()`** (`src/ui.cpp:6585`). The cycle is:
|
||
|
||
1. Add the probe line (with its `want` printed beside the measured value).
|
||
2. Build and run it once; read the **wrong** value. That is the red step.
|
||
3. Implement.
|
||
4. Build and run it once; read the **right** value. That is the green step.
|
||
5. Commit.
|
||
|
||
The run, every time, is two commands. Start the mock in the background, run the app once, **kill the mock**:
|
||
|
||
```bash
|
||
python3 tools/mock_udpstreamer.py --port 8111 --duration 25 &
|
||
MOCK=$!
|
||
xvfb-run -a env UDPSCOPE_ENABLE_ALL_MS=1000 UDPSCOPE_PROBE_MATH_MS=2000 \
|
||
UDPSCOPE_EXIT_AFTER_MS=8000 ./build/udpscope --server 127.0.0.1 --port 8111 \
|
||
2>&1 | grep -E '\[math-probe\]|Gtk-CRITICAL|SIGSEGV'
|
||
kill $MOCK 2>/dev/null
|
||
```
|
||
|
||
**Run one self-check at a time.** Two concurrent `xvfb-run -a` instances land on the same display and the second gets no GL context, which reads as a pipeline failure and is not one. The mock outlives its `--duration`, so always kill it.
|
||
|
||
The build is:
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
## File Structure
|
||
|
||
| File | Responsibility | Change |
|
||
|---|---|---|
|
||
| `src/ui.cpp` | The whole editor: `MathDialog`, the canvas draw, the hit tests, the gestures, the form, the dialog construction, and `math_probe()`. | Modified throughout the math block and the probe. |
|
||
| `src/math_channel.hpp` | `MathOp`, `MathNode`, `MathChannel`, the op metadata accessors. | One addition: `math_op_index(MathOp)` (Task 7). |
|
||
| `src/math_channel.cpp` | The engine and the op metadata. | One addition: `math_op_index()`'s body (Task 7). |
|
||
| `CLAUDE.md` | The probe bullet and the invariants. | Modified (Task 9). |
|
||
| `README.md` | What was built. | Modified (Task 9). |
|
||
| `src/help/11-math.txt` | The user-facing page for the editor. | Modified (Task 9). |
|
||
|
||
`src/ui.cpp` is already by far the largest file in the project and the editor is a coherent block within it; splitting it out is **not** part of this plan. The project's established pattern is one large `ui.cpp`, and a split would make every one of these tasks a merge hazard for no gain.
|
||
|
||
---
|
||
|
||
### Task 1: `math_port_at()` returns the node it found, and Cancel closes
|
||
|
||
This is the bug the user reported as "not possible to add edges", and it is independently shippable: the two-click protocol still works afterwards, it just works *correctly*. The second half is an unrelated live defect found in the same block.
|
||
|
||
**Why it fails today.** `math_port_at()` returns the sentinel `-2` for an output-port hit, discarding **which** node was hit. `math_canvas_pressed()` recovers it with `math_node_at()`, which matches only **inside the box** — but the output square is drawn centred on `x = n.x + kNodeW`, i.e. straddling the right edge, with a `±kPort` (10 px) hit radius against a `±kPort*0.5` (5 px) drawn half-width. So a click on the visible right half of the output square is a port hit whose `math_node_at()` comes back `-1`, and `pending_out = -1` — a **silent disarm**. The first click of the two-click protocol usually does nothing.
|
||
|
||
**Why Cancel is broken.** `src/ui.cpp:3962` connects `gtk_window_destroy` to Cancel. `gtk_window_destroy()` does not emit `close-request`, so the handler that nulls `app.math_dialog` never runs, and every later `open_math_dialog()` takes the "already open" branch and presents a dead widget. `CLAUDE.md` documents this defect as fixed; the fix reached `on_math_apply()` and the probe but not Cancel.
|
||
|
||
**Files:**
|
||
- Modify: `src/ui.cpp:3359-3378` (`math_port_at`)
|
||
- Modify: `src/ui.cpp:3455-3488` (`math_canvas_pressed`)
|
||
- Modify: `src/ui.cpp:3962` (the Cancel connection)
|
||
- Modify: `src/ui.cpp:6688-6743` (the probe, inside the `if (a->math_dialog != nullptr)` block)
|
||
|
||
**Interfaces:**
|
||
- Consumes: nothing from an earlier task.
|
||
- Produces:
|
||
```cpp
|
||
/// A port's identity: the node it belongs to and which port of that node. `which` is
|
||
/// `kPortOut` for the output, 0 or 1 for an input.
|
||
constexpr int kPortOut = -1;
|
||
|
||
/// The port under a canvas position. Returns the **node index**, with `which` set to
|
||
/// `kPortOut` or to the input number; `-1` when no port is under it. Scans topmost-first,
|
||
/// so the node drawn on top wins — the same order as `math_node_at()`.
|
||
int math_port_at(const MathDialog* d, double x, double y, double tol, int& which);
|
||
```
|
||
Tasks 2–5 all call this signature.
|
||
|
||
- [ ] **Step 1: Add the failing probe line**
|
||
|
||
In `src/ui.cpp`, inside `math_probe()`'s `if (a->math_dialog != nullptr)` block — put it immediately **before** the existing `// The editor's two reported failures` drag block, i.e. just after the `[math-probe] reopened:` lines, so it runs on a freshly opened dialog whose node positions are the grid defaults:
|
||
|
||
```cpp
|
||
if (a->math_dialog != nullptr) {
|
||
auto* h = static_cast<MathDialog*>(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog"));
|
||
if (h != nullptr && !h->nodes.empty()) {
|
||
// The output square straddles the box's right edge: it is drawn centred on
|
||
// `n.x + kNodeW` with a 5 px half-width and hit with a 10 px radius. A hit test that
|
||
// recovers the node by a box test therefore misses every pixel past the edge, which
|
||
// is a *silent disarm* of the first click of a wiring gesture.
|
||
double ox = 0.0;
|
||
double oy = 0.0;
|
||
math_output_pos(h->nodes[0], ox, oy);
|
||
int w_in = 0;
|
||
const int n_in = math_port_at(h, ox - 2.0, oy, kPort, w_in);
|
||
int w_out = 0;
|
||
const int n_out = math_port_at(h, ox + 4.0, oy, kPort, w_out);
|
||
std::fprintf(stderr,
|
||
"[math-probe] hit: inside node=%d which=%d, past-edge node=%d which=%d "
|
||
"(want 0 %d 0 %d)\n",
|
||
n_in, w_in, n_out, w_out, kPortOut, kPortOut);
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run it and read the wrong value**
|
||
|
||
It will not compile yet (`kPortOut` and the four-argument `math_port_at` do not exist), which is this project's form of a failing test — the probe names an interface the code has not got. Confirm:
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E "kPortOut|math_port_at" | head -5
|
||
```
|
||
|
||
Expected: errors naming `kPortOut` as undeclared and `math_port_at` as taking 4 arguments where 3 are declared.
|
||
|
||
- [ ] **Step 3: Rewrite `math_port_at()`**
|
||
|
||
Replace `src/ui.cpp:3359-3378` entirely:
|
||
|
||
```cpp
|
||
/// A port's identity: `kPortOut` for a node's output, 0 or 1 for one of its inputs.
|
||
constexpr int kPortOut = -1;
|
||
|
||
/// The port under a canvas position: the **node index** it belongs to, with `which` set to
|
||
/// `kPortOut` or the input number; `-1` when there is no port there. `tol` is the hit radius in
|
||
/// the same units as `x`/`y`.
|
||
///
|
||
/// It returns the node rather than a sentinel because the output square is drawn *straddling* the
|
||
/// box's right edge (centred on `n.x + kNodeW`, half-width `kPort * 0.5`) and hit with a radius of
|
||
/// `kPort` -- so a caller that recovered the node with `math_node_at()`, which matches only inside
|
||
/// the box, got `-1` for every pixel past the edge. That was a silent disarm of the first click of
|
||
/// a wiring gesture, and it is the whole of "edges cannot be added".
|
||
///
|
||
/// Topmost-first, like `math_node_at()`: when two boxes overlap, the one drawn on top owns the port.
|
||
int math_port_at(const MathDialog* d, double x, double y, double tol, int& which) {
|
||
which = kPortOut;
|
||
for (int i = static_cast<int>(d->nodes.size()) - 1; i >= 0; --i) {
|
||
const MathNode& n = d->nodes[static_cast<std::size_t>(i)];
|
||
double px = 0.0;
|
||
double py = 0.0;
|
||
math_output_pos(n, px, py);
|
||
if (std::fabs(x - px) <= tol && std::fabs(y - py) <= tol) {
|
||
which = kPortOut;
|
||
return i;
|
||
}
|
||
for (int k = 0; k < math_op_arity(n.op); ++k) {
|
||
math_input_pos(n, k, px, py);
|
||
if (std::fabs(x - px) <= tol && std::fabs(y - py) <= tol) {
|
||
which = k;
|
||
return i;
|
||
}
|
||
}
|
||
}
|
||
return -1;
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Rewrite `math_canvas_pressed()` for the new signature**
|
||
|
||
Replace `src/ui.cpp:3455-3488` entirely. The two-click protocol is preserved; only the node identification changes. (Task 3 replaces this body again with the drag-based protocol.)
|
||
|
||
```cpp
|
||
void math_canvas_pressed(GtkGestureClick*, int, double x, double y, gpointer user) {
|
||
MathDialog* d = static_cast<MathDialog*>(user);
|
||
int which = kPortOut;
|
||
const int node = math_port_at(d, x, y, kPort, which);
|
||
if (node >= 0 && which == kPortOut) {
|
||
// An output square: arm it. Clicking it again disarms, so a mis-click needs no escape.
|
||
d->pending_out = d->pending_out == node ? -1 : node;
|
||
gtk_widget_queue_draw(d->canvas);
|
||
return;
|
||
}
|
||
if (node >= 0) {
|
||
if (d->pending_out >= 0 && d->pending_out != node) {
|
||
MathNode& n = d->nodes[static_cast<std::size_t>(node)];
|
||
(which == 0 ? n.in_a : n.in_b) = d->pending_out;
|
||
d->pending_out = -1;
|
||
d->selected = node;
|
||
math_dialog_sync(d);
|
||
gtk_widget_queue_draw(d->canvas);
|
||
return;
|
||
}
|
||
d->selected = node;
|
||
math_dialog_sync(d);
|
||
return;
|
||
}
|
||
const int body = math_node_at(d, x, y);
|
||
if (body >= 0) {
|
||
d->selected = body;
|
||
d->pending_out = -1;
|
||
math_dialog_sync(d);
|
||
return;
|
||
}
|
||
d->pending_out = -1;
|
||
gtk_widget_queue_draw(d->canvas);
|
||
}
|
||
```
|
||
|
||
Note `d->pending_out != node` rather than `d->pending_out < node`: a backwards wire is now *written* and left for `math_channel_valid()` to refuse at Apply. Task 5 makes it legal by re-sorting. This is deliberate — it is the state the next tasks build on, and the draw function's `from < i` gate already skips a wire it cannot draw.
|
||
|
||
- [ ] **Step 5: Fix the Cancel button**
|
||
|
||
At `src/ui.cpp:3962`, replace:
|
||
|
||
```cpp
|
||
g_signal_connect_swapped(cancel, "clicked", G_CALLBACK(gtk_window_destroy), window);
|
||
```
|
||
|
||
with:
|
||
|
||
```cpp
|
||
// `gtk_window_close()`, never `gtk_window_destroy()`: only a close emits `close-request`, which
|
||
// is what clears `app.math_dialog`. A destroy left that pointer at a dead widget, and every
|
||
// later open then took the "already open" branch and presented it -- "I cannot edit a math
|
||
// channel again". The same trap the other three windows in this file document.
|
||
g_signal_connect_swapped(cancel, "clicked", G_CALLBACK(gtk_window_close), window);
|
||
```
|
||
|
||
- [ ] **Step 6: Add the Cancel probe line**
|
||
|
||
In `math_probe()`, immediately after the `hit:` block added in Step 1, append:
|
||
|
||
```cpp
|
||
// Cancel must clear the pointer exactly as the close above does: it is the *button* that is
|
||
// clicked here, not the window, because the defect was in what the button was connected to.
|
||
if (a->math_dialog != nullptr) {
|
||
GtkWidget* cancel_button = GTK_WIDGET(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-cancel"));
|
||
if (cancel_button != nullptr) {
|
||
g_signal_emit_by_name(cancel_button, "clicked");
|
||
std::fprintf(stderr, "[math-probe] cancel: dialog=%s (want cleared)\n",
|
||
a->math_dialog == nullptr ? "cleared" : "STILL SET");
|
||
open_math_dialog(*a, 0);
|
||
} else {
|
||
std::fprintf(stderr, "[math-probe] cancel: NO BUTTON (the data key is not set)\n");
|
||
}
|
||
}
|
||
```
|
||
|
||
The probe reaches the button through a data key, so set it where the button is built. At `src/ui.cpp:3962` (just above the `g_signal_connect_swapped` from Step 5), add:
|
||
|
||
```cpp
|
||
// Named so the headless probe can click the *button* rather than call its handler: the defect
|
||
// this guards was in the connection, which a direct call cannot see.
|
||
g_object_set_data(G_OBJECT(window), "udpscope-math-cancel", cancel);
|
||
```
|
||
|
||
The `open_math_dialog(*a, 0)` at the end restores a dialog for the drag/sources/rows blocks that follow.
|
||
|
||
- [ ] **Step 7: Build and run; read the right values**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
Expected: the five documented warnings, no errors.
|
||
|
||
```bash
|
||
python3 tools/mock_udpstreamer.py --port 8111 --duration 25 &
|
||
MOCK=$!
|
||
xvfb-run -a env UDPSCOPE_ENABLE_ALL_MS=1000 UDPSCOPE_PROBE_MATH_MS=2000 \
|
||
UDPSCOPE_EXIT_AFTER_MS=8000 ./build/udpscope --server 127.0.0.1 --port 8111 \
|
||
2>&1 | grep -E '\[math-probe\]|Gtk-CRITICAL'
|
||
kill $MOCK 2>/dev/null
|
||
```
|
||
|
||
Expected, among the existing lines:
|
||
|
||
```
|
||
[math-probe] hit: inside node=0 which=-1, past-edge node=0 which=-1 (want 0 -1 0 -1)
|
||
[math-probe] cancel: dialog=cleared (want cleared)
|
||
```
|
||
|
||
and **no `Gtk-CRITICAL`**. The `past-edge node=0` is the assertion: before the fix it read `-1`.
|
||
|
||
- [ ] **Step 8: Commit**
|
||
|
||
```bash
|
||
git add src/ui.cpp
|
||
git commit -m "$(cat <<'EOF'
|
||
fix: a port hit names its own node, so the first click of a wire lands
|
||
|
||
`math_port_at()` returned a sentinel for an output hit and the caller
|
||
recovered the node with a box test -- but the output square straddles the
|
||
box's right edge, so every pixel past it disarmed the gesture silently.
|
||
That is the whole of "edges cannot be added".
|
||
|
||
Cancel also closed with `gtk_window_destroy()`, which emits no
|
||
`close-request`, so the dialog pointer was left dangling and no channel
|
||
could be edited twice.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 2: The view transform — zoom and pan
|
||
|
||
The second reported failure. A single transform applied to the cairo context, with the inverse used by every hit test.
|
||
|
||
**Files:**
|
||
- Modify: `src/ui.cpp:3290-3319` (`MathDialog`: three fields)
|
||
- Modify: `src/ui.cpp:3330` area (add `math_to_graph`, `math_set_scale`, `math_view_fit`, `math_view_reset`)
|
||
- Modify: `src/ui.cpp:3380-3453` (`math_canvas_draw`: wrap in the transform)
|
||
- Modify: `src/ui.cpp:3455-3512` (the three gesture handlers: convert through the inverse)
|
||
- Modify: `src/ui.cpp:3807-3894` area (the canvas's controllers: scroll and key)
|
||
- Modify: `src/ui.cpp:3946-3972` area (a **Fit** button beside `add node` / `delete node` / `set as output`)
|
||
- Modify: `src/ui.cpp:6688+` (the probe: a `view:` block)
|
||
|
||
**Interfaces:**
|
||
- Consumes: `math_port_at(d, x, y, tol, which)` and `kPortOut` from Task 1.
|
||
- Produces:
|
||
```cpp
|
||
/// Canvas pixels per graph unit, and where graph (0,0) is drawn, in canvas pixels.
|
||
double view_scale = 1.0;
|
||
double view_x = 0.0;
|
||
double view_y = 0.0;
|
||
|
||
/// A canvas position in graph coordinates. The inverse of the transform `math_canvas_draw()`
|
||
/// applies, and the only way a hit test may read the pointer.
|
||
void math_to_graph(const MathDialog* d, double cx, double cy, double& gx, double& gy);
|
||
|
||
/// Sets the scale, clamped to [0.25, 4.0]. Returns what it actually set.
|
||
double math_set_scale(MathDialog* d, double want);
|
||
|
||
/// Frames every node in the canvas, centred.
|
||
void math_view_fit(MathDialog* d);
|
||
|
||
/// Scale 1, offsets 0.
|
||
void math_view_reset(MathDialog* d);
|
||
```
|
||
Tasks 3–5 convert the pointer through `math_to_graph()` and size their tolerances as `kPort / d->view_scale`.
|
||
|
||
- [ ] **Step 1: Add the failing probe lines**
|
||
|
||
In `math_probe()`, after the `cancel:` block from Task 1, append:
|
||
|
||
```cpp
|
||
if (a->math_dialog != nullptr) {
|
||
auto* v = static_cast<MathDialog*>(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog"));
|
||
if (v != nullptr && !v->nodes.empty()) {
|
||
// A wheel zoom must keep the graph point under the pointer under the pointer: that is the
|
||
// one thing a zoom can get wrong without looking broken, and it reads as the canvas
|
||
// sliding away from the hand.
|
||
const double px = 180.0;
|
||
const double py = 140.0;
|
||
double gx0 = 0.0;
|
||
double gy0 = 0.0;
|
||
math_to_graph(v, px, py, gx0, gy0);
|
||
math_canvas_scrolled(nullptr, 0.0, -1.0, v);
|
||
math_canvas_scrolled(nullptr, 0.0, -1.0, v);
|
||
double gx1 = 0.0;
|
||
double gy1 = 0.0;
|
||
math_to_graph(v, px, py, gx1, gy1);
|
||
std::fprintf(stderr,
|
||
"[math-probe] view: zoom scale=%.3f anchor kept dx=%.4f dy=%.4f "
|
||
"(want 0.0000 0.0000)\n",
|
||
v->view_scale, gx1 - gx0, gy1 - gy0);
|
||
|
||
// And the hit tests must go through the inverse. At the identity view a test that forgot
|
||
// it passes anyway, so the port is probed *after* a zoom and a pan -- which is the only
|
||
// arrangement that can fail.
|
||
v->view_x += 37.0;
|
||
v->view_y -= 19.0;
|
||
double ox = 0.0;
|
||
double oy = 0.0;
|
||
math_output_pos(v->nodes[0], ox, oy);
|
||
const double cx = ox * v->view_scale + v->view_x;
|
||
const double cy = oy * v->view_scale + v->view_y;
|
||
int which = 0;
|
||
const int node = math_port_at_canvas(v, cx + 3.0, cy, which);
|
||
std::fprintf(stderr,
|
||
"[math-probe] view: pan+zoom hit node=%d which=%d (want 0 %d)\n", node,
|
||
which, kPortOut);
|
||
|
||
math_view_fit(v);
|
||
int inside = 1;
|
||
int w_px = gtk_widget_get_width(v->canvas);
|
||
int h_px = gtk_widget_get_height(v->canvas);
|
||
if (w_px <= 0) w_px = 420;
|
||
if (h_px <= 0) h_px = 300;
|
||
for (const MathNode& n : v->nodes) {
|
||
const double x0 = n.x * v->view_scale + v->view_x;
|
||
const double y0 = n.y * v->view_scale + v->view_y;
|
||
const double x1 = (n.x + kNodeW) * v->view_scale + v->view_x;
|
||
const double y1 = (n.y + kNodeH) * v->view_scale + v->view_y;
|
||
if (x0 < -0.5 || y0 < -0.5 || x1 > w_px + 0.5 || y1 > h_px + 0.5) inside = 0;
|
||
}
|
||
std::fprintf(stderr, "[math-probe] view: fit scale=%.3f all_inside=%d (want 1)\n",
|
||
v->view_scale, inside);
|
||
|
||
math_view_reset(v);
|
||
std::fprintf(stderr,
|
||
"[math-probe] view: reset scale=%.3f offset=(%.1f,%.1f) "
|
||
"(want 1.000 (0.0,0.0))\n",
|
||
v->view_scale, v->view_x, v->view_y);
|
||
}
|
||
}
|
||
```
|
||
|
||
`math_port_at_canvas()` is a thin wrapper the next step adds, so that the probe — and every gesture handler — converts in exactly one place.
|
||
|
||
- [ ] **Step 2: Run it and read the wrong value**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'math_to_graph|math_canvas_scrolled|math_view_fit|view_scale' | head -8
|
||
```
|
||
|
||
Expected: errors naming `math_to_graph`, `math_canvas_scrolled`, `math_port_at_canvas`, `math_view_fit`, `math_view_reset` and `MathDialog::view_scale` as undeclared.
|
||
|
||
- [ ] **Step 3: Add the three fields to `MathDialog`**
|
||
|
||
In `src/ui.cpp`, inside `struct MathDialog` (after `int drag_node = -1;`):
|
||
|
||
```cpp
|
||
/// The view: canvas pixels per graph unit, and where graph (0,0) is drawn. Applied once to the
|
||
/// cairo context by `math_canvas_draw()`; every hit test runs the inverse (`math_to_graph()`)
|
||
/// rather than comparing a pointer position against a graph one, and sizes its tolerance in
|
||
/// *screen* pixels (`kPort / view_scale`) -- a tolerance left in graph units shrinks with the
|
||
/// view and makes a port unhittable zoomed out.
|
||
double view_scale = 1.0;
|
||
double view_x = 0.0;
|
||
double view_y = 0.0;
|
||
```
|
||
|
||
- [ ] **Step 4: Add the view helpers**
|
||
|
||
In `src/ui.cpp`, immediately after `math_output_pos()` (currently ending at `3349`):
|
||
|
||
```cpp
|
||
/// The smallest and largest the canvas may be scaled to. Below a quarter the op names are
|
||
/// unreadable; above four a box fills the canvas and there is nothing left to navigate.
|
||
constexpr double kMathScaleMin = 0.25;
|
||
constexpr double kMathScaleMax = 4.0;
|
||
constexpr double kMathZoomStep = 1.1;
|
||
|
||
/// A canvas position in graph coordinates: the inverse of the transform the draw applies.
|
||
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;
|
||
}
|
||
|
||
/// Sets the scale, clamped. One helper for all three callers (the wheel, Fit, reset) so the bounds
|
||
/// cannot drift apart. Returns what it actually set.
|
||
double math_set_scale(MathDialog* d, double want) {
|
||
d->view_scale = std::clamp(want, kMathScaleMin, kMathScaleMax);
|
||
return d->view_scale;
|
||
}
|
||
|
||
void math_view_reset(MathDialog* d) {
|
||
math_set_scale(d, 1.0);
|
||
d->view_x = 0.0;
|
||
d->view_y = 0.0;
|
||
if (d->canvas != nullptr) gtk_widget_queue_draw(d->canvas);
|
||
}
|
||
|
||
/// Frames every node in the canvas, centred, at a scale that fits with a little margin.
|
||
void math_view_fit(MathDialog* d) {
|
||
if (d->nodes.empty()) {
|
||
math_view_reset(d);
|
||
return;
|
||
}
|
||
double x0 = d->nodes[0].x;
|
||
double y0 = d->nodes[0].y;
|
||
double x1 = x0 + kNodeW;
|
||
double y1 = y0 + kNodeH;
|
||
for (const MathNode& n : d->nodes) {
|
||
x0 = std::min(x0, n.x);
|
||
y0 = std::min(y0, n.y);
|
||
x1 = std::max(x1, n.x + kNodeW);
|
||
y1 = std::max(y1, n.y + kNodeH);
|
||
}
|
||
// The port squares stick out past the box on both sides, and a wire needs a little air.
|
||
constexpr double kPad = 16.0;
|
||
x0 -= kPad;
|
||
y0 -= kPad;
|
||
x1 += kPad;
|
||
y1 += kPad;
|
||
double w_px = d->canvas != nullptr ? gtk_widget_get_width(d->canvas) : 0;
|
||
double h_px = d->canvas != nullptr ? gtk_widget_get_height(d->canvas) : 0;
|
||
// Before the first allocation the widget has no size; its size request is the honest fallback.
|
||
if (w_px <= 0.0) w_px = 420.0;
|
||
if (h_px <= 0.0) h_px = 300.0;
|
||
const double s = math_set_scale(d, std::min(w_px / (x1 - x0), h_px / (y1 - y0)));
|
||
d->view_x = (w_px - (x1 - x0) * s) * 0.5 - x0 * s;
|
||
d->view_y = (h_px - (y1 - y0) * s) * 0.5 - y0 * s;
|
||
if (d->canvas != nullptr) gtk_widget_queue_draw(d->canvas);
|
||
}
|
||
|
||
/// The port under a *canvas* position, with the tolerance in screen pixels. Every gesture goes
|
||
/// through this rather than converting for itself: `kPort` is a size on screen, so the graph-space
|
||
/// tolerance has to shrink as the view grows, and a handler that got that backwards makes a port
|
||
/// unhittable at one end of the zoom range and greedy at the other.
|
||
int math_port_at_canvas(const MathDialog* d, double cx, double cy, int& which) {
|
||
double gx = 0.0;
|
||
double gy = 0.0;
|
||
math_to_graph(d, cx, cy, gx, gy);
|
||
return math_port_at(d, gx, gy, kPort / d->view_scale, which);
|
||
}
|
||
|
||
/// The node body under a canvas position.
|
||
int math_node_at_canvas(const MathDialog* d, double cx, double cy) {
|
||
double gx = 0.0;
|
||
double gy = 0.0;
|
||
math_to_graph(d, cx, cy, gx, gy);
|
||
return math_node_at(d, gx, gy);
|
||
}
|
||
```
|
||
|
||
`math_port_at_canvas` and `math_node_at_canvas` must be declared **after** `math_port_at` and `math_node_at`; `math_port_at` is at `3361` and `math_node_at` at `3351`, both above `3349`'s insertion point — so move this whole block to immediately **after** `math_port_at()`'s closing brace instead, and keep `math_to_graph`/`math_set_scale`/`math_view_*` with it. (`math_view_fit` reads `kNodeW`, declared at `3286`, so the order is fine.)
|
||
|
||
- [ ] **Step 5: Apply the transform in the draw**
|
||
|
||
In `math_canvas_draw()`, after the background paint and before the wire loop:
|
||
|
||
```cpp
|
||
cairo_set_source_rgb(cr, 0.06, 0.078, 0.094);
|
||
cairo_paint(cr);
|
||
// One transform for the whole graph. Everything below is in graph units; every hit test runs
|
||
// `math_to_graph()`, which is this inverted.
|
||
cairo_save(cr);
|
||
cairo_translate(cr, d->view_x, d->view_y);
|
||
cairo_scale(cr, d->view_scale, d->view_scale);
|
||
```
|
||
|
||
and at the end of the function, before the `(void)width;` lines:
|
||
|
||
```cpp
|
||
cairo_restore(cr);
|
||
```
|
||
|
||
The line widths inside are now in graph units and so scale with the view, which is correct: a zoomed-in graph should have thicker wires, not hairlines.
|
||
|
||
- [ ] **Step 6: Convert the gesture handlers**
|
||
|
||
In `math_canvas_pressed()`, replace the two hit-test calls:
|
||
|
||
```cpp
|
||
int which = kPortOut;
|
||
const int node = math_port_at_canvas(d, x, y, which);
|
||
```
|
||
|
||
and
|
||
|
||
```cpp
|
||
const int body = math_node_at_canvas(d, x, y);
|
||
```
|
||
|
||
In `math_canvas_drag_begin()`, replace:
|
||
|
||
```cpp
|
||
const int node = math_node_at(d, x, y);
|
||
```
|
||
|
||
with:
|
||
|
||
```cpp
|
||
const int node = math_node_at_canvas(d, x, y);
|
||
```
|
||
|
||
In `math_canvas_drag()`, the offset arrives in **canvas** pixels and must be divided by the scale before it moves a graph position:
|
||
|
||
```cpp
|
||
// The gesture's offset is in canvas pixels; a node's position is in graph units.
|
||
n.x = std::max(0.0, d->grab_x + dx / d->view_scale);
|
||
n.y = std::max(0.0, d->grab_y + dy / d->view_scale);
|
||
```
|
||
|
||
- [ ] **Step 7: Add the scroll handler and the key controller**
|
||
|
||
Add, after `math_canvas_drag()`:
|
||
|
||
```cpp
|
||
gboolean math_canvas_scrolled(GtkEventControllerScroll* scroll, double, double dy, gpointer user) {
|
||
MathDialog* d = static_cast<MathDialog*>(user);
|
||
if (dy == 0.0) return TRUE;
|
||
// About the pointer: the graph point under it must still be under it afterwards, or the canvas
|
||
// slides away from the hand. With no controller (the headless probe calls this directly) there
|
||
// is no pointer to read, so the canvas centre is the anchor.
|
||
double px = 0.0;
|
||
double py = 0.0;
|
||
bool have = false;
|
||
if (scroll != nullptr) {
|
||
GdkEvent* event = gtk_event_controller_get_current_event(GTK_EVENT_CONTROLLER(scroll));
|
||
if (event != nullptr) have = gdk_event_get_position(event, &px, &py);
|
||
}
|
||
if (!have) {
|
||
px = d->canvas != nullptr ? gtk_widget_get_width(d->canvas) * 0.5 : 210.0;
|
||
py = d->canvas != nullptr ? gtk_widget_get_height(d->canvas) * 0.5 : 150.0;
|
||
}
|
||
double gx = 0.0;
|
||
double gy = 0.0;
|
||
math_to_graph(d, px, py, gx, gy);
|
||
const double s = math_set_scale(d, d->view_scale * (dy < 0.0 ? kMathZoomStep
|
||
: 1.0 / kMathZoomStep));
|
||
d->view_x = px - gx * s;
|
||
d->view_y = py - gy * s;
|
||
gtk_widget_queue_draw(d->canvas);
|
||
return TRUE;
|
||
}
|
||
|
||
gboolean math_canvas_key(GtkEventControllerKey*, guint keyval, guint, GdkModifierType state,
|
||
gpointer user) {
|
||
MathDialog* d = static_cast<MathDialog*>(user);
|
||
const bool ctrl = (state & GDK_CONTROL_MASK) != 0;
|
||
if (ctrl && (keyval == GDK_KEY_0 || keyval == GDK_KEY_KP_0)) {
|
||
math_view_reset(d);
|
||
return TRUE;
|
||
}
|
||
if (keyval == GDK_KEY_f || keyval == GDK_KEY_F) {
|
||
math_view_fit(d);
|
||
return TRUE;
|
||
}
|
||
return FALSE;
|
||
}
|
||
```
|
||
|
||
Task 3 adds `Escape` and Task 4 adds `Delete` to `math_canvas_key()`.
|
||
|
||
In the dialog construction (after the existing `GtkGestureDrag` is attached to `d->canvas`, around `3860`):
|
||
|
||
```cpp
|
||
// The wheel zooms. A scroll controller is the only way to get one on a drawing area, and
|
||
// `VERTICAL` is what a wheel reports.
|
||
GtkEventController* zoom = gtk_event_controller_scroll_new(GTK_EVENT_CONTROLLER_SCROLL_VERTICAL);
|
||
g_signal_connect(zoom, "scroll", G_CALLBACK(math_canvas_scrolled), d);
|
||
gtk_widget_add_controller(d->canvas, zoom);
|
||
|
||
// A `GtkDrawingArea` is not focusable by default, so a key controller on it never fires. It
|
||
// takes focus on a press rather than only on Tab, because every key this editor binds is about
|
||
// something the pointer just did.
|
||
gtk_widget_set_focusable(d->canvas, TRUE);
|
||
GtkEventController* keys = gtk_event_controller_key_new();
|
||
g_signal_connect(keys, "key-pressed", G_CALLBACK(math_canvas_key), d);
|
||
gtk_widget_add_controller(d->canvas, keys);
|
||
```
|
||
|
||
and at the **top** of `math_canvas_pressed()`:
|
||
|
||
```cpp
|
||
MathDialog* d = static_cast<MathDialog*>(user);
|
||
// The key controller is on the canvas, so the canvas has to hold focus for `Delete`, `Escape`
|
||
// and `Ctrl+0` to reach it.
|
||
gtk_widget_grab_focus(d->canvas);
|
||
```
|
||
|
||
- [ ] **Step 8: Add the Fit button**
|
||
|
||
Beside the existing `add node` / `delete node` / `set as output` buttons (around `3946`):
|
||
|
||
```cpp
|
||
GtkWidget* fit = gtk_button_new_with_label("fit");
|
||
gtk_widget_set_tooltip_text(fit, "Frame the whole graph (f). Ctrl+0 resets the zoom.");
|
||
g_signal_connect(fit, "clicked", G_CALLBACK(+[](GtkButton*, gpointer user) {
|
||
math_view_fit(static_cast<MathDialog*>(user));
|
||
}), d);
|
||
gtk_box_append(GTK_BOX(tools), fit);
|
||
```
|
||
|
||
Use whatever box the three existing buttons are appended to; read the surrounding lines for its name rather than assuming `tools`.
|
||
|
||
- [ ] **Step 9: Build and run; read the right values**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
Expected: the five documented warnings, no errors.
|
||
|
||
Then the probe run from "The TDD cycle" above. Expected:
|
||
|
||
```
|
||
[math-probe] view: zoom scale=1.210 anchor kept dx=0.0000 dy=0.0000 (want 0.0000 0.0000)
|
||
[math-probe] view: pan+zoom hit node=0 which=-1 (want 0 -1)
|
||
[math-probe] view: fit scale=... all_inside=1 (want 1)
|
||
[math-probe] view: reset scale=1.000 offset=(0.0,0.0) (want 1.000 (0.0,0.0))
|
||
```
|
||
|
||
The `fit` scale depends on the canvas allocation and the node spread, so it is **not** an assertion — `all_inside=1` is. `scale=1.210` is `1.1²`; read it as "two steps happened", not as a fixed figure.
|
||
|
||
- [ ] **Step 10: Commit**
|
||
|
||
```bash
|
||
git add src/ui.cpp
|
||
git commit -m "$(cat <<'EOF'
|
||
feat: the node canvas zooms about the pointer and pans, so a graph can be read
|
||
|
||
One view transform applied to the cairo context, with `math_to_graph()` as
|
||
the inverse that every hit test runs -- a tolerance is a screen distance,
|
||
so it is divided by the scale rather than left in graph units. A wheel
|
||
zooms about the pointer, a background drag pans (next commit), `f` fits and
|
||
Ctrl+0 resets. The drawing area is made focusable, without which a key
|
||
controller on it never fires.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 3: The gesture state machine — drag a wire, drag the background to pan
|
||
|
||
Replaces the two-click wiring protocol with the one the user chose: drag from a port to a port. An input port that is already wired is dragged *off*, so detaching needs no separate verb. `pending_out` and the armed-port highlight are deleted.
|
||
|
||
**Files:**
|
||
- Modify: `src/ui.cpp:3290-3319` (`MathDialog`: `MathDrag`, the wire fields; delete `pending_out`)
|
||
- Modify: `src/ui.cpp:3380-3453` (`math_canvas_draw`: the live wire; the armed highlight goes)
|
||
- Modify: `src/ui.cpp:3455-3488` (`math_canvas_pressed`: select only)
|
||
- Modify: `src/ui.cpp:3490-3512` (`math_canvas_drag_begin`, `math_canvas_drag`; add `math_canvas_drag_end`)
|
||
- Modify: `math_canvas_key` (add `Escape`)
|
||
- Modify: the dialog construction (connect `drag-end`)
|
||
- Modify: `src/ui.cpp:6688+` (the probe: a `wire:`/`detach:` block)
|
||
|
||
**Interfaces:**
|
||
- Consumes: `math_port_at_canvas()`, `math_node_at_canvas()`, `math_to_graph()`, `view_scale`, `kPortOut` (Tasks 1–2).
|
||
- Produces:
|
||
```cpp
|
||
/// What a drag on the canvas is doing. Decided once at `drag-begin` from what is under the
|
||
/// press, and not revisited: a gesture that changes its mind mid-drag is unusable.
|
||
enum class MathDrag { None, Node, Wire, Pan };
|
||
|
||
MathDrag drag_mode = MathDrag::None;
|
||
int wire_from = -1; ///< `Wire`: the node whose output is being dragged.
|
||
int wire_detach = -1; ///< `Wire`: the consumer whose input was emptied to start it, or -1.
|
||
int wire_detach_which = 0;
|
||
double wire_x = 0.0; ///< `Wire`: the loose end, in graph units.
|
||
double wire_y = 0.0;
|
||
double pan_x0 = 0.0; ///< `Pan`: `view_x`/`view_y` when the gesture began.
|
||
double pan_y0 = 0.0;
|
||
```
|
||
Task 4 adds `selected_wire`; Task 5 replaces this task's direct `in_a`/`in_b` write with `math_connect()`.
|
||
|
||
- [ ] **Step 1: Add the failing probe lines**
|
||
|
||
In `math_probe()`, after the `view:` block from Task 2, append:
|
||
|
||
```cpp
|
||
if (a->math_dialog != nullptr) {
|
||
auto* g = static_cast<MathDialog*>(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog"));
|
||
// The editor's two graphs are source -> gain, so node 1 has one input and node 0 an output.
|
||
if (g != nullptr && g->nodes.size() >= 2) {
|
||
math_view_reset(g);
|
||
const auto canvas_of = [&](double gx, double gy, double& cx, double& cy) {
|
||
cx = gx * g->view_scale + g->view_x;
|
||
cy = gy * g->view_scale + g->view_y;
|
||
};
|
||
double ox = 0.0;
|
||
double oy = 0.0;
|
||
double ix = 0.0;
|
||
double iy = 0.0;
|
||
math_output_pos(g->nodes[0], ox, oy);
|
||
math_input_pos(g->nodes[1], 0, ix, iy);
|
||
double ocx = 0.0;
|
||
double ocy = 0.0;
|
||
double icx = 0.0;
|
||
double icy = 0.0;
|
||
canvas_of(ox, oy, ocx, ocy);
|
||
canvas_of(ix, iy, icx, icy);
|
||
|
||
// Connect: press the output, release over the input.
|
||
g->nodes[1].in_a = -1;
|
||
math_canvas_drag_begin(nullptr, ocx, ocy, g);
|
||
const int mode_wire = static_cast<int>(g->drag_mode);
|
||
math_canvas_drag(nullptr, icx - ocx, icy - ocy, g);
|
||
math_canvas_drag_end(nullptr, icx - ocx, icy - ocy, g);
|
||
std::fprintf(stderr,
|
||
"[math-probe] wire: connect mode=%d in_a=%d (want %d 0)\n", mode_wire,
|
||
g->nodes[1].in_a, static_cast<int>(MathDrag::Wire));
|
||
|
||
// Detach: a drag that starts on a *wired* input empties it at once, so a release over
|
||
// nothing is the detach. Dropping it back is what the connect above already covers.
|
||
math_canvas_drag_begin(nullptr, icx, icy, g);
|
||
const int emptied = g->nodes[1].in_a;
|
||
math_canvas_drag(nullptr, 0.0, 90.0, g);
|
||
math_canvas_drag_end(nullptr, 0.0, 90.0, g);
|
||
std::fprintf(stderr,
|
||
"[math-probe] detach: emptied_at_begin=%d in_a=%d (want -1 -1)\n", emptied,
|
||
g->nodes[1].in_a);
|
||
|
||
// And Escape during the same gesture must put it back.
|
||
g->nodes[1].in_a = 0;
|
||
math_canvas_drag_begin(nullptr, icx, icy, g);
|
||
math_canvas_drag(nullptr, 0.0, 90.0, g);
|
||
math_canvas_key(nullptr, GDK_KEY_Escape, 0, static_cast<GdkModifierType>(0), g);
|
||
std::fprintf(stderr, "[math-probe] detach: escape restores in_a=%d (want 0)\n",
|
||
g->nodes[1].in_a);
|
||
|
||
// A drag that begins on the background pans and moves nothing.
|
||
const double was_x = g->view_x;
|
||
const double node_x = g->nodes[0].x;
|
||
math_canvas_drag_begin(nullptr, 5.0, 5.0, g);
|
||
const int mode_pan = static_cast<int>(g->drag_mode);
|
||
math_canvas_drag(nullptr, 30.0, 0.0, g);
|
||
math_canvas_drag_end(nullptr, 30.0, 0.0, g);
|
||
std::fprintf(stderr,
|
||
"[math-probe] view: pan mode=%d dview=%.1f dnode=%.1f (want %d 30.0 0.0)\n",
|
||
mode_pan, g->view_x - was_x, g->nodes[0].x - node_x,
|
||
static_cast<int>(MathDrag::Pan));
|
||
math_view_reset(g);
|
||
}
|
||
}
|
||
```
|
||
|
||
The press at `(5.0, 5.0)` is the background only while the view is reset and the first node sits at the grid default `x = 24`; `math_view_reset(g)` above it is what guarantees that.
|
||
|
||
- [ ] **Step 2: Run it and read the wrong value**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'MathDrag|drag_mode|math_canvas_drag_end' | head -6
|
||
```
|
||
|
||
Expected: errors naming `MathDrag`, `MathDialog::drag_mode` and `math_canvas_drag_end` as undeclared.
|
||
|
||
- [ ] **Step 3: Replace `pending_out` with the state machine fields**
|
||
|
||
In `struct MathDialog`, **delete**:
|
||
|
||
```cpp
|
||
/// The node whose output square was clicked and is waiting for an input to connect to.
|
||
int pending_out = -1;
|
||
```
|
||
|
||
and add, beside `drag_node`:
|
||
|
||
```cpp
|
||
/// What a drag is doing, decided once at `drag-begin` from what is under the press and not
|
||
/// revisited: a gesture that changes its mind mid-drag is unusable. `Wire` carries a loose end
|
||
/// in graph units, which the draw shows so the user can see what is being connected.
|
||
MathDrag drag_mode = MathDrag::None;
|
||
int wire_from = -1;
|
||
int wire_detach = -1;
|
||
int wire_detach_which = 0;
|
||
double wire_x = 0.0;
|
||
double wire_y = 0.0;
|
||
double pan_x0 = 0.0;
|
||
double pan_y0 = 0.0;
|
||
```
|
||
|
||
Declare the enum immediately **above** `struct MathDialog`:
|
||
|
||
```cpp
|
||
enum class MathDrag { None, Node, Wire, Pan };
|
||
```
|
||
|
||
- [ ] **Step 4: Rewrite the three drag handlers and add `drag-end`**
|
||
|
||
Replace `src/ui.cpp:3490-3512` (`math_canvas_drag_begin` and `math_canvas_drag`) with:
|
||
|
||
```cpp
|
||
void math_canvas_drag_begin(GtkGestureDrag*, double x, double y, gpointer user) {
|
||
MathDialog* d = static_cast<MathDialog*>(user);
|
||
d->drag_mode = MathDrag::None;
|
||
d->drag_node = -1;
|
||
d->wire_from = -1;
|
||
d->wire_detach = -1;
|
||
// Precedence: a port, then a box, then the background. A port sticks out past the box edge, so
|
||
// asking the box first would make the output square unreachable -- the same geometry that made
|
||
// the old two-click protocol disarm itself.
|
||
int which = kPortOut;
|
||
const int port = math_port_at_canvas(d, x, y, which);
|
||
if (port >= 0) {
|
||
d->drag_mode = MathDrag::Wire;
|
||
double gx = 0.0;
|
||
double gy = 0.0;
|
||
math_to_graph(d, x, y, gx, gy);
|
||
d->wire_x = gx;
|
||
d->wire_y = gy;
|
||
if (which == kPortOut) {
|
||
d->wire_from = port;
|
||
} else {
|
||
// A wired input is dragged *off*: the input is emptied now, so releasing over nothing is
|
||
// itself the detach and needs no second verb. `Escape` puts it back.
|
||
MathNode& n = d->nodes[static_cast<std::size_t>(port)];
|
||
const int from = which == 0 ? n.in_a : n.in_b;
|
||
if (from < 0) {
|
||
d->drag_mode = MathDrag::None;
|
||
return;
|
||
}
|
||
d->wire_from = from;
|
||
d->wire_detach = port;
|
||
d->wire_detach_which = which;
|
||
(which == 0 ? n.in_a : n.in_b) = -1;
|
||
}
|
||
d->selected = d->wire_from;
|
||
math_dialog_sync(d);
|
||
gtk_widget_queue_draw(d->canvas);
|
||
return;
|
||
}
|
||
const int node = math_node_at_canvas(d, x, y);
|
||
if (node >= 0) {
|
||
d->drag_mode = MathDrag::Node;
|
||
d->drag_node = node;
|
||
d->selected = node;
|
||
d->grab_x = d->nodes[static_cast<std::size_t>(node)].x;
|
||
d->grab_y = d->nodes[static_cast<std::size_t>(node)].y;
|
||
math_dialog_sync(d);
|
||
return;
|
||
}
|
||
d->drag_mode = MathDrag::Pan;
|
||
d->pan_x0 = d->view_x;
|
||
d->pan_y0 = d->view_y;
|
||
}
|
||
|
||
void math_canvas_drag(GtkGestureDrag*, double dx, double dy, gpointer user) {
|
||
MathDialog* d = static_cast<MathDialog*>(user);
|
||
// The gesture reports the *total* offset from where it began, so every branch adds it to a value
|
||
// recorded at `drag-begin` and never to one a previous update wrote. Feeding a delta back into
|
||
// the origin is what made a box fly off the canvas.
|
||
switch (d->drag_mode) {
|
||
case MathDrag::Node: {
|
||
if (d->drag_node < 0 || d->drag_node >= static_cast<int>(d->nodes.size())) return;
|
||
MathNode& n = d->nodes[static_cast<std::size_t>(d->drag_node)];
|
||
// The offset is in canvas pixels and a position is in graph units.
|
||
n.x = std::max(0.0, d->grab_x + dx / d->view_scale);
|
||
n.y = std::max(0.0, d->grab_y + dy / d->view_scale);
|
||
break;
|
||
}
|
||
case MathDrag::Wire: {
|
||
// The loose end, so the user can see what is being connected.
|
||
double gx = 0.0;
|
||
double gy = 0.0;
|
||
math_to_graph(d, d->wire_x * d->view_scale + d->view_x + dx,
|
||
d->wire_y * d->view_scale + d->view_y + dy, gx, gy);
|
||
d->wire_x = gx;
|
||
d->wire_y = gy;
|
||
break;
|
||
}
|
||
case MathDrag::Pan:
|
||
d->view_x = d->pan_x0 + dx;
|
||
d->view_y = d->pan_y0 + dy;
|
||
break;
|
||
case MathDrag::None:
|
||
return;
|
||
}
|
||
gtk_widget_queue_draw(d->canvas);
|
||
}
|
||
|
||
void math_canvas_drag_end(GtkGestureDrag*, double dx, double dy, gpointer user) {
|
||
MathDialog* d = static_cast<MathDialog*>(user);
|
||
if (d->drag_mode != MathDrag::Wire) {
|
||
d->drag_mode = MathDrag::None;
|
||
d->drag_node = -1;
|
||
return;
|
||
}
|
||
// Where the release landed, recomputed from the gesture's own offset rather than from `wire_x`:
|
||
// `drag-end` carries the whole offset, and the update that would have moved `wire_x` to it is
|
||
// not guaranteed to have arrived.
|
||
double ox = 0.0;
|
||
double oy = 0.0;
|
||
math_output_pos(d->nodes[static_cast<std::size_t>(d->wire_from)], ox, oy);
|
||
int which = kPortOut;
|
||
const int target = math_port_at_canvas(d, ox * d->view_scale + d->view_x + dx,
|
||
oy * d->view_scale + d->view_y + dy, which);
|
||
if (target >= 0 && which != kPortOut && target != d->wire_from) {
|
||
MathNode& n = d->nodes[static_cast<std::size_t>(target)];
|
||
(which == 0 ? n.in_a : n.in_b) = d->wire_from;
|
||
d->selected = target;
|
||
math_dialog_sync(d);
|
||
}
|
||
// A release over nothing leaves the input the drag emptied empty: that *is* the detach.
|
||
d->drag_mode = MathDrag::None;
|
||
d->wire_from = -1;
|
||
d->wire_detach = -1;
|
||
gtk_widget_queue_draw(d->canvas);
|
||
}
|
||
```
|
||
|
||
The `drag-end` release point is recomputed from the output port rather than reusing `wire_x` because `wire_x` was seeded at the *press* position, which may be a few pixels off the port centre; the release has to be measured from the same origin the offset is relative to. For a detach drag `wire_from` is the original source, so that origin is still a real port.
|
||
|
||
`math_connect()` (Task 5) replaces the direct `in_a`/`in_b` write above; until then a backwards wire is written and refused at Apply, exactly as in Task 1.
|
||
|
||
- [ ] **Step 5: `Escape` restores a detach**
|
||
|
||
In `math_canvas_key()`, before the `return FALSE;`:
|
||
|
||
```cpp
|
||
if (keyval == GDK_KEY_Escape) {
|
||
// A detach empties the input at `drag-begin`, so the undo is to put it back. Anything else is
|
||
// a cancel of a gesture that has changed nothing yet.
|
||
if (d->drag_mode == MathDrag::Wire && d->wire_detach >= 0 &&
|
||
d->wire_detach < static_cast<int>(d->nodes.size())) {
|
||
MathNode& n = d->nodes[static_cast<std::size_t>(d->wire_detach)];
|
||
(d->wire_detach_which == 0 ? n.in_a : n.in_b) = d->wire_from;
|
||
}
|
||
d->drag_mode = MathDrag::None;
|
||
d->wire_from = -1;
|
||
d->wire_detach = -1;
|
||
gtk_widget_queue_draw(d->canvas);
|
||
return TRUE;
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 6: Strip `pending_out` from the press handler and the draw**
|
||
|
||
`math_canvas_pressed()` becomes selection only — the wiring is the drag's job now:
|
||
|
||
```cpp
|
||
void math_canvas_pressed(GtkGestureClick*, int, double x, double y, gpointer user) {
|
||
MathDialog* d = static_cast<MathDialog*>(user);
|
||
// The key controller is on the canvas, so the canvas has to hold focus for `Delete`, `Escape`
|
||
// and `Ctrl+0` to reach it.
|
||
gtk_widget_grab_focus(d->canvas);
|
||
int which = kPortOut;
|
||
const int port = math_port_at_canvas(d, x, y, which);
|
||
if (port >= 0) {
|
||
d->selected = port;
|
||
math_dialog_sync(d);
|
||
return;
|
||
}
|
||
const int node = math_node_at_canvas(d, x, y);
|
||
if (node >= 0) {
|
||
d->selected = node;
|
||
math_dialog_sync(d);
|
||
return;
|
||
}
|
||
gtk_widget_queue_draw(d->canvas);
|
||
}
|
||
```
|
||
|
||
In `math_canvas_draw()`, the armed-port highlight has nothing left to read, so both port colours become unconditional. Replace the input-port block:
|
||
|
||
```cpp
|
||
for (int k = 0; k < math_op_arity(n.op); ++k) {
|
||
double px = 0.0;
|
||
double py = 0.0;
|
||
math_input_pos(n, k, px, py);
|
||
cairo_set_source_rgb(cr, 0.45, 0.52, 0.62);
|
||
cairo_rectangle(cr, px - kPort * 0.5, py - kPort * 0.5, kPort, kPort);
|
||
cairo_fill(cr);
|
||
}
|
||
double ox = 0.0;
|
||
double oy = 0.0;
|
||
math_output_pos(n, ox, oy);
|
||
cairo_set_source_rgb(cr, 0.45, 0.52, 0.62);
|
||
cairo_rectangle(cr, ox - kPort * 0.5, oy - kPort * 0.5, kPort, kPort);
|
||
cairo_fill(cr);
|
||
```
|
||
|
||
and add the live wire at the very end of the transformed block, after the node loop and before `cairo_restore(cr)`:
|
||
|
||
```cpp
|
||
// The wire being dragged, drawn last so it is over the boxes: without it the gesture gives no
|
||
// sign of what it is about to connect.
|
||
if (d->drag_mode == MathDrag::Wire && d->wire_from >= 0 &&
|
||
d->wire_from < static_cast<int>(d->nodes.size())) {
|
||
double ax = 0.0;
|
||
double ay = 0.0;
|
||
math_output_pos(d->nodes[static_cast<std::size_t>(d->wire_from)], ax, ay);
|
||
cairo_set_source_rgb(cr, 1.0, 0.69, 0.13);
|
||
cairo_set_line_width(cr, 1.5);
|
||
cairo_move_to(cr, ax, ay);
|
||
cairo_curve_to(cr, ax + 40.0, ay, d->wire_x - 40.0, d->wire_y, d->wire_x, d->wire_y);
|
||
cairo_stroke(cr);
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 7: Connect `drag-end`**
|
||
|
||
In the dialog construction, beside the existing `drag-begin` and `drag-update` connections:
|
||
|
||
```cpp
|
||
g_signal_connect(drag, "drag-end", G_CALLBACK(math_canvas_drag_end), d);
|
||
```
|
||
|
||
Read the surrounding lines for the gesture variable's actual name rather than assuming `drag`.
|
||
|
||
- [ ] **Step 8: Build and run; read the right values**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
Expected: the five documented warnings, no errors. (`pending_out` is gone from every reference; a leftover is a build error, which is the point of deleting the field rather than leaving it unused.)
|
||
|
||
Then the probe run. Expected:
|
||
|
||
```
|
||
[math-probe] wire: connect mode=2 in_a=0 (want 2 0)
|
||
[math-probe] detach: emptied_at_begin=-1 in_a=-1 (want -1 -1)
|
||
[math-probe] detach: escape restores in_a=0 (want 0)
|
||
[math-probe] view: pan mode=3 dview=30.0 dnode=0.0 (want 3 30.0 0.0)
|
||
```
|
||
|
||
`mode=2` is `MathDrag::Wire` and `mode=3` is `MathDrag::Pan` — the probe prints the enum's own value beside the measurement so a reordering of the enum cannot make the line lie.
|
||
|
||
- [ ] **Step 9: Commit**
|
||
|
||
```bash
|
||
git add src/ui.cpp
|
||
git commit -m "$(cat <<'EOF'
|
||
feat: a wire is dragged from port to port, and a wired input is dragged off
|
||
|
||
One state machine decided at `drag-begin` -- a port starts a wire, a box
|
||
moves it, the background pans -- so the two-click protocol and its armed-port
|
||
highlight are gone. A drag from an input empties it immediately, which makes
|
||
a release over nothing the detach gesture; `Escape` puts it back.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 4: Pick a wire and delete it
|
||
|
||
A wire needs to be selectable to be deleted by anything other than a detach drag, and the user asked for deletion explicitly.
|
||
|
||
**Files:**
|
||
- Modify: `src/ui.cpp:3290-3319` (`MathDialog`: `selected_wire`)
|
||
- Modify: after `math_port_at` (add `MathWireRef`, `math_wire_at`, `math_wire_at_canvas`)
|
||
- Modify: `math_canvas_draw` (the selected wire is drawn highlighted)
|
||
- Modify: `math_canvas_pressed` (a wire is picked between the port test and the box test)
|
||
- Modify: `math_canvas_key` (add `Delete`/`BackSpace`)
|
||
- Modify: `src/ui.cpp:6688+` (the probe: a `wire: pick` line)
|
||
|
||
**Interfaces:**
|
||
- Consumes: `math_port_at_canvas()`, `math_node_at_canvas()`, `math_to_graph()`, `MathDrag` (Tasks 1–3).
|
||
- Produces:
|
||
```cpp
|
||
/// A wire, named by the node that *consumes* it and which of its inputs. The consumer is the
|
||
/// identity because that is where the wire is stored -- a producer may feed several.
|
||
struct MathWireRef {
|
||
int node = -1;
|
||
int which = 0;
|
||
};
|
||
|
||
/// The wire under a graph position, or `{-1, 0}`. `tol` is in graph units.
|
||
MathWireRef math_wire_at(const MathDialog* d, double gx, double gy, double tol);
|
||
|
||
/// The same, from a canvas position, with the tolerance in screen pixels.
|
||
MathWireRef math_wire_at_canvas(const MathDialog* d, double cx, double cy);
|
||
|
||
MathWireRef selected_wire{-1, 0};
|
||
```
|
||
|
||
- [ ] **Step 1: Add the failing probe lines**
|
||
|
||
In `math_probe()`, after Task 3's block:
|
||
|
||
```cpp
|
||
if (a->math_dialog != nullptr) {
|
||
auto* p = static_cast<MathDialog*>(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog"));
|
||
if (p != nullptr && p->nodes.size() >= 2) {
|
||
math_view_reset(p);
|
||
p->nodes[1].in_a = 0;
|
||
// Halfway along the wire in *graph* units: the bezier's own midpoint, which for a cubic
|
||
// with these control points is the average of the two ends plus three eighths of the
|
||
// tread -- so sampling the curve is the only honest way to find a point on it.
|
||
double ax = 0.0;
|
||
double ay = 0.0;
|
||
double bx = 0.0;
|
||
double by = 0.0;
|
||
math_output_pos(p->nodes[0], ax, ay);
|
||
math_input_pos(p->nodes[1], 0, bx, by);
|
||
const double t = 0.5;
|
||
const double u = 1.0 - t;
|
||
const double mx = u * u * u * ax + 3 * u * u * t * (ax + 40.0) +
|
||
3 * u * t * t * (bx - 40.0) + t * t * t * bx;
|
||
const double my = u * u * u * ay + 3 * u * u * t * ay + 3 * u * t * t * by + t * t * t * by;
|
||
const MathWireRef hit = math_wire_at_canvas(p, mx * p->view_scale + p->view_x,
|
||
my * p->view_scale + p->view_y);
|
||
p->selected_wire = hit;
|
||
math_canvas_key(nullptr, GDK_KEY_Delete, 0, static_cast<GdkModifierType>(0), p);
|
||
std::fprintf(stderr,
|
||
"[math-probe] wire: pick node=%d which=%d -> in_a=%d (want 1 0 -1)\n",
|
||
hit.node, hit.which, p->nodes[1].in_a);
|
||
p->nodes[1].in_a = 0;
|
||
p->selected_wire = MathWireRef{-1, 0};
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run it and read the wrong value**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'MathWireRef|math_wire_at_canvas|selected_wire' | head -6
|
||
```
|
||
|
||
Expected: errors naming `MathWireRef`, `math_wire_at_canvas` and `MathDialog::selected_wire` as undeclared.
|
||
|
||
- [ ] **Step 3: Add `MathWireRef` and the hit test**
|
||
|
||
Declare `MathWireRef` **above** `struct MathDialog` (it is a member's type), beside `enum class MathDrag`:
|
||
|
||
```cpp
|
||
/// A wire, named by the node that *consumes* it and which of its inputs. The consumer is the
|
||
/// identity because that is where a wire is stored: `in_a`/`in_b` on the reader. A producer may
|
||
/// feed several inputs and so cannot name one.
|
||
struct MathWireRef {
|
||
int node = -1;
|
||
int which = 0;
|
||
};
|
||
```
|
||
|
||
and add to `struct MathDialog`:
|
||
|
||
```cpp
|
||
/// The wire `Delete` would remove, or `{-1, 0}`.
|
||
MathWireRef selected_wire{-1, 0};
|
||
```
|
||
|
||
Then, after `math_port_at_canvas()` / `math_node_at_canvas()`:
|
||
|
||
```cpp
|
||
/// The wire under a graph position, or `{-1, 0}`. The wire is the cubic the draw lays down, so it
|
||
/// is *sampled* rather than solved: 24 segments is well under a pixel of chord error at the scales
|
||
/// this canvas allows, and a closed form for "nearest point on a cubic" is a quartic root find for
|
||
/// no gain. Topmost-first, matching every other hit test here.
|
||
MathWireRef math_wire_at(const MathDialog* d, double gx, double gy, double tol) {
|
||
constexpr int kSamples = 24;
|
||
for (int i = static_cast<int>(d->nodes.size()) - 1; i >= 0; --i) {
|
||
const MathNode& n = d->nodes[static_cast<std::size_t>(i)];
|
||
const int ins[2] = {n.in_a, n.in_b};
|
||
for (int k = 0; k < math_op_arity(n.op); ++k) {
|
||
const int from = ins[k];
|
||
if (from < 0 || from >= static_cast<int>(d->nodes.size())) continue;
|
||
double ax = 0.0;
|
||
double ay = 0.0;
|
||
double bx = 0.0;
|
||
double by = 0.0;
|
||
math_output_pos(d->nodes[static_cast<std::size_t>(from)], ax, ay);
|
||
math_input_pos(n, k, bx, by);
|
||
const double c1x = ax + 40.0;
|
||
const double c2x = bx - 40.0;
|
||
for (int s = 0; s <= kSamples; ++s) {
|
||
const double t = static_cast<double>(s) / kSamples;
|
||
const double u = 1.0 - t;
|
||
const double px = u * u * u * ax + 3 * u * u * t * c1x + 3 * u * t * t * c2x + t * t * t * bx;
|
||
const double py = u * u * u * ay + 3 * u * u * t * ay + 3 * u * t * t * by + t * t * t * by;
|
||
if (std::fabs(gx - px) <= tol && std::fabs(gy - py) <= tol) {
|
||
return MathWireRef{i, k};
|
||
}
|
||
}
|
||
}
|
||
}
|
||
return MathWireRef{-1, 0};
|
||
}
|
||
|
||
/// The pick radius for a wire, in screen pixels. Smaller than a port's, so a wire that passes
|
||
/// behind a port never wins the press -- the port is tested first anyway, but a user aiming at a
|
||
/// port should not be able to miss it onto a wire either.
|
||
constexpr double kWirePick = 6.0;
|
||
|
||
MathWireRef math_wire_at_canvas(const MathDialog* d, double cx, double cy) {
|
||
double gx = 0.0;
|
||
double gy = 0.0;
|
||
math_to_graph(d, cx, cy, gx, gy);
|
||
return math_wire_at(d, gx, gy, kWirePick / d->view_scale);
|
||
}
|
||
```
|
||
|
||
Note the draw's cubic has `ay` for **both** of the first control point's y and `by` for the second's; the sampler must use the same, or it tests a curve the user cannot see.
|
||
|
||
- [ ] **Step 4: Pick a wire on a press**
|
||
|
||
In `math_canvas_pressed()`, between the port test and the box test:
|
||
|
||
```cpp
|
||
const MathWireRef wire = math_wire_at_canvas(d, x, y);
|
||
if (wire.node >= 0) {
|
||
d->selected_wire = wire;
|
||
d->selected = wire.node;
|
||
math_dialog_sync(d);
|
||
gtk_widget_queue_draw(d->canvas);
|
||
return;
|
||
}
|
||
```
|
||
|
||
and clear it in the two paths that are not a wire — add `d->selected_wire = MathWireRef{-1, 0};` to the port branch, the box branch and the background fall-through.
|
||
|
||
- [ ] **Step 5: Delete the selected wire**
|
||
|
||
In `math_canvas_key()`, before the `return FALSE;`:
|
||
|
||
```cpp
|
||
if (keyval == GDK_KEY_Delete || keyval == GDK_KEY_BackSpace) {
|
||
if (d->selected_wire.node >= 0 &&
|
||
d->selected_wire.node < static_cast<int>(d->nodes.size())) {
|
||
MathNode& n = d->nodes[static_cast<std::size_t>(d->selected_wire.node)];
|
||
(d->selected_wire.which == 0 ? n.in_a : n.in_b) = -1;
|
||
d->selected_wire = MathWireRef{-1, 0};
|
||
math_dialog_sync(d);
|
||
gtk_widget_queue_draw(d->canvas);
|
||
return TRUE;
|
||
}
|
||
}
|
||
```
|
||
|
||
It deliberately does **not** fall through to deleting the selected *node*: a key that removes one thing or another depending on an invisible selection state is how a graph gets destroyed by accident. The `delete node` button stays the only way to remove a node.
|
||
|
||
- [ ] **Step 6: Draw the selected wire highlighted**
|
||
|
||
In `math_canvas_draw()`'s wire loop, replace the stroke colour/width lines:
|
||
|
||
```cpp
|
||
const bool picked = d->selected_wire.node == static_cast<int>(i) &&
|
||
d->selected_wire.which == k;
|
||
cairo_set_source_rgb(cr, picked ? 1.0 : 0.55, picked ? 0.69 : 0.62, picked ? 0.13 : 0.72);
|
||
cairo_set_line_width(cr, picked ? 3.0 : 1.5);
|
||
```
|
||
|
||
- [ ] **Step 7: Build and run; read the right values**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
Expected: the five documented warnings, no errors. Then the probe run. Expected:
|
||
|
||
```
|
||
[math-probe] wire: pick node=1 which=0 -> in_a=-1 (want 1 0 -1)
|
||
```
|
||
|
||
- [ ] **Step 8: Commit**
|
||
|
||
```bash
|
||
git add src/ui.cpp
|
||
git commit -m "$(cat <<'EOF'
|
||
feat: a wire can be picked and deleted, so a graph can be rewired
|
||
|
||
`math_wire_at()` samples the same cubic the draw lays down -- 24 segments,
|
||
well under a pixel of chord error at the scales this canvas allows -- and
|
||
`Delete` removes the picked one. It deliberately does not fall through to
|
||
deleting the selected node: a key whose target depends on an invisible
|
||
selection is how a graph gets destroyed by accident.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 5: `math_connect()` — a backwards wire re-sorts the graph
|
||
|
||
The user chose "reorder silently". The list-walked-in-order invariant stays; the editor becomes what maintains it.
|
||
|
||
**Files:**
|
||
- Modify: `src/ui.cpp` (add `math_connect()` after `math_wire_at_canvas()`)
|
||
- Modify: `math_canvas_drag_end` (call it instead of writing `in_a`/`in_b`)
|
||
- Modify: `src/ui.cpp:6688+` (the probe: `wire: backwards`, `wire: self`, `wire: cycle`)
|
||
|
||
**Interfaces:**
|
||
- Consumes: `MathDrag`, `MathWireRef`, `selected_wire`, `drag_node` (Tasks 3–4).
|
||
- Produces:
|
||
```cpp
|
||
/// Wires `from`'s output into `to`'s input `which`, re-sorting the node list when the wire runs
|
||
/// backwards. Returns false and sets `why` when the wire would make a cycle, leaving the graph
|
||
/// untouched.
|
||
bool math_connect(MathDialog* d, int from, int to, int which, std::string* why);
|
||
```
|
||
Task 6's validation line reads the graph `math_connect()` leaves behind; nothing else calls it.
|
||
|
||
- [ ] **Step 1: Add the failing probe lines**
|
||
|
||
In `math_probe()`, after Task 4's block:
|
||
|
||
```cpp
|
||
if (a->math_dialog != nullptr) {
|
||
auto* c = static_cast<MathDialog*>(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog"));
|
||
if (c != nullptr && c->nodes.size() >= 2) {
|
||
// A wire drawn right-to-left is legal: the editor re-sorts so that the reader still sits
|
||
// after what it reads. Node 0 is the source and node 1 the gain, so wiring the gain into
|
||
// the source is impossible (a source has no inputs) -- a second gain reading the first,
|
||
// placed *before* it, is the case.
|
||
c->nodes.clear();
|
||
MathNode src;
|
||
src.op = MathOp::Source;
|
||
src.source = name;
|
||
MathNode late;
|
||
late.op = MathOp::Gain;
|
||
MathNode early;
|
||
early.op = MathOp::Gain;
|
||
c->nodes = {src, late, early}; // early reads late: backwards
|
||
c->nodes[1].in_a = 0;
|
||
c->output = 2;
|
||
c->selected = 2;
|
||
std::string why;
|
||
const bool ok = math_connect(c, 1, 2, 0, &why);
|
||
// After the sort node 1 must still be the one that reads 0, and the reader of it must sit
|
||
// after it. The op is the same for both, so the *wiring* is the reading.
|
||
std::string order;
|
||
for (std::size_t i = 0; i < c->nodes.size(); ++i) {
|
||
order += (i != 0 ? "," : "");
|
||
order += std::to_string(i) + ":" + std::to_string(c->nodes[i].in_a);
|
||
}
|
||
std::string valid_why;
|
||
const bool valid = math_channel_valid_nodes(c->nodes, c->output, &valid_why);
|
||
std::fprintf(stderr,
|
||
"[math-probe] wire: backwards ok=%d order=[%s] output=%d valid=%d why=[%s] "
|
||
"(want 1 ... 1 [])\n",
|
||
ok ? 1 : 0, order.c_str(), c->output, valid ? 1 : 0, valid_why.c_str());
|
||
|
||
std::string self_why;
|
||
const bool self_ok = math_connect(c, 1, 1, 0, &self_why);
|
||
std::fprintf(stderr, "[math-probe] wire: self refused=%d why=[%s] (want 1)\n",
|
||
self_ok ? 0 : 1, self_why.c_str());
|
||
|
||
// A cycle: the node that already feeds the other is asked to read it back.
|
||
const int reader = c->nodes[1].in_a == 0 ? 2 : 1;
|
||
const int feeder = reader == 2 ? 1 : 2;
|
||
std::string cyc_why;
|
||
const bool cyc_ok = math_connect(c, reader, feeder, 0, &cyc_why);
|
||
std::fprintf(stderr, "[math-probe] wire: cycle refused=%d why=[%s] (want 1)\n",
|
||
cyc_ok ? 0 : 1, cyc_why.c_str());
|
||
}
|
||
}
|
||
```
|
||
|
||
`math_channel_valid_nodes()` does not exist; it is a thin overload the next step adds beside `math_channel_valid()` so the probe can validate a bare node list without building a `MathChannel`.
|
||
|
||
- [ ] **Step 2: Run it and read the wrong value**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'math_connect|math_channel_valid_nodes' | head -4
|
||
```
|
||
|
||
Expected: errors naming `math_connect` and `math_channel_valid_nodes` as undeclared.
|
||
|
||
- [ ] **Step 3: Add `math_channel_valid_nodes()`**
|
||
|
||
In `src/math_channel.hpp`, beside the existing declaration:
|
||
|
||
```cpp
|
||
/// The same check over a bare node list and output index, for a caller that has a graph but not yet
|
||
/// a `MathChannel` -- the editor, and the headless probe that drives it.
|
||
bool math_channel_valid_nodes(const std::vector<MathNode>& nodes, int output,
|
||
std::string* why = nullptr);
|
||
```
|
||
|
||
In `src/math_channel.cpp`, make the existing body the new function and have the old one forward to it. Replace `math_channel_valid()` (currently `218-245`) with:
|
||
|
||
```cpp
|
||
bool math_channel_valid_nodes(const std::vector<MathNode>& nodes, int output, std::string* why) {
|
||
const std::size_t n = nodes.size();
|
||
const auto fail = [&](const std::string& text) {
|
||
if (why != nullptr) *why = text;
|
||
return false;
|
||
};
|
||
if (n == 0) return fail("empty graph");
|
||
for (std::size_t i = 0; i < n; ++i) {
|
||
const MathNode& node = nodes[i];
|
||
if (node.op == MathOp::Source && node.source.empty()) {
|
||
return fail("node " + std::to_string(i + 1) + ": no channel chosen");
|
||
}
|
||
const int arity = math_op_arity(node.op);
|
||
const int ins[2] = {node.in_a, node.in_b};
|
||
for (int k = 0; k < arity; ++k) {
|
||
// Only nodes *before* this one: the evaluator walks the list in order, so a forward or
|
||
// self reference is not a cycle to detect, it is a node reading a value that is not there.
|
||
if (ins[k] < 0 || ins[k] >= static_cast<int>(i)) {
|
||
return fail("node " + std::to_string(i + 1) + ": input " + std::to_string(k + 1) +
|
||
" is not connected");
|
||
}
|
||
}
|
||
}
|
||
if (output < 0 || static_cast<std::size_t>(output) >= n) {
|
||
return fail("no output chosen");
|
||
}
|
||
if (why != nullptr) why->clear();
|
||
return true;
|
||
}
|
||
|
||
bool math_channel_valid(const MathChannel& channel, std::string* why) {
|
||
return math_channel_valid_nodes(channel.nodes, channel.output, why);
|
||
}
|
||
```
|
||
|
||
The `why->clear()` on success is new and load-bearing for Task 6: the validation line prints `why` unconditionally, and a stale message from a previous call would be shown beside a valid graph.
|
||
|
||
- [ ] **Step 4: Add `math_connect()`**
|
||
|
||
After `math_wire_at_canvas()` in `src/ui.cpp`:
|
||
|
||
```cpp
|
||
/// Wires `from`'s output into `to`'s input `which`. The evaluator walks the node list in order, so
|
||
/// a reader must sit after what it reads -- and rather than refuse a wire drawn right to left (the
|
||
/// old editor did not even draw such an input as connectable, which made the order the *user's*
|
||
/// problem), the list is re-sorted so that it holds. Returns false and sets `why` for a wire that
|
||
/// cannot be ordered at all, i.e. one that would close a cycle; nothing is written in that case.
|
||
bool math_connect(MathDialog* d, int from, int to, int which, std::string* why) {
|
||
const int n = static_cast<int>(d->nodes.size());
|
||
if (from < 0 || from >= n || to < 0 || to >= n) return false;
|
||
if (from == to) {
|
||
if (why != nullptr) *why = "a node cannot read itself";
|
||
return false;
|
||
}
|
||
// Kahn's algorithm over the *trial* edge set, so a cycle is found before anything is written.
|
||
std::vector<int> in_a(static_cast<std::size_t>(n));
|
||
std::vector<int> in_b(static_cast<std::size_t>(n));
|
||
for (int i = 0; i < n; ++i) {
|
||
in_a[static_cast<std::size_t>(i)] = d->nodes[static_cast<std::size_t>(i)].in_a;
|
||
in_b[static_cast<std::size_t>(i)] = d->nodes[static_cast<std::size_t>(i)].in_b;
|
||
}
|
||
(which == 0 ? in_a : in_b)[static_cast<std::size_t>(to)] = from;
|
||
|
||
std::vector<int> indegree(static_cast<std::size_t>(n), 0);
|
||
const auto edges_of = [&](int i, int (&out)[2]) {
|
||
const MathNode& node = d->nodes[static_cast<std::size_t>(i)];
|
||
const int arity = math_op_arity(node.op);
|
||
out[0] = arity >= 1 ? in_a[static_cast<std::size_t>(i)] : -1;
|
||
out[1] = arity >= 2 ? in_b[static_cast<std::size_t>(i)] : -1;
|
||
};
|
||
for (int i = 0; i < n; ++i) {
|
||
int e[2] = {-1, -1};
|
||
edges_of(i, e);
|
||
for (int k = 0; k < 2; ++k) {
|
||
if (e[k] >= 0 && e[k] < n && e[k] != i) ++indegree[static_cast<std::size_t>(i)];
|
||
}
|
||
}
|
||
// Ties broken by the current list index, so a graph that is already in order is not permuted:
|
||
// the positions are the user's layout as much as the evaluator's order, and shuffling a valid
|
||
// graph on every wire would read as the editor rearranging itself.
|
||
std::vector<int> order;
|
||
order.reserve(static_cast<std::size_t>(n));
|
||
std::vector<bool> done(static_cast<std::size_t>(n), false);
|
||
for (int step = 0; step < n; ++step) {
|
||
int pick = -1;
|
||
for (int i = 0; i < n; ++i) {
|
||
if (!done[static_cast<std::size_t>(i)] && indegree[static_cast<std::size_t>(i)] == 0) {
|
||
pick = i;
|
||
break;
|
||
}
|
||
}
|
||
if (pick < 0) {
|
||
if (why != nullptr) *why = "that wire would make a loop";
|
||
return false;
|
||
}
|
||
done[static_cast<std::size_t>(pick)] = true;
|
||
order.push_back(pick);
|
||
for (int i = 0; i < n; ++i) {
|
||
if (done[static_cast<std::size_t>(i)]) continue;
|
||
int e[2] = {-1, -1};
|
||
edges_of(i, e);
|
||
for (int k = 0; k < 2; ++k) {
|
||
if (e[k] == pick) --indegree[static_cast<std::size_t>(i)];
|
||
}
|
||
}
|
||
}
|
||
|
||
// `order[p]` is the node that moves to position `p`; `where[old]` is where it ends up. Every
|
||
// index the dialog holds is in the old space and has to be remapped in one block -- a missed one
|
||
// silently renames an operation or re-points a wire.
|
||
std::vector<int> where(static_cast<std::size_t>(n), -1);
|
||
for (int p = 0; p < n; ++p) where[static_cast<std::size_t>(order[static_cast<std::size_t>(p)])] = p;
|
||
std::vector<MathNode> sorted;
|
||
sorted.reserve(static_cast<std::size_t>(n));
|
||
for (int p = 0; p < n; ++p) sorted.push_back(d->nodes[static_cast<std::size_t>(order[static_cast<std::size_t>(p)])]);
|
||
const auto remap = [&](int i) { return i >= 0 && i < n ? where[static_cast<std::size_t>(i)] : -1; };
|
||
for (int p = 0; p < n; ++p) {
|
||
MathNode& node = sorted[static_cast<std::size_t>(p)];
|
||
const int old = order[static_cast<std::size_t>(p)];
|
||
node.in_a = math_op_arity(node.op) >= 1 ? remap(in_a[static_cast<std::size_t>(old)]) : -1;
|
||
node.in_b = math_op_arity(node.op) >= 2 ? remap(in_b[static_cast<std::size_t>(old)]) : -1;
|
||
}
|
||
d->nodes = std::move(sorted);
|
||
d->output = remap(d->output);
|
||
d->selected = remap(d->selected);
|
||
d->drag_node = remap(d->drag_node);
|
||
d->wire_from = remap(d->wire_from);
|
||
d->wire_detach = remap(d->wire_detach);
|
||
d->selected_wire.node = remap(d->selected_wire.node);
|
||
if (why != nullptr) why->clear();
|
||
return true;
|
||
}
|
||
```
|
||
|
||
The `edges_of` lambda masks each input by the op's arity, which matters: a node whose op was changed from binary to unary keeps a stale `in_b`, and counting it would make an indegree that never reaches zero and so report a loop that is not there.
|
||
|
||
- [ ] **Step 5: Call it from `drag-end`**
|
||
|
||
In `math_canvas_drag_end()`, replace the direct write:
|
||
|
||
```cpp
|
||
if (target >= 0 && which != kPortOut && target != d->wire_from) {
|
||
MathNode& n = d->nodes[static_cast<std::size_t>(target)];
|
||
(which == 0 ? n.in_a : n.in_b) = d->wire_from;
|
||
d->selected = target;
|
||
math_dialog_sync(d);
|
||
}
|
||
```
|
||
|
||
with:
|
||
|
||
```cpp
|
||
if (target >= 0 && which != kPortOut && target != d->wire_from) {
|
||
std::string why;
|
||
if (math_connect(d, d->wire_from, target, which, &why)) {
|
||
// `math_connect()` may have permuted the list, so the target's index is no longer `target`.
|
||
math_dialog_sync(d);
|
||
} else if (d->wire_detach >= 0) {
|
||
// A refused wire must not also lose the one the drag detached to start with.
|
||
MathNode& back = d->nodes[static_cast<std::size_t>(d->wire_detach)];
|
||
(d->wire_detach_which == 0 ? back.in_a : back.in_b) = d->wire_from;
|
||
math_dialog_sync(d);
|
||
}
|
||
}
|
||
```
|
||
|
||
`d->selected` is no longer written here: `math_connect()` remaps it and leaving it on `wire_from` — which `drag-begin` set — is the honest answer, since the user grabbed that node.
|
||
|
||
- [ ] **Step 6: Build and run; read the right values**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
Expected: the five documented warnings, no errors. Then the probe run. Expected:
|
||
|
||
```
|
||
[math-probe] wire: backwards ok=1 order=[0:-1,1:0,2:1] output=2 valid=1 why=[] (want 1 ... 1 [])
|
||
[math-probe] wire: self refused=1 why=[a node cannot read itself] (want 1)
|
||
[math-probe] wire: cycle refused=1 why=[that wire would make a loop] (want 1)
|
||
```
|
||
|
||
`valid=1` is the assertion on the first line — the order string is the reading that explains it. `order=[0:-1,1:0,2:1]` says the source feeds the node at position 1 which feeds the node at position 2, which is the chain the graph was wired as regardless of how it was laid out.
|
||
|
||
- [ ] **Step 7: Commit**
|
||
|
||
```bash
|
||
git add src/ui.cpp src/math_channel.cpp src/math_channel.hpp
|
||
git commit -m "$(cat <<'EOF'
|
||
feat: a backwards wire re-sorts the graph instead of being refused
|
||
|
||
The evaluator walks the node list in order, which used to make the order the
|
||
user's problem -- an input of an earlier node was not even drawn as
|
||
connectable. `math_connect()` runs Kahn's algorithm over the trial edge set,
|
||
with ties broken by the current index so a graph already in order is not
|
||
permuted, and remaps every index the dialog holds in one block. A self wire
|
||
and a cycle are refused with a message and change nothing.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 6: The live validation line replaces the modal alerts
|
||
|
||
The user chose "live validation line". The dialog says why it cannot be saved, all the time, instead of refusing with a modal once Apply is pressed. Two latent defects are fixed in the same pass: `math_dialog_sync()` returns early on an out-of-range selection, so the line has to be written **before** that return; and the source dropdown is populated from `signals()` (every signal, including the unacquirable timestamp) while Apply validates against `math_source_names()` (acquired channels only) — an inconsistency that lets a user pick a source they cannot fix.
|
||
|
||
**Files:**
|
||
- Modify: `src/ui.cpp:3290-3319` (`MathDialog`: `problem`, `apply`)
|
||
- Modify: `src/ui.cpp` (add `math_dialog_problem()` above `math_dialog_sync()`)
|
||
- Modify: `src/ui.cpp:3621-3684` (`math_dialog_sync`: the line before the early return; the dropdown's list)
|
||
- Modify: `src/ui.cpp:3571-3590` (`on_math_param_changed`: the same list)
|
||
- Modify: `src/ui.cpp:3686-3719` (`on_math_apply`: the two alerts go)
|
||
- Modify: the dialog construction (the label, and `d->apply`)
|
||
- Modify: `src/ui.cpp:6688+` (the probe: a `validate:` block)
|
||
|
||
**Interfaces:**
|
||
- Consumes: `math_connect()` (Task 5) only in that it reads the graph left behind; `math_channel_valid_nodes()` (Task 5).
|
||
- Produces:
|
||
```cpp
|
||
/// Why the graph as it stands could not be saved, or "" when it could. The graph check and the
|
||
/// acquired-source check in one place, so the line and the Apply button cannot disagree.
|
||
std::string math_dialog_problem(const MathDialog* d);
|
||
|
||
GtkWidget* problem = nullptr; ///< the validation line
|
||
GtkWidget* apply = nullptr; ///< desensitized while `math_dialog_problem()` is non-empty
|
||
```
|
||
|
||
- [ ] **Step 1: Add the failing probe lines**
|
||
|
||
In `math_probe()`, after Task 5's block:
|
||
|
||
```cpp
|
||
if (a->math_dialog != nullptr) {
|
||
auto* v2 = static_cast<MathDialog*>(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog"));
|
||
if (v2 != nullptr && v2->nodes.size() >= 2) {
|
||
// Broken: an unwired input. The line must say so and Apply must be insensitive -- the old
|
||
// behaviour was a modal alert *after* the press, which is a refusal rather than guidance.
|
||
v2->nodes[1].in_a = -1;
|
||
math_dialog_sync(v2);
|
||
const std::string bad = math_dialog_problem(v2);
|
||
const int bad_sensitive = gtk_widget_get_sensitive(v2->apply) ? 1 : 0;
|
||
const char* bad_text = gtk_label_get_text(GTK_LABEL(v2->problem));
|
||
v2->nodes[1].in_a = 0;
|
||
math_dialog_sync(v2);
|
||
const std::string good = math_dialog_problem(v2);
|
||
const int good_sensitive = gtk_widget_get_sensitive(v2->apply) ? 1 : 0;
|
||
std::fprintf(stderr,
|
||
"[math-probe] validate: broken problem=[%s] label=[%s] apply=%d; "
|
||
"fixed problem=[%s] apply=%d (want non-empty 0; empty 1)\n",
|
||
bad.c_str(), bad_text != nullptr ? bad_text : "(null)", bad_sensitive,
|
||
good.c_str(), good_sensitive);
|
||
|
||
// The selection out of range is the case `math_dialog_sync()` returns early on, so the
|
||
// line must be written above that return or it goes stale exactly when it matters.
|
||
const int was_sel = v2->selected;
|
||
v2->selected = 999;
|
||
v2->nodes[1].in_a = -1;
|
||
math_dialog_sync(v2);
|
||
const char* stale = gtk_label_get_text(GTK_LABEL(v2->problem));
|
||
std::fprintf(stderr, "[math-probe] validate: no-selection label=[%s] (want non-empty)\n",
|
||
stale != nullptr ? stale : "(null)");
|
||
v2->selected = was_sel;
|
||
v2->nodes[1].in_a = 0;
|
||
math_dialog_sync(v2);
|
||
|
||
// And the source dropdown must list the *acquired* channels, which is what Apply checks
|
||
// against: listing every signal let a user pick one that could never be made valid.
|
||
const std::vector<std::string> acquired = math_source_names(*a);
|
||
std::size_t signals_n = 0;
|
||
{
|
||
std::lock_guard<std::mutex> lock(a->model->mutex());
|
||
signals_n = a->model->signals().size();
|
||
}
|
||
v2->selected = 0;
|
||
v2->nodes[0].op = MathOp::Source;
|
||
math_dialog_sync(v2);
|
||
GListModel* lm = gtk_drop_down_get_model(GTK_DROP_DOWN(v2->source_drop));
|
||
const guint listed = lm != nullptr ? g_list_model_get_n_items(lm) : 0u;
|
||
std::fprintf(stderr,
|
||
"[math-probe] validate: source list=%u acquired=%zu signals=%zu "
|
||
"(want list==acquired)\n",
|
||
listed, acquired.size(), signals_n);
|
||
v2->selected = was_sel;
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run it and read the wrong value**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'math_dialog_problem|->problem|->apply' | head -5
|
||
```
|
||
|
||
Expected: errors naming `math_dialog_problem`, `MathDialog::problem` and `MathDialog::apply` as undeclared.
|
||
|
||
- [ ] **Step 3: Add the two widget pointers**
|
||
|
||
In `struct MathDialog`, beside `inputs`:
|
||
|
||
```cpp
|
||
/// The validation line and the button it governs. Both are set from `math_dialog_problem()` and
|
||
/// from nowhere else, so what the line says and what the button allows cannot disagree.
|
||
GtkWidget* problem = nullptr;
|
||
GtkWidget* apply = nullptr;
|
||
```
|
||
|
||
- [ ] **Step 4: Add `math_dialog_problem()`**
|
||
|
||
Immediately above `math_dialog_sync()`'s definition (it needs `math_source_names()`, declared at `3324`, and `math_channel_valid_nodes()` from the header):
|
||
|
||
```cpp
|
||
/// Why this graph could not be saved, or "" when it could. Both checks in one place: the graph's own
|
||
/// (an output that exists, every input wired to a node that exists) and that a `Source` names a
|
||
/// channel the scope actually *acquires* -- which the engine's validator cannot see, since it knows
|
||
/// nothing about the acquired list. The two used to be a pair of modal alerts raised after Apply was
|
||
/// pressed, which is a refusal rather than guidance: there is nothing in the message a user could
|
||
/// not have been told while they were still building the graph.
|
||
std::string math_dialog_problem(const MathDialog* d) {
|
||
std::string why;
|
||
if (!math_channel_valid_nodes(d->nodes, d->output, &why)) return why;
|
||
const std::vector<std::string> acquired = math_source_names(*d->app);
|
||
for (std::size_t i = 0; i < d->nodes.size(); ++i) {
|
||
if (d->nodes[i].op != MathOp::Source) continue;
|
||
if (std::find(acquired.begin(), acquired.end(), d->nodes[i].source) != acquired.end()) continue;
|
||
return "node " + std::to_string(i + 1) + ": '" + d->nodes[i].source +
|
||
"' is not an acquired channel";
|
||
}
|
||
return std::string();
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 5: Write the line *before* `math_dialog_sync()`'s early return**
|
||
|
||
At the very top of `math_dialog_sync()`, above the `if (d->selected < 0 ...) return;`:
|
||
|
||
```cpp
|
||
void math_dialog_sync(MathDialog* d) {
|
||
// Before the early return below: an out-of-range selection is exactly the state in which a stale
|
||
// validation line is most misleading -- a graph with no node selected can still be broken, and the
|
||
// user has no form to read instead.
|
||
if (d->problem != nullptr) {
|
||
const std::string why = math_dialog_problem(d);
|
||
gtk_label_set_text(GTK_LABEL(d->problem), why.c_str());
|
||
gtk_widget_set_visible(d->problem, !why.empty());
|
||
if (d->apply != nullptr) gtk_widget_set_sensitive(d->apply, why.empty());
|
||
}
|
||
if (d->selected < 0 || d->selected >= static_cast<int>(d->nodes.size())) {
|
||
if (d->canvas != nullptr) gtk_widget_queue_draw(d->canvas);
|
||
return;
|
||
}
|
||
```
|
||
|
||
The added `queue_draw` on that path is deliberate: the canvas state (a deleted node, a re-sorted list) changes on exactly the frames the selection goes out of range.
|
||
|
||
- [ ] **Step 6: Make both source lists the acquired channels**
|
||
|
||
In `math_dialog_sync()`, replace the list build:
|
||
|
||
```cpp
|
||
std::vector<std::string> names;
|
||
{
|
||
std::lock_guard<std::mutex> lock(d->app->model->mutex());
|
||
for (const SignalInfo& sig : d->app->model->signals()) names.push_back(sig.name);
|
||
}
|
||
```
|
||
|
||
with:
|
||
|
||
```cpp
|
||
// The *acquired* channels, which is what Apply validates against. Listing every signal let a
|
||
// user pick a source that could never be made valid -- and put the stream's timestamp signal,
|
||
// which has no samples at all, at the top of the list.
|
||
const std::vector<std::string> names = math_source_names(*d->app);
|
||
```
|
||
|
||
Do the same in `on_math_param_changed()`: read its current list build (around `3578`) and replace it with `math_source_names(*d->app)`. The variable is used by index against `gtk_drop_down_get_selected()`, so the two lists must be built the same way or the index names a different channel — which is why both have to change in one commit.
|
||
|
||
- [ ] **Step 7: Delete the two alerts from `on_math_apply()`**
|
||
|
||
Replace `src/ui.cpp:3689-3719`'s two blocks with a single guard:
|
||
|
||
```cpp
|
||
// The validation line has said what is wrong for as long as it has been wrong, and Apply is
|
||
// insensitive while it is non-empty -- so this is a guard against a programmatic call (the
|
||
// headless probe makes one), not a message to the user.
|
||
const std::string why = math_dialog_problem(d);
|
||
if (!why.empty()) {
|
||
std::fprintf(stderr, "[math] apply refused: %s\n", why.c_str());
|
||
return;
|
||
}
|
||
MathChannel m;
|
||
m.name = gtk_editable_get_text(GTK_EDITABLE(d->name));
|
||
m.unit = gtk_editable_get_text(GTK_EDITABLE(d->unit));
|
||
m.nodes = d->nodes;
|
||
m.output = d->output;
|
||
```
|
||
|
||
Keep the `on_math_param_changed(nullptr, d);` call at the top. The `std::string why;` and `MathChannel m;` declarations that were above move into this block, so read the whole function and make it one coherent body rather than patching around the old order.
|
||
|
||
- [ ] **Step 8: Build the label and record the Apply button**
|
||
|
||
In the dialog construction, append the label to the form column, below the inputs label:
|
||
|
||
```cpp
|
||
// Why the graph cannot be saved, in the form column where the user is already reading. Wrapped,
|
||
// so a long channel name does not widen the dialog, and hidden when there is nothing to say.
|
||
d->problem = gtk_label_new("");
|
||
gtk_label_set_wrap(GTK_LABEL(d->problem), TRUE);
|
||
gtk_label_set_xalign(GTK_LABEL(d->problem), 0.0f);
|
||
gtk_widget_add_css_class(d->problem, "error");
|
||
gtk_widget_set_visible(d->problem, FALSE);
|
||
gtk_box_append(GTK_BOX(form), d->problem);
|
||
```
|
||
|
||
and where the Apply button is created (around `3964`), record it:
|
||
|
||
```cpp
|
||
d->apply = apply;
|
||
```
|
||
|
||
placed **after** `apply` is created and **before** the `math_dialog_sync(d)` call at the end of the function — that call is what first sets its sensitivity.
|
||
|
||
- [ ] **Step 9: Build and run; read the right values**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
Expected: **four** warnings now, not five — `on_math_apply()` no longer leaves `output_built` unused, because the probe reads it. If it still reads five, check which: an `unused variable 'why'` from the Step 7 rewrite is a finding, not the baseline.
|
||
|
||
Then the probe run. Expected:
|
||
|
||
```
|
||
[math-probe] validate: broken problem=[node 2: input 1 is not connected] label=[node 2: input 1 is not connected] apply=0; fixed problem=[] apply=1 (want non-empty 0; empty 1)
|
||
[math-probe] validate: no-selection label=[node 2: input 1 is not connected] (want non-empty)
|
||
[math-probe] validate: source list=N acquired=N signals=M (want list==acquired)
|
||
```
|
||
|
||
`list == acquired` is the assertion; `signals` is printed only to show that it is the larger number and so that the change is visible.
|
||
|
||
- [ ] **Step 10: Commit**
|
||
|
||
```bash
|
||
git add src/ui.cpp
|
||
git commit -m "$(cat <<'EOF'
|
||
feat: the editor says why a graph cannot be saved instead of refusing on Apply
|
||
|
||
One `math_dialog_problem()` drives both the line and Apply's sensitivity, so
|
||
they cannot disagree, and the two modal alerts are gone -- there was nothing
|
||
in either message a user could not have been told while still building the
|
||
graph. Written above `math_dialog_sync()`'s early return, since an
|
||
out-of-range selection is exactly when a stale line misleads most.
|
||
|
||
The source dropdown now lists the acquired channels, which is what Apply
|
||
validates against: listing every signal let a user pick a source that could
|
||
never be made valid, and put the stream's timestamp signal first.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 7: Pick the op when adding a node
|
||
|
||
The user chose this. `add node` always makes a `MathOp::Gain` today, so building anything else is add-then-change-the-dropdown.
|
||
|
||
**Files:**
|
||
- Modify: `src/math_channel.hpp` (declare `math_op_index`)
|
||
- Modify: `src/math_channel.cpp` (define it)
|
||
- Modify: `src/ui.cpp:3514-3534` (`on_math_node_added` takes an op)
|
||
- Modify: `src/ui.cpp:3666-3667` (`math_dialog_sync` uses it instead of the cast)
|
||
- Modify: the dialog construction (`add node` becomes a `GtkMenuButton`)
|
||
- Modify: `src/ui.cpp:6644` (the probe's existing `on_math_node_added(nullptr, ed)` call)
|
||
|
||
**Interfaces:**
|
||
- Consumes: nothing new.
|
||
- Produces:
|
||
```cpp
|
||
/// An op's position in `math_op_at()`'s order -- the dialog's order, which is not the enum's
|
||
/// declaration order by contract even though it happens to match today.
|
||
int math_op_index(MathOp op);
|
||
```
|
||
```cpp
|
||
/// Adds a node of `op`. The button carries its op in the widget's data.
|
||
void math_add_node(MathDialog* d, MathOp op);
|
||
```
|
||
|
||
- [ ] **Step 1: Add the failing probe line**
|
||
|
||
In `math_probe()`, after Task 6's block:
|
||
|
||
```cpp
|
||
if (a->math_dialog != nullptr) {
|
||
auto* o = static_cast<MathDialog*>(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog"));
|
||
if (o != nullptr) {
|
||
const std::size_t before = o->nodes.size();
|
||
math_add_node(o, MathOp::BandPass);
|
||
const int op_added = o->nodes.empty() ? -1 : math_op_index(o->nodes.back().op);
|
||
// The op picker must make the node the user asked for, and the `rows` the form shows must
|
||
// follow it: a band pass is the one op that shows the Q row, so it is the case worth the
|
||
// check.
|
||
const int q_row = gtk_widget_get_visible(o->rows[3]) ? 1 : 0;
|
||
std::fprintf(stderr,
|
||
"[math-probe] op pick: nodes %zu->%zu op_index=%d q_row=%d "
|
||
"(want %zu %d 1)\n",
|
||
before, o->nodes.size(), op_added, q_row, before + 1,
|
||
math_op_index(MathOp::BandPass));
|
||
// And the identity the whole picker rests on: a round trip through both accessors.
|
||
int round = 1;
|
||
for (int i = 0; i < math_op_count(); ++i) {
|
||
if (math_op_index(math_op_at(i)) != i) round = 0;
|
||
}
|
||
std::fprintf(stderr, "[math-probe] op pick: index round trip=%d (want 1)\n", round);
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run it and read the wrong value**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'math_op_index|math_add_node' | head -4
|
||
```
|
||
|
||
Expected: errors naming `math_op_index` and `math_add_node` as undeclared.
|
||
|
||
- [ ] **Step 3: Add `math_op_index()`**
|
||
|
||
In `src/math_channel.hpp`, beside `math_op_at()`:
|
||
|
||
```cpp
|
||
/// An op's position in `math_op_at()`'s order. That order is the *dialog's* -- what a dropdown or a
|
||
/// menu lists, and what a session file stores -- and it is deliberately not promised to be the
|
||
/// enum's declaration order, so a caller that needs a position must ask rather than cast.
|
||
int math_op_index(MathOp op);
|
||
```
|
||
|
||
In `src/math_channel.cpp`, after `math_op_at()`:
|
||
|
||
```cpp
|
||
int math_op_index(MathOp op) {
|
||
for (int i = 0; i < math_op_count(); ++i) {
|
||
if (math_op_at(i) == op) return i;
|
||
}
|
||
return 0;
|
||
}
|
||
```
|
||
|
||
In `src/ui.cpp`, replace `math_dialog_sync()`'s cast:
|
||
|
||
```cpp
|
||
if (d->op_drop != nullptr) gtk_drop_down_set_selected(GTK_DROP_DOWN(d->op_drop),
|
||
static_cast<guint>(op));
|
||
```
|
||
|
||
with:
|
||
|
||
```cpp
|
||
// Through `math_op_index()`, not a cast: the dropdown is built from `math_op_at()`, whose order
|
||
// is the dialog's rather than the enum's, and the cast worked only because they happen to agree.
|
||
if (d->op_drop != nullptr) {
|
||
gtk_drop_down_set_selected(GTK_DROP_DOWN(d->op_drop),
|
||
static_cast<guint>(math_op_index(op)));
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Make `on_math_node_added()` take an op**
|
||
|
||
Replace `src/ui.cpp:3514-3534` with:
|
||
|
||
```cpp
|
||
/// Adds a node of `op` and selects it. The grid placement is the same one the auto-place uses, so a
|
||
/// node added here lands clear of the others -- 160 is narrower than `kNodeW` plus a wire's tread,
|
||
/// which is why the tidy layout uses a wider pitch; this is only a first guess at a free cell.
|
||
void math_add_node(MathDialog* d, MathOp op) {
|
||
// A node added to a graph that was *linear* -- its output was the last node -- becomes the output,
|
||
// which is what "add a gain and it applies" means. A graph whose output is an earlier node is a
|
||
// deliberate choice and is left alone.
|
||
const bool was_linear =
|
||
d->output == static_cast<int>(d->nodes.size()) - 1 || d->nodes.empty();
|
||
MathNode n;
|
||
n.op = op;
|
||
const std::size_t k = d->nodes.size();
|
||
n.x = 24.0 + 160.0 * static_cast<double>(k % 2u);
|
||
n.y = 20.0 + 58.0 * static_cast<double>(k / 2u);
|
||
const int arity = math_op_arity(n.op);
|
||
if (arity >= 1 && k > 0) n.in_a = static_cast<int>(k) - 1;
|
||
if (arity >= 2 && k > 0) n.in_b = static_cast<int>(k) - 1;
|
||
if (op == MathOp::Source) {
|
||
// Only an *acquired* channel: `signals().front()` is the stream's timestamp signal, which has
|
||
// no samples, so a graph sourced from it validates and then reads NaN for ever.
|
||
const std::vector<std::string> acquired = math_source_names(*d->app);
|
||
if (!acquired.empty()) n.source = acquired.front();
|
||
}
|
||
d->nodes.push_back(n);
|
||
d->selected = static_cast<int>(d->nodes.size()) - 1;
|
||
if (was_linear) d->output = d->selected;
|
||
math_dialog_sync(d);
|
||
}
|
||
|
||
void on_math_node_added(GtkButton*, gpointer user) {
|
||
math_add_node(static_cast<MathDialog*>(user), MathOp::Gain);
|
||
}
|
||
```
|
||
|
||
`on_math_node_added()` is kept because the existing probe block at `src/ui.cpp:6644` calls it and that call is an assertion about the output defaulting to the added node. Leave it.
|
||
|
||
Also delete `d->pending_out = -1;` from `on_math_node_deleted()` (`3557`) — the field is gone as of Task 3, so this is a build error until it is removed. (If Task 3 was done correctly it is already gone; check.)
|
||
|
||
- [ ] **Step 5: Turn `add node` into a menu button**
|
||
|
||
Replace the `add node` button's construction with:
|
||
|
||
```cpp
|
||
// A menu of the twelve ops rather than one button that always makes a gain: building anything
|
||
// else was add-then-change-the-dropdown, which is two steps for the common case.
|
||
GtkWidget* add = gtk_menu_button_new();
|
||
gtk_menu_button_set_label(GTK_MENU_BUTTON(add), "add node");
|
||
GtkWidget* add_menu = gtk_popover_new();
|
||
GtkWidget* add_list = gtk_box_new(GTK_ORIENTATION_VERTICAL, 0);
|
||
for (int i = 0; i < math_op_count(); ++i) {
|
||
const MathOp op = math_op_at(i);
|
||
GtkWidget* item = gtk_button_new_with_label(math_op_name(op));
|
||
gtk_widget_add_css_class(item, "flat");
|
||
gtk_widget_set_halign(gtk_button_get_child(GTK_BUTTON(item)), GTK_ALIGN_START);
|
||
// The op travels in the widget's own data, as an `int` through `math_op_index()`'s space: a
|
||
// pointer-sized enum smuggled through `gpointer` is the kind of cast that survives a
|
||
// reordering of the enum and then means something else.
|
||
g_object_set_data(G_OBJECT(item), "udpscope-math-op", GINT_TO_POINTER(i));
|
||
g_object_set_data(G_OBJECT(item), "udpscope-math-popover", add_menu);
|
||
g_signal_connect(item, "clicked", G_CALLBACK(+[](GtkButton* b, gpointer user) {
|
||
MathDialog* dd = static_cast<MathDialog*>(user);
|
||
const int index =
|
||
GPOINTER_TO_INT(g_object_get_data(G_OBJECT(b), "udpscope-math-op"));
|
||
auto* pop = GTK_POPOVER(
|
||
g_object_get_data(G_OBJECT(b), "udpscope-math-popover"));
|
||
if (pop != nullptr) gtk_popover_popdown(pop);
|
||
math_add_node(dd, math_op_at(index));
|
||
}), d);
|
||
gtk_box_append(GTK_BOX(add_list), item);
|
||
}
|
||
gtk_popover_set_child(GTK_POPOVER(add_menu), add_list);
|
||
gtk_menu_button_set_popover(GTK_MENU_BUTTON(add), add_menu);
|
||
```
|
||
|
||
and append `add` to the same box the old button went into. Delete the old `g_signal_connect(add, "clicked", ...)` line; `on_math_node_added()` now has only the probe as a caller.
|
||
|
||
- [ ] **Step 6: Build and run; read the right values**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
Expected: four warnings, no errors. Then the probe run. Expected:
|
||
|
||
```
|
||
[math-probe] op pick: nodes 3->4 op_index=7 q_row=1 (want 4 7 1)
|
||
[math-probe] op pick: index round trip=1 (want 1)
|
||
```
|
||
|
||
The node counts depend on what the earlier blocks left behind, so read `nodes N->N+1` against the `want`, not against 3→4.
|
||
|
||
- [ ] **Step 7: Commit**
|
||
|
||
```bash
|
||
git add src/ui.cpp src/math_channel.cpp src/math_channel.hpp
|
||
git commit -m "$(cat <<'EOF'
|
||
feat: the op is picked when a node is added, not changed afterwards
|
||
|
||
`add node` made a gain and nothing else, so every other op was two steps. It
|
||
is a menu of the twelve now, each carrying its index through
|
||
`math_op_index()`'s space rather than a cast of the enum -- which is also
|
||
what `math_dialog_sync()` now uses to drive the op dropdown, since that
|
||
order is the dialog's and not the declaration's.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 8: The tidy button
|
||
|
||
The user chose "tidy / auto-layout". The add-node grid wraps at two columns and overlaps as a graph grows; a graph loaded from a session file has whatever positions it was saved with.
|
||
|
||
**Files:**
|
||
- Modify: `src/ui.cpp` (add `math_tidy()` after `math_view_fit()`)
|
||
- Modify: the dialog construction (a `tidy` button)
|
||
- Modify: `src/ui.cpp:6688+` (the probe: a `tidy:` line)
|
||
|
||
**Interfaces:**
|
||
- Consumes: `math_view_fit()` (Task 2).
|
||
- Produces:
|
||
```cpp
|
||
/// Lays the graph out left to right by depth, then frames it.
|
||
void math_tidy(MathDialog* d);
|
||
```
|
||
|
||
- [ ] **Step 1: Add the failing probe line**
|
||
|
||
In `math_probe()`, after Task 7's block:
|
||
|
||
```cpp
|
||
if (a->math_dialog != nullptr) {
|
||
auto* ty = static_cast<MathDialog*>(
|
||
g_object_get_data(G_OBJECT(a->math_dialog), "udpscope-math-dialog"));
|
||
if (ty != nullptr && ty->nodes.size() >= 2) {
|
||
// Pile every node on one spot, so a layout that did nothing would be visible.
|
||
for (MathNode& n : ty->nodes) {
|
||
n.x = 10.0;
|
||
n.y = 10.0;
|
||
}
|
||
math_tidy(ty);
|
||
// No two boxes may overlap -- `math_node_at()` searches topmost-first, so an overlap means
|
||
// a click or a drag lands on the wrong node, which reads as the dialog ignoring the pointer.
|
||
int overlaps = 0;
|
||
std::string xs;
|
||
for (std::size_t i = 0; i < ty->nodes.size(); ++i) {
|
||
xs += (i != 0 ? "," : "") + std::to_string(static_cast<int>(ty->nodes[i].x));
|
||
for (std::size_t j = i + 1; j < ty->nodes.size(); ++j) {
|
||
const MathNode& p = ty->nodes[i];
|
||
const MathNode& q = ty->nodes[j];
|
||
if (p.x < q.x + kNodeW && q.x < p.x + kNodeW && p.y < q.y + kNodeH &&
|
||
q.y < p.y + kNodeH) {
|
||
++overlaps;
|
||
}
|
||
}
|
||
}
|
||
std::fprintf(stderr, "[math-probe] tidy: xs=[%s] overlaps=%d (want 0)\n", xs.c_str(),
|
||
overlaps);
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run it and read the wrong value**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'math_tidy' | head -3
|
||
```
|
||
|
||
Expected: an error naming `math_tidy` as undeclared.
|
||
|
||
- [ ] **Step 3: Add `math_tidy()`**
|
||
|
||
After `math_view_fit()`:
|
||
|
||
```cpp
|
||
/// Lays the graph out left to right by depth: a node sits one column right of the deepest thing it
|
||
/// reads, and its row is its position among the nodes at that depth. It needs no recursion and no
|
||
/// visited set -- the list is already in evaluation order, so a single forward pass has every
|
||
/// input's depth in hand by the time it reaches the node that reads it.
|
||
void math_tidy(MathDialog* d) {
|
||
const std::size_t n = d->nodes.size();
|
||
if (n == 0) return;
|
||
// Wider than the add-node grid's 160: a column pitch of `kNodeW + 48` leaves the wire its 40-unit
|
||
// tread on each side with room to curve. At 160 there were 28 units for a 40-unit tread, so the
|
||
// bezier's control points crossed and the wire doubled back on itself.
|
||
constexpr double kColPitch = kNodeW + 48.0;
|
||
constexpr double kRowPitch = kNodeH + 22.0;
|
||
std::vector<int> depth(n, 0);
|
||
std::vector<int> row(n, 0);
|
||
std::vector<int> used;
|
||
for (std::size_t i = 0; i < n; ++i) {
|
||
const MathNode& node = d->nodes[i];
|
||
int deepest = -1;
|
||
const int ins[2] = {node.in_a, node.in_b};
|
||
for (int k = 0; k < math_op_arity(node.op); ++k) {
|
||
if (ins[k] >= 0 && ins[k] < static_cast<int>(i)) {
|
||
deepest = std::max(deepest, depth[static_cast<std::size_t>(ins[k])]);
|
||
}
|
||
}
|
||
const int dep = deepest + 1;
|
||
depth[i] = dep;
|
||
if (static_cast<std::size_t>(dep) >= used.size()) used.resize(static_cast<std::size_t>(dep) + 1, 0);
|
||
row[i] = used[static_cast<std::size_t>(dep)]++;
|
||
}
|
||
for (std::size_t i = 0; i < n; ++i) {
|
||
d->nodes[i].x = 24.0 + static_cast<double>(depth[i]) * kColPitch;
|
||
d->nodes[i].y = 20.0 + static_cast<double>(row[i]) * kRowPitch;
|
||
}
|
||
// And frame it, since a tidy that leaves the graph off screen has moved the problem rather than
|
||
// solved it.
|
||
math_view_fit(d);
|
||
}
|
||
```
|
||
|
||
A `Source` and an unwired node both get depth 0, which is correct: neither reads anything.
|
||
|
||
- [ ] **Step 4: Add the button**
|
||
|
||
Beside the `fit` button from Task 2:
|
||
|
||
```cpp
|
||
GtkWidget* tidy = gtk_button_new_with_label("tidy");
|
||
gtk_widget_set_tooltip_text(tidy, "Lay the graph out left to right and frame it.");
|
||
g_signal_connect(tidy, "clicked", G_CALLBACK(+[](GtkButton*, gpointer user) {
|
||
math_tidy(static_cast<MathDialog*>(user));
|
||
}), d);
|
||
gtk_box_append(GTK_BOX(tools), tidy);
|
||
```
|
||
|
||
Use the same box name the `fit` button used.
|
||
|
||
- [ ] **Step 5: Build and run; read the right value**
|
||
|
||
```bash
|
||
cmake --build build 2>&1 | grep -E 'warning|error' | sort | uniq -c
|
||
```
|
||
|
||
Expected: four warnings, no errors. Then the probe run. Expected:
|
||
|
||
```
|
||
[math-probe] tidy: xs=[24,204,384,...] overlaps=0 (want 0)
|
||
```
|
||
|
||
`overlaps=0` is the assertion. The `xs` list shows the 180-unit column pitch and is a reading, not an assertion — a graph whose nodes are all at the same depth legitimately reads one x repeated, with the rows separating them.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add src/ui.cpp
|
||
git commit -m "$(cat <<'EOF'
|
||
feat: a tidy button lays the graph out by depth, so no two boxes overlap
|
||
|
||
A single forward pass -- the list is already in evaluation order, so every
|
||
input's depth is in hand by the time the reader is reached -- then a fit. The
|
||
column pitch is `kNodeW + 48` rather than the add-node grid's 160, which left
|
||
28 units for a wire's 40-unit tread and made the bezier double back.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 9: Documentation and the full verification run
|
||
|
||
**Files:**
|
||
- Modify: `CLAUDE.md` (the `UDPSCOPE_PROBE_MATH_MS` bullet under Verification; the math-channel invariant block; the warning baseline)
|
||
- Modify: `README.md` (the math-channel section)
|
||
- Modify: `src/help/11-math.txt`
|
||
|
||
**Interfaces:** consumes everything; produces nothing in code.
|
||
|
||
- [ ] **Step 1: Update the warning baseline in `CLAUDE.md`**
|
||
|
||
Under Verification, the paragraph beginning "Two baselines every run is read against. A clean build emits **five** warnings". Task 6 removes the unused `output_built`. Change the count and the list to **four** — two `-Wmissing-field-initializers` on `MathNode::source` and `-Woverlength-strings` on both generated GResource `.c` files — and say that the fifth was the editor's probe not reading a value it computed, now read by the validation check. **Verify the count from the build before writing it**; if the real figure is not four, write the real figure.
|
||
|
||
- [ ] **Step 2: Extend the `UDPSCOPE_PROBE_MATH_MS` bullet in `CLAUDE.md`**
|
||
|
||
The existing bullet is the `A math channel *is* a channel.` invariant block's final paragraph plus its sub-bullets. Append to the probe's description the new assertions, each with what a failure means:
|
||
|
||
- `hit:` — the output square straddles the box edge, so a hit test that recovers the node by a box test misses every pixel past it. `past-edge node=-1` is the silent disarm that was the whole of "edges cannot be added".
|
||
- `cancel: dialog=cleared` — Cancel must `gtk_window_close()`, not `gtk_window_destroy()`. It clicks the **button**, since the defect was in the connection and a direct handler call cannot see it.
|
||
- `view: zoom … anchor kept dx dy` — must be `0.0000 0.0000`. A wheel zoom that does not keep the graph point under the pointer reads as the canvas sliding away from the hand.
|
||
- `view: pan+zoom hit` — the hit tests run through `math_to_graph()`. **At the identity view a test that forgot the inverse passes anyway**, which is why this one is taken after a pan *and* a zoom.
|
||
- `view: fit … all_inside=1` and `view: reset scale=1.000 offset=(0.0,0.0)`.
|
||
- `view: pan mode=3 dview=30.0 dnode=0.0` — a background drag pans and moves no node.
|
||
- `wire: connect mode=2 in_a=0`, `detach: emptied_at_begin=-1 in_a=-1`, `detach: escape restores in_a=0`.
|
||
- `wire: pick … -> in_a=-1`.
|
||
- `wire: backwards ok=1 … valid=1` — the re-sort. `valid=1` is the assertion; the `order=` string is the reading that explains it.
|
||
- `wire: self refused=1`, `wire: cycle refused=1` — and **nothing is written** on either.
|
||
- `validate: broken problem=[..] apply=0; fixed problem=[] apply=1`, `validate: no-selection label=[..]` (non-empty — the line is written **above** `math_dialog_sync()`'s early return), `validate: source list=N acquired=N`.
|
||
- `op pick: … op_index=.. q_row=1` and `op pick: index round trip=1`.
|
||
- `tidy: … overlaps=0`.
|
||
|
||
- [ ] **Step 3: Add the new invariants to `CLAUDE.md`**
|
||
|
||
Inside the `A math channel *is* a channel.` block, add sub-bullets in that block's existing voice (a rule, then what went wrong, then the guard):
|
||
|
||
- **A port hit must name its own node.** The output square is drawn centred on `n.x + kNodeW`, straddling the box's right edge, with a `±kPort` hit radius against a `±kPort*0.5` drawn half-width — so recovering the node with `math_node_at()`, which matches only inside the box, returns `-1` for every pixel past the edge. `math_port_at()` returns the node directly for that reason.
|
||
- **Every hit test goes through `math_to_graph()`, and every tolerance is a screen distance divided by `view_scale`.** A tolerance left in graph units makes a port unhittable zoomed out and greedy zoomed in. **A probe that tests a hit at the identity view asserts nothing** — the check has to be taken after a pan and a zoom.
|
||
- **A drag's mode is decided once, at `drag-begin`, and the precedence is port, box, background.** Asking the box first makes the output square unreachable, which is the same geometry as the bullet above.
|
||
- **A detach empties the input at `drag-begin`.** That is what makes a release over nothing the detach gesture rather than a second verb, and it is why `Escape` has something to restore.
|
||
- **A backwards wire re-sorts the list; it is not refused.** The evaluator walks the nodes in order, and until now that made the order the *user's* problem — an earlier node's input was not even drawn as connectable. `math_connect()` runs Kahn's algorithm over the **trial** edge set, so a cycle is found before anything is written, breaks ties by the current list index so a graph already in order is not permuted, and remaps **`output`, `selected`, every `in_a`/`in_b`, `drag_node`, `wire_from`, `wire_detach` and `selected_wire.node`** in one block. A missed index silently renames an operation or re-points a wire. Its `edges_of` masks each input by the op's **arity**: a node changed from binary to unary keeps a stale `in_b`, and counting it makes an indegree that never reaches zero and so reports a loop that is not there.
|
||
- **The validation line is written above `math_dialog_sync()`'s early return.** An out-of-range selection is exactly the state in which a stale message misleads most: the graph can still be broken and there is no form to read instead.
|
||
- **`math_op_index()`, never a cast.** `math_op_at()`'s order is the dialog's — what a dropdown lists and what a session file stores — and `static_cast<guint>(op)` worked only because the two happen to agree today. The session-file rule in this file already says an op is written as its index through `math_op_at()`; this is the other direction of the same rule.
|
||
- **A `Delete` key removes the selected *wire* and never falls through to the selected node.** A key whose target depends on an invisible selection state is how a graph gets destroyed by accident; the `delete node` button stays the only way to remove a node.
|
||
- **The tidy layout's column pitch is `kNodeW + 48`, not the add grid's 160.** At 160 a wire has 28 units for its 40-unit tread, so the bezier's control points cross and the wire doubles back on itself.
|
||
|
||
- [ ] **Step 4: Update `README.md`**
|
||
|
||
In the math-channel section, describe the editor as it now is: drag a wire from an output to an input; drag a wired input off to detach it; click a wire and `Delete` to remove it; wheel to zoom about the pointer, drag the background to pan, `f` to fit, `Ctrl+0` to reset; `tidy` to lay out; the op is chosen when the node is added; the line under the form says why the graph cannot be saved and Apply is insensitive while it is there; a wire drawn right to left re-sorts the list silently. State the deviation from the old behaviour explicitly — the two-click protocol and the two modal alerts are gone.
|
||
|
||
- [ ] **Step 5: Update `src/help/11-math.txt`**
|
||
|
||
Read the file first for its Pango-markup conventions and the voice of the other thirteen pages. Rewrite the editor paragraph to describe the gestures above. **Every `&` must be `&`, every `<` `<`** — the body is Pango markup validated with `pango_parse_markup()` at load, and a page that fails renders a one-line stand-in. The `[help]` probe is the check.
|
||
|
||
- [ ] **Step 6: The full verification run**
|
||
|
||
Four runs, **one at a time**, each killing its mock.
|
||
|
||
The math probe, against the fast mock (the two standing assertions for math are `gaps=0` and each `math '<name>': ... N samples vs M time slots` being equal):
|
||
|
||
```bash
|
||
python3 tools/mock_udpstreamer.py --port 8111 --rate 1300000 --batch 2000 --duration 30 &
|
||
MOCK=$!
|
||
xvfb-run -a env UDPSCOPE_ENABLE_ALL_MS=1000 UDPSCOPE_PROBE_MATH_MS=2000 \
|
||
UDPSCOPE_EXIT_AFTER_MS=12000 ./build/udpscope --server 127.0.0.1 --port 8111 \
|
||
2>&1 | grep -E "\[math-probe\]|math '|gaps|math cost|Gtk-CRITICAL"
|
||
kill $MOCK 2>/dev/null
|
||
```
|
||
|
||
The help probe, which is what validates the rewritten page (`parsed` must equal `pages`):
|
||
|
||
```bash
|
||
python3 tools/mock_udpstreamer.py --port 8112 --duration 20 &
|
||
MOCK=$!
|
||
xvfb-run -a env UDPSCOPE_PROBE_HELP_MS=1500 UDPSCOPE_EXIT_AFTER_MS=8000 \
|
||
./build/udpscope --server 127.0.0.1 --port 8112 \
|
||
2>&1 | grep -E '\[help\]|Failed to parse|Gtk-CRITICAL'
|
||
kill $MOCK 2>/dev/null
|
||
```
|
||
|
||
Expected: `[help] pages=14 parsed=14 ...` and **no** `Failed to parse`.
|
||
|
||
A plain exit-dump run, for the layout and icon baselines nothing here should have touched:
|
||
|
||
```bash
|
||
python3 tools/mock_udpstreamer.py --port 8113 --duration 20 &
|
||
MOCK=$!
|
||
xvfb-run -a env UDPSCOPE_EXIT_AFTER_MS=8000 UDPSCOPE_ENABLE_ALL_MS=1000 \
|
||
./build/udpscope --server 127.0.0.1 --port 8113 \
|
||
2>&1 | grep -E 'layout over|axes.w changes|x-desync|toolbar icons|app icons|Gtk-CRITICAL'
|
||
kill $MOCK 2>/dev/null
|
||
```
|
||
|
||
Expected: `x-desync 0`, every icon `(ok)`, `axes.w changes` small, no `Gtk-CRITICAL`.
|
||
|
||
And the measures probe with no unit, which is the run that exercises `format_measure()` on a math channel's unitless values:
|
||
|
||
```bash
|
||
python3 tools/mock_udpstreamer.py --port 8114 --unit '' --duration 25 &
|
||
MOCK=$!
|
||
xvfb-run -a env UDPSCOPE_ENABLE_ALL_MS=1000 UDPSCOPE_PROBE_MEASURES_MS=2500 \
|
||
UDPSCOPE_EXIT_AFTER_MS=10000 ./build/udpscope --server 127.0.0.1 --port 8114 \
|
||
2>&1 | grep -E '\[measures-probe\]|measure [0-9]|measures pane|Gtk-CRITICAL'
|
||
kill $MOCK 2>/dev/null
|
||
```
|
||
|
||
Delete any session files the runs left behind so a later run does not inherit a layout:
|
||
|
||
```bash
|
||
rm -f ~/.config/udpscope/127.0.0.1_811[1-4].toml
|
||
```
|
||
|
||
- [ ] **Step 7: Commit**
|
||
|
||
```bash
|
||
git add CLAUDE.md README.md src/help/11-math.txt
|
||
git commit -m "$(cat <<'EOF'
|
||
docs: the node editor's gestures, its new probe assertions and its invariants
|
||
|
||
Each invariant records what went wrong rather than only the rule: a port hit
|
||
that lost its node, a hit test at the identity view that asserts nothing, a
|
||
`Delete` that must not fall through to the selected node, and a column pitch
|
||
too narrow for a wire's own tread. The warning baseline drops to four.
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
## Self-review
|
||
|
||
**Spec coverage.** Every section of `docs/superpowers/specs/2026-10-06-math-node-editor-design.md` maps to a task: "Why edges cannot be added today" → Task 1; "The view transform" → Task 2; "The gesture state machine" → Task 3; "The port and wire hit tests" → Tasks 1 and 4; "The edge model" → Tasks 3–5; "The live validation line" → Task 6; "The op picker on add" → Task 7; "Tidy / auto-layout" → Task 8; "The `Cancel` button closes rather than destroys" → Task 1; "What `[math-probe]` must assert" → distributed across Tasks 1–8 and summarised in Task 9; "Documentation" → Task 9.
|
||
|
||
**Two things the spec listed as checks are constraints here, not assertions**, because no headless probe can see them: that the canvas takes focus on press (stated in Task 2 Step 7 and Task 3 Step 6, with the reason), and that the live wire tracks the pointer visually (Task 3 Step 6 draws it; nothing asserts its appearance).
|
||
|
||
**Out of scope and deliberately not in any task:** the item-2 feature shortlist (measurement statistics, the extra trigger types, PNG export, FFT, event history, search, reference waveforms) — those need their own design round. The `publish_pause_view()` values/stamps shear is a separate, unrelated defect and is not touched here.
|