Merge pull request #7851 from dotCooCoo/fix/intertwine-hit-walk-limit

Selection: an Intertwine group's members in symbol art count toward a click's object limit

crates/doc/src/hit.rs

@@ -1121,10 +1121,12 @@ fn on_type_path(n: &Node, t: &crate::TextObject, p: Point, tol: f64) -> bool {
     stroke_contains(&path.to_bezpath(), 0.0, tol, p)
 }
 
-/// Hit anywhere in an evaluated subtree (groups recurse; leaves use [`hit_leaf`]).
+/// Hit anywhere in an evaluated subtree (groups recurse; leaves use [`hit_leaf`]). In symbol art
+/// placed by [`FindPart`], each object visited takes one object of the [`ArtWalk`] limit, and
+/// nothing more is tested once the limit is used up.
 fn hit_any(n: &Node, p: Point, opt: HitOptions, symbols: &SymbolBoxes) -> Option<HitKind> {
     // Hidden art does not hit.
-    if !n.visible {
+    if !n.visible || ArtWalk::charged() && !ArtWalk::take(1) {
         return None;
     }
     // Nor art whose reach does not hold `p`, as in [`may_hit`]: a blend's steps far from `p` are
@@ -1257,8 +1259,12 @@ struct FindPart<'a> {
 }
 
 impl FindPart<'_> {
+    /// What a click at `p` hits on object `n` of the art, placed. [`hit_any`] takes `n` and each
+    /// object it visits inside `n` from the [`ArtWalk`] limit.
     fn hit(&self, n: &Node, whole: Option<vectorcraft_geom::Affine>, opt: HitOptions) -> Option<HitKind> {
-        self.art.place(n, whole).and_then(|n| hit_any(&n, self.p, opt, self.symbols))
+        let placed = self.art.place(n, whole)?;
+        let _charged = ArtWalk::charge();
+        hit_any(&placed, self.p, opt, self.symbols)
     }
 
     fn skips_guide(&self, n: &Node) -> bool {
@@ -1268,7 +1274,7 @@ impl FindPart<'_> {
     /// What a click at `p` hits on object `n` of the art, with `path` the child indices that lead
     /// to what it hits. `outline`: the view is in Outline, or a layer around `n` has Preview off.
     fn find(&self, n: &Node, path: &mut Vec<u32>, whole: Option<vectorcraft_geom::Affine>, outline: bool) -> Option<HitKind> {
-        if !n.visible || !self.whole_instance && (n.locked || n.is_template()) || self.skips_guide(n) || !ArtWalk::take(1) {
+        if !n.visible || !self.whole_instance && (n.locked || n.is_template()) || self.skips_guide(n) || ArtWalk::used_up() {
             return None;
         }
         let outline = outline || previews_off(n);
@@ -1277,6 +1283,9 @@ impl FindPart<'_> {
         if path.len() >= crate::symbol_overrides::MAX_PATH_DEPTH {
             return self.hit(n, whole, opt);
         }
+        if !ArtWalk::take(1) {
+            return None;
+        }
         let whole = self.art.whole(n, whole);
         if n.clips()
             && let Some(first) = children.first()
@@ -1370,6 +1379,8 @@ thread_local! {
     /// How many more objects of symbol art the click or marquee under way may visit ([`ArtWalk`]),
     /// `None` outside one.
     static ART_WALK_LEFT: std::cell::Cell<Option<u64>> = const { std::cell::Cell::new(None) };
+    /// Whether [`hit_any`] is testing symbol art that [`FindPart`] placed ([`ArtWalk::charge`]).
+    static ART_WALK_CHARGED: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
 }
 
 #[cfg(test)]
@@ -1420,6 +1431,19 @@ impl ArtWalk {
         ART_WALK_LEFT.set(Some(rest.unwrap_or(0)));
         rest.is_some()
     }
+
+    /// While the guard lives, each object [`hit_any`] visits takes one object from what is left:
+    /// [`FindPart`] holds one while it tests a placed object of symbol art, so that the members of an
+    /// Intertwine group, and the objects of groups nested deeper than
+    /// [`crate::symbol_overrides::MAX_PATH_DEPTH`], count toward the limit.
+    fn charge() -> ArtWalkCharge {
+        ArtWalkCharge { was: ART_WALK_CHARGED.replace(true) }
+    }
+
+    /// Does [`hit_any`] take the objects it visits from what is left ([`Self::charge`])?
+    fn charged() -> bool {
+        ART_WALK_CHARGED.get()
+    }
 }
 
 impl Drop for ArtWalk {
@@ -1430,6 +1454,17 @@ impl Drop for ArtWalk {
     }
 }
 
+/// The guard of [`ArtWalk::charge`].
+struct ArtWalkCharge {
+    was: bool,
+}
+
+impl Drop for ArtWalkCharge {
+    fn drop(&mut self) {
+        ART_WALK_CHARGED.set(self.was);
+    }
+}
+
 /// Is `n` below a layer whose Preview is off, so that the canvas draws it in outline? As for clicks
 /// ([`path_options`]), the Preview of the isolated container `scope` and of the layers around it
 /// does not count.

crates/doc/src/hit/tests.rs

@@ -2140,6 +2140,37 @@ fn a_click_and_a_marquee_walk_repeated_nested_symbol_instances_within_one_limit(
     assert_eq!(with_art_limit(100, || marquee(&d, edge, None, false)).0, [inst], "marquee across the art's edge");
 }
 
+/// The members of an Intertwine group in symbol art count toward the art walk's limit of objects
+/// for a click, as the members of a plain group do. A click on a member's path past the instance's
+/// box is still found within the limit.
+#[test]
+fn a_click_counts_the_members_of_an_intertwine_group_in_symbol_art_toward_one_limit() {
+    set_clip_region(moved_clip_region);
+    // A clip group whose clipping region `test.move` moves to 100..200 × 0..100, past the 0..100
+    // box, over an Intertwine group of 200 squares at 140..160 × 40..60.
+    let mut d = Document::new(600.0, 400.0);
+    let l = d.layers[0].id;
+    let (mut mask, _) = half_clipped(&mut d);
+    mask.appearance.effects.push(test_move());
+    let squares = (0..200)
+        .map(|_| Arc::new(Node::path(d.alloc_id(), shapes::rectangle(Rect::new(140.0, 40.0, 160.0, 60.0)), Appearance::default_art())))
+        .collect();
+    let woven = Arc::new(Node::new(d.alloc_id(), NodeKind::Intertwine { children: squares }));
+    let mut clip = Node::group(d.alloc_id(), vec![Arc::new(mask), woven]);
+    clip.set_clips(true);
+    group_symbol(&mut d, "Woven", vec![Arc::new(clip)]);
+    let inst = instance_of(&mut d, "Woven");
+    let id = inst.id;
+    d.insert(Some(l), 0, Arc::unwrap_or_clone(inst)).unwrap();
+    let opt = HitOptions { path_only: true, ..HitOptions::default() };
+    let at = |x: f64| hit_test(&d, Point::new(x, 50.0), opt).map(|h| h.leaf);
+    // Inside the squares and away from their paths, every member is tested and none is hit.
+    let (hit, leaves, _) = with_art_limit(100, || at(150.0));
+    assert_eq!(hit, None);
+    assert!(leaves <= 100, "{leaves} leaves tested");
+    assert_eq!(with_art_limit(100, || at(140.0)).0, Some(id), "on the squares' paths");
+}
+
 /// In a dynamic symbol's art that placing through its 9-slice guides could make more than
 /// [`crate::MAX_SYMBOL_NODES`] objects of, Direct Selection picks and outlines nothing, as the
 /// canvas draws nothing of it. At the art's natural size the same art is picked.