Code review

libraries/bezier-rs/src/subpath/solvers.rs

@@ -237,34 +237,43 @@ impl<PointId: crate::Identifier> Subpath<PointId> {
 		false
 	}
 
-	/// Returns `true` if this subpath is completely inside the other subpath.
+	/// Returns `true` if this subpath is completely inside the `other` subpath.
 	pub fn is_inside_subpath(&self, other: &Subpath<PointId>, error: Option<f64>, minimum_separation: Option<f64>) -> bool {
-		// Eliminate this subpath if its bounding box is not completely inside other subpath bounding box
-		if !self.is_empty() && !other.is_empty() {
-			let inner_bbox = self.bounding_box().unwrap();
-			let outer_bbox = other.bounding_box().unwrap();
-			// Reasoning:
-			// (min x, min y) of inner subpath is less or equal to the outer (min x, min y) or
-			// (min x, min y) of inner subpath is more or equal to outer (max x, mix y) then the inner is intersecting or is outside the outer. (same will be true for (max x, max y))
-			if !is_rectangle_inside_other(inner_bbox, outer_bbox) {
-				return false;
-			}
-		};
+		// Eliminate any possibility of one being inside the other, if either of them is empty
+		if self.is_empty() || other.is_empty() {
+			return false;
+		}
+
+		// Safe to unwrap because the subpath is not empty
+		let inner_bbox = self.bounding_box().unwrap();
+		let outer_bbox = other.bounding_box().unwrap();
+
+		// Eliminate this subpath if its bounding box is not completely inside the other subpath's bounding box.
+		// Reasoning:
+		// If the (min x, min y) of the inner subpath is less than or equal to the (min x, min y) of the outer subpath,
+		// or if the (min x, min y) of the inner subpath is greater than or equal to the (max x, max y) of the outer subpath,
+		// then the inner subpath is intersecting with or outside the outer subpath. The same logic applies for (max x, max y).
+		if !is_rectangle_inside_other(inner_bbox, outer_bbox) {
+			return false;
+		}
 
-		// Eliminate this if any of its the subpath's anchors is outside other's subpath
+		// Eliminate this subpath if any of its anchors are outside the other subpath.
 		for anchors in self.anchors() {
 			if !other.contains_point(anchors) {
 				return false;
 			}
 		}
 
-		// Eliminate this if its subpath is intersecting wih the other's subpath
-		if !self.subpath_intersections(&other, error, minimum_separation).is_empty() {
+		// Eliminate this subpath if it intersects with the other subpath.
+		if !self.subpath_intersections(other, error, minimum_separation).is_empty() {
 			return false;
 		}
 
-		// here (1) this subpath bbox is inside other bbox, (2) its anchors are inside other subpath and (3) it is not intersecting with other subpath.
-		// hence this this subpath is completely inside given other subpath.
+		// At this point:
+		// (1) This subpath's bounding box is inside the other subpath's bounding box,
+		// (2) Its anchors are inside the other subpath, and
+		// (3) It is not intersecting with the other subpath.
+		// Hence, this subpath is completely inside the given other subpath.
 		true
 	}
 
@@ -298,7 +307,7 @@ impl<PointId: crate::Identifier> Subpath<PointId> {
 		})
 	}
 
