[librsvg: 2/5] Fix which computed values are used to resolve lengths in the filter element and primitives

[Federico Mena Quintero <federico src gnome org>]

commit 0a0083957291e5c62f8a2311257ebef5441865d2
Author: Federico Mena Quintero <federico gnome org>
Date:   Fri Apr 9 14:10:13 2021 -0500

    Fix which computed values are used to resolve lengths in the filter element and primitives
    
    Per my understanding of the spec
    https://www.w3.org/TR/filter-effects/#FilterPrimitiveSubRegion
    
    The x/y/width/height for the <filter> element, and for all <feFoo>
    filter primitive elements, are in the coordinate system determined by
    the values of filterUnits and primitiveUnits, respectively.  That is,
    they should use the current viewport or the current element's bounding
    box depending on those values.
    
    However, font-relative units like "em"/"ex" resolve with respect to
    the font-size of the element where they are used (i.e. the <filter> or
    <feFoo>), *not* the element that referenced the <filter>.  I think
    Firefox handles this incorrectly.
    
    In fact, the test filter-effects-region.svg had a comment along those
    lines - only it considered Firefox's behavior to be correct, and
    Chromium to be in error.  I think it is the other way around, and have
    done two things:
    
    * Tweaked the values in that test file to be easier to eyeball.
    
    * Regenerated the reference file per my understanding of what should
      happen in the "cascading" and "cascading-primitive" cases (upper
      right and center right, respectively).
    
    This also makes it obvious that the computed_from_node_being_filtered
    is not needed at all in the filters code.

 src/drawing_ctx.rs                                 |   1 -
 src/filters/mod.rs                                 |  34 ++++++++++++---------
 .../reftests/filter-effects-region-ref.png         | Bin 1659 -> 1669 bytes
 tests/fixtures/reftests/filter-effects-region.svg  |  17 ++++++-----
 4 files changed, 29 insertions(+), 23 deletions(-)
