[librsvg: 1/3] drawing_ctx: rework visibility control

[Federico Mena Quintero <federico src gnome org>]

commit 786270587a35fc3d1142d47970a9535e9ae3f581
Author: Paolo Borelli <pborelli gnome org>
Date:   Tue Jan 5 10:16:04 2021 +0100

    drawing_ctx: rework visibility control
    
    Distinguish between display=none and visibility=hidden. In the
    first case we avoid recursing into draw() as we were doing before,
    in the latter we call into drawing_ctx to calculate the bounding
    box but we skip actually stroking/filling the cairo path.
    Additionally, the is_displayed check is also added to TRef and
    TSpan elements, since those elements are drawn directly by the
    Text element.

 src/drawing_ctx.rs | 64 +++++++++++++++++++++++++++++++++---------------------
 src/element.rs     |  2 +-
 src/properties.rs  | 16 +++++++++-----
 src/text.rs        |  7 ++++++
 4 files changed, 58 insertions(+), 31 deletions(-)
---
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index 96e624ec..dc8d62e8 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -1254,8 +1254,10 @@ impl DrawingCtx {
                 PathHelper::new(&cr, transform, &shape.path, values.stroke_line_cap());
 
             if clipping {
-                cr.set_fill_rule(cairo::FillRule::from(values.clip_rule()));
-                path_helper.set()?;
+                if values.is_visible() {
+                    cr.set_fill_rule(cairo::FillRule::from(values.clip_rule()));
+                    path_helper.set()?;
+                }
                 return Ok(dc.empty_bbox());
             }
 
@@ -1278,10 +1280,12 @@ impl DrawingCtx {
                         let bbox = bounding_box
                             .get_or_insert_with(|| compute_stroke_and_fill_box(&cr, &values));
 
-                        if target == PaintTarget::Stroke {
-                            dc.stroke(&cr, an, values, &bbox, current_color)?;
-                        } else {
-                            dc.fill(&cr, an, values, &bbox, current_color)?;
+                        if values.is_visible() {
+                            if target == PaintTarget::Stroke {
+                                dc.stroke(&cr, an, values, &bbox, current_color)?;
+                            } else {
+                                dc.fill(&cr, an, values, &bbox, current_color)?;
+                            }
                         }
                     }
                     PaintTarget::Markers if shape.markers == Markers::Yes => {
@@ -1297,6 +1301,26 @@ impl DrawingCtx {
         })
     }
 
+    fn paint_surface(&mut self, surface: &SharedImageSurface, width: f64, height: f64) {
+        let cr = self.cr.clone();
+
+        // We need to set extend appropriately, so can't use cr.set_source_surface().
+        //
+        // If extend is left at its default value (None), then bilinear scaling uses
+        // transparency outside of the image producing incorrect results.
+        // For example, in svg1.1/filters-blend-01-b.svgthere's a completely
+        // opaque 100×1 image of a gradient scaled to 100×98 which ends up
+        // transparent almost everywhere without this fix (which it shouldn't).
+        let ptn = surface.to_cairo_pattern();
+        ptn.set_extend(cairo::Extend::Pad);
+        cr.set_source(&ptn);
+
+        // Clip is needed due to extend being set to pad.
+        clip_to_rectangle(&cr, &Rect::from_size(width, height));
+
+        cr.paint();
+    }
+
     pub fn draw_image(
         &mut self,
         surface: &SharedImageSurface,
@@ -1326,23 +1350,9 @@ impl DrawingCtx {
         self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |_an, dc| {
             dc.with_saved_cr(&mut |dc| {
                 if let Some(_params) = dc.push_new_viewport(Some(vbox), rect, aspect, clip_mode) {
-                    let cr = dc.cr.clone();
-
-                    // We need to set extend appropriately, so can't use cr.set_source_surface().
-                    //
-                    // If extend is left at its default value (None), then bilinear scaling uses
-                    // transparency outside of the image producing incorrect results.
-                    // For example, in svg1.1/filters-blend-01-b.svgthere's a completely
-                    // opaque 100×1 image of a gradient scaled to 100×98 which ends up
-                    // transparent almost everywhere without this fix (which it shouldn't).
-                    let ptn = surface.to_cairo_pattern();
-                    ptn.set_extend(cairo::Extend::Pad);
-                    cr.set_source(&ptn);
-
-                    // Clip is needed due to extend being set to pad.
-                    clip_to_rectangle(&cr, &Rect::from_size(image_width, image_height));
-
-                    cr.paint();
+                    if values.is_visible() {
+                        dc.paint_surface(surface, image_width, image_height);
+                    }
                 }
 
                 // The bounding box for <image> is decided by the values of x, y, w, h
@@ -1402,7 +1412,9 @@ impl DrawingCtx {
                 .map(|had_paint_server| {
                     if had_paint_server {
                         pangocairo::functions::update_layout(&cr, &layout);
-                        pangocairo::functions::show_layout(&cr, &layout);
+                        if values.is_visible() {
+                            pangocairo::functions::show_layout(&cr, &layout);
+                        }
                     };
                 })
             } else {
@@ -1440,8 +1452,10 @@ impl DrawingCtx {
                         let ib = BoundingBox::new()
                             .with_transform(transform)
                             .with_ink_rect(r);
-                        cr.stroke();
                         bbox.insert(&ib);
+                        if values.is_visible() {
+                            cr.stroke();
+                        }
                     }
                 }
             }
diff --git a/src/element.rs b/src/element.rs
index a417046f..de16286d 100644
--- a/src/element.rs
+++ b/src/element.rs
@@ -309,7 +309,7 @@ impl<T: SetAttributes + Draw> Draw for ElementInner<T> {
     ) -> Result<BoundingBox, RenderingError> {
         if !self.is_in_error() {
             let values = cascaded.get();
-            if values.is_visible() {
+            if values.is_displayed() {
                 draw_ctx.with_saved_transform(Some(self.get_transform()), &mut |dc| {
                     self.element_impl
                         .draw(node, acquired_nodes, cascaded, dc, clipping)
diff --git a/src/properties.rs b/src/properties.rs
index 2009f1e7..4009766c 100644
--- a/src/properties.rs
+++ b/src/properties.rs
@@ -109,12 +109,18 @@ impl ComputedValues {
         matches!(self.overflow(), Overflow::Auto | Overflow::Visible)
     }
 
+    /// Whether we should draw the element or skip both space allocation
+    /// and drawing.
+    /// https://www.w3.org/TR/SVG2/render.html#VisibilityControl
+    pub fn is_displayed(&self) -> bool {
+        self.display() != Display::None
+    }
+
+    /// Whether we should draw the element or allocate its space but
+    /// skip drawing.
+    /// https://www.w3.org/TR/SVG2/render.html#VisibilityControl
     pub fn is_visible(&self) -> bool {
-        match (self.display(), self.visibility()) {
-            (Display::None, _) => false,
-            (_, Visibility::Visible) => true,
-            _ => false,
-        }
+        self.visibility() == Visibility::Visible
     }
 }
 
diff --git a/src/text.rs b/src/text.rs
index 18b8fa06..2b02f43a 100644
--- a/src/text.rs
+++ b/src/text.rs
@@ -544,7 +544,11 @@ impl TRef {
         }
 
         let link = self.link.as_ref().unwrap();
+
         let values = cascaded.get();
+        if !values.is_displayed() {
+            return;
+        }
 
         if let Ok(acquired) = acquired_nodes.acquire(link) {
             let c = acquired.get();
@@ -613,6 +617,9 @@ impl TSpan {
         depth: usize,
     ) {
         let values = cascaded.get();
+        if !values.is_displayed() {
+            return;
+        }
 
         let params = draw_ctx.get_view_params();
         let x = self.x.map(|l| l.normalize(&values, &params));