[librsvg: 4/13] with_discrete_layer: take an extra arg with a user-space clipping rectangle

[Federico Mena Quintero <federico src gnome org>]

commit eb46847e4d4feb1446d3b9a8d665cfb3782566f9
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon May 10 20:17:17 2021 -0500

    with_discrete_layer: take an extra arg with a user-space clipping rectangle
    
    This is just used for markers.  I'm trying to concentrate all the
    "isolation group" parameters into the arguments of
    with_discrete_layer().

 src/drawing_ctx.rs | 211 ++++++++++++++++++++++++++++-------------------------
 src/marker.rs      |  13 ++--
 src/structure.rs   |  66 +++++++++++------
 src/text.rs        |  27 ++++---
 4 files changed, 183 insertions(+), 134 deletions(-)
---
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index 906e525b..eca76022 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -577,6 +577,7 @@ impl DrawingCtx {
                 acquired_nodes,
                 values,
                 false,
+                None,
                 &mut |an, dc| mask_node.draw_children(an, &cascaded, dc, false),
             );
 
@@ -598,6 +599,7 @@ impl DrawingCtx {
         acquired_nodes: &mut AcquiredNodes<'_>,
         values: &ComputedValues,
         clipping: bool,
+        clip_rect: Option<Rect>,
         draw_fn: &mut dyn FnMut(
             &mut AcquiredNodes<'_>,
             &mut DrawingCtx,
@@ -630,6 +632,10 @@ impl DrawingCtx {
             let (clip_in_user_space, clip_in_object_space) =
                 get_clip_in_user_and_object_space(acquired_nodes, clip_uri);
 
+            if let Some(rect) = clip_rect {
+                clip_to_rectangle(&saved_cr.draw_ctx.cr, &rect);
+            }
+
             // Here we are clipping in user space, so the bbox doesn't matter
             saved_cr.draw_ctx.clip_to_node(
                 &clip_in_user_space,
@@ -827,26 +833,6 @@ impl DrawingCtx {
         }
     }
 
-    /// if a rectangle is specified, clips and runs the draw_fn, otherwise simply run the draw_fn
-    pub fn with_clip_rect(
-        &mut self,
-        clip: Option<Rect>,
-        draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<BoundingBox, RenderingError>,
-    ) -> Result<BoundingBox, RenderingError> {
-        if let Some(rect) = clip {
-            self.cr.save();
-            clip_to_rectangle(&self.cr, &rect);
-        }
-
-        let res = draw_fn(self);
-
-        if clip.is_some() {
-            self.cr.restore();
-        }
-
-        res
-    }
-
     /// Run the drawing function with the specified opacity
     fn with_alpha(
         &mut self,
@@ -1040,6 +1026,7 @@ impl DrawingCtx {
                         acquired_nodes,
                         pattern_values,
                         false,
+                        None,
                         &mut |an, dc| {
                             pattern.node_with_children.draw_children(
                                 an,
@@ -1213,59 +1200,66 @@ impl DrawingCtx {
             return Ok(self.empty_bbox());
         }
 
-        self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-            let cr = dc.cr.clone();
-            let transform = dc.get_transform();
-            let mut path_helper =
-                PathHelper::new(&cr, transform, &shape.path, values.stroke_line_cap());
-
-            if clipping {
-                if values.is_visible() {
-                    cr.set_fill_rule(cairo::FillRule::from(values.clip_rule()));
-                    path_helper.set()?;
+        self.with_discrete_layer(
+            node,
+            acquired_nodes,
+            values,
+            clipping,
+            None,
+            &mut |an, dc| {
+                let cr = dc.cr.clone();
+                let transform = dc.get_transform();
+                let mut path_helper =
+                    PathHelper::new(&cr, transform, &shape.path, values.stroke_line_cap());
+
+                if clipping {
+                    if values.is_visible() {
+                        cr.set_fill_rule(cairo::FillRule::from(values.clip_rule()));
+                        path_helper.set()?;
+                    }
+                    return Ok(dc.empty_bbox());
                 }
-                return Ok(dc.empty_bbox());
-            }
 
-            cr.set_antialias(cairo::Antialias::from(values.shape_rendering()));
-            dc.setup_cr_for_stroke(&cr, values);
+                cr.set_antialias(cairo::Antialias::from(values.shape_rendering()));
+                dc.setup_cr_for_stroke(&cr, values);
 
-            cr.set_fill_rule(cairo::FillRule::from(values.fill_rule()));
+                cr.set_fill_rule(cairo::FillRule::from(values.fill_rule()));
 
-            let mut bounding_box: Option<BoundingBox> = None;
-            path_helper.unset();
+                let mut bounding_box: Option<BoundingBox> = None;
+                path_helper.unset();
 
-            let view_params = dc.get_view_params();
+                let view_params = dc.get_view_params();
 
-            for &target in &values.paint_order().targets {
-                // fill and stroke operations will preserve the path.
-                // markers operation will clear the path.
-                match target {
-                    PaintTarget::Fill | PaintTarget::Stroke => {
-                        path_helper.set()?;
-                        let bbox = bounding_box.get_or_insert_with(|| {
-                            compute_stroke_and_fill_box(&cr, &values, &view_params)
-                        });
+                for &target in &values.paint_order().targets {
+                    // fill and stroke operations will preserve the path.
+                    // markers operation will clear the path.
+                    match target {
+                        PaintTarget::Fill | PaintTarget::Stroke => {
+                            path_helper.set()?;
+                            let bbox = bounding_box.get_or_insert_with(|| {
+                                compute_stroke_and_fill_box(&cr, &values, &view_params)
+                            });
 
-                        if values.is_visible() {
-                            if target == PaintTarget::Stroke {
-                                dc.stroke(&cr, an, values, &bbox)?;
-                            } else {
-                                dc.fill(&cr, an, values, &bbox)?;
+                            if values.is_visible() {
+                                if target == PaintTarget::Stroke {
+                                    dc.stroke(&cr, an, values, &bbox)?;
+                                } else {
+                                    dc.fill(&cr, an, values, &bbox)?;
+                                }
                             }
                         }
+                        PaintTarget::Markers if shape.markers == Markers::Yes => {
+                            path_helper.unset();
+                            marker::render_markers_for_path(&shape.path, dc, an, values, clipping)?;
+                        }
+                        _ => {}
                     }
-                    PaintTarget::Markers if shape.markers == Markers::Yes => {
-                        path_helper.unset();
-                        marker::render_markers_for_path(&shape.path, dc, an, values, clipping)?;
-                    }
-                    _ => {}
                 }
-            }
 
-            path_helper.unset();
-            Ok(bounding_box.unwrap())
-        })
+                path_helper.unset();
+                Ok(bounding_box.unwrap())
+            },
+        )
     }
 
     fn paint_surface(&mut self, surface: &SharedImageSurface, width: f64, height: f64) {
@@ -1314,25 +1308,32 @@ impl DrawingCtx {
             None
         };
 
-        self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |_an, dc| {
-            let saved_cr = SavedCr::new(dc);
+        self.with_discrete_layer(
+            node,
+            acquired_nodes,
+            values,
+            clipping,
+            None,
+            &mut |_an, dc| {
+                let saved_cr = SavedCr::new(dc);
 
-            if let Some(_params) =
-                saved_cr
-                    .draw_ctx
-                    .push_new_viewport(Some(vbox), rect, aspect, clip_mode)
-            {
-                if values.is_visible() {
+                if let Some(_params) =
                     saved_cr
                         .draw_ctx
-                        .paint_surface(surface, image_width, image_height);
+                        .push_new_viewport(Some(vbox), rect, aspect, clip_mode)
+                {
+                    if values.is_visible() {
+                        saved_cr
+                            .draw_ctx
+                            .paint_surface(surface, image_width, image_height);
+                    }
                 }
-            }
 
-            // The bounding box for <image> is decided by the values of x, y, w, h
-            // and not by the final computed image bounds.
-            Ok(saved_cr.draw_ctx.empty_bbox().with_rect(rect))
-        })
+                // The bounding box for <image> is decided by the values of x, y, w, h
+                // and not by the final computed image bounds.
+                Ok(saved_cr.draw_ctx.empty_bbox().with_rect(rect))
+            },
+        )
     }
 
     pub fn draw_text(
@@ -1612,35 +1613,49 @@ impl DrawingCtx {
                 None
             };
 
-            self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-                let _params = dc.push_new_viewport(
-                    symbol.get_viewbox(),
-                    use_rect,
-                    symbol.get_preserve_aspect_ratio(),
-                    clip_mode,
-                );
-
-                child.draw_children(
-                    an,
-                    &CascadedValues::new_from_values(&child, values),
-                    dc,
-                    clipping,
-                )
-            })
+            self.with_discrete_layer(
+                node,
+                acquired_nodes,
+                values,
+                clipping,
+                None,
+                &mut |an, dc| {
+                    let _params = dc.push_new_viewport(
+                        symbol.get_viewbox(),
+                        use_rect,
+                        symbol.get_preserve_aspect_ratio(),
+                        clip_mode,
+                    );
+
+                    child.draw_children(
+                        an,
+                        &CascadedValues::new_from_values(&child, values),
+                        dc,
+                        clipping,
+                    )
+                },
+            )
         } else {
             // otherwise the referenced node is not a <symbol>; process it generically
 
             let cr = self.cr.clone();
             cr.translate(use_rect.x0, use_rect.y0);
 
-            self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-                child.draw(
-                    an,
-                    &CascadedValues::new_from_values(&child, values),
-                    dc,
-                    clipping,
-                )
-            })
+            self.with_discrete_layer(
+                node,
+                acquired_nodes,
+                values,
+                clipping,
+                None,
+                &mut |an, dc| {
+                    child.draw(
+                        an,
+                        &CascadedValues::new_from_values(&child, values),
+                        dc,
+                        clipping,
+                    )
+                },
+            )
         }
     }
 }
diff --git a/src/marker.rs b/src/marker.rs
index 31e5584d..c79c3bf3 100644
--- a/src/marker.rs
+++ b/src/marker.rs
@@ -160,11 +160,14 @@ impl Marker {
         };
 
         draw_ctx.with_saved_transform(&transform, &mut |dc| {
-            dc.with_clip_rect(clip, &mut |dc| {
-                dc.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-                    node.draw_children(an, &cascaded, dc, clipping)
-                })
-            })
+            dc.with_discrete_layer(
+                node,
+                acquired_nodes,
+                values,
+                clipping,
+                clip,
+                &mut |an, dc| node.draw_children(an, &cascaded, dc, clipping),
+            )
         })
     }
 }
