diff --git a/frontend/src/arthur/domain/pick.cljs b/frontend/src/arthur/domain/pick.cljs index b5a7617..e8c708c 100644 --- a/frontend/src/arthur/domain/pick.cljs +++ b/frontend/src/arthur/domain/pick.cljs @@ -63,8 +63,13 @@ (and (<= 0 u) (< u w) (<= 0 v) (< v h)))) false)) -(defn- op-bounds [{:keys [kind pts n cx cy r size knock lut] :as op}] - (case (when-not (or knock lut) kind) +;; A HOLE AND A LIGHT HAVE BOUNDS LIKE ANYTHING ELSE. A knockout and a remap +;; were both left out of this, which is the rule `hit-op` plays by — a click +;; goes through them to what shows under — carried over to a gesture it is wrong +;; for: a marquee asks what is INSIDE it, not what a point lands on, and leaving +;; them out meant the only way to select one anywhere was its timeline row. +(defn- op-bounds [{:keys [kind pts n cx cy r size] :as op}] + (case kind :trace (let [cs (trace-corners op)] [(apply min (map first cs)) (apply min (map second cs)) (apply max (map first cs)) (apply max (map second cs))]) @@ -98,24 +103,40 @@ "The topmost op in `ops`, in draw order, that SHOWS at stage point `[x y]`, or nil. A knockout is never hit: it is a hole, and a click in it is a click on whatever shows through — so it hides the ops beneath it in its own symbol, of - the colour it clears." - [ops [x y]] - (:op (reduce (fn [holes op] - (cond - ;; A remap is light on what is under it, not a thing. - (or (:lut op) (not (on? op x y))) holes - (:knock op) (conj holes [(pop (path-of op)) (:knock op)]) - (some (fn [[in k]] (and (prefix? in (path-of op)) - (or (neg? k) (== k (:color op))))) - holes) holes - :else (reduced {:op op}))) - [] (let [{traces true drawn false} (group-by #(= :trace (:kind %)) (rseq (vec ops)))] - (concat drawn traces))))) + the colour it clears. A remap is the same kind of thing the other way up: it is + light on what is under it, so a click goes through it to what it lights. + + EXCEPT WHAT IS ALREADY SELECTED, which `selected` is the row path of. Both of + those rules are about reaching PAST a shape, and neither has anything to say + about the shape you have in your hands: a selected remap or knockout answered + no point on the stage at all, so there was nothing to drag it by — the stage's + move gesture is this hit-test on the ops, and `handles` draws a box round + bounds that nothing could then grab. The first press on one started a marquee + instead, which selected nothing and so threw the selection away, and that is + the whole of why a palette swapper could be selected from a timeline row and + then neither moved nor resized. The ordinary depth rule — a click inside what + is selected keeps it, so a deep selection can be dragged — is `choose`'s, and + this is the same rule reaching one step further down." + ([ops point] (hit-op ops point nil)) + ([ops [x y] selected] + (:op (reduce (fn [holes op] + (cond + (not (on? op x y)) holes + (and selected (prefix? selected (path-of op))) (reduced {:op op}) + (:lut op) holes + (:knock op) (conj holes [(pop (path-of op)) (:knock op)]) + (some (fn [[in k]] (and (prefix? in (path-of op)) + (or (neg? k) (== k (:color op))))) + holes) holes + :else (reduced {:op op}))) + [] (let [{traces true drawn false} (group-by #(= :trace (:kind %)) (rseq (vec ops)))] + (concat drawn traces)))))) (defn hit - "The row path of the topmost op that shows at stage point `[x y]`, or nil." - [ops point] - (some-> (hit-op ops point) path-of)) + "The row path of the topmost op that shows at stage point `[x y]`, or nil. + `selected` is what is selected now, which a light or a hole does not hide." + ([ops point] (hit ops point nil)) + ([ops point selected] (some-> (hit-op ops point selected) path-of))) (defn choose "The row path a click on `hit` selects, with `selected` the one selected now." diff --git a/frontend/src/arthur/ui/player.cljs b/frontend/src/arthur/ui/player.cljs index e848ab0..6931491 100644 --- a/frontend/src/arthur/ui/player.cljs +++ b/frontend/src/arthur/ui/player.cljs @@ -220,9 +220,12 @@ "The topmost row path drawn at stage point `point` on the frame last painted, or nil. Read off the ops the canvas was drawn from, so a click picks exactly what is seen: their point buffers stay good until the next paint, and a - pointer event is handled between two." - [point] - (pick/hit (:ops @state) point)) + pointer event is handled between two. + + `selected` is the row path selected now, which a light or a hole it belongs to + does not hide from this: see `pick/hit-op`." + ([point] (at point nil)) + ([point selected] (pick/hit (:ops @state) point selected))) (defn hit-op "The topmost op that shows at `point`, as `at` picks it: what the eraser diff --git a/frontend/src/arthur/ui/stage.cljs b/frontend/src/arthur/ui/stage.cljs index eb38d04..0962702 100644 --- a/frontend/src/arthur/ui/stage.cljs +++ b/frontend/src/arthur/ui/stage.cljs @@ -636,7 +636,10 @@ (do (.setPointerCapture svg (.-pointerId event)) (stroke-begin! ctx tool p size tone event)) - (let [path (pick/choose selected (player/at p) (.-altKey event))] + ;; WITH THE SELECTION, so a shape the hit-test treats as light + ;; or as a hole — a palette swapper, a knockout — can be + ;; dragged by the one you are holding. See `pick/hit-op`. + (let [path (pick/choose selected (player/at p selected) (.-altKey event))] (when (and path (not (.-shiftKey event))) (reset! selection-anchor {:clip-id clip-id :open (:open ctx) :path path})) (cond @@ -672,7 +675,7 @@ :on-double-click (fn [^js event] (when (= :select tool) (let [p (xy (.-currentTarget event) event w h) - hit (player/at p) + hit (player/at p selected) path (pick/deeper selected hit)] (cond (not= path selected) (select! ctx path) diff --git a/frontend/test/arthur/domain/instance_test.cljs b/frontend/test/arthur/domain/instance_test.cljs index 620c783..31196cc 100644 --- a/frontend/test/arthur/domain/instance_test.cljs +++ b/frontend/test/arthur/domain/instance_test.cljs @@ -555,7 +555,14 @@ (is (= (colour [:glasses :frame]) (at 5 10)) "and the frame is still there round it") (is (not= (at 15 10) (at 5 10))) (is (= [:under] (pick/hit ops [15 10])) "a click in the glass is a click on what shows") - (is (= [:glasses :frame] (pick/hit ops [5 10]))))) + (is (= [:glasses :frame] (pick/hit ops [5 10]))) + ;; Reaching past a hole is one thing; the hole you are holding is another. + ;; Without this there is no point on the stage a selected knockout answers, + ;; so the box `handles` draws round it has nothing to drag it by. + (is (= [:glasses :glass] (pick/hit ops [15 10] [:glasses :glass])) + "unless the hole is what is selected, which is how it is dragged") + (is (= [:under] (pick/hit ops [15 10] [:under])) + "something else being selected does not make the hole solid"))) (deftest an-op-goes-into-the-layer-of-the-symbol-it-is-for (let [a {:kind :poly :node :a} b {:kind :poly :node [:g :b]} c {:kind :poly :node [:g :c]} @@ -595,4 +602,10 @@ (is (= (slot 5) (at 7 10)) "the wall under the beam is lit") (is (= (slot 3) (at 12 10)) "the door under it is a slot the map leaves alone") (is (= (slot 2) (at 30 10)) "and the wall outside it is as it was") - (is (= [:wall] (pick/hit ops [7 10])) "a click goes through the light"))) + (is (= [:wall] (pick/hit ops [7 10])) "a click goes through the light") + (is (= [:beam] (pick/hit ops [7 10] [:beam])) + "unless the light is what is selected, which is how it is dragged") + (is (= [:wall] (pick/hit ops [7 10] [:door])) + "something else being selected does not make the light solid") + (is (= [[:wall] [:beam]] (pick/in-rect ops [4 4 6 6] 1)) + "and a marquee reaches it, where a click goes through it"))) diff --git a/frontend/test/browser/remap.mjs b/frontend/test/browser/remap.mjs new file mode 100644 index 0000000..e99e9c2 --- /dev/null +++ b/frontend/test/browser/remap.mjs @@ -0,0 +1,174 @@ +// Browser smoke test for a palette swapper on the stage: a remap shape is +// selected as a timeline row selects it, and the handles over it actually drag. +// +// WHAT A UNIT TEST CANNOT SEE, as `peg.mjs` says of a peg, and for a closely +// related reason. A remap draws no colour of its own — it lights what is under +// it — so `pick/hit-op` let a click go THROUGH it, and the stage's move gesture +// is that hit-test. `handles` meanwhile hangs its box and corners off +// `pick/bounds-of`, which reads the node's points and knows nothing about +// colour. So a remap shape got a full set of handles over a shape that answered +// no point on the stage at all: the first press on it started a marquee, the +// marquee selected nothing, and the selection it had been given by a timeline +// row was thrown away — which looks exactly like handles that do not work. +// Every assertion about the maths still passed, hence this. +// +// A plain shape goes through the same two drags as the control, so a harness +// that has stopped delivering pointer events cannot read as a pass. +// +// Uses an in-memory fixture and performs no server-side writes. +import { spawn } from 'node:child_process'; +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import assert from 'node:assert/strict'; + +const url = process.env.ARTHUR_URL ?? 'http://localhost:8778/'; +const profile = mkdtempSync(join(tmpdir(), 'arthur-remap-')); +const port = 9344; +const chrome = spawn(process.env.CHROME ?? '/usr/bin/chromium', [ + '--headless=new', '--no-sandbox', '--disable-gpu', '--no-first-run', + '--no-default-browser-check', '--mute-audio', '--window-size=1440,1000', + `--user-data-dir=${profile}`, `--remote-debugging-port=${port}`, url, +], { stdio: 'ignore' }); +const sleep = ms => new Promise(r => setTimeout(r, ms)); +let ws, id = 0; +const pending = new Map(); +const send = (method, params) => new Promise(res => { + const n = ++id; pending.set(n, res); + ws.send(JSON.stringify({ id: n, method, params })); +}); +const evalJS = async expr => { + const r = await send('Runtime.evaluate', + { expression: `(function(){${expr}})()`, returnByValue: true, awaitPromise: true }); + if (r.exceptionDetails) throw new Error(r.exceptionDetails.exception?.description ?? JSON.stringify(r.exceptionDetails)); + return r.result.value; +}; +let failures = 0; +const ok = (cond, what, extra = '') => { + console.log(` ${cond ? 'ok ' : 'FAIL'} ${what}${extra ? ' — ' + extra : ''}`); + if (!cond) failures++; +}; + +// A press, two moves and a release, as a hand makes them: the stage only calls +// a gesture a gesture once the pointer has left the press by a pixel. +const drag = async (x, y, dx, dy) => { + for (const [type, fx, fy] of [['mousePressed', 0, 0], ['mouseMoved', dx / 2, dy / 2], + ['mouseMoved', dx, dy], ['mouseReleased', dx, dy]]) { + await send('Input.dispatchMouseEvent', { + type, x: x + fx, y: y + fy, button: 'left', buttons: 1, clickCount: 1, + pointerType: 'mouse', + }); + await sleep(90); + } + await sleep(400); +}; +// The handles as DOM, in client pixels: what a hand would aim at. +const rectOf = async sel => evalJS(` + const q = document.querySelector('${sel}'); + if (!q) return null; + const r = q.getBoundingClientRect(); + return {cx: r.x + r.width/2, cy: r.y + r.height/2, w: r.width, h: r.height}; +`); +const state = async which => evalJS(` + const c = cljs.core, k = c.keyword, v = c.vector; + const db = c.deref(re_frame.db.app_db); + const e = arthur.footage.store.entry(c.get(db, k('clip/current'))); + const n = c.get_in(c.get(e, k('clip')), v(k('symbols'), k('main'), k('nodes'), k('${which}'))); + const ch = p => c.clj__GT_js(c.get_in(n, v(k('channels'), p, k('value')))); + return {pos: ch(v(k('xform'), k('pos'))), scale: ch(v(k('xform'), k('scale'))), + selection: c.clj__GT_js(c.get_in(db, v(k('ui'), k('selection'))))}; +`); + +try { + let target; + for (let i = 0; i < 100 && !target; i++) { + await sleep(100); + try { + target = (await fetch(`http://127.0.0.1:${port}/json/list`).then(r => r.json())) + .find(t => t.type === 'page' && t.url.startsWith(url)); + } catch {} + } + assert(target, 'editor page'); + ws = new WebSocket(target.webSocketDebuggerUrl); + await new Promise(r => ws.addEventListener('open', r)); + ws.addEventListener('message', e => { + const m = JSON.parse(e.data); + if (m.id && pending.has(m.id)) { pending.get(m.id)(m.result); pending.delete(m.id); } + }); + await send('Runtime.enable'); + for (let i = 0; i < 200; i++) { + if (await evalJS('return !!(window.cljs && window.re_frame && window.arthur)')) break; + await sleep(250); + } + + // Local route; a plain shape, and a remap shape beside it on its own ground. + const setup = await evalJS(` + const c = cljs.core, k = c.keyword, v = c.vector, m = c.hash_map; + c.swap_BANG_(re_frame.db.app_db, db => c.assoc(db, k('route'), k('local-test'))); + let doc = arthur.domain.clip.blank(); + doc = arthur.domain.paint.new_shape(doc, k('main'), k('plain'), 0, + v(40, 40, 90, 40, 90, 90, 40, 90), 1); + doc = arthur.domain.paint.new_shape(doc, k('main'), k('light'), 0, + v(140, 40, 200, 40, 200, 100, 140, 100), v(k('remap'), m(1, 2))); + const entry = m(k('clip'), doc, k('store'), m()); + const cid = arthur.footage.store.install_BANG_(entry, 'remap-probe'); + c.swap_BANG_(re_frame.db.app_db, db => + c.assoc(c.assoc_in(db, v(k('ui'), k('open')), k('main')), + k('clip/current'), cid, k('paint/revision'), 0)); + return arthur.domain.symbol.remap_QMARK_( + c.get_in(doc, v(k('symbols'),k('main'),k('nodes'),k('light'), + k('channels'),v(k('style'),k('color')),k('value')))); + `); + ok(setup === true, 'the fixture holds a remap shape'); + await sleep(800); + + // The rule the fix must not break: with nothing selected, a click still goes + // through the light to what it lights. + const through = await evalJS(` + const c = cljs.core, v = c.vector; + const hit = arthur.ui.player.at(v(170, 70)); + return hit ? JSON.stringify(c.clj__GT_js(hit)) : null; + `); + ok(through === null, 'with nothing selected a click goes through the light', + String(through)); + + for (const which of ['light', 'plain']) { + console.log(`\n${which}:`); + // Selected the way a timeline row selects it: a path, and no stage click. + await evalJS(` + const c = cljs.core, k = c.keyword, v = c.vector; + re_frame.core.dispatch_sync(v(k('arthur.events.ui/set-tool'), k('select'))); + re_frame.core.dispatch_sync(v(k('arthur.events.ui/select'), + v(k('node'), k('main'), k('${which}'), v(k('${which}'))))); + return true; + `); + await sleep(500); + + const box = await rectOf('.paint-overlay .handles .box'); + ok(!!box, 'the stage puts a box round it'); + if (!box) continue; + + const before = await state(which); + await drag(box.cx, box.cy, 25, 0); + const moved = await state(which); + ok(Array.isArray(moved.pos) && Math.abs(moved.pos[0] - 0) > 2, + 'dragging its middle moves it', JSON.stringify(moved.pos)); + ok(JSON.stringify(moved.selection) === JSON.stringify(before.selection), + 'and the drag keeps the selection rather than marqueeing it away', + JSON.stringify(moved.selection)); + + const corner = await rectOf('.paint-overlay .handles .corner'); + ok(!!corner, 'and corners to scale by'); + if (!corner) continue; + await drag(corner.cx, corner.cy, 18, 18); + const scaled = await state(which); + ok(Array.isArray(scaled.scale) && Math.abs(scaled.scale[0] - 1) > 0.05, + 'dragging a corner scales it', JSON.stringify(scaled.scale)); + } + + console.log(failures ? `\n${failures} FAILED` + : '\nPASS: a palette swapper can be grabbed, moved and scaled on the stage'); +} finally { + if (ws) ws.close(); + chrome.kill(); +}