[librsvg] Don't recurse into get_dimensions() from get_node_geometry()

[Federico Mena Quintero <federico src gnome org>]

commit bb68856d5776e003454f65b5cd01b4267e397b8c
Author: Federico Mena Quintero <federico gnome org>
Date:   Tue Feb 19 11:43:16 2019 -0600

    Don't recurse into get_dimensions() from get_node_geometry()
    
    As an experiment to remove this recursion, this commit does a few
    things:
    
    * Makes get_node_geometry() not call get_dimensions().  This was a
      semi-recursive call when the toplevel SVG has
      width/height="100%" (or any other percentage-based size), and no
      viewBox.  Instead, start with a 1x1 viewport that matches the
      temporary surface used to run the rendering/measuring loop.
    
    * Make the case where an SVG has no dimensions (no width/height, no
      viewBox, no objects at all) *not* a rendering error.  Remove
      RenderingError::SvgHasNoSize and just render nothing.
    
    This also makes tests/render-crash not check the result of
    rsvg_handle_render_cairo().
    
    We are just checking whether the rendering doesn't crash; not that
    files actually render without errors.
    
    For the same reason, don't even try to figure out the SVG's
    dimensions; just render to a constant-sized surface.

 rsvg_internals/src/error.rs  |  3 ---
 rsvg_internals/src/handle.rs | 22 ++++++++++++++--------
 tests/render-crash.c         | 31 +++++++++++--------------------
 3 files changed, 25 insertions(+), 31 deletions(-)
---
diff --git a/rsvg_internals/src/error.rs b/rsvg_internals/src/error.rs
index 48c86db8..8fd518d1 100644
--- a/rsvg_internals/src/error.rs
+++ b/rsvg_internals/src/error.rs
@@ -104,7 +104,6 @@ pub enum RenderingError {
     InstancingLimit,
     InvalidId(DefsLookupErrorKind),
     InvalidHref,
-    SvgHasNoSize,
     OutOfMemory,
 }
 
@@ -243,7 +242,6 @@ impl error::Error for RenderingError {
             RenderingError::InstancingLimit => "instancing limit",
             RenderingError::InvalidId(_) => "invalid id",
             RenderingError::InvalidHref => "invalid href",
-            RenderingError::SvgHasNoSize => "svg has no size",
             RenderingError::OutOfMemory => "out of memory",
         }
     }
@@ -257,7 +255,6 @@ impl fmt::Display for RenderingError {
             RenderingError::CircularReference
             | RenderingError::InstancingLimit
             | RenderingError::InvalidHref
-            | RenderingError::SvgHasNoSize
             | RenderingError::OutOfMemory => write!(f, "{}", self.description()),
         }
     }
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index 8e8b7742..3ad1c8cd 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -334,13 +334,7 @@ impl Handle {
 
         self.in_loop.set(false);
 
-        res.and_then(|dimensions| {
-            if dimensions.width == 0 || dimensions.height == 0 {
-                Err(RenderingError::SvgHasNoSize)
-            } else {
-                Ok(dimensions)
-            }
-        })
+        res
     }
 
     fn get_dimensions_sub(&self, id: Option<&str>) -> Result<RsvgDimensionData, RenderingError> {
@@ -388,7 +382,14 @@ impl Handle {
         &self,
         node: &RsvgNode,
     ) -> Result<(RsvgRectangle, RsvgRectangle), RenderingError> {
-        let dimensions = self.get_dimensions()?; // replace by 1,1,1,1
+        // This is just to start with an unknown viewport size
+        let dimensions = RsvgDimensionData {
+            width: 1,
+            height: 1,
+            em: 1.0,
+            ex: 1.0,
+        };
+
         let target = ImageSurface::create(cairo::Format::Rgb24, 1, 1)?;
         let cr = cairo::Context::new(&target);
         let mut draw_ctx = self.create_drawing_ctx_for_node(&cr, &dimensions, Some(node));
@@ -507,6 +508,11 @@ impl Handle {
         let dimensions = self.get_dimensions()?;
         let root = self.get_root();
 
+        if dimensions.width == 0 || dimensions.height == 0 {
+            // nothing to render
+            return Ok(())
+        }
+
         cr.save();
         let mut draw_ctx = self.create_drawing_ctx_for_node(cr, &dimensions, node.as_ref());
         let res = draw_ctx.draw_node_from_stack(&root.get_cascaded_values(), &root, false);
diff --git a/tests/render-crash.c b/tests/render-crash.c
index 91f99804..00250cd7 100644
--- a/tests/render-crash.c
+++ b/tests/render-crash.c
@@ -1,4 +1,5 @@
-/* vim: set ts=4 nowrap ai expandtab sw=4: */
+/* vim: set sw=4 sts=4: -*- Mode: C; indent-tabs-mode: t; c-basic-offset: 4; tab-width: 8 -*-
+ */
 
 #define RSVG_DISABLE_DEPRECATION_WARNINGS
 
@@ -12,7 +13,6 @@ test_render_crash (gconstpointer data)
     GFile *file = G_FILE (data);
     RsvgHandle *handle;
     GError *error = NULL;
-    RsvgDimensionData dimensions;
     cairo_surface_t *surface;
     cairo_t *cr;
 
@@ -20,27 +20,18 @@ test_render_crash (gconstpointer data)
     g_assert_no_error (error);
     g_assert (handle != NULL);
 
-    /* rsvg_handle_get_dimensions_sub has a return value we can check */
-    if (rsvg_handle_get_dimensions_sub (handle, &dimensions, NULL)) {
-        RsvgDimensionData dimensions2;
+    surface = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, 100, 100);
+    cr = cairo_create (surface);
 
-        g_assert_cmpint (dimensions.width, >, 0);
-        g_assert_cmpint (dimensions.height, >, 0);
+    g_assert (cairo_status (cr) == CAIRO_STATUS_SUCCESS);
 
-        rsvg_handle_get_dimensions (handle, &dimensions2);
-        g_assert_cmpint (dimensions2.width, ==, dimensions.width);
-        g_assert_cmpint (dimensions2.height, ==, dimensions.height);
+    /* We don't even check the return value of the following function; we are just
+     * trying to check that there are no crashes in the rendering code.
+     */
+    rsvg_handle_render_cairo (handle, cr);
 
-        surface = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, dimensions.width, dimensions.height);
-        cr = cairo_create (surface);
-        g_assert (rsvg_handle_render_cairo (handle, cr));
-
-        cairo_surface_destroy (surface);
-        cairo_destroy (cr);
-    } else {
-        g_assert_cmpint (dimensions.width, ==, 0);
-        g_assert_cmpint (dimensions.height, ==, 0);
-    }
+    cairo_destroy (cr);
+    cairo_surface_destroy (surface);
 
     g_object_unref (handle);
 }