[librsvg: 28/30] filters: do not use BoundingBox to calculate filter bounds

[Federico Mena Quintero <federico src gnome org>]

commit e75f40b15a00a33e0dbfc335227629d258c06605
Author: Paolo Borelli <pborelli gnome org>
Date:   Mon Dec 28 15:03:23 2020 +0100

    filters: do not use BoundingBox to calculate filter bounds
    
    We are building a plain rect, so stick with Rect instead of going
    through the BoundingBox abstraction. This makes code easier to
    follow and even makes it shorter.
    
    I left a FIXME about inversibility of the transform, but not that
    the same panic would already be present also in the bbox code.

 src/filters/bounds.rs  | 68 +++++++++++++++++++++-----------------------------
 src/filters/context.rs | 14 +++++------
 2 files changed, 34 insertions(+), 48 deletions(-)
---
diff --git a/src/filters/bounds.rs b/src/filters/bounds.rs
index 0f56bdcb..3f0bb58b 100644
--- a/src/filters/bounds.rs
+++ b/src/filters/bounds.rs
@@ -1,8 +1,8 @@
 //! Filter primitive subregion computation.
-use crate::bbox::BoundingBox;
 use crate::drawing_ctx::DrawingCtx;
 use crate::length::*;
 use crate::rect::{IRect, Rect};
+use crate::transform::Transform;
 
 use super::context::{FilterContext, FilterInput};
 
@@ -12,8 +12,14 @@ pub struct BoundsBuilder<'a> {
     /// The filter context.
     ctx: &'a FilterContext,
 
-    /// The current bounding box.
-    bbox: BoundingBox,
+    /// The transform to use when generating the rect
+    transform: Transform,
+
+    /// The inverse transform used when adding rects
+    inverse: Transform,
+
+    /// The current bounding rectangle.
+    rect: Option<Rect>,
 
     /// Whether one of the input nodes is standard input.
     standard_input_was_referenced: bool,
