Improve click targets to use compound paths so interior negative space is excluded (#4096)

* Add ClickTargetType::CompoundPath variant for fill-rule-aware compound shape hit testing

* Generate one compound click target per Vector so glyph holes aren't treated as filled

* Fix insidenss logic

editor/src/messages/portfolio/document/document_message_handler.rs

@@ -1636,6 +1636,11 @@ impl DocumentMessageHandler {
 					subpath.apply_transform(layer_transform);
 					subpath.is_inside_subpath(&viewport_polygon, None, None)
 				}
+				ClickTargetType::CompoundPath(subpaths) => subpaths.iter().all(|subpath| {
+					let mut subpath = subpath.clone();
+					subpath.apply_transform(layer_transform);
+					subpath.is_inside_subpath(&viewport_polygon, None, None)
+				}),
 				ClickTargetType::FreePoint(point) => {
 					let mut point = *point;
 					point.apply_transform(layer_transform);

editor/src/messages/portfolio/document/overlays/utility_types_native.rs

@@ -1035,6 +1035,7 @@ impl OverlayContextInternal {
 					self.manipulator_anchor(transform.transform_point2(point.position), false, None);
 				}
 				ClickTargetType::Subpath(subpath) => subpaths.push(subpath.clone()),
+				ClickTargetType::CompoundPath(compound) => subpaths.extend(compound.iter().cloned()),
 			}
 		}
 

editor/src/messages/portfolio/document/overlays/utility_types_web.rs

@@ -986,6 +986,7 @@ impl OverlayContext {
 				self.manipulator_anchor(transform.transform_point2(point.position), false, None);
 			}
 			ClickTargetType::Subpath(subpath) => subpaths.push(subpath.clone()),
