diff --git a/crates/usvg/src/parser/converter.rs b/crates/usvg/src/parser/converter.rs index 38bdbdbf..e3f4202f 100644 --- a/crates/usvg/src/parser/converter.rs +++ b/crates/usvg/src/parser/converter.rs @@ -598,7 +598,7 @@ pub(crate) fn convert_element(node: SvgNode, state: &State, cache: &mut Cache, p return; } - if let Some(g) = convert_group(node, state, false, cache, parent, &|cache, g| { + if let Some(g) = convert_group(node, None, state, false, cache, parent, &|cache, g| { convert_element_impl(tag_name, node, state, cache, g); }) { parent.children.push(Node::Group(Box::new(g))); @@ -679,7 +679,7 @@ pub(crate) fn convert_clip_path_elements( continue; } - if let Some(g) = convert_group(node, state, false, cache, parent, &|cache, g| { + if let Some(g) = convert_group(node, None, state, false, cache, parent, &|cache, g| { convert_clip_path_elements_impl(tag_name, node, state, cache, g); }) { parent.children.push(Node::Group(Box::new(g))); @@ -738,6 +738,7 @@ impl<'a, 'input: 'a> FromValue<'a, 'input> for Isolation { // TODO: explain pub(crate) fn convert_group( node: SvgNode, + transform: Option, state: &State, force: bool, cache: &mut Cache, @@ -752,7 +753,7 @@ pub(crate) fn convert_group( Opacity::ONE }; - let transform = node.resolve_transform(AId::Transform, state); + let transform = transform.unwrap_or_else(|| node.resolve_transform(AId::Transform, state)); let blend_mode: BlendMode = node.attribute(AId::MixBlendMode).unwrap_or_default(); let isolation: Isolation = node.attribute(AId::Isolation).unwrap_or_default(); let isolate = isolation == Isolation::Isolate; diff --git a/crates/usvg/src/parser/switch.rs b/crates/usvg/src/parser/switch.rs index 9b0d892c..784a0d80 100644 --- a/crates/usvg/src/parser/switch.rs +++ b/crates/usvg/src/parser/switch.rs @@ -49,9 +49,11 @@ pub(crate) fn convert( let child = node .children() .find(|n| is_condition_passed(*n, state.opt))?; - if let Some(g) = converter::convert_group(node, state, false, cache, parent, &|cache, g| { - converter::convert_element(child, state, cache, g); - }) { + if let Some(g) = + converter::convert_group(node, None, state, false, cache, parent, &|cache, g| { + converter::convert_element(child, state, cache, g); + }) + { parent.children.push(Node::Group(Box::new(g))); } diff --git a/crates/usvg/src/parser/use_node.rs b/crates/usvg/src/parser/use_node.rs index 74e540f4..04a273b1 100644 --- a/crates/usvg/src/parser/use_node.rs +++ b/crates/usvg/src/parser/use_node.rs @@ -85,14 +85,20 @@ pub(crate) fn convert( if let Some(clip_rect) = get_clip_rect(node, child, &use_state) { let mut g = clip_element(node, clip_rect, orig_ts, &use_state, cache); - g.abs_transform = parent.abs_transform; + g.abs_transform = parent.abs_transform.pre_concat(orig_ts).pre_concat(new_ts); // Make group for `use`. - if let Some(mut g2) = - converter::convert_group(node, &use_state, true, cache, &mut g, &|cache, g2| { + if let Some(mut g2) = converter::convert_group( + node, + None, + &use_state, + true, + cache, + &mut g, + &|cache, g2| { convert_children(child, new_ts, &use_state, cache, false, g2); - }) - { + }, + ) { // We must reset transform, because it was already set // to the group with clip-path. g.is_context_element = true; @@ -115,13 +121,18 @@ pub(crate) fn convert( if linked_to_symbol { // Make group for `use`. - if let Some(mut g) = - converter::convert_group(node, &use_state, false, cache, parent, &|cache, g| { + if let Some(mut g) = converter::convert_group( + node, + Some(orig_ts), + &use_state, + false, + cache, + parent, + &|cache, g| { convert_children(child, orig_ts, &use_state, cache, false, g); - }) - { + }, + ) { g.is_context_element = true; - g.transform = Transform::default(); parent.children.push(Node::Group(Box::new(g))); } } else { @@ -269,13 +280,9 @@ fn convert_children( is_context_element: bool, parent: &mut Group, ) { - // Temporarily adjust absolute transform so `convert_group` would account for `transform`. - let old_abs_transform = parent.abs_transform; - parent.abs_transform = parent.abs_transform.pre_concat(transform); - let required = !transform.is_identity(); if let Some(mut g) = - converter::convert_group(node, state, required, cache, parent, &|cache, g| { + converter::convert_group(node, None, state, required, cache, parent, &|cache, g| { if state.parent_clip_path.is_some() { converter::convert_clip_path_elements(node, state, cache, g); } else { @@ -287,8 +294,6 @@ fn convert_children( g.transform = transform; parent.children.push(Node::Group(Box::new(g))); } - - parent.abs_transform = old_abs_transform; } fn get_clip_rect( diff --git a/crates/usvg/tests/parser.rs b/crates/usvg/tests/parser.rs index a8e65f8f..976d1457 100644 --- a/crates/usvg/tests/parser.rs +++ b/crates/usvg/tests/parser.rs @@ -321,8 +321,8 @@ fn path_transform_in_symbol_no_clip() { // Will be parsed as: // - // - // + // + // // // // @@ -333,7 +333,10 @@ fn path_transform_in_symbol_no_clip() { let group_node1 = &tree.root().children()[0]; assert!(matches!(group_node1, usvg::Node::Group(_))); assert_eq!(group_node1.id(), "use1"); - assert_eq!(group_node1.abs_transform(), usvg::Transform::default()); + assert_eq!( + group_node1.abs_transform(), + usvg::Transform::from_translate(20.0, 0.0) + ); let group1 = match group_node1 { usvg::Node::Group(g) => g, @@ -352,6 +355,11 @@ fn path_transform_in_symbol_no_clip() { _ => unreachable!(), }; + assert_eq!( + group2.transform(), + usvg::Transform::from_translate(20.0, 0.0) + ); + let path = &group2.children()[0]; assert!(matches!(path, usvg::Node::Path(_))); assert_eq!( @@ -380,9 +388,9 @@ fn path_transform_in_symbol_with_clip() { // // // - // + // // - // + // // // // @@ -394,7 +402,10 @@ fn path_transform_in_symbol_with_clip() { let group_node1 = &tree.root().children()[0]; assert!(matches!(group_node1, usvg::Node::Group(_))); assert_eq!(group_node1.id(), "use1"); - assert_eq!(group_node1.abs_transform(), usvg::Transform::default()); + assert_eq!( + group_node1.abs_transform(), + usvg::Transform::from_translate(20.0, 0.0) + ); let group1 = match group_node1 { usvg::Node::Group(g) => g, @@ -403,7 +414,10 @@ fn path_transform_in_symbol_with_clip() { let group_node2 = &group1.children()[0]; assert!(matches!(group_node2, usvg::Node::Group(_))); - assert_eq!(group_node2.abs_transform(), usvg::Transform::default()); + assert_eq!( + group_node2.abs_transform(), + usvg::Transform::from_translate(20.0, 0.0) + ); let group2 = match group_node2 { usvg::Node::Group(g) => g, @@ -412,16 +426,17 @@ fn path_transform_in_symbol_with_clip() { let group_node3 = &group2.children()[0]; assert!(matches!(group_node3, usvg::Node::Group(_))); - assert_eq!( - group_node3.abs_transform(), - usvg::Transform::from_translate(20.0, 0.0) - ); let group3 = match group_node3 { usvg::Node::Group(g) => g, _ => unreachable!(), }; + assert_eq!( + group3.abs_transform(), + usvg::Transform::from_translate(20.0, 0.0) + ); + let path = &group3.children()[0]; assert!(matches!(path, usvg::Node::Path(_))); assert_eq!( @@ -547,3 +562,48 @@ fn no_text_nodes() { let tree = usvg::Tree::from_str(&svg, &usvg::Options::default()).unwrap(); assert!(!tree.has_text_nodes()); } + +#[test] +fn group_abs_transform_should_only_need_to_apply_transform_once() { + let svg = " + + `context-fill` with transform (SVG 2) + + + + + + + "; + + let tree = usvg::Tree::from_str(&svg, &usvg::Options::default()).unwrap(); + + let children = tree.root().children(); + + let usvg::Node::Group(group_node) = &children[0] else { + unreachable!() + }; + let groups = group_node.children(); + + let usvg::Node::Group(original_group) = &groups[0] else { + unreachable!() + }; + + assert_eq!(original_group.transform().get_scale(), (1.0, 1.0)); + assert_eq!(original_group.abs_transform().get_scale(), (1.0, 1.0)); + + let usvg::Node::Group(use_group) = &groups[1] else { + unreachable!() + }; + + assert_eq!(use_group.transform().get_scale(), (0.5, 0.5)); + assert_eq!(use_group.abs_transform().get_scale(), (0.5, 0.5)); +}