Merge branch 'text' into 'master'

text: skip tspan and tref if they are not visible

Closes #667

See merge request GNOME/librsvg!456

6 files changed, 84 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));
diff --git a/tests/fixtures/reftests/bugs/667-tspan-visibility-ref.png b/tests/fixtures/reftests/bugs/667-tspan-visibility-ref.png
new file mode 100644
index 00000000..87de44f7
--- /dev/null
+++ b/tests/fixtures/reftests/bugs/667-tspan-visibility-ref.png
diff --git a/tests/fixtures/reftests/bugs/667-tspan-visibility.svg b/tests/fixtures/reftests/bugs/667-tspan-visibility.svg
new file mode 100644
index 00000000..7919fab3
--- /dev/null
+++ b/tests/fixtures/reftests/bugs/667-tspan-visibility.svg
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<svg viewBox="0 0 300 300" xmlns="http://www.w3.org/2000/svg" version="1.1">
+  <g>
+    <!-- text and tspans all visible -->
+    <text x="10" y="10" font-family="sans" font-size="12">
+      foo 
+      <tspan>bar</tspan>
+      <tspan>baz</tspan>
+    </text>
+
+    <!-- tspan with display none -->
+    <text x="10" y="50" font-family="sans" font-size="12">
+      foo 
+      <tspan display="none">bar</tspan>
+      <tspan>baz</tspan>
+    </text>
+
+    <!-- tspan with visibility hidden -->
+    <text x="10" y="100" font-family="sans" font-size="12">
+      foo 
+      <tspan visibility="hidden">bar</tspan>
+      <tspan>baz</tspan>
+    </text>
+
+  </g>
+</svg>