From 782948f2e9b90698fc0d729a8a42e9282deb3d24 Mon Sep 17 00:00:00 2001 From: Yevhenii Reizner Date: Sun, 11 Feb 2024 23:38:02 +0200 Subject: [PATCH] Try to preserve original gradient and pattern objects when converting units. --- crates/usvg/src/parser/converter.rs | 6 - crates/usvg/src/parser/paint_server.rs | 151 +++++++++++------- ...text-with-generated-gradients-expected.svg | 4 +- 3 files changed, 92 insertions(+), 69 deletions(-) diff --git a/crates/usvg/src/parser/converter.rs b/crates/usvg/src/parser/converter.rs index f6157f47a..d23feab69 100644 --- a/crates/usvg/src/parser/converter.rs +++ b/crates/usvg/src/parser/converter.rs @@ -39,11 +39,8 @@ pub struct Cache { // used for ID generation all_ids: HashSet, - #[cfg(feature = "text")] linear_gradient_index: usize, - #[cfg(feature = "text")] radial_gradient_index: usize, - #[cfg(feature = "text")] pattern_index: usize, clip_path_index: usize, mask_index: usize, @@ -52,7 +49,6 @@ pub struct Cache { impl Cache { // TODO: macros? - #[cfg(feature = "text")] pub(crate) fn gen_linear_gradient_id(&mut self) -> NonEmptyString { loop { self.linear_gradient_index += 1; @@ -64,7 +60,6 @@ impl Cache { } } - #[cfg(feature = "text")] pub(crate) fn gen_radial_gradient_id(&mut self) -> NonEmptyString { loop { self.radial_gradient_index += 1; @@ -76,7 +71,6 @@ impl Cache { } } - #[cfg(feature = "text")] pub(crate) fn gen_pattern_id(&mut self) -> NonEmptyString { loop { self.pattern_index += 1; diff --git a/crates/usvg/src/parser/paint_server.rs b/crates/usvg/src/parser/paint_server.rs index 1322007db..a0891a9a6 100644 --- a/crates/usvg/src/parser/paint_server.rs +++ b/crates/usvg/src/parser/paint_server.rs @@ -632,9 +632,7 @@ fn process_paint(paint: &mut Paint, bbox: Rect, cache: &mut Cache) -> bool { if paint.units() == Units::ObjectBoundingBox || paint.content_units() == Units::ObjectBoundingBox { - if let Some(p) = paint.to_user_coordinates(bbox, cache) { - *paint = p; - } else { + if paint.to_user_coordinates(bbox, cache).is_none() { return false; } } @@ -656,7 +654,7 @@ fn process_text_decoration(style: &mut Option, bbox: Rect, } impl Paint { - fn to_user_coordinates(&self, bbox: Rect, cache: &mut Cache) -> Option { + fn to_user_coordinates(&mut self, bbox: Rect, cache: &mut Cache) -> Option<()> { let name = if matches!(self, Paint::Pattern(_)) { "Pattern" } else { @@ -666,79 +664,110 @@ impl Paint { .to_non_zero_rect() .log_none(|| log::warn!("{} on zero-sized shapes is not allowed.", name))?; - let paint = match self { - Paint::Color(_) => self.clone(), // unreachable - Paint::LinearGradient(ref lg) => { + // `Arc::get_mut()` allow us to modify some paint servers in-place. + // This reduces the amount of cloning and preserves the original ID as well. + match self { + Paint::Color(_) => {} // unreachable + Paint::LinearGradient(ref mut lg) => { let transform = lg.transform.post_concat(Transform::from_bbox(bbox)); - Paint::LinearGradient(Arc::new(LinearGradient { - x1: lg.x1, - y1: lg.y1, - x2: lg.x2, - y2: lg.y2, - base: BaseGradient { - id: cache.gen_linear_gradient_id(), - units: Units::UserSpaceOnUse, - transform, - spread_method: lg.spread_method, - stops: lg.stops.clone(), - }, - })) + if let Some(ref mut lg) = Arc::get_mut(lg) { + lg.base.transform = transform; + lg.base.units = Units::UserSpaceOnUse; + } else { + *lg = Arc::new(LinearGradient { + x1: lg.x1, + y1: lg.y1, + x2: lg.x2, + y2: lg.y2, + base: BaseGradient { + id: cache.gen_linear_gradient_id(), + units: Units::UserSpaceOnUse, + transform, + spread_method: lg.spread_method, + stops: lg.stops.clone(), + }, + }); + } } - Paint::RadialGradient(ref rg) => { + Paint::RadialGradient(ref mut rg) => { let transform = rg.transform.post_concat(Transform::from_bbox(bbox)); - Paint::RadialGradient(Arc::new(RadialGradient { - cx: rg.cx, - cy: rg.cy, - r: rg.r, - fx: rg.fx, - fy: rg.fy, - base: BaseGradient { - id: cache.gen_radial_gradient_id(), - units: Units::UserSpaceOnUse, - transform, - spread_method: rg.spread_method, - stops: rg.stops.clone(), - }, - })) + if let Some(ref mut rg) = Arc::get_mut(rg) { + rg.base.transform = transform; + rg.base.units = Units::UserSpaceOnUse; + } else { + *rg = Arc::new(RadialGradient { + cx: rg.cx, + cy: rg.cy, + r: rg.r, + fx: rg.fx, + fy: rg.fy, + base: BaseGradient { + id: cache.gen_radial_gradient_id(), + units: Units::UserSpaceOnUse, + transform, + spread_method: rg.spread_method, + stops: rg.stops.clone(), + }, + }); + } } - Paint::Pattern(ref patt) => { + Paint::Pattern(ref mut patt) => { let rect = if patt.units == Units::ObjectBoundingBox { patt.rect.bbox_transform(bbox) } else { patt.rect }; - let root = if patt.content_units == Units::ObjectBoundingBox - && patt.view_box().is_none() - { - // No need to shift patterns. - let transform = Transform::from_scale(bbox.width(), bbox.height()); + if let Some(ref mut patt) = Arc::get_mut(patt) { + patt.rect = rect; + patt.units = Units::UserSpaceOnUse; + + if patt.content_units == Units::ObjectBoundingBox && patt.view_box().is_none() { + // No need to shift patterns. + let transform = Transform::from_scale(bbox.width(), bbox.height()); - let mut g = patt.root.clone(); - g.transform = transform; - g.abs_transform = transform; + let mut g = std::mem::replace(&mut patt.root, Group::empty()); + g.transform = transform; + g.abs_transform = transform; - let mut root = Group::empty(); - root.children.push(Node::Group(Box::new(g))); - root.calculate_bounding_boxes(); - root + patt.root.children.push(Node::Group(Box::new(g))); + patt.root.calculate_bounding_boxes(); + } + + patt.content_units = Units::UserSpaceOnUse; } else { - patt.root.clone() - }; + let root = if patt.content_units == Units::ObjectBoundingBox + && patt.view_box().is_none() + { + // No need to shift patterns. + let transform = Transform::from_scale(bbox.width(), bbox.height()); + + let mut g = patt.root.clone(); + g.transform = transform; + g.abs_transform = transform; + + let mut root = Group::empty(); + root.children.push(Node::Group(Box::new(g))); + root.calculate_bounding_boxes(); + root + } else { + patt.root.clone() + }; - Paint::Pattern(Arc::new(Pattern { - id: cache.gen_pattern_id(), - units: Units::UserSpaceOnUse, - content_units: Units::UserSpaceOnUse, - transform: patt.transform, - rect, - view_box: patt.view_box, - root, - })) + *patt = Arc::new(Pattern { + id: cache.gen_pattern_id(), + units: Units::UserSpaceOnUse, + content_units: Units::UserSpaceOnUse, + transform: patt.transform, + rect, + view_box: patt.view_box, + root, + }) + } } - }; + } - Some(paint) + Some(()) } } diff --git a/crates/usvg/tests/files/text-with-generated-gradients-expected.svg b/crates/usvg/tests/files/text-with-generated-gradients-expected.svg index a24612e4f..0a50b9e0a 100644 --- a/crates/usvg/tests/files/text-with-generated-gradients-expected.svg +++ b/crates/usvg/tests/files/text-with-generated-gradients-expected.svg @@ -1,13 +1,13 @@ - + - +