[librsvg: 4/5] DrawingCtx: remove the bbox field; propagate it through Results



commit a2b346312c79bf5376fa22fc297c79895ef0a4e8
Author: Federico Mena Quintero <federico gnome org>
Date:   Wed Sep 18 07:41:21 2019 -0500

    DrawingCtx: remove the bbox field; propagate it through Results
    
    DrawingCtx.bbox was the field that accumulated the bounding box for
    the node that is currently being rendered.  Functions that require
    knowing the node's geometry (e.g. those with objectBoundingBox
    relative scaling) would look at draw_ctx.bbox.
    
    In a couple of places where we *don't* want to accumulate bounding
    boxes (e.g. when rendering a clipping path), we would do horrible
    things like save the old bbox, render the path, then restore the bbox.
    
    Now, the bbox propagated through a Result<BoundingBox> from all the
    drawing-related functions, most notably NodeTrait::draw() and
    ::draw_children(), and DrawingCtx::with_discrete_layer().
    
    This removes a bit of mutability from DrawingCtx, and may make it
    easier to split the geometry computation into a separate stage,
    similar to the get_bbox_contribution() function that Firefox uses for
    its SVG nodes.
    
    This is a big refactor, which must unfortunately land in a single
    step.  All the tests pass.

 rsvg_internals/src/clip_path.rs   | 43 ++++++++++--------
 rsvg_internals/src/drawing_ctx.rs | 92 +++++++++++++++------------------------
 rsvg_internals/src/handle.rs      | 16 +++----
 rsvg_internals/src/image.rs       | 16 +++----
 rsvg_internals/src/link.rs        |  3 +-
 rsvg_internals/src/marker.rs      | 58 ++++++++++++++----------
 rsvg_internals/src/node.rs        | 25 ++++++-----
 rsvg_internals/src/shapes.rs      | 49 +++++++++++----------
 rsvg_internals/src/structure.rs   | 17 ++++----
 rsvg_internals/src/text.rs        | 27 +++++++-----
 10 files changed, 176 insertions(+), 170 deletions(-)
---
diff --git a/rsvg_internals/src/clip_path.rs b/rsvg_internals/src/clip_path.rs
index a6958459..f611fb19 100644
--- a/rsvg_internals/src/clip_path.rs
+++ b/rsvg_internals/src/clip_path.rs
@@ -35,28 +35,35 @@ impl NodeClipPath {
 
         let cascaded = CascadedValues::new_from_node(node);
 
-        draw_ctx.with_saved_matrix(&mut |dc| {
-            let cr = dc.get_cairo_context();
+        draw_ctx
+            .with_saved_matrix(&mut |dc| {
+                let cr = dc.get_cairo_context();
 
-            if self.units == ClipPathUnits(CoordUnits::ObjectBoundingBox) {
-                let bbox_rect = bbox.rect.as_ref().unwrap();
+                if self.units == ClipPathUnits(CoordUnits::ObjectBoundingBox) {
+                    let bbox_rect = bbox.rect.as_ref().unwrap();
 
-                cr.transform(cairo::Matrix::new(
-                    bbox_rect.width,
-                    0.0,
-                    0.0,
-                    bbox_rect.height,
-                    bbox_rect.x,
-                    bbox_rect.y,
-                ))
-            }
+                    cr.transform(cairo::Matrix::new(
+                        bbox_rect.width,
+                        0.0,
+                        0.0,
+                        bbox_rect.height,
+                        bbox_rect.x,
+                        bbox_rect.y,
+                    ))
+                }
+
+                // here we don't push a layer because we are clipping
+                let res = node.draw_children(&cascaded, dc, true);
 
-            // here we don't push a layer because we are clipping
-            let res = node.draw_children(&cascaded, dc, true);
+                cr.clip();
 
-            cr.clip();
-            res
-        })
+                res
+            })
+            .and_then(|_bbox|
+                      // Clipping paths do not contribute to bounding boxes (they should,
+                      // but we need Real Computational Geometry(tm), so ignore the
+                      // bbox from the clip path.
+                      Ok(()))
     }
 }
 
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 172d7669..269eb118 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -2,8 +2,8 @@ use cairo;
 use cairo_sys;
 use glib::translate::*;
 use std::cell::RefCell;
-use std::rc::{Rc, Weak};
 use std::convert::TryFrom;
+use std::rc::{Rc, Weak};
 
 use crate::allowed_url::Fragment;
 use crate::aspect_ratio::AspectRatio;
