Give a remap shape something to drag it by

A remap draws light rather than colour, so `pick/hit-op` let a click go THROUGH
it to what it lights, and `op-bounds` left it out of a marquee's reach. Both of
those rules are about getting PAST a shape, and neither has anything to say
about the shape you have in your hands — but the stage's move gesture IS that
hit-test, while `handles` 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 a timeline row had just given it was thrown
away: selectable in the timeline, and then neither movable nor resizable, with
the handles sitting right there. A knockout was the same bug for the same
reason, and both of them read as handles that do not work.

`hit-op` now takes what is selected, and what is selected is never
click-through. That is `choose`'s own rule — a click inside what is selected
keeps it, so a deep selection can be dragged — reaching one step further down;
clicking a remap you have NOT selected still goes through to what it lights,
which is the point of it. `op-bounds` keeps a hole's and a light's bounds like
anything else, so a marquee reaches one without going to the timeline for it.

`browser/remap.mjs` asserts it as `peg.mjs` does, with real pointer events
through the handles, and for the same reason: every assertion about the maths
passed throughout.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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}]
+;; A HOLE AND A LIGHT HAVE BOUNDS LIKE ANYTHING ELSE. A knockout and a remap
-  (case (when-not (or knock lut) kind)
+;; 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."
+  the colour it clears. A remap is the same kind of thing the other way up: it is
-  [ops [x y]]
+  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
-                   ;; A remap is light on what is under it, not a thing.
+                    (not (on? op x y)) holes
-                   (or (:lut op) (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)))))
+                     (concat drawn traces))))))
 
 (defn hit
-  "The row path of the topmost op that shows at stage point `[x y]`, or nil."
+  "The row path of the topmost op that shows at stage point `[x y]`, or nil.
-  [ops point]
+  `selected` is what is selected now, which a light or a hole does not hide."
-  (some-> (hit-op ops point) path-of))
+  ([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."

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."
+  pointer event is handled between two.
-  [point]
+
-  (pick/hit (:ops @state) point))
+  `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

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)

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")))

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();
+}