From e63a3f76f760f2c5c586a12114db428f93a2be1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Thu, 14 Aug 2025 15:45:09 +0200 Subject: [PATCH] :bug: Fix open/close path detection in wasm (#7110) * :bug: Fix open/close path detection in wasm * :lipstick: Remove leftover code --- render-wasm/src/math.rs | 7 + render-wasm/src/shapes/paths.rs | 26 ++- render-wasm/src/shapes/paths/subpaths.rs | 199 +++++++++++++++++++++++ 3 files changed, 229 insertions(+), 3 deletions(-) create mode 100644 render-wasm/src/shapes/paths/subpaths.rs diff --git a/render-wasm/src/math.rs b/render-wasm/src/math.rs index cf80b4221a..41460a7bae 100644 --- a/render-wasm/src/math.rs +++ b/render-wasm/src/math.rs @@ -24,6 +24,13 @@ pub fn is_close_to(current: f32, value: f32) -> bool { (current - value).abs() <= THRESHOLD } +pub fn are_close_points(a: impl Into<(f32, f32)>, b: impl Into<(f32, f32)>) -> bool { + let (a_x, a_y) = a.into(); + let (b_x, b_y) = b.into(); + + is_close_to(a_x, b_x) && is_close_to(a_y, b_y) +} + pub fn is_close_matrix(m: &Matrix, other: &Matrix) -> bool { is_close_to(m.scale_x(), other.scale_x()) && is_close_to(m.scale_y(), other.scale_y()) diff --git a/render-wasm/src/shapes/paths.rs b/render-wasm/src/shapes/paths.rs index e5489a16e8..da04d49d2c 100644 --- a/render-wasm/src/shapes/paths.rs +++ b/render-wasm/src/shapes/paths.rs @@ -2,6 +2,8 @@ use skia_safe::{self as skia, Matrix}; use crate::math; +mod subpaths; + type Point = (f32, f32); #[derive(Debug, PartialEq, Copy, Clone)] @@ -12,6 +14,24 @@ pub enum Segment { Close, } +impl Segment { + fn xy(&self) -> Option { + match self { + Segment::MoveTo(xy) => Some(*xy), + Segment::LineTo(xy) => Some(*xy), + Segment::CurveTo((_, _, xy)) => Some(*xy), + Segment::Close => None, + } + } + + pub fn is_close_to(&self, other: &Segment) -> bool { + match (self.xy(), other.xy()) { + (Some(a), Some(b)) => math::are_close_points(a, b), + _ => false, + } + } +} + #[derive(Debug, Clone, PartialEq)] pub struct Path { segments: Vec, @@ -39,7 +59,6 @@ fn to_verb(v: u8) -> skia::path::Verb { impl Path { pub fn new(segments: Vec) -> Self { - let mut open = true; let mut skia_path = skia::Path::new(); let mut start = None; @@ -60,7 +79,6 @@ impl Path { } Segment::Close => { skia_path.close(); - open = false; None } }; @@ -70,11 +88,13 @@ impl Path { && math::is_close_to(destination.1, start.1) { skia_path.close(); - open = false; } } } + // TODO: handle error + let open = subpaths::is_open_path(&segments).expect("Failed to determine if path is open"); + Self { segments, skia_path, diff --git a/render-wasm/src/shapes/paths/subpaths.rs b/render-wasm/src/shapes/paths/subpaths.rs new file mode 100644 index 0000000000..9339e0a401 --- /dev/null +++ b/render-wasm/src/shapes/paths/subpaths.rs @@ -0,0 +1,199 @@ +use super::Segment; +use crate::math::are_close_points; + +type Result = std::result::Result; + +#[derive(Debug, Clone, PartialEq)] +pub struct Subpath { + segments: Vec, + closed: Option, +} + +impl Subpath { + pub fn new(segments: Vec) -> Self { + Self { + segments, + closed: None, + } + } + + pub fn starts_in(&self, other_segment: Option<&Segment>) -> bool { + if let (Some(start), Some(end)) = (self.start(), other_segment) { + start.is_close_to(end) + } else { + false + } + } + + pub fn ends_in(&self, other_segment: Option<&Segment>) -> bool { + if let (Some(end), Some(start)) = (self.end(), other_segment) { + end.is_close_to(start) + } else { + false + } + } + + pub fn start(&self) -> Option<&Segment> { + self.segments.first() + } + + pub fn end(&self) -> Option<&Segment> { + self.segments.last() + } + + pub fn is_empty(&self) -> bool { + self.segments.is_empty() + } + + pub fn is_closed(&self) -> bool { + self.closed.unwrap_or_else(|| self.calculate_closed()) + } + + pub fn add_segment(&mut self, segment: Segment) { + self.segments.push(segment); + self.closed = None; + } + + pub fn reversed(&self) -> Self { + let mut reversed = self.clone(); + reversed.segments.reverse(); + reversed + } + + fn calculate_closed(&self) -> bool { + let mut start = None; + for segment in self.segments.iter() { + let destination = match segment { + Segment::MoveTo(xy) => { + start = Some(xy); + None + } + Segment::LineTo(xy) => Some(xy), + Segment::CurveTo((_, _, xy)) => Some(xy), + Segment::Close => { + return true; + } + }; + + if let (Some(&start), Some(&destination)) = (start, destination) { + if are_close_points(start, destination) { + return true; + } + } + } + + false + } +} + +impl Default for Subpath { + fn default() -> Self { + Self::new(vec![]) + } +} + +/// Joins two subpaths into a single subpath +impl TryFrom<(&Subpath, &Subpath)> for Subpath { + type Error = String; + + fn try_from((subpath, other): (&Subpath, &Subpath)) -> Result { + if subpath.is_empty() || other.is_empty() || subpath.end() != other.start() { + return Err("Subpaths cannot be joined".to_string()); + } + + let mut segments = subpath.segments.clone(); + segments.extend_from_slice(&other.segments); + Ok(Subpath::new(segments)) + } +} + +/// Groups segments into subpaths based on MoveTo segments +fn get_subpaths(segments: &[Segment]) -> Vec { + let mut subpaths: Vec = vec![]; + let mut current_subpath = Subpath::default(); + + for segment in segments { + match segment { + Segment::MoveTo(_) => { + subpaths.push(current_subpath); + current_subpath = Subpath::default(); + } + _ => { + current_subpath.add_segment(*segment); + } + } + } + + if !current_subpath.is_empty() { + subpaths.push(current_subpath); + } + + subpaths +} + +/// Computes the merged candidate and the remaining, unmerged subpaths +fn merge_paths(candidate: Subpath, others: Vec) -> Result<(Subpath, Vec)> { + if candidate.is_closed() { + return Ok((candidate, others)); + } + + let mut merged = candidate.clone(); + let mut other_without_merged = vec![]; + + for subpath in others { + if merged.ends_in(subpath.start()) { + merged = Subpath::try_from((&merged, &subpath))?; + } else if merged.starts_in(subpath.end()) { + merged = Subpath::try_from((&subpath, &merged))?; + } else if merged.ends_in(subpath.end()) { + merged = Subpath::try_from((&merged, &subpath.reversed()))?; + } else if merged.starts_in(subpath.start()) { + merged = Subpath::try_from((&subpath.reversed(), &merged))?; + } else { + other_without_merged.push(subpath); + } + } + + Ok((merged, other_without_merged)) +} + +/// Searches a path for potential subpaths that can be closed and merges them +fn closed_subpaths( + current: &Subpath, + others: &[Subpath], + partial: &[Subpath], +) -> Result> { + let mut result = partial.to_vec(); + + let (new_current, new_others) = if current.is_closed() { + (current.clone(), others.to_vec()) + } else { + merge_paths(current.clone(), others.to_vec())? + }; + + // we haven't found any matching subpaths -> advance + if new_current == *current { + result.push(current.clone()); + if new_others.is_empty() { + return Ok(result); + } + + closed_subpaths(&new_others[0], &new_others[1..], &result) + } + // if diffrent, we have to search again with the merged subpaths + else { + closed_subpaths(&new_current, &new_others, &result) + } +} + +pub fn is_open_path(segments: &[Segment]) -> Result { + let subpaths = get_subpaths(segments); + let closed_subpaths = if subpaths.len() > 1 { + closed_subpaths(&subpaths[0], &subpaths[1..], &[])? + } else { + subpaths + }; + + // return true if any subpath is open + Ok(closed_subpaths.iter().any(|subpath| !subpath.is_closed())) +}