GNOME.org
 

[librsvg: 29/30] filters: do not store a FilterCtx ref in the bounds builder.


commit e3c18437ded8db48e536d8e78baf9cf8de494709
Author: Paolo Borelli <pborelli gnome org>
Date:   Mon Dec 28 18:30:08 2020 +0100

    filters: do not store a FilterCtx ref in the bounds builder.
    
    In general I think it us good to untangle Objects with refs to
    other objects. Also in this way we can check transform in the
    caller and return an error instead of panic.
    
    Also change into_rect to return both the clipped and unclipped
    rect: feImage is the only caller and needs both, so avoid the
    need of cloning and calling two methods.

 src/filters/blend.rs              |  2 +-
 src/filters/bounds.rs             | 74 ++++++++++++++++++---------------------
 src/filters/color_matrix.rs       |  2 +-
 src/filters/component_transfer.rs |  2 +-
 src/filters/composite.rs          |  2 +-
 src/filters/convolve_matrix.rs    |  2 +-
 src/filters/displacement_map.rs   |  2 +-
 src/filters/flood.rs              |  2 +-
 src/filters/gaussian_blur.rs      |  2 +-
 src/filters/image.rs              |  4 +--
 src/filters/lighting.rs           |  2 +-
 src/filters/merge.rs              |  2 +-
 src/filters/mod.rs                | 15 +++++---
 src/filters/morphology.rs         |  2 +-
 src/filters/offset.rs             |  2 +-
 src/filters/tile.rs               |  2 +-
 src/filters/turbulence.rs         |  2 +-
 17 files changed, 61 insertions(+), 60 deletions(-)