+			ClickTargetType::CompoundPath(compound) => subpaths.extend(compound.iter().cloned()),
 		});
 
 		if !subpaths.is_empty() {

editor/src/messages/portfolio/document/utility_types/document_metadata.rs

@@ -177,6 +177,10 @@ impl DocumentMetadata {
 			.iter()
 			.filter_map(|click_target| match click_target.target_type() {
 				ClickTargetType::Subpath(subpath) => subpath.loose_bounding_box_with_transform(transform),
+				ClickTargetType::CompoundPath(subpaths) => subpaths
+					.iter()
+					.filter_map(|subpath| subpath.loose_bounding_box_with_transform(transform))
+					.reduce(|[a_min, a_max], [b_min, b_max]| [a_min.min(b_min), a_max.max(b_max)]),
 				ClickTargetType::FreePoint(_) => click_target.bounding_box_with_transform(transform),
 			})
 			.reduce(Quad::combine_bounds)
@@ -219,9 +223,10 @@ impl DocumentMetadata {
 	}
 
 	pub fn layer_outline(&self, layer: LayerNodeIdentifier) -> impl Iterator<Item = &subpath::Subpath<PointId>> {
-		self.visual_targets(layer).unwrap_or(&[]).iter().filter_map(|target| match target.target_type() {
-			ClickTargetType::Subpath(subpath) => Some(subpath),
-			_ => None,
+		self.visual_targets(layer).unwrap_or(&[]).iter().flat_map(|target| match target.target_type() {
+			ClickTargetType::Subpath(subpath) => std::slice::from_ref(subpath),
+			ClickTargetType::CompoundPath(subpaths) => subpaths.as_slice(),
+			ClickTargetType::FreePoint(_) => &[],
 		})
 	}
 

node-graph/libraries/rendering/src/renderer.rs

@@ -1465,23 +1465,33 @@ impl Render for Table<Vector> {
 	}
 }
 
-/// Build click targets (subpaths and free-floating anchors) from a `Vector`, apply the transform, and append to `targets`.
+/// Build one `CompoundPath` (non-zero fill rule, so holes like the inside of an "O" work
+/// correctly) plus one `FreePoint` per disconnected anchor, apply the transform, and append.
 fn extend_targets_from_vector(targets: &mut Vec<ClickTarget>, vector: &Vector, transform: DAffine2) {
 	let stroke_width = vector.style.stroke().as_ref().map_or(0., Stroke::effective_width);
 	let filled = vector.style.fill() != &Fill::None;
-	let fill = |mut subpath: Subpath<_>| {
+	let subpaths: Vec<Subpath<_>> = vector
+		.stroke_bezier_paths()
+		.map(|mut subpath| {
 			if filled {
 				subpath.set_closed(true);
 			}
 			subpath
-	};
-	targets.extend(vector.stroke_bezier_paths().map(fill).map(|subpath| {
-		let mut click_target = ClickTarget::new_with_subpath(subpath, stroke_width);
+		})
+		.collect();
+	if !subpaths.is_empty() {
+		let mut click_target = ClickTarget::new_with_compound_path(subpaths, stroke_width);
 		click_target.apply_transform(transform);
-		click_target
-	}));
+		targets.push(click_target);
+	}
 
-	let single_anchors = vector.point_domain.ids().iter().filter_map(|&point_id| {
+	for click_target in extend_free_point_targets(vector, transform) {
+		targets.push(click_target);
+	}
+}
+
+fn extend_free_point_targets(vector: &Vector, transform: DAffine2) -> impl Iterator<Item = ClickTarget> + '_ {
+	vector.point_domain.ids().iter().filter_map(move |&point_id| {
 		if vector.any_connected(point_id) {
 			return None;
 		}
@@ -1490,8 +1500,7 @@ fn extend_targets_from_vector(targets: &mut Vec<ClickTarget>, vector: &Vector, t
 		let mut click_target = ClickTarget::new_with_free_point(FreePoint::new(point_id, anchor));
 		click_target.apply_transform(transform);
 		Some(click_target)
-	});
-	targets.extend(single_anchors);
+	})
 }
 
 impl Render for Table<Raster<CPU>> {

node-graph/libraries/vector-types/src/vector/click_target.rs

@@ -35,6 +35,9 @@ impl FreePoint {
 pub enum ClickTargetType {
 	Subpath(Subpath<PointId>),
 	FreePoint(FreePoint),
+	/// Multiple subpaths tested as one compound shape using the non-zero fill rule, so holes
+	/// (e.g. the inside of an "O") correctly count as outside the fill.
+	CompoundPath(Vec<Subpath<PointId>>),
 }
 
 /// Fixed-size ring buffer cache for rotated bounding boxes.
@@ -144,6 +147,19 @@ impl ClickTarget {
 		}
 	}
 
+	pub fn new_with_compound_path(subpaths: Vec<Subpath<PointId>>, stroke_width: f64) -> Self {
+		let bounding_box = subpaths
+			.iter()
+			.filter_map(|subpath| subpath.loose_bounding_box())
+			.reduce(|[a_min, a_max], [b_min, b_max]| [a_min.min(b_min), a_max.max(b_max)]);
+		Self {
+			target_type: ClickTargetType::CompoundPath(subpaths),
+			stroke_width,
+			bounding_box,
+			bounding_box_cache: Default::default(),
+		}
+	}
+
 	pub fn new_with_free_point(point: FreePoint) -> Self {
 		const MAX_LENGTH_FOR_NO_WIDTH_OR_HEIGHT: f64 = 1e-4 / 2.;
 		let stroke_width = 10.;
@@ -199,6 +215,10 @@ impl ClickTarget {
 				let mut write_lock = self.bounding_box_cache.write().unwrap();
 				write_lock.add_to_cache(subpath, rotation, scale, translation, fingerprint)
 			}
+			ClickTargetType::CompoundPath(ref subpaths) => subpaths
+				.iter()
+				.filter_map(|subpath| subpath.bounding_box_with_transform(transform))
+				.reduce(|[a_min, a_max], [b_min, b_max]| [a_min.min(b_min), a_max.max(b_max)]),
 			// TODO: use point for calculation of bbox
 			ClickTargetType::FreePoint(_) => self.bounding_box.map(|[a, b]| [transform.transform_point2(a), transform.transform_point2(b)]),
 		}
@@ -209,6 +229,11 @@ impl ClickTarget {
 			ClickTargetType::Subpath(ref mut subpath) => {
 				subpath.apply_transform(affine_transform);
 			}
+			ClickTargetType::CompoundPath(ref mut subpaths) => {
+				for subpath in subpaths {
+					subpath.apply_transform(affine_transform);
+				}
+			}
 			ClickTargetType::FreePoint(ref mut point) => {
 				point.apply_transform(affine_transform);
 			}
@@ -221,6 +246,12 @@ impl ClickTarget {
 			ClickTargetType::Subpath(ref subpath) => {
 				self.bounding_box = subpath.bounding_box();
 			}
+			ClickTargetType::CompoundPath(ref subpaths) => {
+				self.bounding_box = subpaths
+					.iter()
+					.filter_map(|subpath| subpath.bounding_box())
+					.reduce(|[a_min, a_max], [b_min, b_max]| [a_min.min(b_min), a_max.max(b_max)]);
+			}
 			ClickTargetType::FreePoint(ref point) => {
 				self.bounding_box = Some([point.position - DVec2::splat(self.stroke_width / 2.), point.position + DVec2::splat(self.stroke_width / 2.)]);
 			}
@@ -256,6 +287,24 @@ impl ClickTarget {
 				// Check if shape is entirely within selection
 				bezpath_is_inside_bezpath(&subpath.to_bezpath(), &selection, None, None)
 			}
+			ClickTargetType::CompoundPath(subpaths) => {
+				// Outline intersection (catches strokes and both filled/unfilled shapes)
+				let outline_intersects = |path_segment: PathSeg| bezier_iter().any(|line| !filtered_segment_intersections(path_segment, line, None, None).is_empty());
+				if subpaths.iter().flat_map(|subpath| subpath.iter()).any(outline_intersects) {
+					return true;
+				}
+
+				// Selection point inside compound fill (non-zero rule)
+				let combined: BezPath = subpaths.iter().flat_map(|subpath| subpath.to_bezpath()).collect();
+				if bezier_iter().next().is_some_and(|bezier| combined.contains(bezier.start())) {
+					return true;
+				}
+
+				// Build closed selection path, then check if all contours are entirely within it
+				let mut selection = BezPath::from_path_segments(bezier_iter());
+				selection.close_path();
+				subpaths.iter().all(|subpath| bezpath_is_inside_bezpath(&subpath.to_bezpath(), &selection, None, None))
+			}
 			ClickTargetType::FreePoint(point) => bezier_iter().map(|bezier: PathSeg| bezier.winding(dvec2_to_point(point.position))).sum::<i32>() != 0,
 		}
 	}
@@ -288,6 +337,10 @@ impl ClickTarget {
 			// Check if the point is within the shape
 			match self.target_type() {
 				ClickTargetType::Subpath(subpath) => subpath.closed() && subpath.contains_point(point),
+				ClickTargetType::CompoundPath(subpaths) => {
+					let combined: BezPath = subpaths.iter().flat_map(|subpath| subpath.to_bezpath()).collect();
+					combined.contains(dvec2_to_point(point))
+				}
 				ClickTargetType::FreePoint(free_point) => free_point.position == point,
 			}
 		} else {

node-graph/libraries/vector-types/src/vector/vector_types.rs

@@ -158,6 +158,11 @@ impl Vector {
 			match target_type.borrow() {
 				ClickTargetType::Subpath(subpath) => vector.append_subpath(subpath, preserve_id),
 				ClickTargetType::FreePoint(point) => vector.append_free_point(point, preserve_id),
+				ClickTargetType::CompoundPath(subpaths) => {
+					for subpath in subpaths {
+						vector.append_subpath(subpath, preserve_id);
+					}
+				}
 			}
 		}