Fix shape not closed after using Fill Tool (#510)

* Always set shape property 'closed' on fill

* Remove closed property on Shape

* Make color mandatory in Fill

* Fix intersection for filled but open shapes

* Code style tweak

* Add TODO note to rework ClosePath check

graphene/src/intersection.rs

@@ -47,18 +47,24 @@ fn to_point(vec: DVec2) -> Point {
 	Point::new(vec.x, vec.y)
 }
 
-pub fn intersect_quad_bez_path(quad: Quad, shape: &BezPath, closed: bool) -> bool {
+pub fn intersect_quad_bez_path(quad: Quad, shape: &BezPath, filled: bool) -> bool {
+	let mut shape = shape.clone();
+	// for filled shapes act like shape was closed even if it isn't
+	if filled && shape.elements().last() != Some(&kurbo::PathEl::ClosePath) {
+		shape.close_path();
+	}
+
 	// check if outlines intersect
 	if shape.segments().any(|path_segment| quad.lines().iter().any(|line| !path_segment.intersect_line(*line).is_empty())) {
 		return true;
 	}
 	// check if selection is entirely within the shape
-	if closed && shape.contains(to_point(quad.0[0])) {
+	if filled && shape.contains(to_point(quad.0[0])) {
 		return true;
 	}
 
 	// check if shape is entirely within selection
-	get_arbitrary_point_on_path(shape).map(|shape_point| quad.path().contains(shape_point)).unwrap_or_default()
+	get_arbitrary_point_on_path(&shape).map(|shape_point| quad.path().contains(shape_point)).unwrap_or_default()
 }
 
 pub fn get_arbitrary_point_on_path(path: &BezPath) -> Option<Point> {

graphene/src/layers/simple_shape.rs

@@ -17,7 +17,6 @@ pub struct Shape {
 	pub path: BezPath,
 	pub style: style::PathStyle,
 	pub render_index: i32,
-	pub closed: bool,
 }
 
 impl LayerData for Shape {
@@ -54,7 +53,7 @@ impl LayerData for Shape {
 	}
 
 	fn intersects_quad(&self, quad: Quad, path: &mut Vec<LayerId>, intersections: &mut Vec<Vec<LayerId>>) {
-		if intersect_quad_bez_path(quad, &self.path, self.closed) {
+		if intersect_quad_bez_path(quad, &self.path, self.style.fill().is_some()) {
 			intersections.push(path.clone());
 		}
 	}
@@ -75,7 +74,6 @@ impl Shape {
 			path: bez_path,
 			style,
 			render_index: 1,
-			closed,
 		}
 	}
 
@@ -104,12 +102,7 @@ impl Shape {
 
 		path.close_path();
 
-		Self {
-			path,
-			style,
-			render_index: 1,
-			closed: true,
-		}
+		Self { path, style, render_index: 1 }
 	}
 
 	pub fn rectangle(style: PathStyle) -> Self {
@@ -117,7 +110,6 @@ impl Shape {
 			path: kurbo::Rect::new(0., 0., 1., 1.).to_path(0.01),
 			style,
 			render_index: 1,
-			closed: true,
 		}
 	}
 
@@ -126,7 +118,6 @@ impl Shape {
 			path: kurbo::Ellipse::from_rect(kurbo::Rect::new(0., 0., 1., 1.)).to_path(0.01),
 			style,
 			render_index: 1,
-			closed: true,
 		}
 	}
 
@@ -135,7 +126,6 @@ impl Shape {
 			path: kurbo::Line::new((0., 0.), (1., 0.)).to_path(0.01),
 			style,
 			render_index: 1,
-			closed: false,
 		}
 	}
 
@@ -148,11 +138,6 @@ impl Shape {
 			.enumerate()
 			.for_each(|(i, p)| if i == 0 { path.move_to(p) } else { path.line_to(p) });
 
-		Self {
-			path,
-			style,
-			render_index: 0,
-			closed: false,
-		}
+		Self { path, style, render_index: 0 }
 	}
 }

graphene/src/layers/style/mod.rs

@@ -29,25 +29,21 @@ impl Default for ViewMode {
 #[repr(C)]
 #[derive(Debug, Clone, Copy, PartialEq, Default, Serialize, Deserialize)]
 pub struct Fill {
-	color: Option<Color>,
+	color: Color,
 }
 
 impl Fill {
 	pub fn new(color: Color) -> Self {
-		Self { color: Some(color) }
+		Self { color }
 	}
 
-	pub fn color(&self) -> Option<Color> {
+	pub fn color(&self) -> Color {
 		self.color
 	}
 
-	pub const fn none() -> Self {
-		Self { color: None }
-	}
-
-	pub fn render(&self) -> String {
-		match self.color {
-			Some(c) => format!(r##" fill="#{}"{}"##, c.rgb_hex(), format_opacity("fill", c.a())),
+	pub fn render(fill: Option<Fill>) -> String {
+		match fill {
+			Some(c) => format!(r##" fill="#{}"{}"##, c.color.rgb_hex(), format_opacity("fill", c.color.a())),
 			None => r#" fill="none""#.to_string(),
 		}
 	}
@@ -116,9 +112,8 @@ impl PathStyle {
 
 	pub fn render(&self, view_mode: ViewMode) -> String {
 		let fill_attribute = match (view_mode, self.fill) {
-			(ViewMode::Outline, _) => Fill::none().render(),
-			(_, Some(fill)) => fill.render(),
-			(_, None) => String::new(),
+			(ViewMode::Outline, _) => Fill::render(None),
+			(_, fill) => Fill::render(fill),
 		};
 		let stroke_attribute = match (view_mode, self.stroke) {
 			(ViewMode::Outline, _) => Stroke::new(LAYER_OUTLINE_STROKE_COLOR, LAYER_OUTLINE_STROKE_WIDTH).render(),