---
diff --git a/src/filters/blend.rs b/src/filters/blend.rs
index deace8d1..5f73d6b9 100755
--- a/src/filters/blend.rs
+++ b/src/filters/blend.rs
@@ -85,7 +85,7 @@ impl FilterEffect for FeBlend {
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .add_input(&input_2)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let surface =
             input
diff --git a/src/filters/bounds.rs b/src/filters/bounds.rs
index 3f0bb58b..bdfd6885 100644
--- a/src/filters/bounds.rs
+++ b/src/filters/bounds.rs
@@ -7,10 +7,12 @@ use crate::transform::Transform;
 use super::context::{FilterContext, FilterInput};
 
 /// A helper type for filter primitive subregion computation.
-#[derive(Clone, Copy)]
-pub struct BoundsBuilder<'a> {
-    /// The filter context.
-    ctx: &'a FilterContext,
+pub struct BoundsBuilder {
+    /// Filter primitive properties.
+    x: Option<Length<Horizontal>>,
+    y: Option<Length<Vertical>>,
+    width: Option<ULength<Horizontal>>,
+    height: Option<ULength<Vertical>>,
 
     /// The transform to use when generating the rect
     transform: Transform,
@@ -18,40 +20,33 @@ pub struct BoundsBuilder<'a> {
     /// 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,
 
-    /// Filter primitive properties.
-    x: Option<Length<Horizontal>>,
-    y: Option<Length<Vertical>>,
-    width: Option<ULength<Horizontal>>,
-    height: Option<ULength<Vertical>>,
+    /// The current bounding rectangle.
+    rect: Option<Rect>,
 }
 
-impl<'a> BoundsBuilder<'a> {
+impl BoundsBuilder {
     /// Constructs a new `BoundsBuilder`.
     #[inline]
     pub fn new(
-        ctx: &'a FilterContext,
         x: Option<Length<Horizontal>>,
         y: Option<Length<Vertical>>,
         width: Option<ULength<Horizontal>>,
         height: Option<ULength<Vertical>>,
+        transform: Transform,
     ) -> Self {
-        // FIXME: we panic if paffine is not invertible... do we need to check here?
+        // We panic if transform is not invertible. This is checked in the caller.
         Self {
-            ctx,
-            transform: ctx.paffine(),
-            inverse: ctx.paffine().invert().unwrap(),
-            rect: None,
-            standard_input_was_referenced: false,
             x,
             y,
             width,
             height,
+            transform,
+            inverse: transform.invert().unwrap(),
+            standard_input_was_referenced: false,
+            rect: None,
         }
     }
 
@@ -77,32 +72,22 @@ impl<'a> BoundsBuilder<'a> {
         self
     }
 
-    /// Returns the final exact bounds.
-    pub fn into_rect(self, draw_ctx: &mut DrawingCtx) -> Rect {
-        self.into_rect_without_clipping(draw_ctx)
-            .intersection(&self.ctx.effects_region())
-            .unwrap_or_else(Rect::default)
-    }
+    /// Returns the final exact bounds, both with and without clipping to the effects region.
+    pub fn into_rect(self, ctx: &FilterContext, draw_ctx: &mut DrawingCtx) -> (Rect, Rect) {
+        let effects_region = ctx.effects_region();
 
-    /// Returns the final pixel bounds.
-    pub fn into_irect(self, draw_ctx: &mut DrawingCtx) -> IRect {
-        self.into_rect(draw_ctx).into()
-    }
-
-    /// Returns the final exact bounds without clipping to the filter effects region.
-    pub fn into_rect_without_clipping(self, draw_ctx: &mut DrawingCtx) -> Rect {
         // The default value is the filter effects region converted into
-        // the paffine coordinate system.
+        // the ptimitive 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()),
+            _ => self.inverse.transform_rect(&effects_region),
         };
 
         // If any of the properties were specified, we need to respect them.
-        // These replacements are possible because of the paffince coordinate system.
+        // These replacements are possible because of the primitive 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();
+            let params = draw_ctx.push_coord_units(ctx.primitive_units());
+            let values = ctx.get_computed_values_from_node_being_filtered();
 
             if let Some(x) = self.x {
                 let w = rect.width();
@@ -123,6 +108,17 @@ impl<'a> BoundsBuilder<'a> {
         }
 
         // Convert into the surface coordinate system.
-        self.transform.transform_rect(&rect)
+        let unclipped_rect = self.transform.transform_rect(&rect);
+
+        let clipped_rect = unclipped_rect
+            .intersection(&effects_region)
+            .unwrap_or_else(Rect::default);
+
+        (clipped_rect, unclipped_rect)
+    }
+
+    /// Returns the final pixel bounds, clipped to the effects region.
+    pub fn into_irect(self, ctx: &FilterContext, draw_ctx: &mut DrawingCtx) -> IRect {
+        self.into_rect(ctx, draw_ctx).0.into()
     }
 }
diff --git a/src/filters/color_matrix.rs b/src/filters/color_matrix.rs
index e7046316..87ee3e5b 100644
--- a/src/filters/color_matrix.rs
+++ b/src/filters/color_matrix.rs
@@ -150,7 +150,7 @@ impl FilterEffect for FeColorMatrix {
             .base
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let mut surface = ExclusiveImageSurface::new(
             ctx.source_graphic().width(),
diff --git a/src/filters/component_transfer.rs b/src/filters/component_transfer.rs
index 696082d2..280cc63b 100644
--- a/src/filters/component_transfer.rs
+++ b/src/filters/component_transfer.rs
@@ -289,7 +289,7 @@ impl FilterEffect for FeComponentTransfer {
             .base
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         // Create the output surface.
         let mut surface = ExclusiveImageSurface::new(
diff --git a/src/filters/composite.rs b/src/filters/composite.rs
index 1f89f200..3f219659 100644
--- a/src/filters/composite.rs
+++ b/src/filters/composite.rs
@@ -85,7 +85,7 @@ impl FilterEffect for FeComposite {
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .add_input(&input_2)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let surface = if self.operator == Operator::Arithmetic {
             input.surface().compose_arithmetic(
diff --git a/src/filters/convolve_matrix.rs b/src/filters/convolve_matrix.rs
index c7052d33..b333b01c 100644
--- a/src/filters/convolve_matrix.rs
+++ b/src/filters/convolve_matrix.rs
@@ -134,7 +134,7 @@ impl FilterEffect for FeConvolveMatrix {
             .base
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
         let original_bounds = bounds;
 
         let target_x = match self.target_x {
diff --git a/src/filters/displacement_map.rs b/src/filters/displacement_map.rs
index 4eddfef0..ab7c38a5 100644
--- a/src/filters/displacement_map.rs
+++ b/src/filters/displacement_map.rs
@@ -82,7 +82,7 @@ impl FilterEffect for FeDisplacementMap {
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .add_input(&displacement_input)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         // Displacement map's values need to be non-premultiplied.
         let displacement_surface = displacement_input.surface().unpremultiply(bounds)?;
diff --git a/src/filters/flood.rs b/src/filters/flood.rs
index e9bd333e..89c19bfd 100644
--- a/src/filters/flood.rs
+++ b/src/filters/flood.rs
@@ -39,7 +39,7 @@ impl FilterEffect for FeFlood {
         let bounds = self
             .base
             .get_bounds(ctx, node.parent().as_ref())?
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let cascaded = CascadedValues::new_from_node(node);
         let values = cascaded.get();
diff --git a/src/filters/gaussian_blur.rs b/src/filters/gaussian_blur.rs
index a973ca00..8f296fe7 100644
--- a/src/filters/gaussian_blur.rs
+++ b/src/filters/gaussian_blur.rs
@@ -198,7 +198,7 @@ impl FilterEffect for FeGaussianBlur {
             .base
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let (std_x, std_y) = self.std_deviation;
         let (std_x, std_y) = ctx.paffine().transform_distance(std_x, std_y);
diff --git a/src/filters/image.rs b/src/filters/image.rs
index 9af47502..0bb82226 100644
--- a/src/filters/image.rs
+++ b/src/filters/image.rs
@@ -127,7 +127,7 @@ impl FilterEffect for FeImage {
         draw_ctx: &mut DrawingCtx,
     ) -> Result<FilterResult, FilterError> {
         let bounds_builder = self.base.get_bounds(ctx, node.parent().as_ref())?;
-        let bounds = bounds_builder.into_rect(draw_ctx);
+        let (bounds, unclipped_bounds) = bounds_builder.into_rect(ctx, draw_ctx);
 
         let href = self.href.as_ref().ok_or(FilterError::InvalidInput)?;
 
@@ -135,8 +135,6 @@ impl FilterEffect for FeImage {
             // if href has a fragment specified, render as a node
             self.render_node(ctx, acquired_nodes, draw_ctx, bounds, &node_id)
         } else {
-            // if there is no fragment, render as an image
-            let unclipped_bounds = bounds_builder.into_rect_without_clipping(draw_ctx);
             self.render_external_image(
                 ctx,
                 acquired_nodes,
diff --git a/src/filters/lighting.rs b/src/filters/lighting.rs
index e77002db..b6a1cfec 100644
--- a/src/filters/lighting.rs
+++ b/src/filters/lighting.rs
@@ -426,7 +426,7 @@ macro_rules! impl_lighting_filter {
                     .base
                     .get_bounds(ctx, node.parent().as_ref())?
                     .add_input(&input)
-                    .into_irect(draw_ctx);
+                    .into_irect(ctx, draw_ctx);
                 let original_bounds = bounds;
 
                 let scale = self
diff --git a/src/filters/merge.rs b/src/filters/merge.rs
index 48990339..edf667a6 100644
--- a/src/filters/merge.rs
+++ b/src/filters/merge.rs
@@ -98,7 +98,7 @@ impl FilterEffect for FeMerge {
             }
         }
 
-        let bounds = bounds.into_irect(draw_ctx);
+        let bounds = bounds.into_irect(ctx, draw_ctx);
 
         // Now merge them all.
         let mut output_surface = None;
diff --git a/src/filters/mod.rs b/src/filters/mod.rs
index 640da576..2cd66bd4 100644
--- a/src/filters/mod.rs
+++ b/src/filters/mod.rs
@@ -137,8 +137,8 @@ impl Primitive {
         &self,
         ctx: &'a FilterContext,
         parent: Option<&Node>,
-    ) -> Result<BoundsBuilder<'a>, FilterError> {
-        let primitiveunits = parent
+    ) -> Result<BoundsBuilder, FilterError> {
+        let primitive_units = parent
             .and_then(|parent| {
                 assert!(parent.is_element());
                 match *parent.borrow_element() {
@@ -149,19 +149,26 @@ impl Primitive {
             .unwrap_or(CoordUnits::UserSpaceOnUse);
 
         // With ObjectBoundingBox, only fractions and percents are allowed.
-        if primitiveunits == CoordUnits::ObjectBoundingBox {
+        if primitive_units == CoordUnits::ObjectBoundingBox {
             check_units(self.x)?;
             check_units(self.y)?;
             check_units(self.width)?;
             check_units(self.height)?;
         }
 
+        let transform = ctx.paffine();
+        if !transform.is_invertible() {
+            return Err(FilterError::InvalidParameter(
+                "transform is not invertible".to_string(),
+            ));
+        }
+
         Ok(BoundsBuilder::new(
-            ctx,
             self.x,
             self.y,
             self.width,
             self.height,
+            transform,
         ))
     }
 }
diff --git a/src/filters/morphology.rs b/src/filters/morphology.rs
index 65848f16..e64c07b2 100644
--- a/src/filters/morphology.rs
+++ b/src/filters/morphology.rs
@@ -77,7 +77,7 @@ impl FilterEffect for FeMorphology {
             .base
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let (rx, ry) = self.radius;
         let (rx, ry) = ctx.paffine().transform_distance(rx, ry);
diff --git a/src/filters/offset.rs b/src/filters/offset.rs
index 5bfe11f9..b4c2e89b 100644
--- a/src/filters/offset.rs
+++ b/src/filters/offset.rs
@@ -58,7 +58,7 @@ impl FilterEffect for FeOffset {
             .base
             .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let (dx, dy) = ctx.paffine().transform_distance(self.dx, self.dy);
 
diff --git a/src/filters/tile.rs b/src/filters/tile.rs
index 7b3eb3fd..2e7466f8 100644
--- a/src/filters/tile.rs
+++ b/src/filters/tile.rs
@@ -42,7 +42,7 @@ impl FilterEffect for FeTile {
         let bounds = self
             .base
             .get_bounds(ctx, node.parent().as_ref())?
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let surface = match input {
             FilterInput::StandardInput(input_surface) => input_surface,
diff --git a/src/filters/turbulence.rs b/src/filters/turbulence.rs
index fadffeb7..583fc49a 100644
--- a/src/filters/turbulence.rs
+++ b/src/filters/turbulence.rs
@@ -334,7 +334,7 @@ impl FilterEffect for FeTurbulence {
         let bounds = self
             .base
             .get_bounds(ctx, node.parent().as_ref())?
-            .into_irect(draw_ctx);
+            .into_irect(ctx, draw_ctx);
 
         let affine = ctx.paffine().invert().unwrap();
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]