---
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index dc8cccc0..3d1b43d8 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -960,7 +960,6 @@ impl DrawingCtx {
 
                         return filters::render(
                             &filter_node,
-                            values,
                             stroke_paint_source,
                             fill_paint_source,
                             child_surface,
diff --git a/src/filters/mod.rs b/src/filters/mod.rs
index e18154e3..1ffd59a9 100644
--- a/src/filters/mod.rs
+++ b/src/filters/mod.rs
@@ -224,7 +224,6 @@ impl Primitive {
 /// Applies a filter and returns the resulting surface.
 pub fn render(
     filter_node: &Node,
-    computed_from_node_being_filtered: &ComputedValues,
     stroke_paint_source: UserSpacePaintSource,
     fill_paint_source: UserSpacePaintSource,
     source_surface: SharedImageSurface,
@@ -236,19 +235,21 @@ pub fn render(
     let filter_node = &*filter_node;
     assert!(is_element_of_type!(filter_node, Filter));
 
-    if filter_node.borrow_element().is_in_error() {
+    let filter_element = filter_node.borrow_element();
+
+    if filter_element.is_in_error() {
         return Ok(source_surface);
     }
 
-    let filter = borrow_element_as!(filter_node, Filter);
+    let filter_values = filter_element.get_computed_values();
 
-    let values = computed_from_node_being_filtered;
+    let filter = borrow_element_as!(filter_node, Filter);
 
     let resolved_filter = {
         // This is in a temporary scope so we don't leave the coord_units pushed during
         // the execution of all the filter primitives.
         let params = draw_ctx.push_coord_units(filter.get_filter_units());
-        filter.resolve(values, &params)
+        filter.resolve(filter_values, &params)
     };
 
     if let Ok(mut filter_ctx) = FilterContext::new(
@@ -275,20 +276,19 @@ pub fn render(
             // Keep only filter primitives (those that implement the Filter trait)
             .filter(|c| c.borrow_element().as_filter_effect().is_some());
 
-        for c in primitives {
-            let elt = c.borrow_element();
+        for primitive_node in primitives {
+            let elt = primitive_node.borrow_element();
             let filter = elt.as_filter_effect().unwrap();
 
+            let primitive_values = elt.get_computed_values();
+
             let start = Instant::now();
 
             if let Err(err) = filter
-                .resolve(&c)
+                .resolve(&primitive_node)
                 .and_then(|(primitive, params)| {
-                    let resolved_primitive = primitive.resolve(
-                        &filter_ctx,
-                        computed_from_node_being_filtered,
-                        draw_ctx,
-                    )?;
+                    let resolved_primitive =
+                        primitive.resolve(&filter_ctx, primitive_values, draw_ctx)?;
                     Ok((resolved_primitive, params))
                 })
                 .and_then(|(resolved_primitive, params)| {
@@ -307,7 +307,11 @@ pub fn render(
                 })
                 .and_then(|result| filter_ctx.store_result(result))
             {
-                rsvg_log!("(filter primitive {} returned an error: {})", c, err);
+                rsvg_log!(
+                    "(filter primitive {} returned an error: {})",
+                    primitive_node,
+                    err
+                );
 
                 // Exit early on Cairo errors. Continue rendering otherwise.
                 if let FilterError::CairoError(status) = err {
@@ -318,7 +322,7 @@ pub fn render(
             let elapsed = start.elapsed();
             rsvg_log!(
                 "(rendered filter primitive {} in\n    {} seconds)",
-                c,
+                primitive_node,
                 elapsed.as_secs() as f64 + f64::from(elapsed.subsec_nanos()) / 1e9
             );
         }
diff --git a/tests/fixtures/reftests/filter-effects-region-ref.png 
b/tests/fixtures/reftests/filter-effects-region-ref.png
index a5de91fe..bf3086cd 100644
Binary files a/tests/fixtures/reftests/filter-effects-region-ref.png and 
b/tests/fixtures/reftests/filter-effects-region-ref.png differ
diff --git a/tests/fixtures/reftests/filter-effects-region.svg 
b/tests/fixtures/reftests/filter-effects-region.svg
index 577b234c..fcd37944 100644
--- a/tests/fixtures/reftests/filter-effects-region.svg
+++ b/tests/fixtures/reftests/filter-effects-region.svg
@@ -32,15 +32,18 @@
   </filter>
 
   <!-- According to the spec, the "em" here should use the coordinate system
-       of the referencing node. Firefox follows this. Chromium uses the filter node's
-       coordinate system instead. -->
-  <filter id="cascading" filterUnits="userSpaceOnUse" style="font-size: 30pt"
+       of the referencing node, per filterUnits="userSpaceOnUse".  However, the "em" units
+       should resolve with respec to the element's font-size.  Firefox (incorrectly?)
+       uses the font-size of the element that referenced the filter. -->
+  <filter id="cascading" filterUnits="userSpaceOnUse" style="font-size: 20px"
     x="1em" y="1em" width="50" height="50">
     <feFlood flood-color="green"/>
   </filter>
 
-  <!-- Same as above but for the primitive subregion. -->
-  <filter id="cascading-primitive" style="font-size: 30pt">
+  <!-- Same as above but for the default primitiveUnits="userSpaceOnUse"; the primitive's width/height 
default to 100%
+       with respect to the filter region, whose default is filterUnits="objectBoundingBox".
+       Firefox has the same bug here. -->
+  <filter id="cascading-primitive" style="font-size: 20px">
     <feFlood x="1em" y="1em" flood-color="green"/>
   </filter>
 
@@ -64,12 +67,12 @@
     <rect fill="red" x="1" y="1" width="50" height="50" filter="url(#fractions-invalid-primitive)"/>
   </g>
 
-  <g transform="translate(170 20)" style="font-size: 7pt">
+  <g transform="translate(170 20)" style="font-size: 10px">
     <rect fill="red" x="1" y="1" width="50" height="50"/>
     <rect fill="red" x="1" y="1" width="50" height="50" filter="url(#cascading)"/>
   </g>
 
-  <g transform="translate(170 100)" style="font-size: 7pt">
+  <g transform="translate(170 100)" style="font-size: 10px">
     <rect fill="red" x="1" y="1" width="50" height="50"/>
     <rect fill="red" x="1" y="1" width="50" height="50" filter="url(#cascading-primitive)"/>
   </g>