From de85dae5be317032c73a1a25033f6a72642afd41 Mon Sep 17 00:00:00 2001 From: Shreyan C Date: Fri, 4 Sep 2026 22:37:02 +0530 Subject: [PATCH] fix(blocks): a run is drawn where the reader is standing, and a note lives on one level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every path that reads App.states/App.transitions where it should read the view answers about a diagram that is not on screen, and does so silently, because the model stays perfectly consistent. Six surfaces were still doing that, and each is invisible on any machine without a block on it. The playback highlight failed as a whole rather than in part: getSimStepEdgeKeys built its key from a transition's own endpoints and updateSimCanvasHighlights looked its states up by their own ids, so from the first step a run touched a block every mark went at once while the verdict stayed correct. Both halves go through the projection now (viewEdgeKeyFor, visibleNodeIdFor), ids are resolved to drawn nodes before anything is compared, what resolves to nothing is dropped rather than guessed at, and trailUpTo's cache carries the scope. Marks reach one level in, since a box that lights up says "something in here" and nothing more. markPreviewRun writes onto elements drawPreview already built — __pvIndex and __pvEdgeD, stamped in loops it was already running — so nothing is rebuilt; the active edge is a second path rather than a class, and no travelling token goes in there. slideBlockPreview is one function with two callers now: blockPreviewKey is built from the members' positions, so Arrange, a paste, an undo, a nudge, a collision push and the JFLAP importer's spread all moved a box and left its diagram behind. pulseSimNode traces the node's own outline, so an arrival ring on a block is a rect rather than a circle sweeping half the diagram. drawnStateEl/drawnEdgeEl replace the same lookup written out per surface, and fix three more: a crossing edge selected with nothing on screen saying so, a note's anchors lighting its states and none of its edges, and the Language panel highlighting the opposite of the |Q| printed beside it. note.scope is the one thing here that was not a lookup bug. A note is written somewhere, and until it said so it was drawn at every level at once against a state that level does not draw. Absent means the top level, so no serializer changed and there is no migration; a note resolves its anchors against its own level rather than the reader's; ungrouping carries its notes to the parent while noteScopeOf is the safety net for every other way a block can vanish. And viewGraph() is cheap again: a cache hit recomputed a derived block size, falling through to two unindexed filters on the ordinary path, which a select-all over 2000 transitions measured at 617ms with eight blocks against 7ms without. --- CLAUDE.md | 24 ++- css/canvas.css | 106 ++++++++++++ js/blocks-ui.js | 23 ++- js/blocks.js | 51 +++++- js/canvas.js | 104 +++++++---- js/graph-thumb.js | 35 ++-- js/language.js | 40 +++-- js/minimap.js | 18 +- js/notes.js | 127 ++++++++++++-- js/render.js | 114 +++++++++++- js/scope.js | 14 ++ js/simulation.js | 225 +++++++++++++++++++++--- js/ui.js | 15 +- js/view-graph.js | 58 ++++++- tests/block-render.test.js | 54 ++++++ tests/note-scope.test.js | 273 +++++++++++++++++++++++++++++ tests/selection.test.js | 89 ++++++++++ tests/sim-blocks.test.js | 345 +++++++++++++++++++++++++++++++++++++ tests/view-graph.test.js | 78 +++++++++ 19 files changed, 1667 insertions(+), 126 deletions(-) create mode 100644 tests/note-scope.test.js create mode 100644 tests/sim-blocks.test.js diff --git a/CLAUDE.md b/CLAUDE.md index a902a75..864e76b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -433,6 +433,7 @@ Three node kinds share one list, because every one of them is a thing with an `x - **Identity is load-bearing.** `relayout()` refuses the incremental path when `prev.states !== states`, so a projection that rebuilt its arrays every frame would take a full layout pass sixty times a second and undo the whole of the node-shape work. The graph is cached and validated the way `stateIndex()` is — nothing announces that `App.states` changed — and a cache *hit* refreshes the block and port nodes **in place**, so a dragged block moves without any array changing identity. `refresh()` walks `cache.dynamic` and not `cache.states`, because `viewGraph()` is on the hot path: `edgeLabelsHidden()` reaches it once per edge label, and an O(states) loop there would be quadratic. The validation compares field by field against values stamped on the cache rather than against a freshly built signature object, for the same reason. - **A rewritten edge is `Object.create(realTransition)`, not a copy.** `curve` and `loopAngle` are written on the *real* transition by a bend drag, and a copied field would be a stale snapshot the layout pass went on reading for the rest of the session. The proxy owns only `from` and `to`. `edgeGroupFor` resolves through `viewEdgeGroup` rather than filtering `App.transitions` by the drawn key, because `s5|b1` is a pair the model does not contain. +- **A cache hit must not recompute a derived block size, and that is what makes `viewGraph()` cheap enough to be on the hot path at all.** `refresh()` called `blockSize()`, which falls through to `blockMembers()` + `blockChildren()` whenever a record carries no size of its own — and `inlineBlock` leaves those null, so that is the ordinary case rather than the exception. Both are unindexed filters that allocate, and this function is reached per edge label by `edgeLabelsHidden()` and per item by every surface that resolves a machine id to a drawn one. A select-all over 2000 transitions measured **617ms with eight blocks against 7ms without**, and the *root* cost was in a function whose whole job is to be free. Skipping it is sound because what a derived size derives from cannot change without `stillValid()` failing and the projection being rebuilt outright; only a hand-set size can move under a hit, and that is two reads off the record. [tests/view-graph.test.js](tests/view-graph.test.js) pins it on **object identity** rather than on a timer, because an equal box rebuilt every call is exactly the walk being guarded against and looks identical from the outside. - **`ownerMap` is one pass over the machine, not one per block.** A CPU is twenty blocks over three thousand states, and asking each block for its subtree separately is that product; the walk memoises per block on the way up instead. **`largeMachineProfile()` judges the view, not the model.** A CPU is three thousand states in `App.states` while the reader is looking at eight boxes, and the profile as written would strip the edge labels and the eased layout off an eight-node diagram. `machineIsLarge()` reads `drawnSize()`, which `view-graph.js` installs into `state.js` as a **function** — pushed as a value it goes stale the moment a loader replaces `App.states` without the projection having been read since. What scales with the *model* rather than the view — the undo stack's byte budget, the autosave stringify — is still bounded by its own numbers. @@ -472,11 +473,30 @@ Three node kinds share one list, because every one of them is a thing with an `x **It is listed under the Blocks section, not in a dialog.** "The blocks in this machine" and "the blocks I can drop into it" are one question asked twice, and a fourteenth overlay to answer the second half would be a surface to go and find rather than a list already in front of you. Without it the store was **write-only** — saving reported success and nothing anywhere could list, place or delete what it had kept, which is worse than no store, because the reader is told the thing was saved. -**The projection is not optional for anything that draws or is clicked on, and the failure mode is always the same shape.** Every path that reads `App.states`/`App.transitions` where it should read the view answers about a diagram that is not on screen — and it does so *silently*, because the model is still perfectly consistent. The ones that had to move: `minZoom` (keyed on `App.states`, so the floor stayed machine-sized inside a block and the reader could not zoom out to what they were looking at), the marquee's transition sweep (it selected edges from every other scope and could select no crossing edge, since `getState` answers null for a block id), the incoming/outgoing edge highlight (it built the DOM key from the transition's own endpoints, and a crossing edge is registered under `b1|s1`), `isMachineFullyVisible`, `autoLayout`, the minimap's start marker, and `updateFastDOM`'s copy of the start arrow. That last one is worth its own note, because it is the one the reader sees moving: **`startArrowD()` is one function with two callers** — the full render and the drag path — and they had drifted, so the arrow sat still through a whole drag and only caught up on the next full render, which a click on the background happens to cause. +**The projection is not optional for anything that draws or is clicked on, and the failure mode is always the same shape.** Every path that reads `App.states`/`App.transitions` where it should read the view answers about a diagram that is not on screen — and it does so *silently*, because the model is still perfectly consistent. The ones that had to move: `minZoom` (keyed on `App.states`, so the floor stayed machine-sized inside a block and the reader could not zoom out to what they were looking at), the marquee's transition sweep (it selected edges from every other scope and could select no crossing edge, since `getState` answers null for a block id), the incoming/outgoing edge highlight (it built the DOM key from the transition's own endpoints, and a crossing edge is registered under `b1|s1`), `isMachineFullyVisible`, `autoLayout`, the minimap's start marker, the whole of the playback highlight, the selection classes, a note's anchor highlight, the Language panel's tuple and symbol highlights, and `updateFastDOM`'s copy of the start arrow. That last one is worth its own note, because it is the one the reader sees moving: **`startArrowD()` is one function with two callers** — the full render and the drag path — and they had drifted, so the arrow sat still through a whole drag and only caught up on the next full render, which a click on the background happens to cause. + +**The playback highlight was the last one, and it failed as a whole rather than in part.** `getSimStepEdgeKeys` built its key from each transition's own endpoints and `updateSimCanvasHighlights` looked its states up by their own ids — both exactly right about the *machine*, and both naming things the canvas has no node for the moment a block is on it: a step that crosses a boundary is drawn as `x2|b1`, a pair the model does not contain, and a step *inside* a block names a state that is not drawn at all. So from the first step a run touched a block, every mark went at once — no trail, no active edge, no travelling token, no arrival pulse — while the verdict stayed correct throughout, which is what made it read as "the animations are broken" rather than as anything to do with blocks. Both halves go through the projection now (`viewEdgeKeyFor`, `visibleNodeIdFor` — which existed for this and had no callers), and three details follow from it. **Ids are resolved to drawn nodes before anything is compared**, because several states inside one block are one box: compared as machine ids, "is this one already the playhead?" is asked about ids the canvas does not have, and the box gets both the active and the visited mark. **What resolves to nothing is dropped rather than guessed at** — an edge wholly inside a block is not on screen, and the box standing in for it is already lit by the state half. And **`trailUpTo`'s cache now carries the scope**, since its keys are drawn keys: drilling in while a run is paused changes which node every step of it is shown by, so the trail is rebuilt at the cost of a backward scrub rather than carried across as a set of keys for a diagram that is no longer there. `enterBlockScope` repaints the highlight for the same reason — a drill-in evicts every node the previous scope drew, and a run paused inside a block was invisible from the moment you went in to look at it. + +**And it reaches one level in, because a box is a drawing.** Marks that stop at the box say "something in here" and nothing more — the box lights up and the dot that is actually running stays the same grey as the twenty around it. `markPreviewRun` writes onto elements the preview already built: `drawPreview` stamps `__pvIndex` (a state or child block → its dot) and `__pvEdgeD` (a drawn pair → its own subpath) in the loops it was already running, so a step costs a short ancestry walk per mark, a Map get, and one `d` write per box the run is actually inside. **Nothing is rebuilt**, and that is what makes it affordable: a run moves no state and changes no transition, so `blockPreviewKey` does not move and the hundred child elements stay exactly where they were. Three things it is careful about. **`previewNodeIdFor` is the companion to `visibleNodeIdFor`, one level in** — that one says which box shows a state, this one says where in that box, and it has to exist because a preview draws the *immediate* contents only, so a state three levels down is represented by a nested block's rect and has no dot of its own. **The active edge is a second path, not a class**, since every preview edge shares one `d` and a path per edge is a hundred elements per box — the cost the single path exists to avoid; `thumbEdgeSegments` now carries the pair's `key` so the bright subpath is the very one the quiet layer under it drew, rather than a second computation that could land beside it. And **no travelling token goes in there**: at preview scale an interior edge is a dozen pixels and a node is two, so the dot would be larger than the states it travels between — the one part of the canvas animation that does not survive being shrunk. + +**A preview is slid, so something has to slide it, and for a long time only the drag path did.** The interior is laid out in absolute canvas coordinates once and moved with a single transform, which is what keeps a drag frame from rebuilding a hundred elements — but `blockPreviewKey` is built from the *members'* positions, and moving a box changes none of them. So every path that moves a block and ends in a full render rather than in a drag frame left the diagram behind: Arrange, a paste, an undo, an arrow-key nudge, a collision push, the JFLAP importer's `spreadForBlocks`. On a machine whose layout had been rearranged that is a canvas of empty boxes with their diagrams scattered across the background. `slideBlockPreview()` is one function with two callers now, the same shape of fix `startArrowD()` carries its own note about — and the clip rect still stays written at `__previewAt`, because `clipPathUnits` is `userSpaceOnUse` and the transform already carries it. + +**And the arrival ring is asked what shape it is landing on.** `.sim-pulse` is a circle of radius `R` growing 1.7×, which is right for a state and wrong twice over on a block: the ring floats inside a box two hundred pixels wide, reading as a second unexplained mark, and 1.7× of that box sweeps out over half the diagram. `pulseSimNode` traces the node's own outline — a rect for a block or a port, matching its `rx` — and `.sim-pulse.is-box` is the gentler ramp. The pulse also picked up `vector-effect: non-scaling-stroke`, which every other simulation stroke already carried and which matters most here: an arrival is hardest to spot at exactly the zoom where a 2px ring divided by the camera is nothing. + +**The last four were not playback and failed identically, which is the point.** `drawnStateEl` and `drawnEdgeEl` in [js/render.js](js/render.js) are the two lookups every surface that lights something on the canvas needs, in one place because they were being written out per surface — the registry first and a selector second, with the fallback deliberately *not* scoped to `.sn`, since the answer may be a block's box or a port's tab. What they replaced: `syncSelectionClasses`, where an edge crossing into a block sat in `App.selectedTransitions` with nothing on screen saying so and Delete took it anyway — a selection you cannot see is one you cannot check; `highlightNoteAnchors`, where hovering a note lit the states it names and none of its edges, reading as a lost anchor rather than as the diagram having changed shape around it; and `langHighlight`/`langHighlightSymbol`, where hovering **Q** on a machine built out of blocks lit the handful of states left at the top level while the |Q| printed beside it counted every state at every depth — the highlight saying the opposite of the number it sits under. Elements are deduped rather than keys, because several transitions share one drawn edge and several states share one box. + +**A note lives on one level, and `note.scope` is what says which.** It was the one thing in this section that was not a lookup bug: a note is written *somewhere* — at the top level, or inside a block someone had drilled into — and until it said so it was drawn at every level at once, positioned against a state the level showing it does not draw. Dragging the box then stranded it, because a box moves without its members moving: the preview is a *fit* of their coordinates, so where they actually sit is irrelevant to the drawing and nothing pulled the note along. `renderNotes`, `updateNotesDOM`, `includeNoteBounds`, the marquee and select-all all take `visibleNotes()` now; `pruneNoteAnchors*` deliberately does not, being model integrity rather than drawing. + +Four things about it are worth keeping. + +- **Absent means the top level, so no serializer was edited and there is no migration.** `roundForSave` copies a note whole and rounds only the fields it names, and `exportWorkspaceState` is a JSON deep copy — `scope` rides along exactly as `blockId` does on a state — and it is written only when it is *not* null, so a machine with no blocks in it saves the bytes it always saved. **This is deliberately not a `SCHEMA_VERSION` bump**: the rule above says an optional additive field is not one, and `assertReadableSchema` *throws* on a document from the future, so bumping would make every already-shipped build refuse every file saved after the change — to gain nothing an absent field does not already give. +- **`nodeIdAtScope(stateId, scope)` is `visibleNodeIdFor` for an arbitrary level**, which is why it is no longer named after previews. It has two callers wanting different things from one fact: a block's preview marking a running transition, and a note working out where it sits. `visibleNodeIdFor` stays the fast path for the scope on screen — it is a Map get off the cached owner map, where this is a walk up the block tree. +- **A note resolves its anchors against its *own* level, never the reader's**, and getting that wrong is silent. The projection answer is what gives the "one level down" case for free — a note about a state since grouped into a block points at the box and rides along with it. But `pruneNoteAnchorsRemoving` calls the same function to *hold a note still* while its anchors are taken away, and it runs over every note at every level: resolved against wherever the reader happens to be standing, one note answers two positions and the prune freezes it at the one it was never drawn at. Asking about the note's own level gives one answer whoever is asking — and means no view graph is consulted at all, since both a state and a block carry their own coordinates. +- **Ungrouping carries its notes to the parent; a *deleted* block's surface at the top.** Those are different questions and only one of them can be answered on read: a dissolved block knows where its contents went, so `ungroupBlock` re-parents them explicitly, while `noteScopeOf` validating a scope that no longer exists is the safety net for every other way a block can vanish — the rule `liveScope()` and `blockIsIntact()` follow. Losing the note instead would be losing text somebody wrote. `enterBlockScope` also drops from `App.selectedNotes` anything the new level does not draw: before notes had a level a stale selection was at least a visible one, and Delete would otherwise take something nobody can see. **A paste lands in the scope the reader is standing in.** `{...s}` carries the source state's own `blockId`, so states copied while drilled into a block arrived still claiming to belong to it — pasted at the top level they vanished straight back inside the block they came from, with nothing on screen to say where they had gone. And a selected *block* is a node the reader clicked, so Ctrl+C has to mean something for it: it is copied as its **definition** (`outlineBlock`) and pasted through `inlineBlock`, the same path the library uses, rather than a second copier that could disagree with it. -[tests/view-graph.test.js](tests/view-graph.test.js) pins the projection, the array identity across a drag, the profile judging the view, that drilling in moves nothing in the model, and that a port is placed rather than pinned. [tests/block-render.test.js](tests/block-render.test.js) pins the preview against the minimap's own geometry and that a drag rebuilds none of it. [tests/block-actions.test.js](tests/block-actions.test.js) pins that grouping and ungrouping leave the language alone, in one undo step each. +[tests/view-graph.test.js](tests/view-graph.test.js) pins the projection, the array identity across a drag, the profile judging the view, that drilling in moves nothing in the model, and that a port is placed rather than pinned. [tests/block-render.test.js](tests/block-render.test.js) pins the preview against the minimap's own geometry and that a drag rebuilds none of it. [tests/block-actions.test.js](tests/block-actions.test.js) pins that grouping and ungrouping leave the language alone, in one undo step each. [tests/sim-blocks.test.js](tests/sim-blocks.test.js) pins the playback highlight against the projection — the crossing edge's drawn key, the box carrying the playhead for a state nobody can see, several interior states as one mark, the ring's shape, the trail rebuilding across a drill-in, and the preview marks — including that a step touching no block writes into no preview and that marking a run rebuilds none of them — plus the four non-playback surfaces above, since each is invisible on any machine without a block on it. [tests/block-render.test.js](tests/block-render.test.js) pins the slide against a record moved by anything but a drag. [tests/note-scope.test.js](tests/note-scope.test.js) pins the level a note lives on — that the field is absent at the top, that each level draws only its own, the one-level-down anchor and its ride along a dragged box, both ways a block can stop existing, and that the prune still holds a note still when it is run from another level. ### Simulation diff --git a/css/canvas.css b/css/canvas.css index 8ad0605..85e9cd5 100644 --- a/css/canvas.css +++ b/css/canvas.css @@ -241,6 +241,37 @@ vector-effect: non-scaling-stroke; } +/* A run does not stop at a block's edge — it steps inside, where there is + nothing on screen to mark. The box standing in for those states is what the + reader can see, so it takes the playhead's marks: the machine is flat, and + which node is drawn for a state is the projection's answer (js/view-graph.js), + not the run's. Written against `.bn-body` because a block is a rounded rect + where a state is a circle, and the ink is the only difference. */ +.bn.act-st .bn-body, +.pn.act-st .pn-body { + fill: var(--state-active-fill); + stroke: var(--accent); + stroke-width: 2; + filter: var(--state-active-shadow); + vector-effect: non-scaling-stroke; +} + +.bn.rej-st .bn-body, +.pn.rej-st .pn-body { + fill: var(--state-reject-fill); + stroke: var(--red); + stroke-width: 2; + vector-effect: non-scaling-stroke; +} + +.bn.sim-visited-st .bn-body, +.pn.sim-visited-st .pn-body { + stroke: var(--accent); + stroke-opacity: .65; + stroke-width: 1.5; + vector-effect: non-scaling-stroke; +} + .edge-g.sim-trail-t .tarr { stroke: var(--accent); stroke-width: 1.5; @@ -317,9 +348,22 @@ pointer-events: none; transform-box: fill-box; transform-origin: center; + /* Screen-space, like every other simulation stroke above: a 2px ring divided + by the camera is nothing at the zoom a large machine is read at, which is + where an arrival is hardest to spot in the first place. */ + vector-effect: non-scaling-stroke; animation: sim-pulse .5s ease-out forwards; } +/* The same arrival, on a box. The ratio is what carries the meaning, not the + distance: 1.7x of a 22px circle is a ring that reads at a glance and 1.7x of a + 200px block is a sweep across half the diagram. A box is already the largest + thing on the canvas, so it needs the smaller ramp and a little longer to be + read. */ +.sim-pulse.is-box { + animation: sim-pulse-box .55s ease-out forwards; +} + .sim-pulse.rej { stroke: var(--red); } @@ -340,6 +384,18 @@ } } +@keyframes sim-pulse-box { + from { + transform: scale(1); + opacity: .9; + } + + to { + transform: scale(1.14); + opacity: 0; + } +} + @media (prefers-reduced-motion: reduce) { .edge-g.sim-active-t .tarr { animation: none; @@ -1407,6 +1463,56 @@ stroke: var(--gold); } +/* ─── The run, inside a preview ─── + A box on the canvas is a small drawing of the machine inside it, so the + playback marks have to reach one level in: without these the box lights up + and the dot that is actually running stays the same grey as the twenty around + it, which says "something in here" and nothing more. + + The dots are 1.6–7px, so the mark cannot be a stroke weight — at the bottom + of that range a 1px ring is the whole node. It is a fill change plus a glow, + which reads at any of those sizes, and the glow is what carries it when the + dot itself is two pixels across. */ +.bn-pv-node.is-visited { + fill: color-mix(in srgb, var(--accent) 45%, var(--bg2)); +} + +.bn-pv-node.is-active, +.bn-pv-block.is-active { + fill: var(--accent); + filter: drop-shadow(0 0 3px color-mix(in srgb, var(--accent) 85%, transparent)); +} + +.bn-pv-node.is-rej, +.bn-pv-block.is-rej { + fill: var(--red); + filter: drop-shadow(0 0 3px color-mix(in srgb, var(--red) 85%, transparent)); +} + +.bn-pv-block.is-visited { + fill: color-mix(in srgb, var(--accent) 30%, var(--bg2)); +} + +/* The transition being taken, over the quiet edges and under the dots — the + paint order the canvas itself uses. Empty `d` the rest of the time, which is + why it costs nothing on a box the run is nowhere near. It carries no dash + animation: at this scale a 7-5 dash pattern on a twelve-pixel edge is one + dash, flickering. */ +.bn-pv-active { + fill: none; + stroke: var(--accent); + stroke-width: 1.6; + stroke-linecap: round; + vector-effect: non-scaling-stroke; + pointer-events: none; + filter: drop-shadow(0 0 3px color-mix(in srgb, var(--accent) 70%, transparent)); +} + +.bn-pv-active.is-rej { + stroke: var(--red); + filter: drop-shadow(0 0 3px color-mix(in srgb, var(--red) 70%, transparent)); +} + .bn-pv-node.is-start { stroke: var(--green); } diff --git a/js/blocks-ui.js b/js/blocks-ui.js index 6d57f96..e36c291 100644 --- a/js/blocks-ui.js +++ b/js/blocks-ui.js @@ -20,12 +20,13 @@ import { blockAncestry, blockChildren, blockMembers, getBlock, inlineBlock, liveBlocks, - machineSupportsBlocks, outlineBlock, removeBlock, uniqueBlockName, + blockRemovalIds, machineSupportsBlocks, outlineBlock, removeBlock, uniqueBlockName, validateBlockDefinition, blockDefinitionCycle, BLOCK_NAME_SEP } from './blocks.js'; import { clearSelection } from './canvas.js'; import { commit } from './history.js'; import { askConfirm } from './modal.js'; +import { pruneNoteAnchorsExcluding } from './notes.js'; import { openWorkspaceDb } from './persistence.js'; import { enterBlockScope, syncScopeBar } from './scope.js'; import { $, App, getState, stateNameKey } from './state.js'; @@ -173,6 +174,15 @@ export function ungroupBlock(id) { if (parent) s.blockId = parent; else delete s.blockId; } for (const child of blockChildren(id)) child.parent = parent; + // The notes written inside this block come up with its states. Left behind, + // `noteScopeOf` would answer null for a block that no longer exists and they + // would all surface at the *top* level rather than at the one their states + // just landed on — the right rescue for a deleted block and the wrong answer + // for a dissolved one, which knows its own parent. + for (const n of App.notes || []) { + if (n.scope !== id) continue; + if (parent) n.scope = parent; else delete n.scope; + } App.blocks = (App.blocks || []).filter(x => x.id !== id); invalidateViewGraph(); }, Change.GRAPH); @@ -326,7 +336,16 @@ export function ctxDeleteBlock() { confirmLabel: 'Delete', danger: true, onConfirm: () => { - commit(() => { removeBlock(id); invalidateViewGraph(); }, Change.GRAPH); + commit(() => { + // While the ids are still resolvable, so a note *outside* the block that + // anchors into it keeps the position it was drawn at rather than jumping + // to its stored offset. The notes written inside go with the block — + // removeBlock takes those. + const gone = blockRemovalIds(id); + pruneNoteAnchorsExcluding([...gone.states], gone.transitions); + removeBlock(id); + invalidateViewGraph(); + }, Change.GRAPH); showStatus(`Deleted ${b.name}`); } }); diff --git a/js/blocks.js b/js/blocks.js index 194e868..94fb5e8 100644 --- a/js/blocks.js +++ b/js/blocks.js @@ -674,17 +674,62 @@ export function machineAsBlockDefinition(opts = {}) { * Like inlineBlock, this neither snapshots nor emits: deleting a block is one * edit, and the caller owns the undo point. */ -export function removeBlock(id) { +/** + * What removing a block would take with it: every state at every depth behind + * the box, and every transition touching one. + * + * Exported because the *callers* need it before the call. `pruneNoteAnchors- + * Excluding` has to run while the ids are still resolvable, so a note anchored + * into the subtree can be held where it was drawn — and the Delete key's own + * prune names `App.selectedStates`, which for a block holds the box's id and no + * state at all, so it named nothing and the notes settled at their stored + * offsets instead. One declaration, so what is pruned cannot drift from what is + * removed. + */ +export function blockRemovalIds(id) { const subtree = new Set(blockSubtree(id)); - if (!subtree.size) return false; - const doomed = new Set((App.states || []) + const states = new Set((App.states || []) .filter(s => s.blockId && subtree.has(s.blockId)) .map(s => s.id)); + const transitions = (App.transitions || []) + .filter(t => states.has(t.from) || states.has(t.to)) + .map(t => t.id); + return { subtree, states, transitions }; +} + +export function removeBlock(id) { + const { subtree, states: doomed } = blockRemovalIds(id); + if (!subtree.size) return false; if (!doomed.size && !subtree.size) return false; App.states = (App.states || []).filter(s => !doomed.has(s.id)); App.transitions = (App.transitions || []).filter(t => !doomed.has(t.from) && !doomed.has(t.to)); App.blocks = (App.blocks || []).filter(b => !subtree.has(b.id)); + // The notes written *inside* the subtree go with it. Surfacing them at the + // top level instead — which is what `noteScopeOf` does for a scope that has + // simply stopped existing — is the right rescue for a record that vanished by + // accident and the wrong answer for one the reader deliberately deleted: a + // block with thirty notes in it would empty thirty notes onto the machine + // above, at coordinates from another level, every one of them pointing at + // states this call has just removed. Deleting a block means deleting what was + // in it, and one Ctrl+Z brings the notes back with everything else, because + // serializeState carries them. + // + // A note that merely *anchors* into the subtree is left alone: it lives + // outside, so it is not part of what was deleted, and pruneNoteAnchors drops + // its dangling anchors on the next render and freezes it where it was. + // + // Filtered here rather than through removeNotes() because notes.js reaches the + // DOM and this module deliberately imports nothing that does. The two model + // fields that can point at a gone note are cleared with it. + if ((App.notes || []).length) { + const orphaned = new Set(App.notes.filter(n => n.scope && subtree.has(n.scope)).map(n => n.id)); + if (orphaned.size) { + App.notes = App.notes.filter(n => !orphaned.has(n.id)); + orphaned.forEach(id => App.selectedNotes.delete(id)); + if (orphaned.has(App.activeNoteId)) App.activeNoteId = null; + } + } for (const sid of doomed) App.accepts.delete(sid); if (doomed.has(App.startId)) App.startId = App.states[0]?.id || null; invalidateBlockIndex(); diff --git a/js/canvas.js b/js/canvas.js index f735f9d..e64c5c7 100644 --- a/js/canvas.js +++ b/js/canvas.js @@ -7,11 +7,11 @@ import { includeLayoutBounds, resolveNodeOverlaps, startNodeId } from './geometr import { getBlock, inlineBlock, outlineBlock } from './blocks.js'; import { getNode, invalidateViewGraph, isPortNode, scopeId, viewStates, viewTransitions } from './view-graph.js'; import { markDirty, snapshot } from './history.js'; -import { clearActiveNoteHighlight, dragSelectedNotesTo, endNoteResize, getNote, includeNoteBounds, resizeNoteTo, resolveNotePos, syncNoteSelectionClasses } from './notes.js'; +import { clearActiveNoteHighlight, dragSelectedNotesTo, endNoteResize, getNote, includeNoteBounds, resizeNoteTo, resolveNotePos, syncNoteSelectionClasses, visibleNotes } from './notes.js'; import { getWorkspaceData } from './persistence.js'; -import { currentLayoutContext, makeSVG, renderAll, repaintForCamera, scheduleFastDOM, updateFastDOM, updateLPanel, updateRPanel, withFullRender } from './render.js'; +import { currentLayoutContext, drawnEdgeEl, makeSVG, renderAll, repaintForCamera, scheduleFastDOM, updateFastDOM, updateLPanel, updateRPanel, withFullRender } from './render.js'; import { $, App } from './state.js'; -import { createState, deleteState, getState, getTransition, hideContextMenu, newId, newTId, openTransModal } from './states-transitions.js'; +import { createState, deleteState, getState, hideContextMenu, newId, newTId, openTransModal } from './states-transitions.js'; import { Change, emit } from './store.js'; import { scheduleMinimap } from './minimap.js'; import { fitToScreen, markActiveWorkspaceSaved, visibleCanvasBox } from './ui.js'; @@ -425,7 +425,20 @@ wrap.addEventListener('pointerdown', e => { if (e.shiftKey || e.ctrlKey || e.metaKey) clearEdgeDirectionHighlight(); else emit(Change.CANVAS); const pt = svgPt(e); - App.marquee = { start: pt, current: pt }; + // The sweep below rebuilds the selection from this baseline on every move, + // so a marquee dragged back over something it had covered releases it. An + // unmodified press cleared above, so the baseline is empty; a modified one + // keeps what was already selected and the marquee only ever adds to it. + App.marquee = { + start: pt, + current: pt, + base: { + states: new Set(App.selectedStates), + transitions: new Set(App.selectedTransitions), + notes: new Set(App.selectedNotes), + dividers: new Set(App.selectedDividers), + }, + }; App.marqueeRect = makeSVG('rect'); App.marqueeRect.setAttribute('class', 'marquee-rect'); $('cam-g').appendChild(App.marqueeRect); @@ -581,44 +594,54 @@ export function handlePointerMove(e) { const mh = Math.abs(App.marquee.start.y - App.marquee.current.y); App.marqueeRect.setAttribute('x', mx); App.marqueeRect.setAttribute('y', my); App.marqueeRect.setAttribute('width', mw); App.marqueeRect.setAttribute('height', mh); + // Each move rebuilds the four sets from the baseline captured at the press + // rather than adding to whatever the last frame left behind. Written as an + // add-only sweep, the box could take an object in and never give it back: + // shrinking the marquee off a state, or dragging past one and back, left it + // selected with nothing on screen still covering it. + const base = App.marquee.base; + const inBox = (x, y) => x >= mx && x <= mx + mw && y >= my && y <= my + mh; // The drawn graph: a marquee selects the boxes and circles on screen, which // inside a block are its members and not the whole machine. Ports are // derived rather than owned, so there is nothing there to select. + const states = new Set(base.states); viewStates().forEach(s => { if (isPortNode(s)) return; - if (s.x >= mx && s.x <= mx + mw && s.y >= my && s.y <= my + mh) { - if (!App.selectedStates.has(s.id)) { App.selectedStates.add(s.id); hlState(s.id, true); } - } + if (inBox(s.x, s.y)) states.add(s.id); }); // Select transitions whose midpoints are in the marquee. // The drawn edges, not the model's: on App.transitions this swept edges from // every other scope in the machine — their endpoints have coordinates // wherever they were left — and could not select a crossing edge at all, // since getState() answers null for a block id. + const transitions = new Set(base.transitions); viewTransitions().forEach(t => { if (t.port) return; const from = getNode(t.from), to = getNode(t.to); if (!from || !to) return; // Approximate center including potential curve - const midX = (from.x + to.x) / 2, midY = (from.y + to.y) / 2; - if (midX >= mx && midX <= mx + mw && midY >= my && midY <= my + mh) { - if (!App.selectedTransitions.has(t.id)) { - App.selectedTransitions.add(t.id); - const el = App.domCache.transitions.get(t.from + '|' + t.to); - if (el) el.classList.add('sel-t'); - } - } + if (inBox((from.x + to.x) / 2, (from.y + to.y) / 2)) transitions.add(t.id); }); - App.notes.forEach(n => { + // The drawn ones, for the reason the transition sweep above gives: a marquee + // that selects what is not on screen makes the next Delete a surprise. + const notes = new Set(base.notes); + visibleNotes().forEach(n => { const pos = resolveNotePos(n); - if (pos.x >= mx && pos.x <= mx + mw && pos.y >= my && pos.y <= my + mh) App.selectedNotes.add(n.id); + if (inBox(pos.x, pos.y)) notes.add(n.id); }); + const dividers = new Set(base.dividers); App.dividers.forEach(d => { const mid = dividerMid(d); - if (mid.x >= mx && mid.x <= mx + mw && mid.y >= my && mid.y <= my + mh) App.selectedDividers.add(d.id); + if (inBox(mid.x, mid.y)) dividers.add(d.id); }); - syncNoteSelectionClasses(); - syncDividerSelectionClasses(); + replaceSet(App.selectedStates, states); + replaceSet(App.selectedTransitions, transitions); + replaceSet(App.selectedNotes, notes); + replaceSet(App.selectedDividers, dividers); + // Repainted wholesale rather than per hit: a release has to reach a node + // the last frame highlighted, and parallel edges share one drawn element, + // so which edges are still selected decides whether it keeps its class. + syncSelectionClasses(); checkAutoPan(e); return; } @@ -1063,6 +1086,18 @@ export function clearSelection() { clearActiveNoteHighlight(); } +// Brings a selection set to `next` by the difference rather than by clearing +// and refilling it. These are reactive sets and the marquee rebuilds them on +// every frame of a drag, so a wholesale clear would announce the whole +// selection as gone and back again sixty times a second. +function replaceSet(set, next) { + // Snapshotted before the deletes: a ReactiveSet writes a signal on every + // mutation, and mutating one through its own live iterator is not something + // to rely on. + for (const id of [...set]) if (!next.has(id)) set.delete(id); + for (const id of next) if (!set.has(id)) set.add(id); +} + export function selectionCount() { return App.selectedStates.size + App.selectedTransitions.size + App.selectedNotes.size + App.selectedDividers.size; @@ -1081,18 +1116,20 @@ export function syncSelectionClasses() { for (const [, n] of App.domCache.states) n.classList.remove('sel-st'); for (const [, n] of App.domCache.transitions) n.classList.remove('sel-t'); App.selectedStates.forEach(id => hlState(id, true)); - // Parallel transitions share one drawn edge, so the keys are collected first: - // selecting five edges between the same pair must not mean five lookups and - // five class writes on the same node. - const keys = new Set(); + // Parallel transitions share one drawn edge, so the elements are collected + // first: selecting five edges between the same pair must not mean five lookups + // and five class writes on the same node. Resolved through the projection, + // because a selected edge that crosses into a block is drawn as `s5|b1` — a + // pair the model does not contain, so a key built from the transition's own + // endpoints found nothing and the edge stayed unhighlighted while very much + // being in `App.selectedTransitions`. Delete then took it, which is the worst + // version of that: a selection you cannot see is one you cannot check. + const els = new Set(); App.selectedTransitions.forEach(tid => { - const t = getTransition(tid); - if (t) keys.add(t.from + '|' + t.to); + const el = drawnEdgeEl(tid); + if (el) els.add(el); }); - for (const key of keys) { - const el = App.domCache.transitions.get(key) || document.querySelector(`[data-edge="${key}"]`); - if (el) el.classList.add('sel-t'); - } + for (const el of els) el.classList.add('sel-t'); syncNoteSelectionClasses(); syncDividerSelectionClasses(); } @@ -1224,16 +1261,17 @@ export function ctxHighlightIncoming() { // ══════════════════════════════════════════════════════════════════ export function selectAllStates() { const drawn = viewStates().filter(s => !isPortNode(s)); - if (!drawn.length && !App.notes.length && !App.dividers.length) return; + const notes = visibleNotes(); + if (!drawn.length && !notes.length && !App.dividers.length) return; clearEdgeDirectionHighlight(); // What is on screen, which inside a block is that block's contents. Select-all // reaching states the reader cannot see would make the next Delete a surprise. App.selectedStates = new Set(drawn.map(s => s.id)); App.selectedTransitions = new Set(viewTransitions().filter(t => t.id && !t.port).map(t => t.id)); - App.selectedNotes = new Set(App.notes.map(n => n.id)); + App.selectedNotes = new Set(notes.map(n => n.id)); App.selectedDividers = new Set(App.dividers.map(d => d.id)); emit(Change.CANVAS); - const extra = App.notes.length + App.dividers.length; + const extra = notes.length + App.dividers.length; const n = drawn.length; showStatus(`Selected ${n} item${n === 1 ? '' : 's'}${extra ? ` and ${extra} annotation${extra === 1 ? '' : 's'}` : ''}`); } diff --git a/js/graph-thumb.js b/js/graph-thumb.js index ea3fa2b..174fc26 100644 --- a/js/graph-thumb.js +++ b/js/graph-thumb.js @@ -113,6 +113,11 @@ export function thumbEdgePairs(transitions) { * { kind: 'line', ax, ay, bx, by } * { kind: 'curve', ax, ay, cx, cy, bx, by } quadratic * + * Each carries the `key` of the pair it was built from, so a caller that wants + * to draw one of them differently — a block's preview marking the transition a + * run is taking right now — can find it without a second pass that could + * disagree with this one about which edges there are. The minimap ignores it. + * * The routing decision is mirrored cheaply rather than reproduced: a hand-set * bend wins, and otherwise a pair with a reverse edge splays so the two are * distinguishable. The collision-avoidance detour is deliberately left out — at @@ -123,7 +128,7 @@ export function thumbEdgeSegments(pairs, byId, project, nodeR, curveOff = 45) { const { px, py, scale } = project; const loopR = Math.max(1.4, nodeR * 0.62); const out = []; - for (const [, e] of pairs) { + for (const [key, e] of pairs) { const from = byId.get(e.from), to = byId.get(e.to); if (!from || !to) continue; @@ -131,6 +136,7 @@ export function thumbEdgeSegments(pairs, byId, project, nodeR, curveOff = 45) { // Up is the layout's default direction; a dragged loop stores its own. const a = Number.isFinite(e.loopAngle) ? e.loopAngle : -Math.PI / 2; out.push({ + key, kind: 'loop', cx: px(from.x) + Math.cos(a) * (nodeR + loopR * 0.55), cy: py(from.y) + Math.sin(a) * (nodeR + loopR * 0.55), @@ -144,10 +150,10 @@ export function thumbEdgeSegments(pairs, byId, project, nodeR, curveOff = 45) { const dist = Math.hypot(dx, dy); if (!dist) continue; const crv = e.curve !== null ? e.curve : (pairs.has(e.to + '|' + e.from) ? curveOff : 0); - if (!crv) { out.push({ kind: 'line', ax, ay, bx, by }); continue; } + if (!crv) { out.push({ key, kind: 'line', ax, ay, bx, by }); continue; } const nx = -dy / dist, ny = dx / dist; out.push({ - kind: 'curve', ax, ay, bx, by, + key, kind: 'curve', ax, ay, bx, by, cx: (ax + bx) / 2 + nx * crv * scale, cy: (ay + by) / 2 + ny * crv * scale }); @@ -165,20 +171,21 @@ export function thumbEdgeSegments(pairs, byId, project, nodeR, curveOff = 45) { */ export function thumbEdgePath(segments) { const d = []; - for (const s of segments) { - if (s.kind === 'loop') { - d.push(`M ${r2(s.cx - s.r)} ${r2(s.cy)}`); - d.push(`a ${r2(s.r)} ${r2(s.r)} 0 1 0 ${r2(s.r * 2)} 0`); - d.push(`a ${r2(s.r)} ${r2(s.r)} 0 1 0 ${r2(-s.r * 2)} 0`); - } else if (s.kind === 'line') { - d.push(`M ${r2(s.ax)} ${r2(s.ay)} L ${r2(s.bx)} ${r2(s.by)}`); - } else { - d.push(`M ${r2(s.ax)} ${r2(s.ay)} Q ${r2(s.cx)} ${r2(s.cy)} ${r2(s.bx)} ${r2(s.by)}`); - } - } + for (const s of segments) d.push(thumbSubpath(s)); return d.join(' '); } +/** One segment as its own subpath — the piece thumbEdgePath is built from. */ +export function thumbSubpath(s) { + if (s.kind === 'loop') { + return `M ${r2(s.cx - s.r)} ${r2(s.cy)}` + + ` a ${r2(s.r)} ${r2(s.r)} 0 1 0 ${r2(s.r * 2)} 0` + + ` a ${r2(s.r)} ${r2(s.r)} 0 1 0 ${r2(-s.r * 2)} 0`; + } + if (s.kind === 'line') return `M ${r2(s.ax)} ${r2(s.ay)} L ${r2(s.bx)} ${r2(s.by)}`; + return `M ${r2(s.ax)} ${r2(s.ay)} Q ${r2(s.cx)} ${r2(s.cy)} ${r2(s.bx)} ${r2(s.by)}`; +} + // Two decimals. A preview is a few dozen pixels across, so the third one is // noise that only makes the attribute longer — and these strings end up in // every export of every diagram that has a block on it. diff --git a/js/language.js b/js/language.js index 16272bf..231fa0f 100644 --- a/js/language.js +++ b/js/language.js @@ -3,7 +3,7 @@ import { createMemo, reactiveRoot } from './reactive.js'; import { Change, changed } from './store.js'; import { openExportCodeModal } from './export-ui.js'; -import { _regexCacheKey, updateDefBoxOverflowShadow } from './render.js'; +import { _regexCacheKey, drawnEdgeEl, drawnStateEl, updateDefBoxOverflowShadow } from './render.js'; import { runSim } from './simulation.js'; import { decideWord, inFamily, machineFormal } from './machines/index.js'; import { langStepBudget } from './machines/runtime.js'; @@ -772,19 +772,28 @@ export function langClearHighlight() { document.querySelectorAll('.edge-g.list-hover-t').forEach(el => el.classList.remove('list-hover-t')); } +// Hovering a symbol in the tuple lights what it stands for. The panel reports +// the *machine* — |Q| counts every state at every depth, which is the honest +// number there — so the highlight has to project: a state inside a collapsed +// block is shown by that block's box, and an edge crossing into one is drawn as +// `s5|b1`. Unprojected, hovering Q on a machine built out of blocks lit the +// handful of states left at the top level and nothing else, which says the +// opposite of what the number beside it says. export function langHighlight(sym) { langClearHighlight(); - const litStates = (ids) => ids.forEach(id => { - const el = App.domCache.states.get(id) || document.querySelector(`.sn[data-id="${id}"]`); - if (el) el.classList.add('list-hover-st'); - }); + const litStates = (ids) => { + const els = new Set(); + ids.forEach(id => { const el = drawnStateEl(id); if (el) els.add(el); }); + els.forEach(el => el.classList.add('list-hover-st')); + }; const litEdges = (pred) => { - const keys = new Set(); - App.transitions.forEach(t => { if (pred(t)) keys.add(t.from + '|' + t.to); }); - keys.forEach(k => { - const el = App.domCache.transitions.get(k) || document.querySelector(`.edge-g[data-edge="${k}"]`); - if (el) el.classList.add('list-hover-t'); + const els = new Set(); + App.transitions.forEach(t => { + if (!pred(t)) return; + const el = drawnEdgeEl(t.id); + if (el) els.add(el); }); + els.forEach(el => el.classList.add('list-hover-t')); }; if (sym === 'Q') litStates(App.states.map(s => s.id)); else if (sym === 'F') litStates([...App.accepts]); @@ -795,12 +804,13 @@ export function langHighlight(sym) { // Highlight every transition carrying one particular input symbol. export function langHighlightSymbol(sym, on) { - const keys = new Set(); - App.transitions.forEach(t => { if (t.symbol === sym) keys.add(t.from + '|' + t.to); }); - keys.forEach(k => { - const el = App.domCache.transitions.get(k) || document.querySelector(`.edge-g[data-edge="${k}"]`); - if (el) el.classList.toggle('list-hover-t', on); + const els = new Set(); + App.transitions.forEach(t => { + if (t.symbol !== sym) return; + const el = drawnEdgeEl(t.id); + if (el) els.add(el); }); + els.forEach(el => el.classList.toggle('list-hover-t', on)); } // ── rendering ───────────────────────────────────────────────────── diff --git a/js/minimap.js b/js/minimap.js index f5f742e..3de8b66 100644 --- a/js/minimap.js +++ b/js/minimap.js @@ -39,7 +39,7 @@ import { includeDividerBounds, isRectDivider } from './dividers.js'; import { markDirty } from './history.js'; import { includeNoteBounds, noteBoxLayout, resolveNotePos } from './notes.js'; import { $, App, largeMachineProfile } from './state.js'; -import { viewGraph, viewStates, viewTransitions } from './view-graph.js'; +import { viewGraph, viewStates, viewTransitions, visibleNodeIdFor } from './view-graph.js'; import { thumbEdgePairs, thumbEdgeSegments, thumbNodeRadius } from './graph-thumb.js'; import { startNodeId } from './geometry.js'; import { Change, subscribe } from './store.js'; @@ -311,12 +311,22 @@ function roundRectPath(ctx, x, y, w, h, r) { // record a single `state`, the subset/nondeterministic ones an array — this is // the whole reason the minimap is worth looking at mid-run on a big machine, // where the active state is usually off screen. +// Answered in *drawn* node ids, because that is what drawStates compares +// against: the map paints the projection, and a run that has stepped inside a +// block names states the projection draws no node for. Resolved, the box +// standing in for them carries the halo — unresolved, the marker simply +// disappeared for the whole of the time the run was inside. function simActiveStates() { const step = App.simSteps && App.simSteps[App.simIdx]; if (!step) return null; - if (Array.isArray(step.states)) return step.states.length ? new Set(step.states) : null; - if (step.state) return new Set([step.state]); - return null; + const ids = Array.isArray(step.states) ? step.states : (step.state ? [step.state] : []); + if (!ids.length) return null; + const drawn = new Set(); + for (const id of ids) { + const node = visibleNodeIdFor(id); + if (node) drawn.add(node); + } + return drawn.size ? drawn : null; } function paint(dt) { diff --git a/js/notes.js b/js/notes.js index 13960a1..8de7885 100644 --- a/js/notes.js +++ b/js/notes.js @@ -1,8 +1,10 @@ import { beginSelectionDrag, clearSelection, hideCanvasContextMenu, pickObject, svgPt } from './canvas.js'; import { snapshot } from './history.js'; import { closeModal, registerModal, showOverlay } from './modal.js'; -import { makeSVG, renderAll } from './render.js'; +import { drawnEdgeEl, drawnStateEl, makeSVG, renderAll } from './render.js'; import { $, App } from './state.js'; +import { getBlock } from './blocks.js'; +import { nodeIdAtScope, scopeId } from './view-graph.js'; import { getState, getTransition, hideContextMenu, showContextMenu } from './states-transitions.js'; import { showStatus } from './utils.js'; @@ -30,20 +32,91 @@ export function normalizeNoteColor(color) { return color === 'purple' ? 'violet' : (color || 'default'); } +// ══════════════════════════════════════════════════════════════════ +// WHICH LEVEL A NOTE LIVES ON +// ══════════════════════════════════════════════════════════════════ +// A note is written *somewhere* — at the top level, or inside a block a reader +// had drilled into — and until it said so it was drawn at every level at once. +// That is not a highlight bug like the ones around it; it is a note being in +// the wrong place. `note.scope` is the block it belongs to, absent for the top +// level, which is what every note written before this existed already was. +// +// **Absent means the top level, so nothing had to be added to a serializer.** +// `roundForSave` copies a note whole and rounds only the fields it names, and +// `exportWorkspaceState` is a JSON deep copy — so `scope` rides along exactly +// as `blockId` does on a state, and the field is written only when it is not +// null, which keeps a file with no blocks in it byte-identical to before. + +/** The scope a note belongs to, with a block that has since gone dropped. */ +export function noteScopeOf(note) { + const id = note && note.scope; + // Validated on read rather than invalidated: nothing announces that a block + // record went, and a note whose block was deleted must surface at the top + // level rather than become invisible everywhere. The same rule liveScope() + // and blockIsIntact() follow. + return id && getBlock(id) ? id : null; +} + +/** True when a note belongs on the level currently being drawn. */ +export function noteInScope(note) { + return noteScopeOf(note) === scopeId(); +} + +/** The notes this level draws. */ +export function visibleNotes() { + return (App.notes || []).filter(noteInScope); +} + // ── Anchoring: a note's stored (x, y) is an absolute point when it has no // anchors, or an offset from its anchors' centroid when it does. This way an // anchored note rides along automatically whenever a state it's pinned to // moves, without having to track every drag separately. ── + +/** + * Where an anchor sits, as seen from one particular level. + * + * **The level is the note's own, never the reader's**, and that is the whole of + * what makes this safe. A note anchored to a state that has since been grouped + * into a block should point at the box — that is the "one level down" case and + * the reason any of this projects at all. But `pruneNoteAnchorsRemoving` calls + * this to *hold a note still* while its anchors are taken away, and it runs over + * every note at every level: resolved against whatever scope the reader happens + * to be standing on, the same note answers two different positions and the prune + * freezes it at the wrong one. Asking about the note's own level gives one + * answer whoever is asking. + * + * Which also means no view graph is consulted: `nodeIdAtScope` walks the block + * tree, and both a state and a block carry their own coordinates. Nothing here + * depends on what is currently drawn. + */ +function anchorPoint(stateId, scope) { + const nodeId = nodeIdAtScope(stateId, scope); + // Not under this level at all, or under it directly: either way the state's + // own position is the honest answer — the first is a note whose anchor has + // gone somewhere else entirely, and freezing it where the state is keeps the + // prune's bookkeeping exactly as it was. + if (!nodeId || nodeId === stateId) { + const s = getState(stateId); + return s ? { x: s.x, y: s.y } : null; + } + const b = getBlock(nodeId); + return b ? { x: b.x || 0, y: b.y || 0 } : null; +} + export function noteAnchorPoints(note) { + const scope = noteScopeOf(note); const pts = []; (note.anchorStates || []).forEach(id => { - const s = getState(id); - if (s) pts.push({ x: s.x, y: s.y }); + const p = anchorPoint(id, scope); + if (p) pts.push(p); }); (note.anchorTransitions || []).forEach(id => { const t = getTransition(id); if (!t) return; - const from = getState(t.from), to = getState(t.to); + const from = anchorPoint(t.from, scope), to = anchorPoint(t.to, scope); + // Both ends against the same level, so an edge crossing into a block is a + // midpoint between the state outside and the box — the line the reader can + // actually see — rather than between one drawn node and one that is not. if (from && to) pts.push({ x: (from.x + to.x) / 2, y: (from.y + to.y) / 2 }); }); return pts; @@ -88,7 +161,10 @@ export function noteBoxLayout(note) { // their world bounding box, so a free-floating note never gets scrolled out // of view when a saved workspace is loaded. export function includeNoteBounds(cb) { - App.notes.forEach(note => { + // The drawn ones. Fit-to-screen and a cropped export frame what is on screen, + // and a note two levels down would otherwise pull the frame out to wherever + // its anchors happen to sit. + visibleNotes().forEach(note => { const pos = resolveNotePos(note); const { w, h } = noteBoxLayout(note); cb(pos.x - w / 2, pos.y - h / 2, pos.x + w / 2, pos.y + h / 2); @@ -278,7 +354,7 @@ export function renderNotes() { const g = $('notes-g'); if (!g) return; g.innerHTML = ''; - App.notes.forEach(note => renderOneNote(g, note)); + visibleNotes().forEach(note => renderOneNote(g, note)); } // Fills `textEl` with one tspan per styled run, laid out as wrapped lines. @@ -429,7 +505,7 @@ export function updateOneNoteDOM(note, { refillText = true } = {}) { }); } export function updateNotesDOM() { - App.notes.forEach(updateOneNoteDOM); + visibleNotes().forEach(updateOneNoteDOM); } // ══════════════════════════════════════════════════════════════════ @@ -472,15 +548,6 @@ export function attachNoteHandlers(grp, note) { }); } -export function getNoteTransitionGroupKeys(note) { - const keys = new Set(); - (note.anchorTransitions || []).forEach(id => { - const t = getTransition(id); - if (t) keys.add(`${t.from}|${t.to}`); - }); - return keys; -} - export function clearNoteAnchorHighlight(noteId = null) { const noteSelector = noteId ? `.note-g[data-note-id="${noteId}"]` : '.note-g'; document.querySelectorAll(`${noteSelector}.note-link-active`).forEach(el => el.classList.remove('note-link-active')); @@ -496,13 +563,16 @@ export function highlightNoteAnchors(id, pin = false) { const noteEl = App.domCache.notes.get(id) || document.querySelector(`.note-g[data-note-id="${id}"]`); if (noteEl && (pin || App.activeNoteId === id)) noteEl.classList.add('note-link-active'); + // The box a state is inside when it is not drawn itself — the same rule the + // playback highlight follows. A note anchored to something in a block should + // point at the block, not at nothing. (note.anchorStates || []).forEach(stateId => { - const el = App.domCache.states.get(stateId) || document.querySelector(`.sn[data-id="${stateId}"]`); + const el = drawnStateEl(stateId); if (el) el.classList.add('note-link-st'); }); - getNoteTransitionGroupKeys(note).forEach(key => { - const el = App.domCache.transitions.get(key) || document.querySelector(`.edge-g[data-edge="${key}"]`); + (note.anchorTransitions || []).forEach(id => { + const el = drawnEdgeEl(id); if (el) el.classList.add('note-link-t'); }); } @@ -618,6 +688,21 @@ export function deleteNote(id) { // Drops notes without taking an undo point — the caller owns the snapshot, // which is what lets Delete remove a mixed selection in one history step. +/** + * Drops from the selection any note this level does not draw. + * + * Called on a scope change. Before notes had a level they were all drawn, so a + * stale selection was at least a visible one; now Delete over a selection made + * on the way past would take a note nobody can see. Narrow on purpose — it is + * about the kind this change made invisible, and says nothing about the rest of + * the selection. + */ +export function dropOffscreenNoteSelection() { + if (!App.selectedNotes || !App.selectedNotes.size) return; + const here = new Set(visibleNotes().map(n => n.id)); + for (const id of [...App.selectedNotes]) if (!here.has(id)) App.selectedNotes.delete(id); +} + export function removeNotes(ids) { const gone = new Set(ids); if (!gone.size) return; @@ -639,6 +724,10 @@ export function createNote(x, y, anchorStates = [], anchorTransitions = []) { } else { note = { id, text: '', color: 'yellow', anchorStates: [], anchorTransitions: [], x, y }; } + // Written only when it is not the top level, so a machine with no blocks in + // it saves exactly the bytes it saved before. + const scope = scopeId(); + if (scope) note.scope = scope; App.notes.push(note); renderAll(); return note; diff --git a/js/render.js b/js/render.js index 72d7cd8..15ace04 100644 --- a/js/render.js +++ b/js/render.js @@ -9,10 +9,10 @@ import { scheduleMinimap } from './minimap.js'; import { renderLanguagePanel } from './language.js'; import { highlightNoteAnchors, pruneNoteAnchors, renderNotes, updateNotesDOM } from './notes.js'; import { $, App, OmegaAcceptance, R, SVG_NS, edgeLabelsHidden, getMachineConfig, isDeterministicOmega, omegaAcceptanceOf, statePriority, usesParityPriorities, wrapStateLabelsOn } from './state.js'; -import { BLOCK_STRIP_H, blockPreviewGraph, blockPreviewKey, getNode, viewEdgeGroup, viewStates } from './view-graph.js'; +import { BLOCK_STRIP_H, blockPreviewGraph, blockPreviewKey, getNode, viewEdgeGroup, viewEdgeKeyFor, viewStates, visibleNodeIdFor } from './view-graph.js'; import { machineSupportsBlocks } from './machines/index.js'; import { allBlocks } from './blocks-ui.js'; -import { thumbBounds, thumbEdgePairs, thumbEdgePath, thumbEdgeSegments, thumbFit, thumbNodeRadius } from './graph-thumb.js'; +import { thumbBounds, thumbEdgePairs, thumbEdgePath, thumbEdgeSegments, thumbSubpath, thumbFit, thumbNodeRadius } from './graph-thumb.js'; import { enterBlockScope } from './scope.js'; import { edgeTipFor, getState, openTransModal, showContextMenu, transLabel, transLabelDescriptive, transLabelParts } from './states-transitions.js'; import { Change, changed, emit, subscribe } from './store.js'; @@ -139,6 +139,40 @@ function edgeGroupFor(key) { return { from, to, ts, grp: { from: fromId, to: toId, ts } }; } +// ── a machine id, resolved to the thing on screen ───────────────── +// The two lookups every surface that lights something on the canvas needs, in +// one place because they were being written out per surface and the model's own +// ids are *almost* always right — which is what makes getting them wrong so +// quiet. A state inside a collapsed block has no node of its own, and an edge +// that crosses a block's boundary is registered under `s5|b1`, a pair the model +// does not contain. Built from the model's ids, both lookups simply come back +// empty, and the caller lights nothing at all with nothing to say it failed. +// +// The registry first and a selector second, the way every other lookup here is +// written: after culling only the drawn window has nodes, and a state that is +// currently off screen has nothing to mark. The fallback is deliberately not +// scoped to `.sn` — the answer may be a block's box or a port's tab. + +/** + * The element standing for a real state: its own circle, or the box of whichever + * block it is inside. Null when it is in some other branch of the tree entirely. + */ +export function drawnStateEl(stateId) { + const id = visibleNodeIdFor(stateId); + if (!id) return null; + return App.domCache.states.get(id) || document.querySelector(`[data-id="${id}"]`); +} + +/** + * The edge group a real transition is drawn as, or null when it is not drawn — + * which an edge wholly inside a collapsed block genuinely is not. + */ +export function drawnEdgeEl(transitionId) { + const key = viewEdgeKeyFor(transitionId); + if (!key) return null; + return App.domCache.transitions.get(key) || document.querySelector(`.edge-g[data-edge="${key}"]`); +} + // How big the label for a group of transitions will be, before it is written to // the DOM. geometry.js needs the box to place it clear of everything else, and // this module is the one that knows which of the two label styles is on and what @@ -849,8 +883,36 @@ function moveBlockNode(grp, node) { // the preview. Writing the new position onto the rect as well moved it twice, // and two boxes' worth of offset puts the clip clean off the block, which // clips the whole interior away. + slideBlockPreview(grp, x, y); +} + +/** + * Puts a block's preview back under its box. + * + * **One function because there are two callers**, and they had drifted — the + * same shape of bug `startArrowD()` carries its own note about. The drag path + * called it; the full render did not, and a full render is the *only* thing + * that runs after Arrange, a paste, an undo, an arrow-key nudge, a collision + * push or the JFLAP importer's spread. Every one of those writes a block + * record's coordinates without a member state moving — which is what + * `blockPreviewKey` is built from, so the preview was neither rebuilt nor + * re-translated and simply stayed where the box used to be. On a machine whose + * layout had been rearranged, that is a canvas of empty boxes with their + * diagrams scattered across the background. + * + * The translate is a delta from `__previewAt` — where the preview was *drawn* — + * rather than an absolute position, because the interior is laid out in + * absolute canvas coordinates once and then slid, which is what keeps a drag + * frame from rebuilding a hundred child elements. The clip rect rides along: + * `clipPathUnits` defaults to `userSpaceOnUse`, so this transform establishes + * the space the clip resolves in, which is why the rect stays written at + * `__previewAt` and must never be moved as well. + */ +function slideBlockPreview(grp, x, y) { const at = grp.__previewAt; - if (at) p.preview.setAttribute('transform', `translate(${x - at.x} ${y - at.y})`); + if (!at) return; + const dx = x - at.x, dy = y - at.y; + grp.__parts.preview.setAttribute('transform', dx || dy ? `translate(${dx} ${dy})` : ''); } function movePortNode(grp, node) { @@ -1150,6 +1212,14 @@ function createBlockNode(id) { const pvEdges = makeSVG('path'); pvEdges.classList.add('bn-pv-edges'); preview.appendChild(pvEdges); + // The edge a run is taking right now, drawn over the quiet ones and under the + // node dots — the order the canvas itself uses. It is a *second path* rather + // than a class on the first because every preview edge shares one `d`: at this + // scale a path per edge is a hundred elements per box, which is exactly the + // cost the single path exists to avoid. One more path is one more element. + const pvActive = makeSVG('path'); + pvActive.classList.add('bn-pv-active'); + preview.appendChild(pvActive); const pvNodes = makeSVG('g'); pvNodes.classList.add('bn-pv-nodes'); preview.appendChild(pvNodes); @@ -1169,7 +1239,7 @@ function createBlockNode(id) { count.classList.add('bn-count'); g.appendChild(count); - g.__parts = { body, title, count, preview, pvEdges, pvNodes, clip, clipRect }; + g.__parts = { body, title, count, preview, pvEdges, pvActive, pvNodes, clip, clipRect }; g.__previewKey = null; // Opening is decided on `pointerdown`, from two presses of our own, and NOT @@ -1278,6 +1348,11 @@ function syncBlockNode(g, node, lod) { if (stale) { p.preview.setAttribute('transform', ''); g.__previewAt = { x, y }; + } else { + // The box may have moved since the preview was drawn, by any of the paths + // that end in a full render rather than in a drag frame. See + // slideBlockPreview — this is the half that was missing. + slideBlockPreview(g, x, y); } // The clip is the body below the strip: the preview must never paint over the @@ -1303,13 +1378,17 @@ function syncBlockNode(g, node, lod) { if (lod || !inside) { p.pvEdges.setAttribute('d', ''); + p.pvActive.setAttribute('d', ''); p.pvNodes.innerHTML = ''; + g.__pvIndex = null; + g.__pvEdgeD = null; return; } - drawPreview(p, inside, pv); + drawPreview(g, inside, pv); } -function drawPreview(p, inside, box) { +function drawPreview(g, inside, box) { + const p = g.__parts; const nodes = inside.nodes.slice(0, PREVIEW_MAX_NODES); const shown = new Set(nodes.map(n => n.id)); const bounds = thumbBounds(nodes, R, n => (n.box ? Math.hypot(n.box.w, n.box.h) / 2 : R)); @@ -1317,8 +1396,26 @@ function drawPreview(p, inside, box) { const r = thumbNodeRadius(fit.scale, R); const pairs = thumbEdgePairs(inside.edges.filter(e => shown.has(e.from) && shown.has(e.to))); - p.pvEdges.setAttribute('d', thumbEdgePath( - thumbEdgeSegments(pairs, inside.byId, fit, r, App.config.render.curveOff))); + const segments = thumbEdgeSegments(pairs, inside.byId, fit, r, App.config.render.curveOff); + p.pvEdges.setAttribute('d', thumbEdgePath(segments)); + p.pvActive.setAttribute('d', ''); + + // ── what the playback highlight addresses ── + // Two lookups, built in the loops that were already running rather than by a + // second pass: a state's dot, and one drawn edge's subpath. Both live only as + // long as the preview does — they are dropped whenever it is rebuilt or the + // LOD blanks it, and the node is evicted with the box when it scrolls off. + // + // The *graph* is deliberately still not retained (see the note in + // syncBlockNode): these hold ids and elements that are alive anyway, plus one + // short string per drawn edge — the same characters `pvEdges` already carries, + // split up. Both are bounded by PREVIEW_MAX_NODES, which is what bounds the + // preview itself. + const index = new Map(); + const edgeD = new Map(); + for (const s of segments) edgeD.set(s.key, thumbSubpath(s)); + g.__pvIndex = index; + g.__pvEdgeD = edgeD; // A nested block draws as a tiny rect rather than a circle, so the silhouette // itself says there is another level below this one. @@ -1340,6 +1437,7 @@ function drawPreview(p, inside, box) { if (App.accepts.has(n.id)) el.classList.add('is-accept'); if (n.id === App.startId) el.classList.add('is-start'); } + index.set(n.id, el); p.pvNodes.appendChild(el); } } diff --git a/js/scope.js b/js/scope.js index 5e16a43..3ccf4bf 100644 --- a/js/scope.js +++ b/js/scope.js @@ -21,6 +21,8 @@ import { $, App } from './state.js'; import { Change, emit, subscribe } from './store.js'; import { invalidateViewGraph, liveScope, scopeTrail } from './view-graph.js'; import { showStatus } from './utils.js'; +import { updateSimCanvasHighlights } from './simulation.js'; +import { dropOffscreenNoteSelection } from './notes.js'; // Where the camera was, per scope path. Session state rather than document // state: it is a property of this reader's navigation, not of the machine, so @@ -71,6 +73,18 @@ export function enterBlockScope(blockId, opts = {}) { } renderBreadcrumb(); + // A drill-in evicts every node the previous scope drew, and the playhead's + // marks went with them — so a run paused inside a block was invisible from + // the moment you went in to look at it, which is the one time you would. + // Repainted rather than carried, because *which* node shows a given state is + // exactly what the scope change decides (js/view-graph.js). + const step = App.simSteps && App.simSteps[App.simIdx]; + if (step) updateSimCanvasHighlights(step); + + // A note belongs to one level now, so a selection made before the move can no + // longer be seen — and Delete would take it anyway. + dropOffscreenNoteSelection(); + const b = next.length ? getBlock(next[next.length - 1]) : null; showStatus(b ? `Inside ${b.name}` : 'Back to the top level'); return true; diff --git a/js/simulation.js b/js/simulation.js index a017b4c..9941529 100644 --- a/js/simulation.js +++ b/js/simulation.js @@ -25,6 +25,7 @@ import { poolSize, runParallel, shouldParallelize } from './parallel/pool.js'; import { renderTracker, resetTracker } from './tape-view.js'; import { isPainterSuppressed, setSimStepPainter, withPainterSuppressed } from './machines/paint.js'; import { makeRun } from './machines/run.js'; +import { nodeIdAtScope, viewEdgeKeyFor, viewGraph, visibleNodeIdFor } from './view-graph.js'; export function runSim() { resetSim(); @@ -482,30 +483,87 @@ export function simMotionOk() { } export function findSimEdgeGroup(key) { + if (!key) return null; return App.domCache.transitions.get(key) || document.querySelector(`.edge-g[data-edge="${key}"]`); } +// ── the run, projected onto what is drawn ───────────────────────── +// The machine is flat and the canvas is a projection of it (js/view-graph.js), +// so a run's own ids are not always ids the canvas has anything under. A step +// inside a block names a state that is not drawn, and a step that crosses a +// block's boundary names a transition drawn as `x2|b1` — a pair the *model* +// does not contain. Built from the transition's own endpoints, every one of +// those lookups came back empty: no trail, no active edge, no travelling token +// and no pulse, from the first step a run touched a block onward. The run was +// correct throughout; the whole of what was lost was the drawing of it. +// +// So both halves go through the projection. What resolves to nothing is dropped +// rather than guessed at: an edge *wholly inside* a block is not on screen, and +// the box standing in for it is already lit by the state half below. + +/** The drawn edge a real transition is part of, or null when it is not drawn. */ +function drawnEdgeKey(t) { + return t ? viewEdgeKeyFor(t.id) : null; +} + +/** The drawn node a real state is shown by — itself, or the box it is inside. */ +function drawnNodeId(stateId) { + return visibleNodeIdFor(stateId) || null; +} + // Edge(s) traversed to arrive at step `idx`, as "from|to" keys matching the // grouped edge DOM. Path-style machines record the transition id on the step; // NFA-style set steps are reconstructed from the previous state set (symbol // move + ε-closure). NDTM exploration steps carry no path information — // consecutive steps are BFS order, not a run — so they highlight states only. export function getSimStepEdgeKeys(idx) { + const keyOf = viewGraph().keyOf; + const keys = new Set(); + for (const t of simStepTransitions(idx)) { + const key = keyOf.get(t.id); + // The *drawn* edge, which several real transitions can share — a Set is + // what keeps a block's four incoming rules one highlight. What resolves to + // nothing is an edge wholly inside a block, which is not on screen at all. + if (key) keys.add(key); + } + return [...keys]; +} + +/** + * The real transitions a step was taken along. + * + * Split out from the keys because the two halves of the highlight want + * different things from it: the canvas wants the *drawn* edge, and a block's + * preview wants the transition itself, to find the mark for it one level in. + * Written twice they would be two answers to "which edge fired", and the + * preview would light one the canvas did not. + */ +function simStepTransitions(idx) { const step = App.simSteps[idx]; if (!step) return []; if (step.tid) { const t = getTransition(step.tid); - return t ? [t.from + '|' + t.to] : []; + return t ? [t] : []; } - if (step.states) return getNfaSimStepEdgeKeys(idx); + if (step.states) return nfaStepTransitions(idx); return []; } export function getNfaSimStepEdgeKeys(idx) { + const keyOf = viewGraph().keyOf; + const keys = new Set(); + for (const t of nfaStepTransitions(idx)) { + const key = keyOf.get(t.id); + if (key) keys.add(key); + } + return [...keys]; +} + +function nfaStepTransitions(idx) { const eps = App.config.sym.eps, any = App.config.sym.any; + const out = []; const step = App.simSteps[idx]; const cur = new Set(step.states); - const keys = new Set(); let seed; if (idx === 0) { seed = new Set([App.startId]); @@ -522,7 +580,7 @@ export function getNfaSimStepEdgeKeys(idx) { if (sym !== null) { prevStates.forEach(sid => App.transitions.forEach(t => { if (t.from === sid && (t.symbol === sym || t.symbol === any) && cur.has(t.to)) { - keys.add(t.from + '|' + t.to); + out.push(t); seed.add(t.to); } })); @@ -534,18 +592,24 @@ export function getNfaSimStepEdgeKeys(idx) { const s = stk.pop(); App.transitions.forEach(t => { if (t.from === s && t.symbol === eps && cur.has(t.to)) { - keys.add(t.from + '|' + t.to); + out.push(t); if (!seen.has(t.to)) { seen.add(t.to); stk.push(t.to); } } }); } - return [...keys]; + return out; } // What the last paint lit, so undoing it is a walk over a few dozen elements // rather than four document-wide selector matches per step of playback. let simLit = []; +// The preview paths whose `d` the last paint wrote. Kept apart from simLit +// because what has to be undone there is an attribute rather than a class — +// and blanking every block's path unconditionally would mean a write per box on +// screen per step, on boxes with no run anywhere near them. +let simLitPaths = []; + function litAdd(el, ...classes) { if (!el) return; el.classList.add(...classes); @@ -555,6 +619,8 @@ function litAdd(el, ...classes) { export function clearSimCanvasHighlights() { for (const [el, classes] of simLit) el.classList.remove(...classes); simLit = []; + for (const el of simLitPaths) el.setAttribute('d', ''); + simLitPaths = []; // Pulses are transient rings the animation appends and removes itself; the // sweep is a safety net for the ones whose animationend never fired, and it is // scoped to the states layer rather than the document. @@ -573,9 +639,15 @@ export function clearSimCanvasHighlights() { // Only a jump backwards costs a rebuild, which is what a scrub is and is // bounded by where it lands. function trailUpTo(idx) { + // The keys are *drawn* keys, so the scope is part of what makes the cache + // valid: drilling into a block while a run is paused changes which node every + // step of it is shown by, and a trail carried across that would be a set of + // keys for a diagram that is no longer on screen. Rebuilding costs what a + // backward scrub costs, and only on a scope change. + const scopeKey = (App.scope || []).join('/'); let c = App._simTrail; - if (!c || c.run !== App.simSteps || c.upTo > idx) { - c = { run: App.simSteps, upTo: 0, visited: new Set(), keys: new Set() }; + if (!c || c.run !== App.simSteps || c.upTo > idx || c.scope !== scopeKey) { + c = { run: App.simSteps, scope: scopeKey, upTo: 0, visited: new Set(), keys: new Set() }; } for (let i = c.upTo; i < idx; i++) { const s = App.simSteps[i]; @@ -602,13 +674,20 @@ export function updateSimCanvasHighlights(step) { const activeKeys = getSimStepEdgeKeys(App.simIdx); const activeSet = new Set(activeKeys); const hl = step.state ? [step.state] : (step.states || []); - const hlSet = new Set(hl); + + // Resolved to *drawn* nodes before anything is compared, and deduped there: + // several states inside one block are one box on screen, so a set of real ids + // would light it once per member and — worse — the "is this one already + // active?" test below would answer about ids the canvas does not have. + const hlNodes = new Set(); + hl.forEach(id => { const n = drawnNodeId(id); if (n) hlNodes.add(n); }); + const visitedNodes = new Set(); + visited.forEach(id => { const n = drawnNodeId(id); if (n && !hlNodes.has(n)) visitedNodes.add(n); }); // The registries rather than the document: after culling only the drawn // window has nodes, and a state the trail passed through that is currently // off screen has nothing to mark. - visited.forEach(id => { - if (hlSet.has(id)) return; + visitedNodes.forEach(id => { litAdd(App.domCache.states.get(id), 'sim-visited-st'); }); trailKeys.forEach(k => { @@ -616,7 +695,7 @@ export function updateSimCanvasHighlights(step) { litAdd(findSimEdgeGroup(k), 'sim-trail-t'); }); - hl.forEach(id => { + hlNodes.forEach(id => { litAdd(App.domCache.states.get(id) || document.querySelector(`[data-id="${id}"]`), step.final === 'reject' ? 'rej-st' : 'act-st'); }); @@ -626,12 +705,22 @@ export function updateSimCanvasHighlights(step) { litAdd(document.getElementById(`pill-lbl-${k}`), 'sim-active-lbl'); }); + // ── one level in ── + // A box on the canvas is a small drawing of the machine inside it, so the + // marks above stop one level short of what the reader can actually see: the + // box lights, and the dot that is really running stays the same grey as the + // twenty around it. These write onto elements the preview already built, so + // the cost is a class per mark and nothing is rebuilt — see markPreviewRun. + markPreviewRun(hl, visited, simStepTransitions(App.simIdx), step); + // Motion: a token slides along each newly-taken edge, then the arrival // state pulses (verdict-colored on the final step). Only on a single // forward step — scrubbing and jumps update instantly. if (!simMotionOk()) return; const tone = step.final === 'reject' ? 'rej' : step.final === 'accept' ? 'acc' : ''; - const pulseAll = () => hl.forEach(id => pulseSimState(id, tone)); + // Over the drawn nodes, so a run that has stepped inside a block pulses the + // box once rather than pulsing nothing four times. + const pulseAll = () => hlNodes.forEach(id => pulseSimNode(id, tone)); if (advancedOne && activeKeys.length) { const dur = App.autoTimer ? Math.max(160, Math.min(App.config.autoSpeed * 0.6, 500)) @@ -644,6 +733,66 @@ export function updateSimCanvasHighlights(step) { } } +/** + * The run, marked inside the previews the blocks on screen are drawing. + * + * **Nothing here rebuilds a preview**, and that is the whole of why it is + * affordable. `renderAll` draws a preview only when `blockPreviewKey` changes, + * and a run changes no position and no transition — so the dots and the edge + * subpaths this writes to were built once and are still there. Per step the + * work is: a short ancestry walk per marked state (nodeIdAtScope), a Map get + * per mark, and one `d` write per box the run is actually inside. It runs on a + * step change rather than on a frame, so it is off the render path entirely. + * + * What it deliberately does not do is send a travelling token in there. At + * preview scale an interior edge is a dozen pixels and a node is two, so the + * dot would be larger than the states it travels between — the one part of the + * canvas animation that does not survive being shrunk. + */ +function markPreviewRun(active, visited, transitions, step) { + const boxes = App.domCache.states; + const activeCls = step.final === 'reject' ? 'is-rej' : 'is-active'; + + // A state is marked in a preview only when it is *inside* a box — a state at + // this scope is drawn as itself and already has the canvas mark. + const markState = (stateId, cls) => { + const boxId = drawnNodeId(stateId); + if (!boxId || boxId === stateId) return; + const g = boxes.get(boxId); + if (!g || !g.__pvIndex) return; // off screen, or blanked by the zoom LOD + const pvId = nodeIdAtScope(stateId, boxId); + const el = pvId && g.__pvIndex.get(pvId); + if (el) litAdd(el, cls); + }; + + visited.forEach(id => markState(id, 'is-visited')); + active.forEach(id => markState(id, activeCls)); + + // The edge, which is only drawn when both ends are immediate members of the + // same box: an edge deeper than that is inside the nested rect the state half + // has already lit, and one crossing the box's own boundary is the canvas edge + // above. + const byBox = new Map(); + for (const t of transitions) { + const boxId = drawnNodeId(t.from); + if (!boxId || boxId === t.from || drawnNodeId(t.to) !== boxId) continue; + const g = boxes.get(boxId); + if (!g || !g.__pvEdgeD) continue; + const a = nodeIdAtScope(t.from, boxId), b = nodeIdAtScope(t.to, boxId); + if (!a || !b || a === b) continue; + const d = g.__pvEdgeD.get(a + '|' + b); + if (!d) continue; + const acc = byBox.get(g); + if (acc) acc.push(d); else byBox.set(g, [d]); + } + for (const [g, parts] of byBox) { + const el = g.__parts.pvActive; + el.setAttribute('d', parts.join(' ')); + simLitPaths.push(el); + if (step.final === 'reject') litAdd(el, 'is-rej'); + } +} + export function removeSimTokens() { (App._simTokens || []).forEach(t => { cancelAnimationFrame(t.raf); t.el.remove(); }); App._simTokens = []; @@ -695,14 +844,42 @@ export function animateSimToken(edgeKey, dur, onDone) { token.raf = requestAnimationFrame(tick); } -export function pulseSimState(id, tone = '') { - const grp = App.domCache.states.get(id) || document.querySelector(`[data-id="${id}"]`); - const c = grp && grp.querySelector('circle.bd'); - if (!c) return; - const ring = makeSVG('circle'); - ring.setAttribute('cx', c.getAttribute('cx')); - ring.setAttribute('cy', c.getAttribute('cy')); - ring.setAttribute('r', R); +/** + * The arrival ring, on a *drawn* node — which is a circle for a state and a box + * for a block or a port. + * + * The shape is asked of the node rather than assumed, because the ring has to + * trace the outline the reader can see: a circle of radius R centred on a box + * two hundred pixels wide is a ring floating inside it, which reads as a second + * unexplained mark rather than as "control arrived here". + * + * The scale is the shape's too. `.sim-pulse` grows 1.7x from its own centre, + * which is right for a 22px circle and far too much for a block's box — it + * would sweep out over half the diagram. `.is-box` is the gentler ramp. + */ +export function pulseSimNode(nodeId, tone = '') { + const grp = App.domCache.states.get(nodeId) || document.querySelector(`[data-id="${nodeId}"]`); + if (!grp) return; + const parts = grp.__parts || {}; + const box = (grp.__kind === 'block' || grp.__kind === 'port') ? parts.body : null; + let ring; + if (box) { + ring = makeSVG('rect'); + ring.setAttribute('x', box.getAttribute('x')); + ring.setAttribute('y', box.getAttribute('y')); + ring.setAttribute('width', box.getAttribute('width')); + ring.setAttribute('height', box.getAttribute('height')); + const rx = box.getAttribute('rx'); + if (rx) ring.setAttribute('rx', rx); + ring.classList.add('is-box'); + } else { + const c = parts.circle || (grp.querySelector && grp.querySelector('circle.bd')); + if (!c) return; + ring = makeSVG('circle'); + ring.setAttribute('cx', c.getAttribute('cx')); + ring.setAttribute('cy', c.getAttribute('cy')); + ring.setAttribute('r', R); + } ring.classList.add('sim-pulse'); if (tone) ring.classList.add(tone); grp.appendChild(ring); @@ -710,6 +887,12 @@ export function pulseSimState(id, tone = '') { setTimeout(() => ring.remove(), 900); // safety net if animations are disabled } +/** The same, addressed by a *machine* state id. */ +export function pulseSimState(id, tone = '') { + const nodeId = drawnNodeId(id); + if (nodeId) pulseSimNode(nodeId, tone); +} + // ── Scrubber / transport ── export function updateSimScrubber() { const row = $('sim-scrubber-row'), scrubber = $('sim-scrubber'), counter = $('sim-step-counter'); diff --git a/js/ui.js b/js/ui.js index 7de2326..703b720 100644 --- a/js/ui.js +++ b/js/ui.js @@ -2,7 +2,7 @@ import { utmStepBack, utmStepFwd, utmToggleAuto } from './algorithms-fa.js'; import { renderGamma } from './alphabet.js'; import { settleAll } from './anim.js'; import { applyCamera, clampZoom, clearEdgeDirectionHighlight, clearSelection, clearTempLine, copySelection, duplicateSelection, getContentBounds, hideCanvasContextMenu, hlState, minZoom, nudgeSelected, pasteClipboard, selectAllStates, selectionCount, toggleSnapToGrid, wrap } from './canvas.js'; -import { getBlock, removeBlock } from './blocks.js'; +import { blockRemovalIds, getBlock, removeBlock } from './blocks.js'; import { viewStates } from './view-graph.js'; import { leaveBlockScope, syncScopeBar } from './scope.js'; import { isQuickSettingsOpen, positionQuickSettings, refreshQuickSettings } from './quick-settings.js'; @@ -981,7 +981,18 @@ document.addEventListener('keydown', e => { App.transitions.forEach(t => { if (App.selectedStates.has(t.from) || App.selectedStates.has(t.to)) removedTransIds.add(t.id); }); - pruneNoteAnchorsExcluding([...App.selectedStates], [...removedTransIds]); + // A selected *block* is one id standing for a whole subtree, so naming + // the selection alone named the box and not one state inside it: a note + // out here anchored into the block was never seen by the prune and + // settled at its stored offset instead of holding where it was drawn. + const removedStateIds = new Set(App.selectedStates); + App.selectedStates.forEach(id => { + if (!getBlock(id)) return; + const gone = blockRemovalIds(id); + gone.states.forEach(sid => removedStateIds.add(sid)); + gone.transitions.forEach(tid => removedTransIds.add(tid)); + }); + pruneNoteAnchorsExcluding([...removedStateIds], [...removedTransIds]); } App.selectedStates.forEach(id => { // A selected block id deletes the whole subtree behind the box — every diff --git a/js/view-graph.js b/js/view-graph.js index 2752eb0..cb5e07a 100644 --- a/js/view-graph.js +++ b/js/view-graph.js @@ -269,9 +269,27 @@ function refresh(g) { // nothing to copy here — and copying them is exactly what used to undo // every drag, nudge and collision push a frame after it happened. node.name = b.name; - const size = blockSize(b); - node.box = size; - node.r = boxRadius(size.w, size.h); + // **A derived size is never recomputed here, and that is the difference + // between this function being O(1) and being O(blocks x states).** + // blockSize() falls through to blockMembers() + blockChildren() when the + // record carries no size of its own — and inlineBlock leaves them null, so + // that is the ordinary case rather than the exception. Both of those are + // unindexed filters that allocate, and viewGraph() is on the hot path: + // edgeLabelsHidden() reaches it once per edge label, and every surface + // that resolves a machine id to a drawn one reaches it per item. A + // select-all over 2000 transitions on a machine with eight blocks measured + // at 617ms against 7ms without them. + // + // It is safe to skip because what a derived size is derived *from* — the + // states and blocks — cannot change without stillValid() failing and the + // whole projection being rebuilt. Only a hand-set size can change under a + // cache hit, and reading two numbers off the record is what that costs. + if (Number.isFinite(b.w) && Number.isFinite(b.h)) { + if (node.box.w !== b.w || node.box.h !== b.h) { + node.box = { w: b.w, h: b.h }; + node.r = boxRadius(b.w, b.h); + } + } } else { const anchor = getState(node.anchor); if (!anchor) continue; @@ -588,6 +606,40 @@ export function blockPreviewGraph(blockId) { return { nodes, byId, edges }; } +/** + * Which node the level `scope` draws for a real state — the state itself when it + * is an immediate member of that level, or the box of whichever child block + * contains it. Null when the state is not under that level at all. + * + * **`visibleNodeIdFor` for an arbitrary level**, which is why it is not called + * something about previews any more. It has two callers that want different + * things from the same fact: a block's preview marking the transition a run is + * taking (there the level is the block, and the answer is a dot or a nested + * rect), and a note working out where it sits (there the level is the note's + * own, which is deliberately *not* the one the reader is standing on). + * + * A walk up the ancestry rather than an ownerMap, because the map is built for + * one scope and cached, and this is asked about others — for a handful of ids at + * a time, so depth is what it costs and depth is small. `visibleNodeIdFor` stays + * the fast path for the scope actually on screen. + */ +export function nodeIdAtScope(stateId, scope) { + const s = getState(stateId); + if (!s) return null; + const at = scope || null; + let cur = s.blockId || null; + if (cur === at) return stateId; + const seen = new Set(); + while (cur && !seen.has(cur)) { + seen.add(cur); + const b = getBlock(cur); + if (!b) return null; + if ((b.parent || null) === at) return cur; + cur = b.parent || null; + } + return null; +} + /** * A signature of what a block's preview draws, so it is rebuilt when the * interior changes and not on every graph emit. diff --git a/tests/block-render.test.js b/tests/block-render.test.js index 0c73885..e175789 100644 --- a/tests/block-render.test.js +++ b/tests/block-render.test.js @@ -413,3 +413,57 @@ test('the start arrow follows a block being dragged, without a full render', () assert.equal(arrow.getAttribute('d'), after, 'and the drag path put it exactly where a full render does'); }); + +// ── the preview follows its box ─────────────────────────────────── +// +// The interior is laid out in absolute canvas coordinates once and then *slid*, +// which is what keeps a drag frame from rebuilding a hundred child elements. So +// something has to write that translate, and for a long time only the drag path +// did — while `blockPreviewKey` is built from the *members'* positions, which a +// block moving does not change. Every path that moves a box and ends in a full +// render therefore left the diagram behind: Arrange, a paste, an undo, an +// arrow-key nudge, a collision push, the JFLAP importer's spread. On a machine +// whose layout had been rearranged that is a canvas of empty boxes with their +// diagrams scattered across the background. + +test('a block moved by anything but a drag takes its preview with it', () => { + tmCanvas(); + const block = placeBlock('seek', 4, { x: 100, y: 100 }); + context.renderAll(); + const g = blockNode(block.id); + const drawnAt = { ...g.__previewAt }; + + // What autoLayout, paste, undo and spreadForBlocks all do: write the record. + block.x = 900; + block.y = 640; + context.renderAll(); + + const box = { x: Number(g.__parts.body.getAttribute('x')), y: Number(g.__parts.body.getAttribute('y')) }; + assert.equal(g.__parts.preview.getAttribute('transform'), + `translate(${box.x - drawnAt.x} ${box.y - drawnAt.y})`, + 'the preview is slid under the box it belongs to'); + + // And the clip stays written where the preview was *drawn*. clipPathUnits is + // userSpaceOnUse, so the transform above already carries it — writing the new + // position onto the rect as well moves it twice, and two boxes' worth of + // offset clips the whole interior away. + assert.equal(Number(g.__parts.clipRect.getAttribute('x')), drawnAt.x + 1); +}); + +test('a rebuild resets the slide rather than compounding it', () => { + tmCanvas(); + const block = placeBlock('seek', 4, { x: 100, y: 100 }); + context.renderAll(); + const g = blockNode(block.id); + + block.x = 500; + context.renderAll(); + assert.notEqual(g.__parts.preview.getAttribute('transform'), ''); + + // Moving a member changes the key, so the interior is laid out afresh at the + // box's current position — and the translate has to go with it. + context.blockMembers(block.id)[0].x += 40; + context.renderAll(); + assert.equal(g.__parts.preview.getAttribute('transform'), ''); + assert.equal(g.__previewAt.x, Number(g.__parts.body.getAttribute('x'))); +}); diff --git a/tests/note-scope.test.js b/tests/note-scope.test.js new file mode 100644 index 0000000..641bebb --- /dev/null +++ b/tests/note-scope.test.js @@ -0,0 +1,273 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createHarness, context } from './harness.js'; + +// Which level a note lives on. +// +// A note is written *somewhere* — at the top level, or inside a block someone +// had drilled into — and until it said so it was drawn at every level at once, +// positioned against a state the level it was showing on does not draw. Dragging +// the box then left it behind entirely, because a box moves without its members +// moving: the preview is a *fit* of their coordinates, so where they actually +// sit is irrelevant to the drawing and nothing pulled the note along. +// +// That is not a lookup bug like the highlights around it. It is a note being in +// the wrong place, and the fix is a property of the note rather than of the +// thing that draws it. + +const harness = createHarness(); +const { App } = context; + +function def(name) { + return { + name, machine: 'DFA', sigma: ['a'], + states: [{ id: 'd0', x: 0, y: 0, name: 'i' }, { id: 'd1', x: 90, y: 0, name: 'j' }], + transitions: [{ id: 'e0', from: 'd0', to: 'd1', symbol: 'a' }], + startId: 'd0', entry: 'd0', accepts: ['d1'], version: 1 + }; +} + +function fixture() { + harness.resetApp(); + App.machine = 'DFA'; + App.sigma = new Set(['a']); + App.config.render.animateLayout = false; + App.states.push({ id: 'x1', x: 0, y: 0, name: 'A' }); + App.states.push({ id: 'x2', x: 200, y: 0, name: 'B' }); + App.transitions.push({ id: 'u1', from: 'x1', to: 'x2', symbol: 'a' }); + App.startId = 'x1'; + const { block } = context.inlineBlock(def('sub'), { x: 600, y: 400 }); + App.transitions.push({ id: 'u2', from: 'x2', to: block.entry, symbol: 'a' }); + context.invalidateViewGraph(); + return { block, inside: context.blockMembers(block.id) }; +} + +const top = () => context.enterBlockScope(null, { to: [] }); +const drawnIds = () => context.visibleNotes().map(n => n.id); + +// ── the field ───────────────────────────────────────────────────── + +test('a note written at the top level carries no scope at all', () => { + fixture(); + const note = context.createNote(10, 10); + // Absent means the top level, which is what every note written before this + // existed already was — so a machine with no blocks in it saves exactly the + // bytes it saved before and needs no migration. + assert.equal('scope' in note, false); + assert.equal(context.noteScopeOf(note), null); +}); + +test('a note written inside a block says so', () => { + const { block } = fixture(); + context.enterBlockScope(block.id); + const note = context.createNote(10, 10); + assert.equal(note.scope, block.id); +}); + +// ── what each level draws ───────────────────────────────────────── + +test('a level draws its own notes and nobody else’s', () => { + const { block } = fixture(); + const outer = context.createNote(10, 10); + context.enterBlockScope(block.id); + const inner = context.createNote(10, 10); + + assert.deepEqual(drawnIds(), [inner.id], 'inside the block, only the note written there'); + top(); + assert.deepEqual(drawnIds(), [outer.id], 'and back out, only the one written out here'); +}); + +test('only the drawn notes are rendered', () => { + const { block } = fixture(); + context.createNote(10, 10); + context.enterBlockScope(block.id); + const inner = context.createNote(10, 10); + context.renderAll(); + + const layer = context.$('notes-g'); + assert.equal(layer.children.length, 1, 'one note element on the level, not two'); + assert.equal(layer.children[0].getAttribute('data-note-id'), inner.id); +}); + +test('fit-to-screen frames the notes on screen, not the ones below', () => { + const { block } = fixture(); + context.enterBlockScope(block.id); + context.createNote(9000, 9000); + top(); + + let far = false; + context.includeNoteBounds((x0, y0, x1, y1) => { if (x1 > 5000) far = true; }); + assert.equal(far, false, 'a note two levels down does not pull the frame out to it'); +}); + +// ── one level down ──────────────────────────────────────────────── + +test('a note anchored to a state since grouped away points at the box', () => { + const { block, inside } = fixture(); + // The case the whole thing exists for: the note was written out here about a + // state that is now inside a block. `nodeIdAtScope` already answers which node + // this level draws for it, so there is no second rule — the note follows the + // box, and the box is what the reader can see. + const note = { id: 'n9', x: 0, y: 0, text: '', anchorStates: [inside[1].id], anchorTransitions: [] }; + App.notes.push(note); + assert.deepEqual(context.resolveNotePos(note), { x: block.x, y: block.y }); + + block.x = 2400; block.y = 1600; + context.invalidateViewGraph(); + assert.deepEqual(context.resolveNotePos(note), { x: 2400, y: 1600 }, + 'and it rides along, which it could not do while it was pinned to a hidden state'); +}); + +test('an anchored edge that crosses a boundary takes the box as one end', () => { + const { block } = fixture(); + const note = { id: 'n9', x: 0, y: 0, text: '', anchorStates: [], anchorTransitions: ['u2'] }; + App.notes.push(note); + const pos = context.resolveNotePos(note); + // The midpoint of the line the reader can see — x2 to the box — rather than + // of one drawn node and one that is not. + assert.deepEqual(pos, { x: (200 + block.x) / 2, y: (0 + block.y) / 2 }); +}); + +// ── blocks coming apart ─────────────────────────────────────────── + +test('ungrouping carries a note up to the level its states landed on', () => { + const { block } = fixture(); + const outer = context.inlineBlock(def('deep'), { x: 0, y: 0, parent: block.id }).block; + context.enterBlockScope(outer.id); + const note = context.createNote(10, 10); + top(); + + context.ungroupBlock(outer.id); + // The parent, not the top: a dissolved block knows where its contents went, + // and the read-validation below cannot, because the record is gone by then. + assert.equal(note.scope, block.id); +}); + +test('deleting a block deletes the notes written inside it', () => { + const { block } = fixture(); + context.enterBlockScope(block.id); + const inside = [context.createNote(10, 10), context.createNote(20, 20), context.createNote(30, 30)]; + top(); + const outside = context.createNote(0, 0); + App.selectedNotes = new Set(inside.map(n => n.id)); + + context.removeBlock(block.id); + context.invalidateViewGraph(); + + // Surfacing them instead would empty every note in the block onto the machine + // above — at coordinates from another level, each one pointing at states this + // call has just removed. On a block with thirty notes in it that is the + // workspace buried. Deleting a block means deleting what was in it. + assert.deepEqual(App.notes.map(n => n.id), [outside.id]); + assert.equal(App.selectedNotes.size, 0, 'and nothing is left selected that is gone'); +}); + +test('the whole subtree goes, not just the level named', () => { + const { block } = fixture(); + const deep = context.inlineBlock(def('deep'), { x: 0, y: 0, parent: block.id }).block; + context.enterBlockScope(deep.id); + context.createNote(10, 10); + top(); + + context.removeBlock(block.id); + assert.deepEqual(App.notes, [], 'a note two levels down is inside the block too'); +}); + +test('a note that only anchors into a deleted block is kept, where it was', () => { + const { block, inside } = fixture(); + const note = context.createNote(0, 0, [inside[1].id], []); + const was = context.resolveNotePos(note); + + // What both delete paths do, and the reason blockRemovalIds is exported: the + // prune has to run while the ids are still resolvable. `App.selectedStates` + // holds the *box's* id for a selected block and no state inside it, so naming + // the selection alone named nothing and the note jumped to its stored offset. + const gone = context.blockRemovalIds(block.id); + context.pruneNoteAnchorsExcluding([...gone.states], gone.transitions); + context.removeBlock(block.id); + context.invalidateViewGraph(); + + // The note lives out here, so it is not part of what was deleted — only its + // anchor was. It stays, loses the dangling anchor, and holds its position. + assert.deepEqual(App.notes.map(n => n.id), [note.id]); + assert.deepEqual(note.anchorStates, []); + assert.deepEqual(context.resolveNotePos(note), was); +}); + +test('a scope that vanished some other way still surfaces its notes', () => { + const { block } = fixture(); + context.enterBlockScope(block.id); + const note = context.createNote(10, 10); + top(); + + // Not a deletion: an algorithm result, an import or StateMate's apply replaces + // App.states wholesale and blockIsIntact prunes the record on the next read. + // Nothing announces that, so noteScopeOf validating on read is what stops the + // note being invisible at every level — text somebody wrote, silently gone. + App.blocks = []; + context.invalidateViewGraph(); + assert.equal(context.noteScopeOf(note), null); + assert.deepEqual(drawnIds(), [note.id]); +}); + +// ── the bookkeeping this must not disturb ───────────────────────── + +test('pruning an anchor holds a note still, even one on another level', () => { + const { block, inside } = fixture(); + context.enterBlockScope(block.id); + const note = context.createNote(400, 300, [inside[1].id], []); + const was = context.resolveNotePos(note); + top(); + + // pruneNoteAnchorsRemoving runs over every note at every level to preserve + // each one's position while its anchors go — so it is running from out here + // on a note that lives in there. Resolved against the *reader's* level this + // note answers the box's position rather than its own, and the prune freezes + // it at a place it was never drawn. Against the note's own level there is one + // answer whoever is asking. + context.pruneNoteAnchorsRemoving([inside[1].id], []); + assert.deepEqual(context.resolveNotePos(note), was); + assert.deepEqual(note.anchorStates, []); +}); + +// ── selection ───────────────────────────────────────────────────── + +test('a selection does not survive going where it cannot be seen', () => { + const { block } = fixture(); + const note = context.createNote(10, 10); + App.selectedNotes = new Set([note.id]); + + context.enterBlockScope(block.id); + // Before a note had a level they were all drawn, so a stale selection was at + // least a visible one. Now Delete would take something nobody can see. + assert.equal(App.selectedNotes.size, 0); +}); + +test('select-all and the marquee take only what is drawn', () => { + const { block } = fixture(); + context.createNote(10, 10); + context.enterBlockScope(block.id); + const inner = context.createNote(10, 10); + + context.selectAllStates(); + assert.deepEqual([...App.selectedNotes], [inner.id]); +}); + +// ── the file ────────────────────────────────────────────────────── + +test('the level a note lives on survives a round trip', () => { + const { block } = fixture(); + context.enterBlockScope(block.id); + const note = context.createNote(10, 10); + top(); + + // No serializer was edited for this: roundForSave copies a note whole and + // rounds only the fields it names, so `scope` rides along exactly as `blockId` + // does on a state. + const data = JSON.parse(JSON.stringify(context.getWorkspaceData())); + const saved = data.notes.find(n => n.id === note.id); + assert.equal(saved.scope, block.id); + + context.loadData(data); + assert.equal(context.getNote(note.id).scope, block.id); +}); diff --git a/tests/selection.test.js b/tests/selection.test.js index 3601449..85915d0 100644 --- a/tests/selection.test.js +++ b/tests/selection.test.js @@ -146,3 +146,92 @@ test('a restored snapshot drops selected objects it no longer holds', () => { assert.ok(!App.selectedNotes.has('n99')); assert.ok(!App.selectedDividers.has('d99')); }); + +// ── the marquee ─────────────────────────────────────────────────── +// The sweep rebuilds the selection from the baseline captured at the press on +// every move. Written as an add-only sweep it could take an object in and +// never give it back: dragging past a state and back, or shrinking the box off +// one, left it selected with nothing on screen still covering it — and the +// next Delete took it. + +/** Drives the real pointerdown/move listeners over the canvas background. */ +function marquee(from, to) { + const wrap = context.wrap; + wrap.setPointerCapture = () => { }; + const at = (x, y) => ({ + target: wrap, button: 0, pointerId: 1, pointerType: 'mouse', + clientX: x, clientY: y, preventDefault() { } + }); + wrap._listeners.pointerdown(at(from.x, from.y)); + const drag = { to(x, y) { context.handlePointerMove(at(x, y)); return drag; } }; + return drag.to(to.x, to.y); +} + +test('a marquee releases what it is dragged back off', () => { + reset(); + const { App, createState } = context; + App.tool = 'pointer'; + const near = createState(0, 0); + const far = createState(300, 0); + + const drag = marquee({ x: -50, y: -50 }, { x: 400, y: 50 }); + assert.deepStrictEqual([...App.selectedStates].sort(), [near.id, far.id].sort()); + + // Back over the near state only: the far one is no longer in the box, so it + // is no longer selected. + drag.to(50, 50); + assert.deepStrictEqual([...App.selectedStates], [near.id]); + assert.strictEqual( + App.domCache.states.get(far.id).classList.contains('sel-st'), false, + 'and the node it had highlighted is repainted' + ); +}); + +test('a marquee that covers nothing ends with nothing selected', () => { + reset(); + const { App, createState } = context; + App.tool = 'pointer'; + createState(0, 0); + + marquee({ x: -50, y: -50 }, { x: 50, y: 50 }).to(-40, -40); + assert.strictEqual(App.selectedStates.size, 0); +}); + +test('a modified marquee keeps what was selected before it started', () => { + reset(); + const { App, createState } = context; + App.tool = 'pointer'; + const kept = createState(0, 0); + const swept = createState(300, 0); + App.selectedStates.add(kept.id); + + const wrap = context.wrap; + wrap.setPointerCapture = () => { }; + const at = (x, y) => ({ + target: wrap, button: 0, pointerId: 1, pointerType: 'mouse', shiftKey: true, + clientX: x, clientY: y, preventDefault() { } + }); + wrap._listeners.pointerdown(at(200, -50)); + context.handlePointerMove(at(400, 50)); + assert.deepStrictEqual([...App.selectedStates].sort(), [kept.id, swept.id].sort()); + + // Shrunk back off the swept state — the baseline it was added to survives. + context.handlePointerMove(at(210, -40)); + assert.deepStrictEqual([...App.selectedStates], [kept.id]); +}); + +test('a marquee releases notes and dividers too', () => { + reset(); + const { App, createNote, createDivider } = context; + App.tool = 'pointer'; + const note = createNote(100, 100); + const region = createDivider('rect', 100, 100, 300, 300); + + const drag = marquee({ x: -50, y: -50 }, { x: 500, y: 500 }); + assert.ok(App.selectedNotes.has(note.id)); + assert.ok(App.selectedDividers.has(region.id)); + + drag.to(-40, -40); + assert.strictEqual(App.selectedNotes.size, 0); + assert.strictEqual(App.selectedDividers.size, 0); +}); diff --git a/tests/sim-blocks.test.js b/tests/sim-blocks.test.js new file mode 100644 index 0000000..de38ffc --- /dev/null +++ b/tests/sim-blocks.test.js @@ -0,0 +1,345 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createHarness, context } from './harness.js'; + +// The run, drawn onto a canvas that is showing blocks. +// +// The machine is flat and the canvas is a projection of it (js/view-graph.js), +// and the playback highlight is the one consumer that had never been told. It +// built its edge keys from each transition's own endpoints and looked its states +// up by their own ids — both perfectly correct about the *machine*, and both +// naming things the canvas has no node for the moment a block is on it. So from +// the first step a run touched a block, every mark went missing at once: no +// trail, no active edge, no travelling token, no arrival pulse. Nothing threw, +// and the verdict was right throughout — the whole of what was lost was the +// drawing of it, which is exactly the failure a test has to pin, because looking +// at the machine tells you nothing. + +const harness = createHarness(); +const { App } = context; + +function canvas() { + harness.resetApp(); + App.machine = 'DFA'; + App.sigma = new Set(['a', 'b']); + App.config.render.animateLayout = false; +} + +function def(name) { + return { + name, machine: 'DFA', sigma: ['a', 'b'], + states: [ + { id: 'd0', x: 0, y: 0, name: 'i' }, + { id: 'd1', x: 90, y: 0, name: 'j' } + ], + transitions: [{ id: 'e0', from: 'd0', to: 'd1', symbol: 'a' }], + startId: 'd0', entry: 'd0', accepts: ['d1'], version: 1 + }; +} + +/** Two plain states, a block beside them, and an edge that crosses into it. */ +function withBlock() { + canvas(); + App.states.push({ id: 'x1', x: 0, y: 0, name: 'A' }); + App.states.push({ id: 'x2', x: 200, y: 0, name: 'B' }); + App.transitions.push({ id: 'u1', from: 'x1', to: 'x2', symbol: 'a' }); + App.startId = 'x1'; + const { block } = context.inlineBlock(def('sub'), { x: 460, y: 0 }); + App.transitions.push({ id: 'u2', from: 'x2', to: block.entry, symbol: 'b' }); + context.invalidateViewGraph(); + context.renderAll(); + const inside = context.blockMembers(block.id); + return { block, inside }; +} + +/** The run this fixture describes: A -u1-> B -u2-> into the block -> out. */ +function stepsFor(block, inside) { + const inner = App.transitions.find(t => t.from === inside[0].id && t.to === inside[1].id); + return [ + { state: 'x1', note: '' }, + { state: 'x2', tid: 'u1', note: '' }, + { state: inside[0].id, tid: 'u2', note: '' }, + { state: inside[1].id, tid: inner.id, note: '', final: 'accept' } + ]; +} + +function has(id, cls) { + const n = App.domCache.states.get(id); + return !!n && n.classList.contains(cls); +} + +// ── the edge keys ───────────────────────────────────────────────── + +test('a step that crosses into a block names the edge the canvas drew', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + + // The model's own answer is `x2|s1`, which is a pair the *projection* does + // not contain: the block's box stands in for everything inside it, so the + // drawn edge is registered under the box's id. + const keys = context.getSimStepEdgeKeys(2); + assert.deepEqual(keys, [`x2|${block.id}`]); + assert.ok(context.findSimEdgeGroup(keys[0]), 'and that key has a node behind it'); +}); + +test('a step wholly inside a block names no edge at all', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + + // Not a failure to resolve — there is genuinely nothing drawn for it. The box + // is what is on screen, and the state half below is what lights it. + assert.deepEqual(context.getSimStepEdgeKeys(3), []); +}); + +test('a step outside every block is unchanged', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + assert.deepEqual(context.getSimStepEdgeKeys(1), ['x1|x2']); +}); + +// ── what gets lit ───────────────────────────────────────────────── + +test('a run inside a block lights the box standing in for it', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + App.simIdx = 3; + context.updateSimCanvasHighlights(App.simSteps[3]); + + assert.ok(has(block.id, 'act-st'), + 'the block carries the playhead for a state nobody can see'); + assert.ok(has('x1', 'sim-visited-st'), 'and the route in is still a trail'); + assert.ok(has('x2', 'sim-visited-st')); +}); + +test('several states inside one block are one mark, not four', () => { + const { block, inside } = withBlock(); + // A subset-style step standing on both interior states at once. They are one + // box on screen, so a set of *machine* ids would resolve to the same node + // twice — and the "already active?" test would be asking about ids the canvas + // does not have. + App.simSteps = [{ states: [inside[0].id, inside[1].id], note: '' }]; + App.simIdx = 0; + context.updateSimCanvasHighlights(App.simSteps[0]); + + assert.ok(has(block.id, 'act-st')); + assert.ok(!has(block.id, 'sim-visited-st'), + 'the active box is not also marked as merely visited'); +}); + +test('the marks come off again', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + App.simIdx = 3; + context.updateSimCanvasHighlights(App.simSteps[3]); + context.clearSimCanvasHighlights(); + assert.ok(!has(block.id, 'act-st')); + assert.ok(!has('x1', 'sim-visited-st')); +}); + +// ── the arrival pulse ───────────────────────────────────────────── + +test('the arrival ring traces the shape of the node it arrives at', () => { + const { block, inside } = withBlock(); + + context.pulseSimState(inside[1].id, 'acc'); + const boxRing = [...App.domCache.states.get(block.id).childNodes] + .filter(n => n.classList && n.classList.contains('sim-pulse')); + assert.equal(boxRing.length, 1, 'a state inside a block pulses its box'); + // A ring of radius R centred on a box two hundred pixels wide is a circle + // floating inside it, which reads as a second unexplained mark rather than as + // "control arrived here". + assert.equal(boxRing[0].tagName.toLowerCase(), 'rect'); + assert.ok(boxRing[0].classList.contains('is-box'), 'and takes the gentler ramp'); + + context.pulseSimState('x1'); + const stRing = [...App.domCache.states.get('x1').childNodes] + .filter(n => n.classList && n.classList.contains('sim-pulse')); + assert.equal(stRing.length, 1); + assert.equal(stRing[0].tagName.toLowerCase(), 'circle', 'a plain state still pulses as a circle'); + assert.ok(!stRing[0].classList.contains('is-box')); +}); + +// ── the map ─────────────────────────────────────────────────────── + +test('the minimap marks the run in drawn ids too', () => { + const { block, inside } = withBlock(); + // drawStates compares against the projection's nodes, so an answer in the + // machine's own ids never matched, and the marker vanished for the whole of + // the time the run was inside a block — on the one surface whose reason to + // exist is finding a playhead that is off screen. + assert.equal(context.visibleNodeIdFor(inside[1].id), block.id); + assert.equal(context.visibleNodeIdFor('x1'), 'x1'); +}); + +// ── the trail across a scope change ─────────────────────────────── + +test('drilling in rebuilds the trail rather than carrying stale keys', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + App.simIdx = 3; + context.updateSimCanvasHighlights(App.simSteps[3]); + const top = App._simTrail; + assert.ok(top.keys.has('x1|x2')); + + context.enterBlockScope(block.id); + context.renderAll(); + context.updateSimCanvasHighlights(App.simSteps[3]); + + // Which node shows a given state is exactly what the scope change decides, so + // a trail carried across it would be a set of keys for a diagram that is no + // longer on screen. + assert.notEqual(App._simTrail, top, 'the cache was rebuilt'); + assert.ok(!App._simTrail.keys.has('x1|x2'), 'and holds nothing from the level above'); + assert.ok(has(inside[1].id, 'act-st'), + 'the playhead is on the real state now that the real state is drawn'); +}); + +// ── one level in: the preview draws the run too ─────────────────── +// +// A box on the canvas is a small drawing of the machine inside it, so marks that +// stop at the box say "something in here" and nothing more. These reach the dot +// that is actually running — and cost a class each, because the preview was +// drawn once and a run moves nothing it is keyed on. + +function pvEl(blockId, stateId) { + const g = App.domCache.states.get(blockId); + return g && g.__pvIndex ? g.__pvIndex.get(stateId) : null; +} + +test('the dot that is really running lights inside the preview', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + App.simIdx = 3; + context.updateSimCanvasHighlights(App.simSteps[3]); + + assert.ok(pvEl(block.id, inside[1].id).classList.contains('is-active'), + 'the interior state has its own mark, not just the box'); + assert.ok(pvEl(block.id, inside[0].id).classList.contains('is-visited'), + 'and the one it came from is on the trail'); +}); + +test('the interior transition is drawn on its own path', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + App.simIdx = 3; + context.updateSimCanvasHighlights(App.simSteps[3]); + + const active = App.domCache.states.get(block.id).__parts.pvActive; + const d = active.getAttribute('d'); + assert.ok(d && d.startsWith('M'), 'the edge inside the box is drawn'); + // The same subpath the quiet layer under it carries, so the bright edge lies + // exactly on the one it replaces rather than beside it. + assert.ok(App.domCache.states.get(block.id).__parts.pvEdges.getAttribute('d').includes(d), + 'and it is the very segment the base path already had'); +}); + +test('the preview marks come off with everything else', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + App.simIdx = 3; + context.updateSimCanvasHighlights(App.simSteps[3]); + context.clearSimCanvasHighlights(); + + assert.ok(!pvEl(block.id, inside[1].id).classList.contains('is-active')); + assert.equal(App.domCache.states.get(block.id).__parts.pvActive.getAttribute('d'), '', + 'the active path is blanked, not left lit on a box the run has left'); +}); + +test('a step at the top level writes nothing into any preview', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + App.simIdx = 1; + context.updateSimCanvasHighlights(App.simSteps[1]); + + // The cheapness of this is the point: a box the run is nowhere near is not + // touched at all, so previews cost nothing on the machines that have many. + assert.equal(App.domCache.states.get(block.id).__parts.pvActive.getAttribute('d'), ''); + assert.ok(!pvEl(block.id, inside[0].id).classList.contains('is-active')); +}); + +test('marking the run rebuilds no preview', () => { + const { block, inside } = withBlock(); + App.simSteps = stepsFor(block, inside); + const g = App.domCache.states.get(block.id); + const before = { key: g.__previewKey, index: g.__pvIndex, dot: pvEl(block.id, inside[1].id) }; + + for (let i = 0; i < App.simSteps.length; i++) { + App.simIdx = i; + context.updateSimCanvasHighlights(App.simSteps[i]); + } + + // A run moves no state and changes no transition, so nothing the preview key + // is built from has moved — and rebuilding a hundred child elements per box + // per step is exactly the cost this feature must not have. + assert.equal(g.__previewKey, before.key); + assert.equal(g.__pvIndex, before.index, 'the index is the same object'); + assert.equal(pvEl(block.id, inside[1].id), before.dot, 'and the dot is the same element'); +}); + +// ── the other three surfaces that light something ───────────────── +// +// Not playback, but the same failure exactly: a key or an id taken from the +// model, a lookup that comes back empty, and a surface that lights nothing with +// nothing to say it tried. They are collected here because the shape is one +// thing, and because each is invisible on any machine without a block on it. + +function edgeEl(key) { return App.domCache.transitions.get(key); } + +test('a selected edge that crosses into a block is drawn selected', () => { + const { block } = withBlock(); + App.selectedTransitions = new Set(['u2']); + context.syncSelectionClasses(); + + // The worst version of the bug: `u2` is in App.selectedTransitions and Delete + // will take it, while nothing on screen says so — a selection you cannot see + // is one you cannot check. + assert.ok(edgeEl(`x2|${block.id}`).classList.contains('sel-t')); +}); + +test('a note anchored across a boundary lights the edge and the box', () => { + const { block, inside } = withBlock(); + App.notes = [{ + id: 'n1', x: 0, y: 0, text: 'why', + anchorStates: [inside[0].id], anchorTransitions: ['u2'] + }]; + context.renderAll(); + context.highlightNoteAnchors('n1', true); + + assert.ok(edgeEl(`x2|${block.id}`).classList.contains('note-link-t'), + 'the crossing edge is what the anchor is on'); + assert.ok(App.domCache.states.get(block.id).classList.contains('note-link-st'), + 'and a state inside the block points at the block, not at nothing'); +}); + +test('hovering Q lights every state, including the ones inside blocks', () => { + const { block } = withBlock(); + context.langHighlight('Q'); + + // The panel beside it reports the machine's own |Q| — every state at every + // depth — so a highlight that lit only the top level would say the opposite + // of the number it sits under. + assert.ok(App.domCache.states.get('x1').classList.contains('list-hover-st')); + assert.ok(App.domCache.states.get(block.id).classList.contains('list-hover-st'), + 'the box stands for the states it contains'); +}); + +test('hovering a symbol lights a crossing edge once, not zero times', () => { + const { block } = withBlock(); + context.langHighlightSymbol('b', true); + assert.ok(edgeEl(`x2|${block.id}`).classList.contains('list-hover-t')); + + context.langHighlightSymbol('b', false); + assert.ok(!edgeEl(`x2|${block.id}`).classList.contains('list-hover-t')); +}); + +test('an edge wholly inside a block lights nothing anywhere', () => { + const { block, inside } = withBlock(); + const inner = App.transitions.find(t => t.from === inside[0].id && t.to === inside[1].id); + App.selectedTransitions = new Set([inner.id]); + context.syncSelectionClasses(); + + // It is genuinely not on screen. The honest answer is no mark, not a mark on + // the box — which already carries the states' own. + assert.ok(!App.domCache.states.get(block.id).classList.contains('sel-t')); + assert.equal(context.viewEdgeKeyFor(inner.id), null); +}); diff --git a/tests/view-graph.test.js b/tests/view-graph.test.js index fa88984..db27ea4 100644 --- a/tests/view-graph.test.js +++ b/tests/view-graph.test.js @@ -362,3 +362,81 @@ test('ports reach no serializer', () => { assert.ok(!blob.includes('__in__'), 'the entry tab is derived, never stored'); assert.ok(!blob.includes('__out__'), 'and so is every exit tab'); }); + +// ── a cache hit has to actually be cheap ────────────────────────── +// +// viewGraph() is on the hot path in a way that is easy to forget: the layout +// pass runs it per frame, edgeLabelsHidden() reaches it once per edge label, and +// every surface that resolves a machine id to a drawn one reaches it per item. +// So "the cache hit" is not an optimisation on top of a correct answer — it is +// the answer, and anything O(machine) inside it is a stall rather than a slow +// frame. +// +// What made it one: refresh() called blockSize(), which falls through to +// blockMembers() + blockChildren() whenever a record carries no size of its own +// — and inlineBlock leaves those null, so that is the ordinary case, not the +// exception. Both are unindexed filters that allocate. A select-all over 2000 +// transitions measured 617ms with eight blocks against 7ms without. + +test('a cache hit recomputes no derived size', () => { + tmCanvas(); + const { block } = context.inlineBlock(def('seek', 6), { x: 200, y: 200 }); + context.invalidateViewGraph(); + const node = context.viewGraph().byId.get(block.id); + const box = node.box; + + for (let i = 0; i < 5; i++) context.viewGraph(); + + // Object identity, not equality: an equal box rebuilt each time is exactly the + // walk over the machine this is here to catch, and it looks identical from the + // outside. What a derived size derives from cannot change without stillValid() + // failing and the projection being rebuilt outright. + assert.equal(context.viewGraph().byId.get(block.id).box, box); +}); + +test('a hand-set size is still picked up on a cache hit', () => { + tmCanvas(); + const { block } = context.inlineBlock(def('seek', 6), { x: 200, y: 200 }); + context.invalidateViewGraph(); + context.viewGraph(); + + // The one thing that *can* change without any array changing identity: the + // reader resizing the box. Two reads off the record is what that costs. + block.w = 260; block.h = 180; + const node = context.viewGraph().byId.get(block.id); + assert.deepEqual(node.box, { w: 260, h: 180 }); +}); + +test('blocks do not make a selection sweep scale with the machine', () => { + const build = (nStates, nBlocks) => { + harness.resetApp(); + const { App } = context; + App.machine = 'DFA'; + App.sigma = new Set(['a']); + for (let i = 0; i < nStates; i++) App.states.push({ id: 's' + i, x: i * 5, y: (i % 40) * 5, name: 'q' + i }); + for (let i = 0; i + 1 < nStates; i++) App.transitions.push({ id: 't' + i, from: 's' + i, to: 's' + (i + 1), symbol: 'a' }); + App.startId = 's0'; + const per = Math.floor(nStates / (nBlocks + 1)); + for (let b = 0; b < nBlocks; b++) { + const id = 'blk' + b; + App.blocks.push({ id, name: 'B' + b, parent: null, entry: 's' + (b * per + 1), exits: [], x: b * 300, y: 900, w: null, h: null, collapsed: true }); + for (let i = b * per + 1; i < (b + 1) * per; i++) App.states[i].blockId = id; + } + context.invalidateViewGraph(); + App.selectedTransitions = new Set(App.transitions.map(t => t.id)); + }; + const time = (nStates, nBlocks) => { + build(nStates, nBlocks); + context.syncSelectionClasses(); // warm + const t = Date.now(); + for (let i = 0; i < 3; i++) context.syncSelectionClasses(); + return Date.now() - t; + }; + + const plain = time(2000, 0); + const withBlocks = time(2000, 8); + // Wall-clock, and deliberately loose — it is here to catch a reintroduced walk + // over the machine, not to pin a budget. The regression it guards was 88x. + assert.ok(withBlocks <= Math.max(60, plain * 6), + `selection with blocks (${withBlocks}ms) should not dwarf without (${plain}ms)`); +});