Merge pull request #3460 from dotCooCoo/fix/live-paint-tools-skip-hidden-faces

Live Paint: the Live Paint tools and livePaint.fill skip hidden faces

crates/engine/src/cmd/buildcmds.rs

@@ -102,7 +102,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Live Paint Bucket",
             [],
             None,
-            "{group?: id | ids?|id?: [id…] (paths to make live first, as livePaint.make does), point: [x,y], color?|swatch?|gradient?|none? (default: current fill), tint?: 0..100, keepModel?: bool (both as paint.setFill)} fill the face containing point → {group, face}",
+            "{group?: id | ids?|id?: [id…] (paths to make live first, as livePaint.make does), point: [x,y], color?|swatch?|gradient?|none? (default: current fill), tint?: 0..100, keepModel?: bool (both as paint.setFill)} fill the face containing point, skipping hidden faces → {group, face}",
             has_doc,
             lp_fill
         ),
@@ -764,13 +764,24 @@ fn lp_fill(s: &mut Session, p: &Value) -> Result<Value> {
                 }
                 g
             }
-            None => match roots.iter().copied().find(|r| d.node(*r).is_some_and(|n| is_live_paint(n) && face_at(n, pt).is_some())) {
-                Some(g) => g,
-                None => make_live(d, sel, &roots)?.0,
-            },
+            None => {
+                let live_at = |face: fn(&Node, Point) -> Option<&Arc<Node>>| {
+                    roots.iter().copied().find(|r| d.node(*r).is_some_and(|n| is_live_paint(n) && face(n, pt).is_some()))
+                };
+                match (live_at(b::visible_face_at), live_at(face_at)) {
+                    (Some(g), _) => g,
+                    // At a point on a hidden face of a Live Paint group among the objects, the command returns an error
+                    // and does not merge the objects.
+                    (None, Some(_)) => return Err(EngineError::Other("no Live Paint face at that point".into())),
+                    (None, None) => make_live(d, sel, &roots)?.0,
+                }
+            }
         };
-        let fid =
-            d.node(gid).and_then(|n| face_at(n, pt)).map(|f| f.id).ok_or_else(|| EngineError::Other("no Live Paint face at that point".into()))?;
+        let fid = d
+            .node(gid)
+            .and_then(|n| b::visible_face_at(n, pt))
+            .map(|f| f.id)
+            .ok_or_else(|| EngineError::Other("no Live Paint face at that point".into()))?;
         d.node_mut(fid).ok_or(EngineError::NoNode(fid))?.appearance.set_fill(paint.clone());
         Ok((gid, fid))
     })?;

crates/engine/src/tests_build.rs

@@ -689,6 +689,32 @@ fn live_paint_stroke_edge_skips_hidden_edges() {
     assert!(s.execute("livePaint.strokeEdge", &json!({"group": g.0, "point": [99, 50]})).is_err());
 }
 
