Puppet: a click on deformed-away space creates nothing (#2865)

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

crates/effects/src/puppet.rs

@@ -395,6 +395,27 @@ pub fn contains(mesh: &Mesh, deformed: &[[f64; 2]], p: [f64; 2]) -> bool {
     })
 }
 
+/// Whether `at` (effect space) is art a new mesh with this `expansion` could take: on the expanded
+/// alpha itself, and not what a mesh of `params` already covers at rest, even if its deformation
+/// has carried that art away. A mesh there would only rebuild the same outline: a full copy (#2865).
+pub fn unmeshed_art_at(buf: &Buf, params: &Params, at: [f64; 2], expansion: f64) -> bool {
+    let (w, h) = (buf.img.width as usize, buf.img.height as usize);
+    let alpha: Vec<f32> = buf.img.data.iter().map(|p| p[3]).collect();
+    let mut outline = mesh::outline_under(&alpha, w, h, buf.scale.max(1e-9), buf.offset, at, expansion);
+    if outline.is_empty() {
+        return false;
+    }
+    // As `triangulate` winds it. The simplified outline can cut a corner past `at`, so a mesh can
+    // miss the very art it was built from: the same outline is the same art.
+    if mesh::signed_area(&outline) < 0.0 {
+        outline.reverse();
+    }
+    !parse(params).iter().any(|m| {
+        let mesh = mesh_for(buf, m);
+        mesh.outline == outline || contains(&mesh, &mesh.verts, at)
+    })
+}
+
 #[inline]
 fn bary(a: [f64; 2], b: [f64; 2], c: [f64; 2], p: [f64; 2]) -> Option<[f64; 3]> {
     let d = (b[1] - c[1]) * (a[0] - c[0]) + (c[0] - b[0]) * (a[1] - c[1]);

crates/effects/src/puppet/mesh.rs

@@ -194,6 +194,11 @@ pub fn region(g: &Grid, seed: [f64; 2]) -> Option<Grid> {
     Some(Grid { w, h, on: outside.iter().map(|o| !o).collect() })
 }
 
+/// The region under `seed` only when `seed` is on a cell: `region` without its nearest fallback.
+pub fn region_under(g: &Grid, seed: [f64; 2]) -> Option<Grid> {
+    if g.at(seed[0].floor() as i64, seed[1].floor() as i64) { region(g, seed) } else { None }
+}
+
 /// Trace the outer boundary of a hole-free region along pixel cracks: a closed polygon through
 /// pixel corners, region on the left when walking (counter-clockwise in y-down coordinates).
 pub fn trace(g: &Grid) -> Vec<[f64; 2]> {
@@ -617,6 +622,25 @@ pub fn triangulate(outline: &[[f64; 2]], opts: &MeshOpts) -> Mesh {
 /// The outline of the alpha region under `seed` (buffer pixels → layer space via `scale` and
 /// `offset`), expanded by `expansion` layer pixels.
 pub fn outline(alpha: &[f32], w: usize, h: usize, scale: f64, offset: [f64; 2], seed: [f64; 2], expansion: f64) -> Vec<[f64; 2]> {
+    outline_of(alpha, w, h, scale, offset, seed, expansion, region)
+}
+
+/// As `outline`, but empty unless `seed` is on the expanded alpha itself.
+pub fn outline_under(alpha: &[f32], w: usize, h: usize, scale: f64, offset: [f64; 2], seed: [f64; 2], expansion: f64) -> Vec<[f64; 2]> {
+    outline_of(alpha, w, h, scale, offset, seed, expansion, region_under)
+}
+
+#[allow(clippy::too_many_arguments)]
+fn outline_of(
+    alpha: &[f32],
+    w: usize,
+    h: usize,
+    scale: f64,
+    offset: [f64; 2],
+    seed: [f64; 2],
+    expansion: f64,
+    pick: fn(&Grid, [f64; 2]) -> Option<Grid>,
+) -> Vec<[f64; 2]> {
     // Pad so expansion can grow past the buffer edge.
     let pad = (expansion.max(0.0) * scale).ceil() as usize + 2;
     let (pw, ph) = (w + 2 * pad, h + 2 * pad);
@@ -628,7 +652,7 @@ pub fn outline(alpha: &[f32], w: usize, h: usize, scale: f64, offset: [f64; 2],
     }
     let g = expand(&Grid { w: pw, h: ph, on }, expansion * scale);
     let sp = [seed[0] * scale + offset[0] + pad as f64, seed[1] * scale + offset[1] + pad as f64];
-    let Some(r) = region(&g, sp) else { return vec![] };
+    let Some(r) = pick(&g, sp) else { return vec![] };
     let c = trace(&r);
     let c = simplify(&c, 0.75);
     c.iter().map(|p| [(p[0] - pad as f64 - offset[0]) / scale, (p[1] - pad as f64 - offset[1]) / scale]).collect()

crates/effects/src/puppet/tests.rs

@@ -323,3 +323,40 @@ fn single_pin_on_a_traced_solid_does_not_rotate_it() {
         }
     }
 }
+
+/// #2865: only art no mesh has takes a new mesh, however close the rest is.
+#[test]
+fn unmeshed_art_is_strictly_under_the_point_and_not_already_meshed() {
+    // Two discs; the mesh is on the left one, at rest.
+    let (w, h) = (96usize, 48usize);
+    let (l, r) = (disc_alpha(w, h, [24.0, 24.0], 18.0), disc_alpha(w, h, [72.0, 24.0], 12.0));
+    let mut img = Image::new(w as u32, h as u32);
+    for (i, p) in img.data.iter_mut().enumerate() {
+        *p = [1.0, 1.0, 1.0, l[i].max(r[i])];
+    }
+    let buf = Buf { img, offset: [0.0; 2], scale: 1.0 };
+    let params = flatten_params(&puppet_fx(&[(PinKind::Position, [24.0, 24.0], [24.0, 24.0])]), &mut |p| p.value.clone());
+    let e = 3.0;
+    assert!(unmeshed_art_at(&buf, &params, [72.0, 24.0], e), "the other disc");
+    assert!(!unmeshed_art_at(&buf, &params, [24.0, 24.0], e), "the meshed disc");
+    assert!(!unmeshed_art_at(&buf, &params, [48.0, 24.0], e), "between them: no nearest fallback");
+    assert!(!unmeshed_art_at(&buf, &params, [500.0, 24.0], e), "off the buffer");
+    // Pixel corners of the meshed disc that simplifying the outline left just outside the triangles:
+    // on its art, off its mesh, and a mesh there would be the same mesh again.
+    let m = parse(&params).remove(0);
+    let mesh = mesh_for(&buf, &m);
+    let mut slivers = 0;
+    for y in 0..h {
+        for x in 0..48 {
+            for (dx, dy) in [(0.02, 0.02), (0.98, 0.02), (0.02, 0.98), (0.98, 0.98)] {
+                let p = [x as f64 + dx, y as f64 + dy];
+                if contains(&mesh, &mesh.verts, p) || mesh::outline_under(&l, w, h, 1.0, [0.0; 2], p, e).is_empty() {
+                    continue;
+                }
+                slivers += 1;
+                assert!(!unmeshed_art_at(&buf, &params, p, e), "{p:?}");
+            }
+        }
+    }
+    assert!(slivers > 0, "no point of the art falls outside the mesh: this test proves nothing");
+}

crates/engine/src/cmd/puppet.rs

@@ -7,7 +7,8 @@
 //!
 //! Pin positions are in effect space. A new pin is attached to the mesh under it: the click is
 //! mapped back through the current deformation to the rest mesh (its hidden rest position), and
-//! Position / Advanced pins get a Position keyframe at the current time, as in After Effects.
+//! Position / Advanced pins get a Position keyframe at the current time, as in After Effects. Off
+//! every mesh a pin starts a new mesh only on art no mesh has yet; elsewhere nothing happens (#2865).
 
 use effectcraft_effects::puppet::{self, MeshOpts, PinKind};
 use effectcraft_effects::{Buf, Params};
@@ -62,6 +63,19 @@ fn layer_time(s: &Session, cid: ItemId, lid: LayerId, t: Tick) -> Result<Tick> {
     Ok(l.layer_time(t))
 }
 
+/// Whether a pin at `at` (effect space) goes on this mesh as `def` deforms it: on it or beside it.
+fn on_mesh(mesh: &puppet::Mesh, def: &[[f64; 2]], at: [f64; 2]) -> bool {
+    !mesh.tris.is_empty() && (puppet::contains(mesh, def, at) || def.iter().map(|v| (v[0] - at[0]).hypot(v[1] - at[1])).fold(f64::MAX, f64::min) < 12.0)
+}
+
+/// Whether `puppet.addPin` at `at` (effect space) places a pin on a layer with a Puppet effect: on
+/// a mesh, on art no mesh has, or anywhere when there is no mesh yet (the first pin).
+pub fn places_pin(s: &Session, cid: ItemId, lid: LayerId, at: [f64; 2], t: Tick) -> bool {
+    let Some((buf, params)) = find_puppet(s, cid, lid).and_then(|fu| puppet_eval(s, cid, lid, fu, t)) else { return true };
+    let meshes = puppet::overlay(&buf, &params);
+    meshes.is_empty() || meshes.iter().any(|(_, mesh, def, _)| on_mesh(mesh, def, at)) || puppet::unmeshed_art_at(&buf, &params, at, s.state.puppet.expansion)
+}
+
 fn add_pin(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "puppet.addPin";
     let (cid, lid) = layer_p(s, p, cmd)?;
@@ -78,14 +92,17 @@ fn add_pin(s: &mut Session, p: &Value) -> Result<Value> {
             return Err(bad(cmd, "the Puppet tools work on footage, solid, text, shape and precomp layers"));
         }
     }
+    let opts = MeshOpts {
+        density: f_p(p, "density").unwrap_or(s.state.puppet.density),
+        expansion: f_p(p, "expansion").unwrap_or(s.state.puppet.expansion),
+        triangles: f_p(p, "triangles").unwrap_or(s.state.puppet.triangles),
+    };
     // Which mesh: the one under the click (through the current deformation), else a new one.
     let fx_uid = find_puppet(s, cid, lid);
     let want_mesh = p.get("mesh").and_then(Value::as_u64);
-    let new_mesh = p.get("newMesh").and_then(Value::as_bool).unwrap_or(false);
     // (mesh, the pin's rest point, where it is now)
     let mut target: Option<(Uid, [f64; 2], [f64; 2])> = None;
     if let Some(fu) = fx_uid
-        && !new_mesh
         && let Some((buf, params)) = puppet_eval(s, cid, lid, fu, t)
     {
         let meshes = puppet::overlay(&buf, &params);
@@ -93,9 +110,7 @@ fn add_pin(s: &mut Session, p: &Value) -> Result<Value> {
             if want_mesh.is_some_and(|w| w != *uid) || mesh.tris.is_empty() {
                 continue;
             }
-            let inside = puppet::contains(mesh, def, at);
-            let near = def.iter().map(|v| (v[0] - at[0]).hypot(v[1] - at[1])).fold(f64::MAX, f64::min) < 12.0;
-            if inside || near || want_mesh.is_some() {
+            if on_mesh(mesh, def, at) || want_mesh.is_some() {
                 let rest = puppet::unmap(mesh, def, at);
                 // A moving pin holds its nearest mesh vertex at `position + (vertex - rest)`
                 // (puppet::solve). Choose the position that keeps that vertex where it is now, so
@@ -120,12 +135,11 @@ fn add_pin(s: &mut Session, p: &Value) -> Result<Value> {
         {
             return Err(bad(cmd, format!("no mesh {w}")));
         }
+        // Nothing to pin here: empty space, or art a mesh has already carried away (#2865).
+        if target.is_none() && !meshes.is_empty() && !puppet::unmeshed_art_at(&buf, &params, at, opts.expansion) {
+            return Ok(json!({"pin": null, "changed": false}));
+        }
     }
-    let opts = MeshOpts {
-        density: f_p(p, "density").unwrap_or(s.state.puppet.density),
-        expansion: f_p(p, "expansion").unwrap_or(s.state.puppet.expansion),
-        triangles: f_p(p, "triangles").unwrap_or(s.state.puppet.triangles),
-    };
     let size = {
         let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?;
         let l = comp.layer(lid).ok_or(EngineError::NoComp)?;
@@ -652,7 +666,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Add Puppet Pin",
             [],
             None,
-            "{layer?, kind?: position|advanced|bend|starch|overlap, position: [x,y] (effect space), time? (s) | frame?, mesh?: uid, newMesh?, density?, expansion?, triangles?}",
+            "{layer?, kind?: position|advanced|bend|starch|overlap, position: [x,y] (effect space), time? (s) | frame?, mesh?: uid, density?, expansion?, triangles?}",
             has_comp,
             add_pin
         ),

crates/engine/src/tests/paint.rs

@@ -225,6 +225,60 @@ fn a_pin_added_to_a_bent_mesh_moves_nothing() {
     }
 }
 
+fn mesh_count(s: &mut Session, id: u64) -> usize {
+    s.execute("puppet.info", json!({"layer": id})).unwrap()["meshes"].as_array().unwrap().len()
+}
+
+/// #2865: where the deformed layer is empty a click makes nothing: no copy of the art it has moved
+/// off, no undo step, no error.
+#[test]
+fn a_pin_where_the_deformed_layer_is_empty_creates_nothing() {
+    let (mut s, id) = setup();
+    s.execute("puppet.addPin", json!({"layer": id, "position": [10, 30]})).unwrap();
+    s.execute("puppet.addPin", json!({"layer": id, "position": [90, 30]})).unwrap();
+    s.set_time(Tick::from_seconds_f64(1.0));
+    s.execute("puppet.movePin", json!({"layer": id, "pin": "Puppet Pin 1", "position": [10, 70]})).unwrap();
+    s.execute("puppet.movePin", json!({"layer": id, "pin": "Puppet Pin 2", "position": [90, 70]})).unwrap();
+    let (cid, t) = (s.active_comp_id().unwrap(), Tick::from_seconds_f64(1.0));
+    let before = s.render(cid, t, RenderOpts::default());
+    let revision = s.revision;
+    let lid = effectcraft_project::LayerId(id);
+    // The solid's top, which the pins have carried 40 px down; and well off it.
+    for at in [[50, 10], [300, 300]] {
+        assert!(!crate::commands::puppet::places_pin(&s, cid, lid, [at[0] as f64, at[1] as f64], t), "{at:?}");
+        let r = s.execute("puppet.addPin", json!({"layer": id, "position": at})).unwrap();
+        assert_eq!(r["pin"], Value::Null, "{at:?}");
+        assert_eq!(s.revision, revision, "{at:?} changed the project");
+    }
+    assert_eq!((mesh_count(&mut s, id), pin_count(&s, id)), (1, 2));
+    assert_eq!(s.render(cid, t, RenderOpts::default()), before);
+    // On the deformed shape a pin still goes on the mesh.
+    assert!(crate::commands::puppet::places_pin(&s, cid, lid, [50.0, 70.0], t));
+    assert!(s.execute("puppet.addPin", json!({"layer": id, "position": [50, 70]})).unwrap()["pin"].is_u64());
+    assert_eq!((mesh_count(&mut s, id), pin_count(&s, id)), (1, 3));
+}
+
+/// #2865: art no mesh has still takes its own mesh; the gap beside it takes none. The first pin of
+/// a layer is as ever: it finds the art nearest the click.
+#[test]
+fn only_unmeshed_art_takes_a_new_mesh() {
+    let (mut s, id) = setup();
+    for x in [[0, 30], [60, 100]] {
+        s.execute("mask.new", json!({"layer": id, "vertices": [[x[0], 0], [x[1], 0], [x[1], 60], [x[0], 60]], "closed": true})).unwrap();
+    }
+    s.execute("puppet.addPin", json!({"layer": id, "position": [46, 30]})).unwrap();
+    assert_eq!(mesh_count(&mut s, id), 1, "the first pin");
+    s.undo();
+    s.execute("puppet.addPin", json!({"layer": id, "position": [10, 30]})).unwrap();
+    // In the gap, past reach of the left piece's mesh (Expansion takes it to x 33).
+    assert_eq!(s.execute("puppet.addPin", json!({"layer": id, "position": [46, 30]})).unwrap()["pin"], Value::Null);
+    assert_eq!((mesh_count(&mut s, id), pin_count(&s, id)), (1, 1));
+    s.execute("puppet.addPin", json!({"layer": id, "position": [80, 30]})).unwrap();
+    assert_eq!((mesh_count(&mut s, id), pin_count(&s, id)), (2, 2), "the right piece gets its own mesh");
+    s.execute("puppet.addPin", json!({"layer": id, "position": [80, 50]})).unwrap();
+    assert_eq!((mesh_count(&mut s, id), pin_count(&s, id)), (2, 3), "and the next pin goes on it");
+}
+
 #[test]
 fn puppet_pin_kinds_and_mesh_options() {
     let (mut s, id) = setup();

crates/ui-egui/src/panels/puppet_tool.rs

@@ -84,8 +84,8 @@ pub fn overlay(app: &EffectcraftApp, ctx: &egui::Context, cid: ItemId, layer: &L
 
 /// The layer a Puppet tool press at comp point `cpt` works on, of the selected `layers` with a
 /// Puppet effect (stack order): the one whose mesh, as deformed now, is under it (pins go on the
-/// deformed shape, also where it reaches past the layer's own bounds), else the one whose art is
-/// (a new mesh there). Never a layer without a Puppet effect: the tools stay on the layers being
+/// deformed shape, also where it reaches past the layer's own bounds), else the one `on_art` takes
+/// it (see `takes_pin`). Never a layer without a Puppet effect: the tools stay on the layers being
 /// rigged, as in After Effects (#284).
 pub fn target(ectx: &EvalCtx, layers: &[(&Layer, Overlay)], cpt: [f64; 2], on_art: impl Fn(&Layer) -> bool) -> Option<LayerId> {
     let on_mesh = |l: &Layer, ov: &Overlay| {
@@ -96,6 +96,14 @@ pub fn target(ectx: &EvalCtx, layers: &[(&Layer, Overlay)], cpt: [f64; 2], on_ar
     layers.iter().find(|(l, ov)| on_mesh(l, ov)).or_else(|| layers.iter().find(|(l, _)| on_art(l))).map(|(l, _)| l.id)
 }
 
+/// Whether a press at comp point `cpt` places a pin on `layer`. Where it would not (the deformed
+/// layer is empty there) the press is on nothing: a click does nothing, a drag is a marquee (#2865).
+pub fn takes_pin(app: &EffectcraftApp, ectx: &EvalCtx, layer: &Layer, cpt: [f64; 2]) -> bool {
+    let Some(inv) = fx2c(ectx, layer).inverse() else { return false };
+    let p = inv.apply(gv2(cpt[0], cpt[1]));
+    effectcraft_engine::commands::puppet::places_pin(&app.session, ectx.comp_id, layer.id, [p.x, p.y], ectx.time)
+}
+
 /// Keep a pin's keyframed Position in view in the Timeline after placing, moving or recording the
 /// pin in the viewer (#273).
 pub fn reveal_pin(app: &mut EffectcraftApp, layer: LayerId, pin: u64) {

crates/ui-egui/src/panels/viewer.rs

@@ -1092,7 +1092,9 @@ fn phase_dragstart(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx,
         if puppet_layers.is_empty() {
             return pick(app, ectx, cpt, false);
         }
-        super::puppet_tool::target(ectx, &puppet_layers, cpt, |l| layers_at(ectx, cpt).any(|h| h.id == l.id))
+        super::puppet_tool::target(ectx, &puppet_layers, cpt, |l| {
+            layers_at(ectx, cpt).any(|h| h.id == l.id) && super::puppet_tool::takes_pin(app, ectx, l, cpt)
+        })
     };
     if resp.drag_started()
         && !text_owns
@@ -1680,7 +1682,9 @@ fn phase_clicked(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response
         if puppet_layers.is_empty() {
             return pick(app, ectx, cpt, false);
         }
-        super::puppet_tool::target(ectx, &puppet_layers, cpt, |l| layers_at(ectx, cpt).any(|h| h.id == l.id))
+        super::puppet_tool::target(ectx, &puppet_layers, cpt, |l| {
+            layers_at(ectx, cpt).any(|h| h.id == l.id) && super::puppet_tool::takes_pin(app, ectx, l, cpt)
+        })
     };
     if resp.clicked()
         && !text_owns

crates/ui-egui/tests/it/ui_viewer.rs

@@ -1657,6 +1657,42 @@ fn puppet_pins_go_on_the_deformed_mesh_and_the_tool_stays_on_the_rigged_layer()
     assert!(!rigged(&h, plate));
 }
 
+/// #2865: a press where the deformed layer is empty, though on its original bounds, adds nothing:
+/// a click leaves the rig alone and a drag from there is a pin marquee.
+#[test]
+fn puppet_press_where_the_deformed_layer_is_empty_is_a_marquee_not_a_copy() {
+    let mut h = harness();
+    let s = &mut h.state_mut().session;
+    s.execute("edit.clear", json!({"layers": ["Plate"]})).unwrap();
+    s.execute("layer.select", json!({"layers": ["Box"]})).unwrap();
+    let id = s.state.selected_layers[0].0;
+    let a = s.execute("puppet.addPin", json!({"layer": id, "position": [10, 40]})).unwrap()["pin"].as_u64().unwrap();
+    let b = s.execute("puppet.addPin", json!({"layer": id, "position": [70, 40]})).unwrap()["pin"].as_u64().unwrap();
+    // Carry the Box 60 px right: its left part (comp x 280..340) is now empty.
+    s.execute("puppet.movePin", json!({"layer": id, "pin": a, "position": [70, 40]})).unwrap();
+    s.execute("puppet.movePin", json!({"layer": id, "pin": b, "position": [130, 40]})).unwrap();
+    h.state_mut().ui.tool = Tool::Puppet;
+    h.run_steps(3);
+    let rig = |h: &mut Harness<'_, EffectcraftApp>| {
+        let info = h.state_mut().session.execute("puppet.info", json!({"layer": id})).unwrap();
+        let meshes = info["meshes"].as_array().unwrap().clone();
+        (meshes.len(), meshes.iter().map(|m| m["pins"].as_array().unwrap().len()).sum::<usize>())
+    };
+    let at = screen(&h, [285.0, 180.0]);
+    click(&mut h, at);
+    assert_eq!(rig(&mut h), (1, 2), "the click made nothing");
+    let comp = rect(&h, "viewer.comp");
+    drag(&mut h, at, comp.max - vec2(4.0, 4.0));
+    assert_eq!(rig(&mut h), (1, 2), "nor did the drag");
+    let mut sel: Vec<u64> = h.state().session.state.selected_props.iter().map(|(_, u)| *u).collect();
+    sel.sort();
+    assert_eq!(sel, [a, b], "the drag's marquee selected the pins");
+    // On the deformed shape a click still pins it.
+    let at = screen(&h, [380.0, 180.0]);
+    click(&mut h, at);
+    assert_eq!(rig(&mut h), (1, 3));
+}
+
 /// Filled circles painted this frame (flattening nested shape lists).
 fn circles(h: &Harness<'_, EffectcraftApp>) -> Vec<egui::epaint::CircleShape> {
     fn walk(s: &egui::Shape, out: &mut Vec<egui::epaint::CircleShape>) {