[librsvg: 1/15] Split the logic in get_svg_size into physical dimensions and viewBox

[Federico Mena Quintero <federico src gnome org>]

commit a98ce5261ea8602d6f6e877f854258abfd39ae51
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon Oct 26 19:39:39 2020 -0600

    Split the logic in get_svg_size into physical dimensions and viewBox
    
    We compute a pixel size from physical dimensions immediately.
    Otherwise, if any of width/height are percentages, we defer to a
    second function.
    
    THIS IS A BEHAVIOR CHANGE:  That second function only handles the case
    where width and height are 100%, in which case we return the viewBox
    size.  It used to be that we multiplied percentage width and height by
    the viewBox size, which doesn't really make much sense.
    
    Other cases aren't handled --- different percentages are not
    resolvable here; lack of intrinsic dimensions needs the caller to do
    something else.

 rsvg_internals/src/handle.rs | 94 +++++++++++++++++++++++++++++++++++---------
 tests/api.c                  | 10 ++---
 2 files changed, 80 insertions(+), 24 deletions(-)
---
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index 12824dab..dfdf1543 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -2,6 +2,8 @@
 //!
 //! This module provides the primitives on which the public APIs are implemented.
 
+use float_cmp::approx_eq;
+
 use crate::bbox::BoundingBox;
 use crate::css::{Origin, Stylesheet};
 use crate::document::{AcquiredNodes, Document};
@@ -10,7 +12,6 @@ use crate::drawing_ctx::{draw_tree, DrawingMode, ViewParams};
 use crate::error::{DefsLookupErrorKind, LoadingError, RenderingError};
 use crate::length::*;
 use crate::node::{CascadedValues, Node, NodeBorrow};
-use crate::parsers::Parse;
 use crate::rect::Rect;
 use crate::structure::IntrinsicDimensions;
 use crate::url_resolver::{AllowedUrl, Href, UrlResolver};