+#[test]
+fn live_paint_fill_skips_hidden_faces() {
+    let mut s = session();
+    let g = live(&mut s);
+    // The face from x=50 to x=100 is hidden on its own; the group stays visible.
+    let hidden = vectorcraft_tools::builder::face_at(&node(&s, g), vectorcraft_geom::Point::new(75.0, 50.0)).unwrap().id;
+    s.execute("object.setProps", &json!({"ids": [hidden.0], "visible": false})).unwrap();
+    let green = Paint::solid(Color::from_hex("#00ff00").unwrap());
+    let children = node(&s, g).children().unwrap().len();
+    // livePaint.fill at a point on the hidden face returns an error and fills no face, with `group`
+    // and with the group in `ids`, and it does not rebuild the group.
+    for p in [json!({"group": g.0, "point": [75, 50], "color": "#00ff00"}), json!({"ids": [g.0], "point": [75, 50], "color": "#00ff00"})] {
+        assert!(s.execute("livePaint.fill", &p).is_err(), "{p}");
+        let face = node(&s, hidden);
+        assert!(!face.visible && face.appearance.fill_paint() != green, "{p}");
+        assert_eq!(node(&s, g).children().unwrap().len(), children, "{p}");
+    }
+    // Given another Live Paint group with a face at that point as well, that face is filled.
+    let other = rect(&mut s, 60.0, 40.0, 20.0, 20.0);
+    let other = NodeId(s.execute("livePaint.make", &json!({"ids": [other.0]})).unwrap()["id"].as_u64().unwrap());
+    let r = s.execute("livePaint.fill", &json!({"ids": [g.0, other.0], "point": [75, 50], "color": "#00ff00"})).unwrap();
+    assert_eq!(r["group"], other.0);
+    assert_eq!(node(&s, NodeId(r["face"].as_u64().unwrap())).appearance.fill_paint(), green);
+    assert!(!node(&s, hidden).visible);
+}
+
 #[test]
 fn live_paint_release_and_expand() {
     let mut s = session();

crates/tools/src/builder.rs

@@ -72,7 +72,17 @@ pub fn path_contains(path: &PathData, rule: FillRule, p: Point) -> bool {
 
 /// The face of `group` under `p` (front-most).
 pub fn face_at(group: &Node, p: Point) -> Option<&Arc<Node>> {
-    faces(group).into_iter().rev().find(|f| f.path_data().is_some_and(|pd| path_contains(pd, FillRule::NonZero, p)))
+    front_face(faces(group).into_iter(), p)
+}
+
+/// The face of `group` under `p` (front-most) among its faces that are not hidden.
+pub fn visible_face_at(group: &Node, p: Point) -> Option<&Arc<Node>> {
+    front_face(faces(group).into_iter().filter(|f| f.visible), p)
+}
+
+/// The face of `faces` (paint order) under `p` (front-most).
+fn front_face<'a>(faces: impl DoubleEndedIterator<Item = &'a Arc<Node>>, p: Point) -> Option<&'a Arc<Node>> {
+    faces.rev().find(|f| f.path_data().is_some_and(|pd| path_contains(pd, FillRule::NonZero, p)))
 }
 
 /// The edge of `group` nearest to `p` within `tol`.
@@ -99,11 +109,11 @@ fn nearest_edge<'a>(edges: impl IntoIterator<Item = &'a Arc<Node>>, p: Point, to
     best.map(|b| b.1)
 }
 