@@ -35,10 +41,12 @@ impl<'a> BoundsBuilder<'a> {
         width: Option<ULength<Horizontal>>,
         height: Option<ULength<Vertical>>,
     ) -> Self {
+        // FIXME: we panic if paffine is not invertible... do we need to check here?
         Self {
             ctx,
-            // The transform is paffine because we're using that fact in apply_properties().
-            bbox: BoundingBox::new().with_transform(ctx.paffine()),
+            transform: ctx.paffine(),
+            inverse: ctx.paffine().invert().unwrap(),
+            rect: None,
             standard_input_was_referenced: false,
             x,
             y,
@@ -51,8 +59,7 @@ impl<'a> BoundsBuilder<'a> {
     #[inline]
     pub fn add_input(mut self, input: &FilterInput) -> Self {
         // If a standard input was referenced, the default value is the filter effects region
-        // regardless of other referenced inputs. This means we can skip computing the bounding
-        // box.
+        // regardless of other referenced inputs. This means we can skip computing the bounds.
         if self.standard_input_was_referenced {
             return self;
         }
@@ -62,8 +69,8 @@ impl<'a> BoundsBuilder<'a> {
                 self.standard_input_was_referenced = true;
             }
             FilterInput::PrimitiveOutput(ref output) => {
-                let input_bbox = BoundingBox::new().with_rect(output.bounds.into());
-                self.bbox.insert(&input_bbox);
+                let input_rect = self.inverse.transform_rect(&Rect::from(output.bounds));
+                self.rect = Some(self.rect.map_or(input_rect, |r| input_rect.union(&r)));
             }
         }
 
@@ -72,12 +79,9 @@ impl<'a> BoundsBuilder<'a> {
 
     /// Returns the final exact bounds.
     pub fn into_rect(self, draw_ctx: &mut DrawingCtx) -> Rect {
-        let mut bbox = self.apply_properties(draw_ctx);
-
-        let effects_region = self.ctx.effects_region();
-        bbox.clip(&effects_region);
-
-        bbox.rect.unwrap()
+        self.into_rect_without_clipping(draw_ctx)
+            .intersection(&self.ctx.effects_region())
+            .unwrap_or_else(Rect::default)
     }
 
     /// Returns the final pixel bounds.
@@ -85,35 +89,21 @@ impl<'a> BoundsBuilder<'a> {
         self.into_rect(draw_ctx).into()
     }
 
-    /// Returns the final pixel bounds without clipping to the filter effects region.
-    ///
-    /// Used by feImage.
+    /// Returns the final exact bounds without clipping to the filter effects region.
     pub fn into_rect_without_clipping(self, draw_ctx: &mut DrawingCtx) -> Rect {
-        self.apply_properties(draw_ctx).rect.unwrap()
-    }
-
-    /// Applies the filter primitive properties.
-    fn apply_properties(mut self, draw_ctx: &mut DrawingCtx) -> BoundingBox {
-        if self.bbox.rect.is_none() || self.standard_input_was_referenced {
-            // The default value is the filter effects region.
-            let effects_region = self.ctx.effects_region();
-
-            // Clear out the rect.
-            self.bbox.clear();
-
-            // Convert into the paffine coordinate system.
-            self.bbox.insert(&effects_region);
-        }
+        // The default value is the filter effects region converted into
+        // the paffine coordinate system.
+        let mut rect = match self.rect {
+            Some(r) if !self.standard_input_was_referenced => r,
+            _ => self.inverse.transform_rect(&self.ctx.effects_region()),
+        };
 
         // If any of the properties were specified, we need to respect them.
+        // These replacements are possible because of the paffince coordinate system.
         if self.x.is_some() || self.y.is_some() || self.width.is_some() || self.height.is_some() {
             let params = draw_ctx.push_coord_units(self.ctx.primitive_units());
             let values = self.ctx.get_computed_values_from_node_being_filtered();
 
-            // These replacements are correct only because self.bbox is used with the
-            // paffine transform.
-            let rect = self.bbox.rect.as_mut().unwrap();
-
             if let Some(x) = self.x {
                 let w = rect.width();
                 rect.x0 = x.normalize(values, &params);
@@ -133,8 +123,6 @@ impl<'a> BoundsBuilder<'a> {
         }
 
         // Convert into the surface coordinate system.
-        let mut bbox = BoundingBox::new();
-        bbox.insert(&self.bbox);
-        bbox
+        self.transform.transform_rect(&rect)
     }
 }
diff --git a/src/filters/context.rs b/src/filters/context.rs
index d83eee46..fec42c84 100644
--- a/src/filters/context.rs
+++ b/src/filters/context.rs
@@ -62,7 +62,7 @@ pub struct FilterContext {
     /// Primtive units
     primitive_units: CoordUnits,
     /// The filter effects region.
-    effects_region: BoundingBox,
+    effects_region: Rect,
     /// Whether the currently rendered filter primitive uses linear RGB for color operations.
     ///
     /// This affects `get_input()` and `store_result()` which should perform linearization and
@@ -153,7 +153,7 @@ impl FilterContext {
             let other_bbox = BoundingBox::new().with_rect(rect);
             bbox.clip(&other_bbox);
 
-            bbox
+            bbox.rect.unwrap()
         };
 
         Self {
@@ -260,7 +260,7 @@ impl FilterContext {
 
     /// Returns the filter effects region.
     #[inline]
-    pub fn effects_region(&self) -> BoundingBox {
+    pub fn effects_region(&self) -> Rect {
         self.effects_region
     }
 
@@ -291,7 +291,7 @@ impl FilterContext {
 
             Input::SourceAlpha => self
                 .source_graphic()
-                .extract_alpha(self.effects_region().rect.unwrap().into())
+                .extract_alpha(self.effects_region().into())
                 .map_err(FilterError::CairoError)
                 .map(FilterInput::StandardInput),
 
@@ -303,7 +303,7 @@ impl FilterContext {
                 .background_image(draw_ctx)
                 .and_then(|surface| {
                     surface
-                        .extract_alpha(self.effects_region().rect.unwrap().into())
+                        .extract_alpha(self.effects_region().into())
                         .map_err(FilterError::CairoError)
                 })
                 .map(FilterInput::StandardInput),
@@ -356,9 +356,7 @@ impl FilterContext {
 
         // Convert the input surface to the desired format.
         let (surface, bounds) = match raw {
-            FilterInput::StandardInput(ref surface) => {
-                (surface, self.effects_region().rect.unwrap().into())
-            }
+            FilterInput::StandardInput(ref surface) => (surface, self.effects_region().into()),
             FilterInput::PrimitiveOutput(FilterOutput {
                 ref surface,
                 ref bounds,