-	/// Return the min and max corners that represent the bounding box of the subpath.
+	/// Return the min and max corners that represent the bounding box of the subpath. Return `None` if the subpath is empty.
 	/// <iframe frameBorder="0" width="100%" height="300px" src="https://graphite.rs/libraries/bezier-rs#subpath/bounding-box/solo" title="Bounding Box Demo"></iframe>
 	pub fn bounding_box(&self) -> Option<[DVec2; 2]> {
 		self.iter().map(|bezier| bezier.bounding_box()).reduce(|bbox1, bbox2| [bbox1[0].min(bbox2[0]), bbox1[1].max(bbox2[1])])
@@ -909,24 +918,24 @@ mod tests {
 
 	#[test]
 	fn is_inside_subpath() {
-		let lasso_polygon = [DVec2::new(100., 100.), DVec2::new(500., 100.), DVec2::new(500., 500.), DVec2::new(100., 500.)].to_vec();
-		let lasso_polygon = Subpath::from_anchors_linear(lasso_polygon, true);
+		let boundary_polygon = [DVec2::new(100., 100.), DVec2::new(500., 100.), DVec2::new(500., 500.), DVec2::new(100., 500.)].to_vec();
+		let boundary_polygon = Subpath::from_anchors_linear(boundary_polygon, true);
 
 		let curve = Bezier::from_quadratic_dvec2(DVec2::new(189., 289.), DVec2::new(9., 286.), DVec2::new(45., 410.));
 		let curve_intersecting = Subpath::<EmptyId>::from_bezier(&curve);
-		assert_eq!(curve_intersecting.is_inside_subpath(&lasso_polygon, None, None), false);
+		assert_eq!(curve_intersecting.is_inside_subpath(&boundary_polygon, None, None), false);
 
 		let curve = Bezier::from_quadratic_dvec2(DVec2::new(115., 37.), DVec2::new(51.4, 91.8), DVec2::new(76.5, 242.));
 		let curve_outside = Subpath::<EmptyId>::from_bezier(&curve);
-		assert_eq!(curve_outside.is_inside_subpath(&lasso_polygon, None, None), false);
+		assert_eq!(curve_outside.is_inside_subpath(&boundary_polygon, None, None), false);
 
 		let curve = Bezier::from_cubic_dvec2(DVec2::new(210.1, 133.5), DVec2::new(150.2, 436.9), DVec2::new(436., 285.), DVec2::new(247.6, 240.7));
 		let curve_inside = Subpath::<EmptyId>::from_bezier(&curve);
-		assert_eq!(curve_inside.is_inside_subpath(&lasso_polygon, None, None), true);
+		assert_eq!(curve_inside.is_inside_subpath(&boundary_polygon, None, None), true);
 
 		let line = Bezier::from_linear_dvec2(DVec2::new(101., 101.5), DVec2::new(150.2, 499.));
 		let line_inside = Subpath::<EmptyId>::from_bezier(&line);
-		assert_eq!(line_inside.is_inside_subpath(&lasso_polygon, None, None), true);
+		assert_eq!(line_inside.is_inside_subpath(&boundary_polygon, None, None), true);
 	}
 
 	#[test]

libraries/bezier-rs/src/utils.rs

@@ -171,21 +171,21 @@ pub fn solve_cubic(a: f64, b: f64, c: f64, d: f64) -> [Option<f64>; 3] {
 	}
 }
 
-/// Check if two rectangles have any overlap. The rectangles are represented by a pair of coordinates that designate the top left and bottom right corners (in a graphical coordinate system).
+/// Determines if two rectangles have any overlap. The rectangles are represented by a pair of coordinates that designate the top left and bottom right corners (in a graphical coordinate system).
 pub fn do_rectangles_overlap(rectangle1: [DVec2; 2], rectangle2: [DVec2; 2]) -> bool {
 	let [bottom_left1, top_right1] = rectangle1;
 	let [bottom_left2, top_right2] = rectangle2;
 
 	top_right1.x >= bottom_left2.x && top_right2.x >= bottom_left1.x && top_right2.y >= bottom_left1.y && top_right1.y >= bottom_left2.y
 }
 
-/// Check if a point is completely inside rectangle which is respresented as pair of coordinates [top-left, bottom-right].
+/// Determines if a point is completely inside a rectangle, which is represented as a pair of coordinates [top-left, bottom-right].
 pub fn is_point_inside_rectangle(rect: [DVec2; 2], point: DVec2) -> bool {
-	let [top_left, bottom_rigth] = rect;
-	point.x > top_left.x && point.x < bottom_rigth.x && point.y > top_left.y && point.y < bottom_rigth.y
+	let [top_left, bottom_right] = rect;
+	point.x > top_left.x && point.x < bottom_right.x && point.y > top_left.y && point.y < bottom_right.y
 }
 
-/// Check if inner rectangle is completely inside outer rectangle. The rectangles are represented as pair of coordinates [top-left, bottom-right].
+/// Determines if the inner rectangle is completely inside the outer rectangle. The rectangles are represented as pairs of coordinates [top-left, bottom-right].
 pub fn is_rectangle_inside_other(inner: [DVec2; 2], outer: [DVec2; 2]) -> bool {
 	is_point_inside_rectangle(outer, inner[0]) && is_point_inside_rectangle(outer, inner[1])
 }