Fix a derivative causing NaNs in boolean ops (#934)

* Fix a derivative causing NaNs

* Better error messages around boolean ops

* Tweak error dialog wording

Co-authored-by: Keavon Chambers <keavon@keavon.com>

document-legacy/src/boolean_ops.rs

@@ -18,7 +18,7 @@ pub enum BooleanOperation {
 	SubtractBack,
 }
 
-#[derive(Debug, Clone, Copy, Deserialize, Serialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)]
 pub enum BooleanOperationError {
 	InvalidSelection,
 	InvalidIntersections,

document-legacy/src/error.rs

@@ -14,11 +14,11 @@ pub enum DocumentError {
 	NotAnImage,
 	NotAnImaginate,
 	InvalidFile(String),
+	BooleanOperationError(BooleanOperationError),
 }
 
-// TODO: change how BooleanOperationErrors are handled
 impl From<BooleanOperationError> for DocumentError {
 	fn from(err: BooleanOperationError) -> Self {
-		DocumentError::InvalidFile(format!("{:?}", err))
+		DocumentError::BooleanOperationError(err)
 	}
 }

document-legacy/src/intersection.rs

@@ -454,7 +454,10 @@ pub fn extend_curve(curve: &mut PathSeg, distance: f64) {
 		let mut c_prime = c.deriv().eval(0.0);
 		c_prime.x *= d / c_prime.distance(Point::ORIGIN);
 		c_prime.y *= d / c_prime.distance(Point::ORIGIN);
-		let es_vec = c.eval(0.0) - c_prime;
+
+		// Don't apply a subtraction if c_prime is not finite (to prevent NaNs)
+		let es_vec = if c_prime.is_finite() { c.eval(0.0) - c_prime } else { c.eval(0.0).to_vec2() };
+
 		Point { x: es_vec.x, y: es_vec.y }
 	}
 	match curve {
@@ -849,6 +852,27 @@ pub fn valid_t(t: f64) -> bool {
 	t > -F64PRECISE && t < (1.0 - F64PRECISE)
 }
 
+// Tests that a very simple line curve intersection computes two intersections
+#[test]
+fn line_curve_intersection() {
+	let mut alpha = BezPath::new();
+
+	alpha.move_to((10., 10.));
+	alpha.curve_to((10., 10.), (30., 10.), (30., 10.));
+	alpha.curve_to((30., 10.), (30., 30.), (30., 30.));
+	alpha.curve_to((30., 30.), (10., 30.), (10., 30.));
+	alpha.curve_to((10., 30.), (10., 10.), (10., 10.));
+	alpha.close_path();
+	let mut beta = BezPath::new();
+	beta.move_to((0., 0.));
+	beta.line_to((20., 0.));
+	beta.line_to((20., 20.));
+	beta.line_to((0., 20.));
+	beta.close_path();
+
+	assert_eq!(intersections(&alpha, &beta).len(), 2);
+}
+
 /// Each of these tests have been visually, but not mathematically, verified.
 /// These tests are all ignored because each test looks for exact floating point comparisons, so isn't tolerant to small adjustments in the algorithm.
 mod tests {

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

@@ -21,6 +21,7 @@ use crate::messages::portfolio::document::utility_types::vectorize_layer_metadat
 use crate::messages::portfolio::utility_types::PersistentData;
 use crate::messages::prelude::*;
 
+use document_legacy::boolean_ops::BooleanOperationError;
 use document_legacy::color::Color;
 use document_legacy::document::Document as DocumentLegacy;
 use document_legacy::layers::blend_mode::BlendMode;
@@ -141,6 +142,14 @@ impl MessageHandler<DocumentMessage, (u64, &InputPreprocessorMessageHandler, &Pe
 						responses.push_back(BroadcastEvent::DocumentIsDirty.into());
 					}
 				}
+				// Display boolean operation error to the user (except if it is a nothing done error).
+				Err(DocumentError::BooleanOperationError(boolean_operation_error)) if boolean_operation_error != BooleanOperationError::NothingDone => responses.push_back(
+					DialogMessage::DisplayDialogError {
+						title: "Failed to calculate boolean operation".into(),
+						description: format!("Unfortunately, this feature not that robust yet.\n\nError: {boolean_operation_error:?}"),
+					}
+					.into(),
+				),
 				Err(e) => error!("DocumentError: {:?}", e),
 				Ok(_) => (),
 			},
@@ -236,6 +245,7 @@ impl MessageHandler<DocumentMessage, (u64, &InputPreprocessorMessageHandler, &Pe
 			BooleanOperation(op) => {
 				// Convert Vec<&[LayerId]> to Vec<Vec<&LayerId>> because Vec<&[LayerId]> does not implement several traits (Debug, Serialize, Deserialize, ...) required by DocumentOperation enum
 				responses.push_back(StartTransaction.into());
+				responses.push_back(BroadcastEvent::ToolAbort.into());
 				responses.push_back(
 					DocumentOperation::BooleanOperation {
 						operation: op,