[librsvg] Remove DrawingCtx::pop_view_box() in favor of an impl Drop for it

[Federico Mena Quintero <federico src gnome org>]

commit 063066aecb171997815933c571cba89f6aa88d1e
Author: Federico Mena Quintero <federico gnome org>
Date:   Tue Sep 4 18:10:56 2018 -0500

    Remove DrawingCtx::pop_view_box() in favor of an impl Drop for it
    
    ViewParams and DrawingCtx now store an Rc<RefCell<...>> of the
    view_box_stack.
    
    Thanks to Antoni Boucher for the suggestion on how to avoid lifetimes here.

 rsvg_internals/src/drawing_ctx.rs     | 41 +++++++++++++++++++++--------------
 rsvg_internals/src/filters/context.rs |  5 -----
 rsvg_internals/src/gradient.rs        |  8 -------
 rsvg_internals/src/marker.rs          |  2 --
 rsvg_internals/src/mask.rs            |  8 -------
 rsvg_internals/src/pattern.rs         | 17 ++-------------
 rsvg_internals/src/viewport.rs        |  2 --
 7 files changed, 27 insertions(+), 56 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index c9ced2c1..d5049ca9 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -48,6 +48,7 @@ pub struct ViewParams {
     dpi_y: f64,
     view_box_width: f64,
     view_box_height: f64,
+    view_box_stack: Option<Rc<RefCell<Vec<ViewBox>>>>,
 }
 
 impl ViewParams {
@@ -58,6 +59,7 @@ impl ViewParams {
             dpi_y,
             view_box_width,
             view_box_height,
+            view_box_stack: None,
         }
     }
 
@@ -78,6 +80,14 @@ impl ViewParams {
     }
 }
 
+impl Drop for ViewParams {
+    fn drop(&mut self) {
+        if let Some(ref stack) = self.view_box_stack {
+            stack.borrow_mut().pop();
+        }
+    }
+}
+
 pub enum RsvgDrawingCtx {}
 
 pub struct DrawingCtx<'a> {
@@ -103,7 +113,7 @@ pub struct DrawingCtx<'a> {
 
     surfaces_stack: Vec<cairo::ImageSurface>,
 
-    view_box_stack: RefCell<Vec<ViewBox>>,
+    view_box_stack: Rc<RefCell<Vec<ViewBox>>>,
 
     bbox: BoundingBox,
     bbox_stack: Vec<BoundingBox>,
@@ -160,7 +170,7 @@ impl<'a> DrawingCtx<'a> {
             cr: cr.clone(),
             initial_cr: cr.clone(),
             surfaces_stack: Vec::new(),
-            view_box_stack: RefCell::new(view_box_stack),
+            view_box_stack: Rc::new(RefCell::new(view_box_stack)),
             bbox: BoundingBox::new(&affine),
             bbox_stack: Vec::new(),
             drawsub_stack: Vec::new(),
@@ -211,33 +221,32 @@ impl<'a> DrawingCtx<'a> {
         }
     }
 
