Repository: GraphiteEditor/Graphite
File: graphene/src/layers/vector/subpath.rs (Rust)

Code before:
	pub fn should_close_shape(&self) -> bool {
		if self.last_point(ManipulatorType::Anchor).is_none() {
			return false;
		}

		self.first_point(ManipulatorType::Anchor)
			.unwrap()
			.position
			.distance(self.last_point(ManipulatorType::Anchor).unwrap().position)
			< 0.001 // TODO Replace with constant, a small epsilon
	}

	/// Close the shape if able
	pub fn close_shape(&mut self) {
		if self.should_close_shape() {
			self.manipulator_groups_mut().push_end(ManipulatorGroup::closed());
		}
	}

	/// An alias for `self.0` mutable
	pub fn manipulator_groups_mut(&mut self) -> &mut IdBackedVec<ManipulatorGroup> {
		&mut self.0
	}

	/// Return the bounding box of the shape
	pub fn bounding_box(&self) -> Option<[DVec2; 2]> {
		self.bezier_iter()
			.map(|bezier| bezier.internal.bounding_box())
			.reduce(|[a_min, a_max], [b_min, b_max]| [a_min.min(b_min), a_max.max(b_max)])
	}

	/// Convert path to svg
	pub fn to_svg(&mut self) -> String {
		fn write_positions(result: &mut String, values: [Option<DVec2>; 3]) {
			use std::fmt::Write;
			let count = values.into_iter().flatten().count();
			for (index, pos) in values.into_iter().flatten().enumerate() {
				write!(result, "{},{}", pos.x, pos.y).unwrap();
				if index != count - 1 {
					result.push(' ');
				}
			}
		}

		let mut result = String::new();
		// The out position from the previous ManipulatorGroup
		let mut last_out_handle = None;
		// The values from the last moveto (for closing the path)
		let (mut first_in_handle, mut first_in_anchor) = (None, None);
		// Should the next element be a moveto?
		let mut start_new_contour = true;

Review comment (suggestion):
We should definitely add this function to the `bezier-rs` implementation of `Subpath`. I'm wondering if it'd be beneficial to do it as part of this PR.

If you were to add this code into `libraries/bezier-rs/subpath/solvers.rs` and use the `bezier-rs` `Subpath` instead, I think you'd be able to avoid needing `PathIter` for this part of the code.

Something similar can be said for the projection part in `closest_segement`.


Code after:
	pub fn should_close_shape(&self) -> bool {
		if self.last_point(ManipulatorType::Anchor).is_none() {
			return false;
		}

		self.first_point(ManipulatorType::Anchor)
			.unwrap()
			.position
			.distance(self.last_point(ManipulatorType::Anchor).unwrap().position)
			< 0.001 // TODO Replace with constant, a small epsilon
	}

	/// Close the shape if able
	pub fn close_shape(&mut self) {
		if self.should_close_shape() {
			self.manipulator_groups_mut().push_end(ManipulatorGroup::closed());
		}
	}

	/// An alias for `self.0` mutable
	pub fn manipulator_groups_mut(&mut self) -> &mut IdBackedVec<ManipulatorGroup> {
		&mut self.0
	}

	/// Return the bounding box of the shape.
	pub fn bounding_box(&self) -> Option<[DVec2; 2]> {
		self.bezier_iter()
			.map(|bezier| bezier.internal.bounding_box())
			.reduce(|[a_min, a_max], [b_min, b_max]| [a_min.min(b_min), a_max.max(b_max)])
	}

	/// Generate an SVG `path` elements's `d` attribute: `<path d="...">`.
	pub fn to_svg(&mut self) -> String {
		fn write_positions(result: &mut String, values: [Option<DVec2>; 3]) {
			use std::fmt::Write;
			let count = values.into_iter().flatten().count();
			for (index, pos) in values.into_iter().flatten().enumerate() {
				write!(result, "{},{}", pos.x, pos.y).unwrap();
				if index != count - 1 {
					result.push(' ');
				}
			}
		}

		let mut result = String::new();
		// The out position from the previous ManipulatorGroup
		let mut last_out_handle = None;
		// The values from the last moveto (for closing the path)
		let (mut first_in_handle, mut first_in_anchor) = (None, None);
		// Should the next element be a moveto?
		let mut start_new_contour = true;