@@ -21,12 +21,7 @@ use crate::paint_server::{PaintServer, PaintSource};
 use crate::pattern::NodePattern;
 use crate::properties::ComputedValues;
 use crate::property_defs::{
-    ClipRule,
-    FillRule,
-    ShapeRendering,
-    StrokeDasharray,
-    StrokeLinecap,
-    StrokeLinejoin,
+    ClipRule, FillRule, ShapeRendering, StrokeDasharray, StrokeLinecap, StrokeLinejoin,
 };
 use crate::rect::RectangleExt;
 use crate::surface_utils::shared_surface::SharedImageSurface;
@@ -105,8 +100,6 @@ pub struct DrawingCtx {
 
     view_box_stack: Rc<RefCell<Vec<ViewBox>>>,
 
-    bbox: BoundingBox,
-
     drawsub_stack: Vec<RsvgNode>,
 
     acquired_nodes: AcquiredNodes,
@@ -174,7 +167,6 @@ impl DrawingCtx {
             cr_stack: Vec::new(),
             cr: cr.clone(),
             view_box_stack: Rc::new(RefCell::new(view_box_stack)),
-            bbox: BoundingBox::new(&initial_affine),
             drawsub_stack: Vec::new(),
             acquired_nodes,
             measuring,
@@ -204,6 +196,10 @@ impl DrawingCtx {
         self.cr.clone()
     }
 
+    pub fn empty_bbox(&self) -> BoundingBox {
+        BoundingBox::new(&self.cr.get_matrix())
+    }
+
     // FIXME: Usage of this function is more less a hack... The caller
     // manually saves and then restore the draw_ctx.cr.
     // It would be better to have an explicit push/pop for the cairo_t, or
@@ -334,14 +330,6 @@ impl DrawingCtx {
             })
     }
 
-    pub fn insert_bbox(&mut self, bbox: &BoundingBox) {
-        self.bbox.insert(bbox);
-    }
-
-    pub fn get_bbox(&self) -> &BoundingBox {
-        &self.bbox
-    }
-
     pub fn acquired_nodes(&self) -> &AcquiredNodes {
         &self.acquired_nodes
     }
@@ -370,21 +358,15 @@ impl DrawingCtx {
         }
     }
 