-    // FIXME: the result of this is only valid within a
-    // push_view_box() / pop_view_box() pair.  How do we make this
-    // safe?
     pub fn get_view_params(&self) -> ViewParams {
         let view_box_stack = self.view_box_stack.borrow();
         let last = view_box_stack.len() - 1;
+        let stack_top = &view_box_stack[last];
 
         ViewParams {
             dpi_x: self.dpi_x,
             dpi_y: self.dpi_y,
-            view_box_width: view_box_stack[last].0.width,
-            view_box_height: view_box_stack[last].0.height,
+            view_box_width: stack_top.0.width,
+            view_box_height: stack_top.0.height,
+            view_box_stack: None,
         }
     }
 
     pub fn push_view_box(&self, width: f64, height: f64) -> ViewParams {
-        {
-            let mut view_box_stack = self.view_box_stack.borrow_mut();
+        self.view_box_stack
+            .borrow_mut()
+            .push(ViewBox::new(0.0, 0.0, width, height));
 
-            view_box_stack.push(ViewBox::new(0.0, 0.0, width, height));
+        ViewParams {
+            dpi_x: self.dpi_x,
+            dpi_y: self.dpi_y,
+            view_box_width: width,
+            view_box_height: height,
+            view_box_stack: Some(self.view_box_stack.clone()),
         }
-
-        self.get_view_params()
-    }
-
-    pub fn pop_view_box(&self) {
-        self.view_box_stack.borrow_mut().pop();
     }
 
     pub fn insert_bbox(&mut self, bbox: &BoundingBox) {
diff --git a/rsvg_internals/src/filters/context.rs b/rsvg_internals/src/filters/context.rs
index 1a5aecbc..90a2bb4e 100644
--- a/rsvg_internals/src/filters/context.rs
+++ b/rsvg_internals/src/filters/context.rs
@@ -149,10 +149,6 @@ fn compute_effects_region(
         }
     };
 
-    if filter.filterunits.get() == CoordUnits::ObjectBoundingBox {
-        draw_ctx.pop_view_box();
-    }
-
     let other_bbox = BoundingBox::new(&affine).with_rect(Some(rect));
 
     // At this point all of the previous viewbox and matrix business gets converted to pixel
@@ -447,7 +443,6 @@ impl FilterContext {
         if filter.primitiveunits.get() == CoordUnits::ObjectBoundingBox {
             let _params = draw_ctx.push_view_box(1.0, 1.0);
             let rv = f(Box::new(Length::get_unitless));
-            draw_ctx.pop_view_box();
 
             rv
         } else {
diff --git a/rsvg_internals/src/gradient.rs b/rsvg_internals/src/gradient.rs
index bcde6f06..ae219f81 100644
--- a/rsvg_internals/src/gradient.rs
+++ b/rsvg_internals/src/gradient.rs
@@ -516,10 +516,6 @@ fn set_linear_gradient_on_pattern(
                 y2.as_ref().unwrap().normalize(values, &params),
             );
 
-            if units == GradientUnits(CoordUnits::ObjectBoundingBox) {
-                draw_ctx.pop_view_box();
-            }
-
             set_common_on_pattern(gradient, draw_ctx, &mut pattern, bbox, opacity);
         }
     } else {
@@ -603,10 +599,6 @@ fn set_radial_gradient_on_pattern(
 
             let mut pattern = cairo::RadialGradient::new(new_fx, new_fy, 0.0, n_cx, n_cy, n_r);
 
-            if units == GradientUnits(CoordUnits::ObjectBoundingBox) {
-                draw_ctx.pop_view_box();
-            }
-
             set_common_on_pattern(gradient, draw_ctx, &mut pattern, bbox, opacity);
         }
     } else {
diff --git a/rsvg_internals/src/marker.rs b/rsvg_internals/src/marker.rs
index fd2269ca..d2d0ddc9 100644
--- a/rsvg_internals/src/marker.rs
+++ b/rsvg_internals/src/marker.rs
@@ -195,8 +195,6 @@ impl NodeMarker {
             node.draw_children(&cascaded, dc, clipping)
         });
 
-        draw_ctx.pop_view_box();
-
         cr.restore();
 
         res
diff --git a/rsvg_internals/src/mask.rs b/rsvg_internals/src/mask.rs
index 010e8294..8fcece25 100644
--- a/rsvg_internals/src/mask.rs
+++ b/rsvg_internals/src/mask.rs
@@ -95,10 +95,6 @@ impl NodeMask {
             let w = self.width.get().normalize(&values, &params);
             let h = self.height.get().normalize(&values, &params);
 
-            if mask_units == CoordUnits::ObjectBoundingBox {
-                draw_ctx.pop_view_box();
-            }
-
             (x, y, w, h)
         };
 
@@ -156,10 +152,6 @@ impl NodeMask {
                     node.draw_children(&cascaded, dc, false)
                 });
 
-                if content_units == CoordUnits::ObjectBoundingBox {
-                    draw_ctx.pop_view_box();
-                }
-
                 draw_ctx.set_cairo_context(&save_cr);
 
                 res
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index 4eb79f88..a259ffed 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -280,10 +280,6 @@ fn set_pattern_on_draw_context(
         let pattern_width = pattern.width.unwrap().normalize(values, &params);
         let pattern_height = pattern.height.unwrap().normalize(values, &params);
 
-        if units == PatternUnits(CoordUnits::ObjectBoundingBox) {
-            draw_ctx.pop_view_box();
-        }
-
         (pattern_x, pattern_y, pattern_width, pattern_height)
     };
 
@@ -366,9 +362,7 @@ fn set_pattern_on_draw_context(
 
         caffine = cairo::Matrix::new(w / vbox.0.width, 0.0, 0.0, h / vbox.0.height, x, y);
 
-        let params = draw_ctx.push_view_box(vbox.0.width, vbox.0.height);
-        pushed_view_box = true;
-        params
+        draw_ctx.push_view_box(vbox.0.width, vbox.0.height)
     } else if content_units == PatternContentUnits(CoordUnits::ObjectBoundingBox) {
         // If coords are in terms of the bounding box, use them
         let bbrect = bbox.rect.unwrap();
@@ -376,12 +370,9 @@ fn set_pattern_on_draw_context(
         caffine = cairo::Matrix::identity();
         caffine.scale(bbrect.width, bbrect.height);
 
-        let params = draw_ctx.push_view_box(1.0, 1.0);
-        pushed_view_box = true;
-        params
+        draw_ctx.push_view_box(1.0, 1.0)
     } else {
         caffine = cairo::Matrix::identity();
-        pushed_view_box = false;
         draw_ctx.get_view_params()
     };
 
@@ -425,10 +416,6 @@ fn set_pattern_on_draw_context(
 
     draw_ctx.set_cairo_context(&cr_save);
 
-    if pushed_view_box {
-        draw_ctx.pop_view_box();
-    }
-
     // Set the final surface as a Cairo pattern into the Cairo context
 
     let surface_pattern = cairo::SurfacePattern::create(&surface);
diff --git a/rsvg_internals/src/viewport.rs b/rsvg_internals/src/viewport.rs
index 7c3a762b..9d617eba 100644
--- a/rsvg_internals/src/viewport.rs
+++ b/rsvg_internals/src/viewport.rs
@@ -82,8 +82,6 @@ pub fn draw_in_viewport(
 
         let res = draw_fn(dc);
 
-        dc.pop_view_box();
-
         res
     })
 }