@@ -387,34 +388,89 @@ fn unit_rectangle() -> Rect {
 /// The legacy C API can compute an SVG document's size from the
 /// `width`, `height`, and `viewBox` attributes of the toplevel `<svg>`
 /// element.  If these are not available, then the size must be computed
-/// by actually measuring the geometries of elements in the document.
+/// by actually measuring the geometries of elements in the document; this last
+/// case is implemented by the caller.
+///
+/// See https://www.w3.org/TR/css-images-3/#sizing-terms for terminology and logic.
 fn get_svg_size(
     dimensions: &IntrinsicDimensions,
     cascaded: &CascadedValues,
     dpi: Dpi,
 ) -> Option<(f64, f64)> {
-    let values = cascaded.get();
+    size_from_intrinsic_dimensions(dimensions, cascaded, dpi)
+        .or_else(|| size_in_pixels_from_vbox(dimensions))
+}
 
-    // these defaults are per the spec
-    let w = dimensions
-        .width
-        .unwrap_or_else(|| Length::<Horizontal>::parse_str("100%").unwrap());
-    let h = dimensions
-        .height
-        .unwrap_or_else(|| Length::<Vertical>::parse_str("100%").unwrap());
+/// Computes a size from `width` and `height` in physical units, or returns None.
+///
+/// If any of the width/height are percentages, we cannot compute the size here.  Here
+/// just normalize lengths with physical units, or units based on the font size.
+fn size_from_intrinsic_dimensions(
+    dimensions: &IntrinsicDimensions,
+    cascaded: &CascadedValues,
+    dpi: Dpi,
+) -> Option<(f64, f64)> {
+    if let (None, None) = (dimensions.width, dimensions.height) {
+        return None;
+    }
 
-    match (w, h, dimensions.vbox) {
-        (w, h, Some(vbox)) => {
-            let params = ViewParams::new(dpi, vbox.width(), vbox.height());
+    let w = dimensions.width.unwrap();
+    let h = dimensions.height.unwrap();
 
-            Some((w.normalize(values, &params), h.normalize(values, &params)))
-        }
+    use crate::length::LengthUnit::*;
 
-        (w, h, None) if w.unit != LengthUnit::Percent && h.unit != LengthUnit::Percent => {
-            let params = ViewParams::new(dpi, 0.0, 0.0);
+    if w.unit == Percent || h.unit == Percent {
+        return None;
+    }
 
-            Some((w.normalize(values, &params), h.normalize(values, &params)))
+    let params = ViewParams::new(dpi, 0.0, 0.0);
+    let values = cascaded.get();
+
+    Some((w.normalize(values, &params), h.normalize(values, &params)))
+}
+
+/// If the width and height do not exist (so default to 100% per the spec) or if they
+/// exist and are in percentage units, computes a size equal to the `viewBox`'s aspect
+/// ratio if it exists, or else returns None.
+///
+/// For example, a `viewBox="0 0 100 200"` will yield `Some(100.0, 200.0)`.
+///
+/// Note that this only checks that the width and height are in percentage units, but
+/// it actually ignores their values.  This is because at the point this function is
+/// called, there is no viewport to embed the SVG document in, so those percentage
+/// units cannot be resolved against anything in particular.  The idea is to return
+/// some dimensions with the correct aspect ratio.
+fn size_in_pixels_from_vbox(dim: &IntrinsicDimensions) -> Option<(f64, f64)> {
+    let IntrinsicDimensions {
+        width,
+        height,
+        vbox,
+    } = *dim;
+
+    use crate::length::LengthUnit::*;
+
+    // If width and height don't exist, or if they are both 100%, just use the vbox size as a pixel size.
+    // This gives a size with the correct aspect ratio.
+
+    match (width, height, vbox) {
+        (None, None, Some(vbox)) => Some((vbox.width(), vbox.height())),
+
+        (
+            Some(Length {
+                length: w,
+                unit: Percent,
+                ..
+            }),
+            Some(Length {
+                length: h,
+                unit: Percent,
+                ..
+            }),
+            Some(vbox),
+        ) if approx_eq!(f64, w, 1.0) && approx_eq!(f64, h, 1.0) => {
+            Some((vbox.width(), vbox.height()))
         }
-        (_, _, _) => None,
+
+        _ => None,
     }
 }
diff --git a/tests/api.c b/tests/api.c
index eb7cc115..c344d32b 100644
--- a/tests/api.c
+++ b/tests/api.c
@@ -1488,28 +1488,28 @@ test_dimensions (DimensionsFixtureData *fixture)
 static DimensionsFixtureData dimensions_fixtures[] =
 {
     {
-        "/dimensions/no viewbox, width and height",
+        "/dimensions/viewbox_only",
         "dimensions/bug608102.svg",
         NULL,
         0, 0, 16, 16,
         FALSE, TRUE
     },
     {
-        "/dimensions/100% width and height",
+        "/dimensions/hundred_percent_width_and_height",
         "dimensions/bug612951.svg",
         NULL,
         0, 0, 47, 47.14,
         FALSE, TRUE
     },
     {
-        "/dimensions/viewbox only",
+        "/dimensions/viewbox_only_2",
         "dimensions/bug614018.svg",
         NULL,
         0, 0, 972, 546,
         FALSE, TRUE
     },
     {
-        "/dimensions/sub/rect no unit",
+        "/dimensions/sub/rect_no_unit",
         "dimensions/sub-rect-no-unit.svg",
         "#rect-no-unit",
         0, 0, 44, 45,
@@ -1523,7 +1523,7 @@ static DimensionsFixtureData dimensions_fixtures[] =
         TRUE, FALSE
     },
     {
-        "/dimensions/with-viewbox",
+        "/dimensions/with_viewbox",
         "dimensions/521-with-viewbox.svg",
         "#foo",
         50.0, 60.0, 70.0, 80.0,