[librsvg: 5/13] Remove DrawingCtx::with_saved_transform(); pass the transform to with_discrete_layer()

[Federico Mena Quintero <federico src gnome org>]

commit 075179faa9305d1b9689351464dd22be606cc650
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon May 10 20:37:57 2021 -0500

    Remove DrawingCtx::with_saved_transform(); pass the transform to with_discrete_layer()
    
    Element::draw() used to set the draw_ctx's transform itself by calling
    DrawingCtx::with_saved_transform().  So, the code flow was this:
    
      Element.draw() {
          with_saved_transform(
              element.transform,
              {
                  element_impl.draw() {
                      draw_ctx.with_discrete_layer({ ... })
                  }
              }
          )
      }
    
    Now, we pass the transform to use directly to with_discrete_layer().
    The code flow becomes this:
    
      Element.draw() {
          element_impl.draw() {
              draw_ctx.with_discrete_layer({ element.transform, ... })
          }
      }
    
    This requires two notable changes:
    
    1. draw_from_use_node(): must now apply the <use> element's transform
    in the same fashion as with_discrete_layer().
    
    2. "impl Draw for Text" is what starts the recursive process of
    creating text chunks and spans.  It used to be that generating the
    chunks/spans was done outside the call to
    DrawingCtx::with_discrete_layer().
    
    However, now that the draw_ctx.cr is *not* transformed until the call
    to DrawingCtx::with_discrete_layer(), I had to move the code that
    creates the chunks/spans to be inside the call to
    with_discrete_layer().  This is because creating a Pango layout
    requires the cr to already have its final transform.

 src/drawing_ctx.rs | 67 ++++++++++++++++++++++++++++++------------------------
 src/element.rs     |  6 ++---
 src/marker.rs      | 19 ++++++++--------
 src/structure.rs   |  4 ++++
 src/text.rs        | 47 +++++++++++++++++++-------------------
 5 files changed, 76 insertions(+), 67 deletions(-)
---
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index eca76022..199c6399 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -578,6 +578,7 @@ impl DrawingCtx {
                 values,
                 false,
                 None,
+                Transform::identity(),
                 &mut |an, dc| mask_node.draw_children(an, &cascaded, dc, false),
             );
 
@@ -600,12 +601,17 @@ impl DrawingCtx {
         values: &ComputedValues,
         clipping: bool,
         clip_rect: Option<Rect>,
+        transform: Transform,
         draw_fn: &mut dyn FnMut(
             &mut AcquiredNodes<'_>,
             &mut DrawingCtx,
         ) -> Result<BoundingBox, RenderingError>,
     ) -> Result<BoundingBox, RenderingError> {
-        if clipping {
+        let orig_transform = self.get_transform();
+
+        self.cr.transform(transform.into());
+
+        let res = if clipping {
             draw_fn(acquired_nodes, self)
         } else {
             let saved_cr = SavedCr::new(self);
@@ -793,34 +799,7 @@ impl DrawingCtx {
             } else {
                 draw_fn(acquired_nodes, saved_cr.draw_ctx)
             }
-        }
-    }
-
-    fn initial_transform_with_offset(&self) -> Transform {
-        let rect = self.toplevel_viewport();
-
-        self.initial_viewport
-            .transform
-            .pre_translate(rect.x0, rect.y0)
-    }
-
-    /// Saves the current transform, applies a new transform, runs the
-    /// draw_fn, and restores the original transform
-    ///
-    /// This is slightly cheaper than a `cr.save()` / `cr.restore()`
-    /// pair, but more importantly, it does not reset the whole
-    /// graphics state, i.e. it leaves a clipping path in place if it
-    /// was set by the `draw_fn`.
-    pub fn with_saved_transform(
-        &mut self,
-        transform: &Transform,
-        draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<BoundingBox, RenderingError>,
-    ) -> Result<BoundingBox, RenderingError> {
-        let orig_transform = self.get_transform();
-
-        self.cr.transform((*transform).into());
-
-        let res = draw_fn(self);
+        };
 
         self.cr.set_matrix(orig_transform.into());
 
@@ -833,6 +812,14 @@ impl DrawingCtx {
         }
     }
 