diff --git a/src/structure.rs b/src/structure.rs
index e92801e1..f85f4e23 100644
--- a/src/structure.rs
+++ b/src/structure.rs
@@ -33,9 +33,14 @@ impl Draw for Group {
     ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
-        draw_ctx.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-            node.draw_children(an, cascaded, dc, clipping)
-        })
+        draw_ctx.with_discrete_layer(
+            node,
+            acquired_nodes,
+            values,
+            clipping,
+            None,
+            &mut |an, dc| node.draw_children(an, cascaded, dc, clipping),
+        )
     }
 }
 
@@ -66,16 +71,23 @@ impl Draw for Switch {
     ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
-        draw_ctx.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-            if let Some(child) = node.children().filter(|c| c.is_element()).find(|c| {
-                let elt = c.borrow_element();
-                elt.get_cond() && !elt.is_in_error()
-            }) {
-                child.draw(an, &CascadedValues::new(cascaded, &child), dc, clipping)
-            } else {
-                Ok(dc.empty_bbox())
-            }
-        })
+        draw_ctx.with_discrete_layer(
+            node,
+            acquired_nodes,
+            values,
+            clipping,
+            None,
+            &mut |an, dc| {
+                if let Some(child) = node.children().filter(|c| c.is_element()).find(|c| {
+                    let elt = c.borrow_element();
+                    elt.get_cond() && !elt.is_in_error()
+                }) {
+                    child.draw(an, &CascadedValues::new(cascaded, &child), dc, clipping)
+                } else {
+                    Ok(dc.empty_bbox())
+                }
+            },
+        )
     }
 }
 