-    fn clip_to_node(&mut self, clip_node: &Option<RsvgNode>) -> Result<(), RenderingError> {
+    fn clip_to_node(
+        &mut self,
+        clip_node: &Option<RsvgNode>,
+        bbox: &BoundingBox,
+    ) -> Result<(), RenderingError> {
         if let Some(node) = clip_node {
-            let orig_bbox = self.bbox;
-
             let node_data = node.borrow();
             let clip_path = node_data.get_impl::<NodeClipPath>();
-            let res = clip_path.to_cairo_context(&node, self, &orig_bbox);
-
-            // FIXME: this is an EPIC HACK to keep the clipping context from
-            // accumulating bounding boxes.  We'll remove this later, when we
-            // are able to extract bounding boxes from outside the
-            // general drawing loop.
-            self.bbox = orig_bbox;
-
-            res
+            clip_path.to_cairo_context(&node, self, &bbox)
         } else {
             Ok(())
         }
@@ -395,8 +377,8 @@ impl DrawingCtx {
         node: &RsvgNode,
         values: &ComputedValues,
         clipping: bool,
-        draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<(), RenderingError>,
-    ) -> Result<(), RenderingError> {
+        draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<BoundingBox, RenderingError>,
+    ) -> Result<BoundingBox, RenderingError> {
         if clipping {
             draw_fn(self)
         } else {
@@ -418,7 +400,8 @@ impl DrawingCtx {
                 let (clip_in_user_space, clip_in_object_space) =
                     dc.get_clip_in_user_and_object_space(clip_uri);
 
-                dc.clip_to_node(&clip_in_user_space)?;
+                // Here we are clipping in user space, so the bbox doesn't matter
+                dc.clip_to_node(&clip_in_user_space, &dc.empty_bbox())?;
 
                 let needs_temporary_surface = !(opacity == 1.0
                     && filter.is_none()
@@ -448,22 +431,23 @@ impl DrawingCtx {
 
                     dc.push_cairo_context(cr);
 
-                    // Create temporary bbox with the cr's affine
-
-                    let prev_bbox = dc.bbox;
-
-                    dc.bbox = BoundingBox::new(&affines.for_temporary_surface);
-
                     // Draw!
 
                     let mut res = draw_fn(dc);
 
+                    let bbox = if let Ok(ref bbox) = res {
+                        *bbox
+                    } else {
+                        BoundingBox::new(&affines.for_temporary_surface)
+                    };
+
                     // Filter
 
                     let source_surface = if let Some(filter_uri) = filter {
-                        let child_surface = cairo::ImageSurface::try_from(dc.cr.get_target()).unwrap();
+                        let child_surface =
+                            cairo::ImageSurface::try_from(dc.cr.get_target()).unwrap();
                         let img_surface =
-                            dc.run_filter(filter_uri, node, values, &child_surface, dc.bbox)?;
+                            dc.run_filter(filter_uri, node, values, &child_surface, bbox)?;
                         // turn into a Surface
                         (*img_surface).clone()
                     } else {
@@ -480,7 +464,7 @@ impl DrawingCtx {
                     // Clip
 
                     dc.cr.set_matrix(affines.outside_temporary_surface);
-                    dc.clip_to_node(&clip_in_object_space)?;
+                    let _: () = dc.clip_to_node(&clip_in_object_space, &bbox)?;
 
                     // Mask
 
@@ -491,12 +475,12 @@ impl DrawingCtx {
                         {
                             let mask_node = acquired.get();
 
-                            res = res.and_then(|_| {
-                                let bbox = dc.bbox;
+                            res = res.and_then(|bbox| {
                                 mask_node
                                     .borrow()
                                     .get_impl::<NodeMask>()
                                     .generate_cairo_mask(&mask_node, &affines, dc, &bbox)
+                                    .map(|_: ()| bbox)
                             });
                         } else {
                             rsvg_log!("element {} references nonexistent mask \"{}\"", node, mask);
@@ -515,10 +499,6 @@ impl DrawingCtx {
 
                     dc.cr.set_matrix(affine_at_start);
 
-                    let bbox = dc.bbox;
-                    dc.bbox = prev_bbox;
-                    dc.bbox.insert(&bbox);
-
                     res
                 } else {
                     draw_fn(dc)
@@ -541,8 +521,8 @@ impl DrawingCtx {
     /// was set by the `draw_fn`.
     pub fn with_saved_matrix(
         &mut self,
-        draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<(), RenderingError>,
-    ) -> Result<(), RenderingError> {
+        draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<BoundingBox, RenderingError>,
+    ) -> Result<BoundingBox, RenderingError> {
         let matrix = self.cr.get_matrix();
         let res = draw_fn(self);
         self.cr.set_matrix(matrix);
@@ -552,8 +532,8 @@ impl DrawingCtx {
     /// Saves the current Cairo context, runs the draw_fn, and restores the context
     pub fn with_saved_cr(
         &mut self,
-        draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<(), RenderingError>,
-    ) -> Result<(), RenderingError> {
+        draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<BoundingBox, RenderingError>,
+    ) -> Result<BoundingBox, RenderingError> {
         self.cr.save();
         let res = draw_fn(self);
         self.cr.restore();
@@ -808,7 +788,7 @@ impl DrawingCtx {
         surface: &cairo::ImageSurface,
         width: f64,
         height: f64,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let save_cr = self.cr.clone();
         let save_rect = self.rect;
         let save_affine = self.get_cairo_context().get_matrix();
@@ -835,7 +815,7 @@ impl DrawingCtx {
         cascaded: &CascadedValues<'_>,
         node: &RsvgNode,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let stack_top = self.drawsub_stack.pop();
 
         let draw = if let Some(ref top) = stack_top {
@@ -848,14 +828,14 @@ impl DrawingCtx {
         let res = if draw && values.is_visible() {
             node.draw(cascaded, self, clipping)
         } else {
-            Ok(())
+            Ok(self.empty_bbox())
         };
 
         if let Some(top) = stack_top {
             self.drawsub_stack.push(top);
         }
 
-        res.and_then(|_| self.check_limits())
+        res.and_then(|bbox| self.check_limits().and_then(|_| Ok(bbox)))
     }
 
     pub fn add_node_and_ancestors_to_stack(&mut self, node: &RsvgNode) {
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index 04358d60..d1107dd6 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -295,9 +295,7 @@ impl Handle {
         );
         let root = self.svg.root();
 
-        draw_ctx.draw_node_from_stack(&CascadedValues::new_from_node(&root), &root, false)?;
-
-        let bbox = draw_ctx.get_bbox();
+        let bbox = draw_ctx.draw_node_from_stack(&CascadedValues::new_from_node(&root), &root, false)?;
 
         let ink_rect = bbox
             .ink_rect
@@ -463,7 +461,9 @@ impl Handle {
             is_testing,
         );
         let cascaded = CascadedValues::new_from_node(&root);
-        let res = draw_ctx.draw_node_from_stack(&cascaded, &root, false);
+        let res = draw_ctx
+            .draw_node_from_stack(&cascaded, &root, false)
+            .map(|_bbox| ());
         cr.restore();
 
         res
@@ -488,8 +488,7 @@ impl Handle {
             is_testing,
         );
 
-        draw_ctx.draw_node_from_stack(&CascadedValues::new_from_node(node), node, false)?;
-        Ok(draw_ctx.get_bbox().clone())
+        draw_ctx.draw_node_from_stack(&CascadedValues::new_from_node(node), node, false)
     }
 
     /// Returns (ink_rect, logical_rect)
@@ -575,8 +574,9 @@ impl Handle {
             is_testing,
         );
 
-        let res =
-            draw_ctx.draw_node_from_stack(&CascadedValues::new_from_node(&node), &node, false);
+        let res = draw_ctx
+            .draw_node_from_stack(&CascadedValues::new_from_node(&node), &node, false)
+            .map(|_bbox| ());
 
         cr.restore();
 
diff --git a/rsvg_internals/src/image.rs b/rsvg_internals/src/image.rs
index ef86cd3d..f054d5a2 100644
--- a/rsvg_internals/src/image.rs
+++ b/rsvg_internals/src/image.rs
@@ -65,7 +65,7 @@ impl NodeTrait for NodeImage {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
         let params = draw_ctx.get_view_params();
 
@@ -75,14 +75,14 @@ impl NodeTrait for NodeImage {
         let h = self.h.normalize(values, &params);
 
         if w.approx_eq_cairo(0.0) || h.approx_eq_cairo(0.0) {
-            return Ok(());
+            return Ok(draw_ctx.empty_bbox());
         }
 
         draw_ctx.with_discrete_layer(node, values, clipping, &mut |dc| {
             let surface = if let Some(Href::PlainUrl(ref url)) = self.href {
                 dc.lookup_image(&url)?
             } else {
-                return Ok(());
+                return Ok(dc.empty_bbox());
             };
 
             let clip_mode = if !values.is_overflow() && self.aspect.is_slice() {
@@ -94,12 +94,12 @@ impl NodeTrait for NodeImage {
             let image_width = surface.width();
             let image_height = surface.height();
             if clipping || image_width == 0 || image_height == 0 {
-                return Ok(());
+                return Ok(dc.empty_bbox());
             }
 
             // The bounding box for <image> is decided by the values of x, y, w, h and not by
             // the final computed image bounds.
-            let bbox = BoundingBox::new(&dc.get_cairo_context().get_matrix()).with_rect(Some(
+            let bbox = dc.empty_bbox().with_rect(Some(
                 cairo::Rectangle {
                     x,
                     y,
@@ -138,11 +138,7 @@ impl NodeTrait for NodeImage {
                     cr.paint();
                 }
 
-                Ok(())
-            })
-            .and_then(|()| {
-                dc.insert_bbox(&bbox);
-                Ok(())
+                Ok(bbox)
             })
         })
     }
diff --git a/rsvg_internals/src/link.rs b/rsvg_internals/src/link.rs
index cb0aeb11..7c5ab3d9 100644
--- a/rsvg_internals/src/link.rs
+++ b/rsvg_internals/src/link.rs
@@ -3,6 +3,7 @@ use markup5ever::local_name;
 use regex::{Captures, Regex};
 use std::borrow::Cow;
 
+use crate::bbox::BoundingBox;
 use crate::drawing_ctx::DrawingCtx;
 use crate::error::RenderingError;
 use crate::node::*;
@@ -31,7 +32,7 @@ impl NodeTrait for NodeLink {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let cascaded = CascadedValues::new(cascaded, node);
         let values = cascaded.get();
 
diff --git a/rsvg_internals/src/marker.rs b/rsvg_internals/src/marker.rs
index 6a75fc65..127829d4 100644
--- a/rsvg_internals/src/marker.rs
+++ b/rsvg_internals/src/marker.rs
@@ -8,6 +8,7 @@ use markup5ever::local_name;
 use crate::allowed_url::Fragment;
 use crate::angle::Angle;
 use crate::aspect_ratio::*;
+use crate::bbox::BoundingBox;
 use crate::drawing_ctx::DrawingCtx;
 use crate::error::*;
 use crate::float_eq_cairo::ApproxEqCairo;
@@ -124,7 +125,7 @@ impl NodeMarker {
         computed_angle: Angle,
         line_width: f64,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let cascaded = CascadedValues::new_from_node(&node);
         let values = cascaded.get();
 
@@ -136,7 +137,7 @@ impl NodeMarker {
         if marker_width.approx_eq_cairo(0.0) || marker_height.approx_eq_cairo(0.0) {
             // markerWidth or markerHeight set to 0 disables rendering of the element
             // https://www.w3.org/TR/SVG/painting.html#MarkerWidthAttribute
-            return Ok(());
+            return Ok(draw_ctx.empty_bbox());
         }
 
         draw_ctx.with_saved_cr(&mut |dc| {
@@ -162,7 +163,7 @@ impl NodeMarker {
                 );
 
                 if vbox.width.approx_eq_cairo(0.0) || vbox.height.approx_eq_cairo(0.0) {
-                    return Ok(());
+                    return Ok(dc.empty_bbox());
                 }
 
                 cr.scale(w / vbox.width, h / vbox.height);
@@ -586,7 +587,7 @@ fn emit_marker_by_name(
     computed_angle: Angle,
     line_width: f64,
     clipping: bool,
-) -> Result<(), RenderingError> {
+) -> Result<BoundingBox, RenderingError> {
     if let Some(acquired) = draw_ctx
         .acquired_nodes()
         .get_node_of_type(Some(name), NodeType::Marker)
@@ -604,7 +605,7 @@ fn emit_marker_by_name(
         )
     } else {
         rsvg_log!("marker \"{}\" not found", name);
-        Ok(())
+        Ok(draw_ctx.empty_bbox())
     }
 }
 
@@ -620,9 +621,9 @@ fn emit_marker<E>(
     marker_type: MarkerType,
     orient: Angle,
     emit_fn: &mut E,
-) -> Result<(), RenderingError>
+) -> Result<BoundingBox, RenderingError>
 where
-    E: FnMut(MarkerType, f64, f64, Angle) -> Result<(), RenderingError>,
+    E: FnMut(MarkerType, f64, f64, Angle) -> Result<BoundingBox, RenderingError>,
 {
     let (x, y) = match *segment {
         Segment::Degenerate { x, y } => (x, y),
@@ -641,14 +642,14 @@ pub fn render_markers_for_path_builder(
     draw_ctx: &mut DrawingCtx,
     values: &ComputedValues,
     clipping: bool,
-) -> Result<(), RenderingError> {
+) -> Result<BoundingBox, RenderingError> {
     let line_width = values
         .stroke_width
         .0
         .normalize(values, &draw_ctx.get_view_params());
 
     if line_width.approx_eq_cairo(0.0) {
-        return Ok(());
+        return Ok(draw_ctx.empty_bbox());
     }
 
     let marker_start = &values.marker_start.0;
@@ -656,12 +657,13 @@ pub fn render_markers_for_path_builder(
     let marker_end = &values.marker_end.0;
 
     match (marker_start, marker_mid, marker_end) {
-        (&IRI::None, &IRI::None, &IRI::None) => return Ok(()),
+        (&IRI::None, &IRI::None, &IRI::None) => return Ok(draw_ctx.empty_bbox()),
         _ => (),
     }
 
     emit_markers_for_path_builder(
         builder,
+        draw_ctx.empty_bbox(),
         &mut |marker_type: MarkerType, x: f64, y: f64, computed_angle: Angle| {
             if let &IRI::Resource(ref marker) = match marker_type {
                 MarkerType::Start => &values.marker_start.0,
@@ -670,7 +672,7 @@ pub fn render_markers_for_path_builder(
             } {
                 emit_marker_by_name(draw_ctx, marker, x, y, computed_angle, line_width, clipping)
             } else {
-                Ok(())
+                Ok(draw_ctx.empty_bbox())
             }
         },
     )
@@ -678,16 +680,19 @@ pub fn render_markers_for_path_builder(
 
 fn emit_markers_for_path_builder<E>(
     builder: &PathBuilder,
+    empty_bbox: BoundingBox,
     emit_fn: &mut E,
-) -> Result<(), RenderingError>
+) -> Result<BoundingBox, RenderingError>
 where
-    E: FnMut(MarkerType, f64, f64, Angle) -> Result<(), RenderingError>,
+    E: FnMut(MarkerType, f64, f64, Angle) -> Result<BoundingBox, RenderingError>,
 {
     enum SubpathState {
         NoSubpath,
         InSubpath,
     };
 
+    let mut bbox = empty_bbox;
+
     // Convert the path to a list of segments and bare points
     let segments = Segments::from(builder);
 
@@ -702,23 +707,25 @@ where
                     // Got a lone point after a subpath; render the subpath's end marker first
                     let (_, incoming_vx, incoming_vy) =
                         segments.find_incoming_directionality_backwards(i - 1);
-                    emit_marker(
+                    let marker_bbox = emit_marker(
                         &segments[i - 1],
                         MarkerEndpoint::End,
                         MarkerType::End,
                         Angle::from_vector(incoming_vx, incoming_vy),
                         emit_fn,
                     )?;
+                    bbox.insert(&marker_bbox);
                 }
 
                 // Render marker for the lone point; no directionality
-                emit_marker(
+                let marker_bbox = emit_marker(
                     segment,
                     MarkerEndpoint::Start,
                     MarkerType::Middle,
                     Angle::new(0.0),
                     emit_fn,
                 )?;
+                bbox.insert(&marker_bbox);
 
                 subpath_state = SubpathState::NoSubpath;
             }
@@ -729,13 +736,14 @@ where
                     SubpathState::NoSubpath => {
                         let (_, outgoing_vx, outgoing_vy) =
                             segments.find_outgoing_directionality_forwards(i);
-                        emit_marker(
+                        let marker_bbox = emit_marker(
                             segment,
                             MarkerEndpoint::Start,
                             MarkerType::Start,
                             Angle::from_vector(outgoing_vx, outgoing_vy),
                             emit_fn,
                         )?;
+                        bbox.insert(&marker_bbox);
 
                         subpath_state = SubpathState::InSubpath;
                     }
@@ -763,13 +771,14 @@ where
                             angle = Angle::new(0.0);
                         }
 
-                        emit_marker(
+                        let marker_bbox = emit_marker(
                             segment,
                             MarkerEndpoint::Start,
                             MarkerType::Middle,
                             angle,
                             emit_fn,
                         )?;
+                        bbox.insert(&marker_bbox);
                     }
                 }
             }
@@ -795,17 +804,18 @@ where
                 }
             };
 
-            emit_marker(
+            let marker_bbox = emit_marker(
                 segment,
                 MarkerEndpoint::End,
                 MarkerType::End,
                 angle,
                 emit_fn,
             )?;
+            bbox.insert(&marker_bbox);
         }
     }
 
-    Ok(())
+    Ok(bbox)
 }
 
 #[cfg(test)]
@@ -1138,13 +1148,14 @@ mod marker_tests {
 
         assert!(emit_markers_for_path_builder(
             &builder,
+            BoundingBox::new(&cairo::Matrix::identity()),
             &mut |marker_type: MarkerType,
                   x: f64,
                   y: f64,
                   computed_angle: Angle|
-             -> Result<(), RenderingError> {
+             -> Result<BoundingBox, RenderingError> {
                 v.push((marker_type, x, y, computed_angle));
-                Ok(())
+                Ok(BoundingBox::new(&cairo::Matrix::identity()))
             }
         )
         .is_ok());
@@ -1173,13 +1184,14 @@ mod marker_tests {
 
         assert!(emit_markers_for_path_builder(
             &builder,
+            BoundingBox::new(&cairo::Matrix::identity()),
             &mut |marker_type: MarkerType,
                   x: f64,
                   y: f64,
                   computed_angle: Angle|
-             -> Result<(), RenderingError> {
+             -> Result<BoundingBox, RenderingError> {
                 v.push((marker_type, x, y, computed_angle));
-                Ok(())
+                Ok(BoundingBox::new(&cairo::Matrix::identity()))
             }
         )
         .is_ok());
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index 1e389bf4..fa3f7b26 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -5,6 +5,7 @@ use std::cell::Ref;
 use std::collections::HashSet;
 use std::fmt;
 
+use crate::bbox::BoundingBox;
 use crate::cond::{RequiredExtensions, RequiredFeatures, SystemLanguage};
 use crate::css::CssRules;
 use crate::drawing_ctx::DrawingCtx;
@@ -355,11 +356,11 @@ pub trait NodeTrait: Downcast {
         &self,
         _node: &RsvgNode,
         _cascaded: &CascadedValues<'_>,
-        _draw_ctx: &mut DrawingCtx,
+        draw_ctx: &mut DrawingCtx,
         _clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         // by default nodes don't draw themselves
-        Ok(())
+        Ok(draw_ctx.empty_bbox())
     }
 
     /// Returns the Filter trait if this node is a filter primitive
@@ -480,14 +481,14 @@ pub trait NodeDraw {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError>;
+    ) -> Result<BoundingBox, RenderingError>;
 
     fn draw_children(
         &self,
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError>;
+    ) -> Result<BoundingBox, RenderingError>;
 }
 
 impl NodeDraw for RsvgNode {
@@ -496,7 +497,7 @@ impl NodeDraw for RsvgNode {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         if !self.borrow().is_in_error() {
             draw_ctx.with_saved_matrix(&mut |dc| {
                 let cr = dc.get_cairo_context();
@@ -509,7 +510,8 @@ impl NodeDraw for RsvgNode {
         } else {
             rsvg_log!("(not rendering element {} because it is in error)", self);
 
-            Ok(()) // maybe we should actually return a RenderingError::NodeIsInError here?
+            // maybe we should actually return a RenderingError::NodeIsInError here?
+            Ok(draw_ctx.empty_bbox())
         }
     }
 
@@ -518,15 +520,18 @@ impl NodeDraw for RsvgNode {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
+        let mut bbox = draw_ctx.empty_bbox();
+
         for child in self.children() {
-            draw_ctx.draw_node_from_stack(
+            let child_bbox = draw_ctx.draw_node_from_stack(
                 &CascadedValues::new(cascaded, &child),
                 &child,
                 clipping,
             )?;
+            bbox.insert(&child_bbox);
         }
 
-        Ok(())
+        Ok(bbox)
     }
 }
diff --git a/rsvg_internals/src/shapes.rs b/rsvg_internals/src/shapes.rs
index f368ac95..0e475efe 100644
--- a/rsvg_internals/src/shapes.rs
+++ b/rsvg_internals/src/shapes.rs
@@ -2,6 +2,7 @@ use cairo;
 use markup5ever::local_name;
 use std::ops::Deref;
 
+use crate::bbox::BoundingBox;
 use crate::drawing_ctx::DrawingCtx;
 use crate::error::*;
 use crate::length::*;
@@ -21,30 +22,30 @@ fn render_path_builder(
     values: &ComputedValues,
     render_markers: bool,
     clipping: bool,
-) -> Result<(), RenderingError> {
+) -> Result<BoundingBox, RenderingError> {
     if !builder.empty() {
-        draw_ctx.with_discrete_layer(node, values, clipping, &mut |dc| {
+        let bbox = draw_ctx.with_discrete_layer(node, values, clipping, &mut |dc| {
             let cr = dc.get_cairo_context();
 
             builder.to_cairo(&cr)?;
 
             if clipping {
                 cr.set_fill_rule(cairo::FillRule::from(values.clip_rule));
+                Ok(dc.empty_bbox())
             } else {
                 cr.set_fill_rule(cairo::FillRule::from(values.fill_rule));
-                let bbox = dc.stroke_and_fill(&cr, values)?;
-                dc.insert_bbox(&bbox);
+                dc.stroke_and_fill(&cr, values)
             }
-
-            Ok(())
         })?;
 
         if render_markers {
             marker::render_markers_for_path_builder(builder, draw_ctx, values, clipping)?;
         }
-    }
 
-    Ok(())
+        Ok(bbox)
+    } else {
+        Ok(draw_ctx.empty_bbox())
+    }
 }
 
 fn render_ellipse(
@@ -56,10 +57,10 @@ fn render_ellipse(
     node: &RsvgNode,
     values: &ComputedValues,
     clipping: bool,
-) -> Result<(), RenderingError> {
+) -> Result<BoundingBox, RenderingError> {
     // Per the spec, rx and ry must be nonnegative
     if rx <= 0.0 || ry <= 0.0 {
-        return Ok(());
+        return Ok(draw_ctx.empty_bbox());
     }
 
     // 4/3 * (1-cos 45°)/sin 45° = 4/3 * sqrt(2) - 1
@@ -142,14 +143,14 @@ impl NodeTrait for NodePath {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         if let Some(ref builder) = self.builder {
-            render_path_builder(builder, draw_ctx, node, values, true, clipping)?;
+            render_path_builder(builder, draw_ctx, node, values, true, clipping)
+        } else {
+            Ok(draw_ctx.empty_bbox())
         }
-
-        Ok(())
     }
 }
 
@@ -237,7 +238,7 @@ impl NodeTrait for NodePoly {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         if let Some(ref points) = self.points {
@@ -255,10 +256,10 @@ impl NodeTrait for NodePoly {
                 builder.close_path();
             }
 
-            render_path_builder(&builder, draw_ctx, node, values, true, clipping)?;
+            render_path_builder(&builder, draw_ctx, node, values, true, clipping)
+        } else {
+            Ok(draw_ctx.empty_bbox())
         }
-
-        Ok(())
     }
 }
 
@@ -291,7 +292,7 @@ impl NodeTrait for NodeLine {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         let mut builder = PathBuilder::new();
@@ -357,7 +358,7 @@ impl NodeTrait for NodeRect {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         let params = draw_ctx.get_view_params();
@@ -394,12 +395,12 @@ impl NodeTrait for NodeRect {
 
         // Per the spec, w,h must be >= 0
         if w <= 0.0 || h <= 0.0 {
-            return Ok(());
+            return Ok(draw_ctx.empty_bbox());
         }
 
         // ... and rx,ry must be nonnegative
         if rx < 0.0 || ry < 0.0 {
-            return Ok(());
+            return Ok(draw_ctx.empty_bbox());
         }
 
         let half_w = w / 2.0;
@@ -560,7 +561,7 @@ impl NodeTrait for NodeCircle {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         let params = draw_ctx.get_view_params();
@@ -606,7 +607,7 @@ impl NodeTrait for NodeEllipse {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         let params = draw_ctx.get_view_params();
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index 08254b23..c9da97e7 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -3,6 +3,7 @@ use markup5ever::local_name;
 
 use crate::allowed_url::Fragment;
 use crate::aspect_ratio::*;
+use crate::bbox::BoundingBox;
 use crate::dpi::Dpi;
 use crate::drawing_ctx::{ClipMode, DrawingCtx, ViewParams};
 use crate::error::{AttributeResultExt, RenderingError};
@@ -30,7 +31,7 @@ impl NodeTrait for NodeGroup {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         draw_ctx.with_discrete_layer(node, values, clipping, &mut |dc| {
@@ -66,7 +67,7 @@ impl NodeTrait for NodeSwitch {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         draw_ctx.with_discrete_layer(node, values, clipping, &mut |dc| {
@@ -77,7 +78,7 @@ impl NodeTrait for NodeSwitch {
             {
                 dc.draw_node_from_stack(&CascadedValues::new(cascaded, &child), &child, clipping)
             } else {
-                Ok(())
+                Ok(dc.empty_bbox())
             }
         })
     }
@@ -212,7 +213,7 @@ impl NodeTrait for NodeSvg {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         let params = draw_ctx.get_view_params();
@@ -305,11 +306,11 @@ impl NodeTrait for NodeUse {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
 
         if self.link.is_none() {
-            return Ok(());
+            return Ok(draw_ctx.empty_bbox());
         }
 
         let link = self.link.as_ref().unwrap();
@@ -321,7 +322,7 @@ impl NodeTrait for NodeUse {
             acquired.get().clone()
         } else {
             rsvg_log!("element {} references nonexistent \"{}\"", node, link,);
-            return Ok(());
+            return Ok(draw_ctx.empty_bbox());
         };
 
         if node.ancestors().any(|ancestor| ancestor == child) {
@@ -353,7 +354,7 @@ impl NodeTrait for NodeUse {
         // width or height set to 0 disables rendering of the element
         // https://www.w3.org/TR/SVG/struct.html#UseElementWidthAttribute
         if nw.approx_eq_cairo(0.0) || nh.approx_eq_cairo(0.0) {
-            return Ok(());
+            return Ok(draw_ctx.empty_bbox());
         }
 
         let viewport = Rectangle::new(nx, ny, nw, nh);
diff --git a/rsvg_internals/src/text.rs b/rsvg_internals/src/text.rs
index 6e52e3c5..8de8424b 100644
--- a/rsvg_internals/src/text.rs
+++ b/rsvg_internals/src/text.rs
@@ -272,7 +272,7 @@ impl PositionedSpan {
         }
     }
 
-    fn draw(&self, draw_ctx: &mut DrawingCtx, clipping: bool) -> Result<(), RenderingError> {
+    fn draw(&self, draw_ctx: &mut DrawingCtx, clipping: bool) -> Result<BoundingBox, RenderingError> {
         draw_ctx.with_saved_cr(&mut |dc| {
             let cr = dc.get_cairo_context();
 
@@ -281,14 +281,14 @@ impl PositionedSpan {
             let gravity = self.layout.get_context().unwrap().get_gravity();
             let bbox = self.compute_text_bbox(&affine, gravity);
             if bbox.is_none() {
-                return Ok(());
+                return Ok(dc.empty_bbox());
             }
 
-            let bbox = bbox.unwrap();
-
-            if !clipping {
-                dc.insert_bbox(&bbox);
-            }
+            let mut bbox = if clipping {
+                dc.empty_bbox()
+            } else {
+                bbox.unwrap()
+            };
 
             cr.set_antialias(cairo::Antialias::from(self.values.text_rendering));
 
@@ -349,13 +349,13 @@ impl PositionedSpan {
                             let ib =
                                 BoundingBox::new(&affine).with_ink_extents(cr.stroke_extents());
                             cr.stroke();
-                            dc.insert_bbox(&ib);
+                            bbox.insert(&ib);
                         }
                     }
                 }
             }
 
-            res
+            res.and_then(|_: ()| Ok(bbox))
         })
     }
 
@@ -609,7 +609,7 @@ impl NodeTrait for NodeText {
         cascaded: &CascadedValues<'_>,
         draw_ctx: &mut DrawingCtx,
         clipping: bool,
-    ) -> Result<(), RenderingError> {
+    ) -> Result<BoundingBox, RenderingError> {
         let values = cascaded.get();
         let params = draw_ctx.get_view_params();
 
@@ -641,13 +641,16 @@ impl NodeTrait for NodeText {
         }
 
         draw_ctx.with_discrete_layer(node, values, clipping, &mut |dc| {
+            let mut bbox = dc.empty_bbox();
+
             for chunk in &positioned_chunks {
                 for span in &chunk.spans {
-                    span.draw(dc, clipping)?;
+                    let span_bbox = span.draw(dc, clipping)?;
+                    bbox.insert(&span_bbox);
                 }
             }
 
-            Ok(())
+            Ok(bbox)
         })
     }
 }



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]