-/// The visible Live Paint group with a face under `p` (front-most in paint order).
+/// The visible Live Paint group with a face under `p` that is not hidden (front-most in paint order).
 pub fn live_paint_at(doc: &Document, p: Point) -> Option<NodeId> {
     let mut hit = None;
     doc.walk(|n| {
-        if is_live_paint(n) && doc.is_visible(n.id) && face_at(n, p).is_some() {
+        if is_live_paint(n) && doc.is_visible(n.id) && visible_face_at(n, p).is_some() {
             hit = Some(n.id);
         }
     });
@@ -780,14 +790,14 @@ enum LpHover {
     Region(usize),
 }
 
-/// The Live Paint group and part the Live Paint tools hover and click at `p`. Inside a face of a
-/// visible Live Paint group ([`live_paint_at`]) it is that group with its face under `p`, or with
-/// `edge` its visible edge nearest `p` within `tol` (None when there is none). Outside every face,
-/// with `edge`, it is the visible edge nearest `p` within `tol` among the visible Live Paint groups
-/// (the front-most group's on a tie). None when no group is found.
+/// The Live Paint group and part the Live Paint tools hover and click at `p`. Inside a visible face
+/// of a visible Live Paint group ([`live_paint_at`]) it is that group with its visible face under
+/// `p`, or with `edge` its visible edge nearest `p` within `tol` (None when there is none). Outside
+/// every visible face, with `edge`, it is the visible edge nearest `p` within `tol` among the
+/// visible Live Paint groups (the front-most group's on a tie). None when no group is found.
 fn lp_part_at(doc: &Document, p: Point, tol: f64, edge: bool) -> Option<(NodeId, Option<&Arc<Node>>)> {
     if let Some(g) = live_paint_at(doc, p).and_then(|id| doc.node(id)) {
-        return Some((g.id, if edge { visible_edge_near(g, p, tol) } else { face_at(g, p) }));
+        return Some((g.id, if edge { visible_edge_near(g, p, tol) } else { visible_face_at(g, p) }));
     }
     if !edge {
         return None;
@@ -1417,6 +1427,49 @@ mod tests {
         }
     }
 
+    #[test]
+    fn live_paint_tools_skip_hidden_faces() {
+        let mut d = Document::new(500.0, 500.0);
+        let (back, [_, _, back_right, _]) = live_square(&mut d, Rect::new(0.0, 0.0, 100.0, 100.0));
+        let (front, _) = live_square(&mut d, Rect::new(50.0, 0.0, 150.0, 100.0));
+        let face = |g: NodeId| faces(d.node(g).unwrap())[0].id;
+        let (back_face, hidden) = (face(back), face(front));
+        // The front group's face is hidden on its own; both groups stay visible.
+        d.node_mut(hidden).unwrap().visible = false;
+        let s = Selection::default();
+        let p = paint();
+        let cx = cx(&d, &s, &p);
+        let alt = Mods { alt: true, ..Default::default() };
+        let part = |ov: &[Overlay]| match ov {
+            [] => None,
+            [Overlay::Path { path, .. }] => Some(path.bounding_box()),
+            _ => panic!("{ov:?}"),
+        };
+        // Where the faces overlap, both tools take the back group's face under the hidden one.
+        let mut bucket = LivePaintBucketTool::default();
+        bucket.pointer(&cx, &ev(PointerKind::Move, 75.0, 50.0));
+        assert_eq!(part(&bucket.overlays(&cx)), Some(Rect::new(0.0, 0.0, 100.0, 100.0)));
+        let fill = Action::Exec("livePaint.fill".into(), json!({ "group": back.0, "point": [75.0, 50.0] }));
+        assert_eq!(bucket.pointer(&cx, &ev(PointerKind::Down, 75.0, 50.0)), vec![fill]);
+        let mut sel = LivePaintSelectionTool::default();
+        sel.pointer(&cx, &ev(PointerKind::Move, 75.0, 50.0));
+        assert_eq!(part(&sel.overlays(&cx)), Some(Rect::new(0.0, 0.0, 100.0, 100.0)));
+        assert_eq!(sel.pointer(&cx, &ev(PointerKind::Down, 75.0, 50.0)), vec![Action::Exec("select.set".into(), json!({ "ids": [back_face.0] }))]);
+        // Over the hidden face alone, neither tool highlights or picks a face.
+        bucket.pointer(&cx, &ev(PointerKind::Move, 125.0, 50.0));
+        assert_eq!(part(&bucket.overlays(&cx)), None);
+        assert!(bucket.pointer(&cx, &ev(PointerKind::Down, 125.0, 50.0)).is_empty());
+        sel.pointer(&cx, &ev(PointerKind::Move, 125.0, 50.0));
+        assert_eq!(part(&sel.overlays(&cx)), None);
+        assert_eq!(sel.pointer(&cx, &ev(PointerKind::Down, 125.0, 50.0)), vec![Action::Exec("select.none".into(), json!({}))]);
+        // Inside the hidden face, 1.5 pt from the back group's right side, Alt-click takes that side
+        // as it does outside every face.
+        sel.pointer(&cx, &ev(PointerKind::Move, 101.5, 50.0).with_mods(alt));
+        assert_eq!(part(&sel.overlays(&cx)), Some(Rect::new(100.0, 0.0, 100.0, 100.0)));
+        let picked = sel.pointer(&cx, &ev(PointerKind::Down, 101.5, 50.0).with_mods(alt));
+        assert_eq!(picked, vec![Action::Exec("select.set".into(), json!({ "ids": [back_right.0] }))]);
+    }
+
     #[test]
     fn live_paint_selection_options_set_the_highlight() {
         let (d, ..) = live_group_doc();

docs/mcp.md

@@ -1349,6 +1349,9 @@ already walks Image Trace. Short agent entry points:
   `livePaint.merge` keeps the bottom Live Paint group's id, name, lock, visibility, transparency, appearance, opacity
   mask and graphic style link. With a Live Paint group among the objects, `livePaint.make` adds the others to it as
   `livePaint.merge` does, and so does `livePaint.fill {ids}` when `point` is outside that group's faces.
+  `livePaint.fill` does not fill a face that is hidden on its own. With `group`, a point on a hidden face is an
+  error. With `ids`, at a point on a hidden face it fills the face there of another Live Paint group among the
+  objects; otherwise it returns an error and does not merge the objects.
 
 `brush.selectionArt {ids?|id?}` (a query) replies `{art}`: the object that `brush.new` given the same `ids` or `id`,
 or the same selection, takes as a new scatter or art brush's art and as a new pattern brush's side tile. Several