@@ -233,10 +245,17 @@ impl Draw for Svg {
     ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
-        draw_ctx.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-            let _params = self.push_viewport(node, cascaded, dc);
-            node.draw_children(an, cascaded, dc, clipping)
-        })
+        draw_ctx.with_discrete_layer(
+            node,
+            acquired_nodes,
+            values,
+            clipping,
+            None,
+            &mut |an, dc| {
+                let _params = self.push_viewport(node, cascaded, dc);
+                node.draw_children(an, cascaded, dc, clipping)
+            },
+        )
     }
 }
 
@@ -474,13 +493,18 @@ impl Draw for Link {
         let cascaded = CascadedValues::new(cascaded, node);
         let values = cascaded.get();
 
-        draw_ctx.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-            match self.link.as_ref() {
+        draw_ctx.with_discrete_layer(
+            node,
+            acquired_nodes,
+            values,
+            clipping,
+            None,
+            &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))
                 }
                 _ => node.draw_children(an, &cascaded, dc, clipping),
-            }
-        })
+            },
+        )
     }
 }
diff --git a/src/text.rs b/src/text.rs
index 30757a07..805568cb 100644
--- a/src/text.rs
+++ b/src/text.rs
@@ -514,18 +514,25 @@ impl Draw for Text {
             positioned_chunks.push(positioned);
         }
 
-        draw_ctx.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
-            let mut bbox = dc.empty_bbox();
-
-            for chunk in &positioned_chunks {
-                for span in &chunk.spans {
-                    let span_bbox = span.draw(an, dc, clipping)?;
-                    bbox.insert(&span_bbox);
+        draw_ctx.with_discrete_layer(
+            node,
+            acquired_nodes,
+            values,
+            clipping,
+            None,
+            &mut |an, dc| {
+                let mut bbox = dc.empty_bbox();
+
+                for chunk in &positioned_chunks {
+                    for span in &chunk.spans {
+                        let span_bbox = span.draw(an, dc, clipping)?;
+                        bbox.insert(&span_bbox);
+                    }
                 }
-            }
 
-            Ok(bbox)
-        })
+                Ok(bbox)
+            },
+        )
     }
 }