From 60e1600caa258d8307747e9ad5360f6c21d85803 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 2 Nov 2026 02:49:36 -0800 Subject: [PATCH 1/6] SVG: circles and ellipses import as four-anchor ellipses usvg turns each and into arcs and then into a number of cubics that depends on f32 rounding: an ellipse that export wrote as could come back with eight anchors instead of the four it was written from. The renderer flattens the two outlines differently, so a round trip of a flat filled ellipse redrew 158 edge pixels by up to 43 levels, and prop_svg::plain_art_survives_visually failed on main. Import now records the ids of and elements in the prepass and rebuilds each as the ellipse VectorCraft draws: four smooth anchors on the axes, starting at the right as SVG defines, with handles KAPPA of the radii long, then transformed as before. Tests: prop_svg::filled_ellipse_on_the_artboard_edge_survives_visually (the failing case) and basic_shapes::circles_and_ellipses_import_as_the_ellipse_drawn fail without the change. --- crates/svg/src/import.rs | 3 ++ crates/svg/src/import/convert.rs | 45 ++++++++++++++++++++++++++++- crates/svg/src/import/importer.rs | 9 ++++-- crates/svg/src/import/prepass.rs | 12 ++++++++ crates/svg/tests/it/basic_shapes.rs | 45 +++++++++++++++++++++++++++++ crates/svg/tests/it/prop_svg.rs | 28 ++++++++++++++++++ docs/target-app-parity.md | 1 + 7 files changed, 139 insertions(+), 4 deletions(-) diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index 18f4f088d..0201c591e 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -161,6 +161,7 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions, max_nodes: usize) -> Resul foreign: found.foreign, sublayers: found.sublayers, sublayer_groups: HashSet::new(), + ellipses: found.ellipses, screen: k, converted_nodes: 1, max_nodes, @@ -318,6 +319,8 @@ struct Importer { sublayers: HashSet, /// The groups made of them, made sublayers where they sit in a layer ([`promote_sublayers`]). sublayer_groups: HashSet, + /// Ids of the `` and `` elements ([`prepass`]). + ellipses: HashSet, /// Points per screen pixel (the root's user unit before its `viewBox`): what a non-scaling /// stroke's width is measured in. screen: f64, diff --git a/crates/svg/src/import/convert.rs b/crates/svg/src/import/convert.rs index 40c2cf577..a154a233f 100644 --- a/crates/svg/src/import/convert.rs +++ b/crates/svg/src/import/convert.rs @@ -1,4 +1,6 @@ -use super::{Affine, Arc, BezPath, BlendMode, Color, FillRule, GradientStop, HashMap, Node, Point, XNode, roxmltree}; +use super::{Affine, Arc, BezPath, BlendMode, Color, FillRule, GradientStop, HashMap, Node, PathData, Point, XNode, roxmltree}; +use vectorcraft_geom::path::{Anchor, AnchorKind, SubPath}; +use vectorcraft_geom::shapes::KAPPA; /// `n` with every object id (its descendants' and its mask art's too) zeroed, to compare art. pub(super) fn without_ids(n: &Node) -> Node { @@ -135,6 +137,47 @@ pub(super) fn bezpath(p: &usvg::tiny_skia_path::Path, m: Affine) -> BezPath { bp } +/// The path usvg makes of a `` or `` (`p`: one closed outline of arcs from the +/// right end of the horizontal axis round through the bottom) as the ellipse VectorCraft draws: four +/// smooth anchors on the axes in the same order, each quarter a cubic with handles [`KAPPA`] of the +/// radii long, the ellipse export writes as ``. `None` when `p` is not such an outline. +pub(super) fn drawn_ellipse(p: &usvg::tiny_skia_path::Path) -> Option { + use usvg::tiny_skia_path::PathSegment as S; + let mut ends = vec![]; + let mut closed = false; + for s in p.segments() { + match s { + S::MoveTo(q) if ends.is_empty() => ends.push(Point::new(q.x as f64, q.y as f64)), + S::LineTo(q) | S::QuadTo(_, q) | S::CubicTo(_, _, q) if !ends.is_empty() && !closed => ends.push(Point::new(q.x as f64, q.y as f64)), + S::Close if !closed => closed = true, + _ => return None, + } + } + let (x0, x1) = ends.iter().fold((f64::INFINITY, f64::NEG_INFINITY), |(lo, hi), q| (lo.min(q.x), hi.max(q.x))); + let (y0, y1) = ends.iter().fold((f64::INFINITY, f64::NEG_INFINITY), |(lo, hi), q| (lo.min(q.y), hi.max(q.y))); + let (c, rx, ry) = (Point::new((x0 + x1) / 2.0, (y0 + y1) / 2.0), (x1 - x0) / 2.0, (y1 - y0) / 2.0); + // usvg works in f32: the tolerance follows the size of the numbers. + let tol = 1e-5 * (c.x.abs() + c.y.abs() + rx + ry).max(1.0); + let axes = [Point::new(x1, c.y), Point::new(c.x, y1), Point::new(x0, c.y), Point::new(c.x, y0)]; + let on_ellipse = |q: &Point| (((q.x - c.x) / rx).powi(2) + ((q.y - c.y) / ry).powi(2) - 1.0).abs() <= 1e-3; + if !closed + || rx <= tol + || ry <= tol + || ends.first().is_none_or(|s| s.distance(axes[0]) > tol) + || !axes.iter().all(|a| ends.iter().any(|q| q.distance(*a) <= tol)) + || !ends.iter().all(on_ellipse) + { + return None; + } + let anchors = (0..4) + .map(|i| { + let (p, next, prev) = (axes[i], axes[(i + 1) % 4], axes[(i + 3) % 4]); + Anchor { p, h_in: p + (prev - c) * KAPPA, h_out: p + (next - c) * KAPPA, kind: AnchorKind::Smooth } + }) + .collect(); + Some(PathData::single(SubPath::new(anchors, true))) +} + /// The shapes of clip path `cp` on an element whose user space `ts` maps to the document. pub(super) fn clip_shapes(cp: &usvg::ClipPath, ts: Affine) -> Vec<(BezPath, FillRule)> { fn collect(g: &usvg::Group, m: Affine, out: &mut Vec<(BezPath, FillRule)>) { diff --git a/crates/svg/src/import/importer.rs b/crates/svg/src/import/importer.rs index 0e0b8127e..eeb0a4804 100644 --- a/crates/svg/src/import/importer.rs +++ b/crates/svg/src/import/importer.rs @@ -1,4 +1,4 @@ -use super::convert::{aff, bezpath, blend, clip_shapes, gradient_stops, is_plain, made_up, rule, uncut, unique_name, without_ids}; +use super::convert::{aff, bezpath, blend, clip_shapes, drawn_ellipse, gradient_stops, is_plain, made_up, rule, uncut, unique_name, without_ids}; use super::layers::filter_names; use super::prepass::link; use super::{ @@ -601,7 +601,8 @@ impl Importer { if !p.is_visible() { return None; } - let path = PathData::from_bezpath(&bezpath(p.data(), acc)); + let ellipse = if self.ellipses.contains(p.id()) { drawn_ellipse(p.data()) } else { None }; + let path = ellipse.map_or_else(|| PathData::from_bezpath(&bezpath(p.data(), acc)), |e| e.transformed(acc)); if path.is_empty() { return None; } @@ -683,9 +684,10 @@ impl Importer { std::mem::take(&mut self.non_scaling), std::mem::take(&mut self.data), std::mem::take(&mut self.events), + std::mem::take(&mut self.ellipses), ); let art = self.group_node(tree.root(), acc); - (self.links, self.hidden, self.uses, self.labels, self.non_scaling, self.data, self.events) = outer; + (self.links, self.hidden, self.uses, self.labels, self.non_scaling, self.data, self.events, self.ellipses) = outer; art } } @@ -720,6 +722,7 @@ mod tests { foreign: HashMap::new(), sublayers: HashSet::new(), sublayer_groups: HashSet::new(), + ellipses: HashSet::new(), screen: 1.0, converted_nodes: 0, max_nodes: 1000, diff --git a/crates/svg/src/import/prepass.rs b/crates/svg/src/import/prepass.rs index 8018138dc..2afa651a5 100644 --- a/crates/svg/src/import/prepass.rs +++ b/crates/svg/src/import/prepass.rs @@ -28,6 +28,9 @@ pub(super) struct Found { pub(super) scripts: Vec, /// Ids of the `` elements marked as layers ([`marked_layer`]). pub(super) sublayers: HashSet, + /// Ids of the `` and `` elements, which come back as the ellipse VectorCraft + /// draws ([`drawn_ellipse`](super::convert::drawn_ellipse)). + pub(super) ellipses: HashSet, pub(super) warnings: Vec, } @@ -115,6 +118,8 @@ impl Edits { /// * objects with event attributes get an id when they have none, recorded with them /// ([`user_events`]), and the `