+    fn initial_transform_with_offset(&self) -> Transform {
+        let rect = self.toplevel_viewport();
+
+        self.initial_viewport
+            .transform
+            .pre_translate(rect.x0, rect.y0)
+    }
+
     /// Run the drawing function with the specified opacity
     fn with_alpha(
         &mut self,
@@ -1027,6 +1014,7 @@ impl DrawingCtx {
                         pattern_values,
                         false,
                         None,
+                        Transform::identity(),
                         &mut |an, dc| {
                             pattern.node_with_children.draw_children(
                                 an,
@@ -1206,6 +1194,7 @@ impl DrawingCtx {
             values,
             clipping,
             None,
+            node.borrow_element().get_transform(),
             &mut |an, dc| {
                 let cr = dc.cr.clone();
                 let transform = dc.get_transform();
@@ -1314,6 +1303,7 @@ impl DrawingCtx {
             values,
             clipping,
             None,
+            node.borrow_element().get_transform(),
             &mut |_an, dc| {
                 let saved_cr = SavedCr::new(dc);
 
@@ -1597,7 +1587,12 @@ impl DrawingCtx {
 
         let child = acquired.get();
 
-        if is_element_of_type!(child, Symbol) {
+        let orig_transform = self.get_transform();
+
+        self.cr
+            .transform(node.borrow_element().get_transform().into());
+
+        let res = if is_element_of_type!(child, Symbol) {
             // if the <use> references a <symbol>, it gets handled specially
 
             let elt = child.borrow_element();
@@ -1619,6 +1614,7 @@ impl DrawingCtx {
                 values,
                 clipping,
                 None,
+                Transform::identity(),
                 &mut |an, dc| {
                     let _params = dc.push_new_viewport(
                         symbol.get_viewbox(),
@@ -1647,6 +1643,7 @@ impl DrawingCtx {
                 values,
                 clipping,
                 None,
+                Transform::identity(),
                 &mut |an, dc| {
                     child.draw(
                         an,
@@ -1656,6 +1653,16 @@ impl DrawingCtx {
                     )
                 },
             )
+        };
+
+        self.cr.set_matrix(orig_transform.into());
+
+        if let Ok(bbox) = res {
+            let mut res_bbox = BoundingBox::new().with_transform(orig_transform);
+            res_bbox.insert(&bbox);
+            Ok(res_bbox)
+        } else {
+            res
         }
     }
 }
diff --git a/src/element.rs b/src/element.rs
index 1956c2ac..3c68804d 100644
--- a/src/element.rs
+++ b/src/element.rs
@@ -281,10 +281,8 @@ impl<T: SetAttributes + Draw> Draw for ElementInner<T> {
         if !self.is_in_error() {
             let values = cascaded.get();
             if values.is_displayed() {
-                draw_ctx.with_saved_transform(&self.get_transform(), &mut |dc| {
-                    self.element_impl
-                        .draw(node, acquired_nodes, cascaded, dc, clipping)
-                })
+                self.element_impl
+                    .draw(node, acquired_nodes, cascaded, draw_ctx, clipping)
             } else {
                 Ok(draw_ctx.empty_bbox())
             }
diff --git a/src/marker.rs b/src/marker.rs
index c79c3bf3..068800b4 100644
--- a/src/marker.rs
+++ b/src/marker.rs
@@ -159,16 +159,15 @@ impl Marker {
             )
         };
 
-        draw_ctx.with_saved_transform(&transform, &mut |dc| {
-            dc.with_discrete_layer(
-                node,
-                acquired_nodes,
-                values,
-                clipping,
-                clip,
-                &mut |an, dc| node.draw_children(an, &cascaded, dc, clipping),
-            )
-        })
+        draw_ctx.with_discrete_layer(
+            node,
+            acquired_nodes,
+            values,
+            clipping,
+            clip,
+            transform,
+            &mut |an, dc| node.draw_children(an, &cascaded, dc, clipping),
+        )
     }
 }
 
diff --git a/src/structure.rs b/src/structure.rs
index f85f4e23..82d7ff98 100644
--- a/src/structure.rs
+++ b/src/structure.rs
@@ -39,6 +39,7 @@ impl Draw for Group {
             values,
             clipping,
             None,
+            node.borrow_element().get_transform(),
             &mut |an, dc| node.draw_children(an, cascaded, dc, clipping),
         )
     }
@@ -77,6 +78,7 @@ impl Draw for Switch {
             values,
             clipping,
             None,
+            node.borrow_element().get_transform(),
             &mut |an, dc| {
                 if let Some(child) = node.children().filter(|c| c.is_element()).find(|c| {
                     let elt = c.borrow_element();
@@ -251,6 +253,7 @@ impl Draw for Svg {
             values,
             clipping,
             None,
+            node.borrow_element().get_transform(),
             &mut |an, dc| {
                 let _params = self.push_viewport(node, cascaded, dc);
                 node.draw_children(an, cascaded, dc, clipping)
@@ -499,6 +502,7 @@ impl Draw for Link {
             values,
             clipping,
             None,
+            node.borrow_element().get_transform(),
             &mut |an, dc| match self.link.as_ref() {
                 Some(l) if !l.is_empty() => {
                     dc.with_link_tag(l, &mut |dc| node.draw_children(an, &cascaded, dc, clipping))
diff --git a/src/text.rs b/src/text.rs
index 805568cb..fa3d0891 100644
--- a/src/text.rs
+++ b/src/text.rs
@@ -491,36 +491,37 @@ impl Draw for Text {
         let view_params = draw_ctx.get_view_params();
         let params = NormalizeParams::new(&values, &view_params);
 
-        let mut x = self.x.to_user(&params);
-        let mut y = self.y.to_user(&params);
-
-        let chunks = self.make_chunks(node, acquired_nodes, cascaded, draw_ctx, x, y);
-
-        let mut measured_chunks = Vec::new();
-        for chunk in &chunks {
-            measured_chunks.push(MeasuredChunk::from_chunk(chunk, draw_ctx));
-        }
-
-        let mut positioned_chunks = Vec::new();
-        for chunk in &measured_chunks {
-            let chunk_x = chunk.x.unwrap_or(x);
-            let chunk_y = chunk.y.unwrap_or(y);
-
-            let positioned = PositionedChunk::from_measured(&chunk, draw_ctx, chunk_x, chunk_y);
-
-            x = positioned.next_chunk_x;
-            y = positioned.next_chunk_y;
-
-            positioned_chunks.push(positioned);
-        }
-
         draw_ctx.with_discrete_layer(
             node,
             acquired_nodes,
             values,
             clipping,
             None,
+            node.borrow_element().get_transform(),
             &mut |an, dc| {
+                let mut x = self.x.to_user(&params);
+                let mut y = self.y.to_user(&params);
+
+                let chunks = self.make_chunks(node, an, cascaded, dc, x, y);
+
+                let mut measured_chunks = Vec::new();
+                for chunk in &chunks {
+                    measured_chunks.push(MeasuredChunk::from_chunk(chunk, dc));
+                }
+
+                let mut positioned_chunks = Vec::new();
+                for chunk in &measured_chunks {
+                    let chunk_x = chunk.x.unwrap_or(x);
+                    let chunk_y = chunk.y.unwrap_or(y);
+
+                    let positioned = PositionedChunk::from_measured(&chunk, dc, chunk_x, chunk_y);
+
+                    x = positioned.next_chunk_x;
+                    y = positioned.next_chunk_y;
+
+                    positioned_chunks.push(positioned);
+                }
+
                 let mut bbox = dc.empty_bbox();
 
                 for chunk in &positioned_chunks {