fix: group abs_transform should only need to apply transform once

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<Transform>,
     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;

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)));
     }
 

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(

crates/usvg/tests/parser.rs

@@ -321,8 +321,8 @@ fn path_transform_in_symbol_no_clip() {
 
     // Will be parsed as:
     // <svg width="100" height="100" viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
-    //     <g id="use1">
-    //         <g transform="matrix(1 0 0 1 20 0)">
+    //     <g id="use1" transform="matrix(1 0 0 1 20 0)">
+    //         <g>
     //             <path fill="#000000" stroke="none" d="M 0 0 L 10 0 L 10 10 L 0 10 Z"/>
     //         </g>
     //     </g>
@@ -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() {
     //             <path fill="#000000" stroke="none" d="M 20 0 L 120 0 L 120 100 L 20 100 Z"/>
     //         </clipPath>
     //     </defs>
-    //     <g id="use1" clip-path="url(#clipPath1)">
+    //     <g id="use1" clip-path="url(#clipPath1)" transform="matrix(1 0 0 1 20 0)">
     //         <g>
-    //             <g transform="matrix(1 0 0 1 20 0)">
+    //             <g>
     //                 <path fill="#000000" stroke="none" d="M 0 0 L 10 0 L 10 10 L 0 10 Z"/>
     //             </g>
     //         </g>
@@ -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 = "
+      <svg
+        id='svg1'
+        viewBox='-100 -100 200 200'
+        xmlns='http://www.w3.org/2000/svg'
+        xmlns:xlink='http://www.w3.org/1999/xlink'
+      >
+        <title>`context-fill` with transform (SVG 2)</title>
+        <g id='point'>
+          <path d='m0,0 v-98 l15,83z' style='fill:#337599;' />
+          <path
+            d='m0,0 v-98 l-15,83z'
+            style='fill:#9fc9df; stroke:#337599; stroke-width:0.5;'
+          />
+        </g>
+        <use xlink:href='#point' transform='scale(0.5)' />
+      </svg>
+    ";
+
+    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));
+}