[librsvg: 4/15] Move the legacy sizing logic to c_api

[Federico Mena Quintero <federico src gnome org>]

commit 08c68dd881392b374d634773904623a18a4d5b48
Author: Federico Mena Quintero <federico gnome org>
Date:   Tue Oct 27 10:54:02 2020 -0600

    Move the legacy sizing logic to c_api
    
    Export a Handle::get_intrinsic_size_in_pixels(), which resolves
    non-percent lengths for the <svg>'s width/height attributes.
    
    For other dimensions (percentages, nonexitent width/height, etc.), we
    do the legacy logic in c_api.

 Cargo.lock                   |   1 +
 librsvg/Cargo.toml           |   1 +
 librsvg/c_api.rs             | 147 ++++++++++++++++++++++++++++++++++++++-----
 librsvg/pixbuf_utils.rs      |   2 +-
 rsvg_internals/src/handle.rs | 138 ++++++++--------------------------------
 5 files changed, 160 insertions(+), 129 deletions(-)
---
diff --git a/Cargo.lock b/Cargo.lock
index 4c2687be..efa21792 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -815,6 +815,7 @@ dependencies = [
  "cairo-sys-rs",
  "cast",
  "criterion",
+ "float-cmp",
  "gdk-pixbuf",
  "gdk-pixbuf-sys",
  "gio",
diff --git a/librsvg/Cargo.toml b/librsvg/Cargo.toml
index 0032c304..ff94bc87 100644
--- a/librsvg/Cargo.toml
+++ b/librsvg/Cargo.toml
@@ -16,6 +16,7 @@ bitflags = "1.0"
 cairo-rs = { version="0.8.0", features=["v1_16"] }
 cairo-sys-rs = "0.9.0"
 cast = "0.2.3"
+float-cmp = "0.8.0"
 gdk-pixbuf = "0.8.0"
 gdk-pixbuf-sys = "0.9.0"
 glib = "0.9.0"
diff --git a/librsvg/c_api.rs b/librsvg/c_api.rs
index 1a2262c9..edd5c990 100644
--- a/librsvg/c_api.rs
+++ b/librsvg/c_api.rs
@@ -8,14 +8,13 @@ use std::str;
 use std::sync::Once;
 use std::{f64, i32};
 
+use bitflags::bitflags;
+use float_cmp::approx_eq;
 use gdk_pixbuf::Pixbuf;
+use gio::prelude::*;
 use glib::error::ErrorDomain;
 use url::Url;
 
-use bitflags::bitflags;
-
-use gio::prelude::*;
-
 use glib::object::ObjectClass;
 use glib::subclass;
 use glib::subclass::object::ObjectClassSubclassExt;
@@ -32,7 +31,7 @@ use glib::types::instance_of;
 use gobject_sys::{GEnumValue, GFlagsValue};
 
 use rsvg_internals::{
-    rsvg_log, DefsLookupErrorKind, Handle, IntrinsicDimensions, LoadOptions, LoadingError, Rect,
+    rsvg_log, DefsLookupErrorKind, Handle, IntrinsicDimensions, Length, LoadOptions, LoadingError,
     RenderingError, RsvgLength, SharedImageSurface, SurfaceType, UrlResolver, ViewBox,
 };
 
@@ -580,6 +579,21 @@ impl Drop for SizeCallback {
     }
 }
 
+trait CairoRectangleExt {
+    fn from_size(width: f64, height: f64) -> Self;
+}
+
+impl CairoRectangleExt for cairo::Rectangle {
+    fn from_size(width: f64, height: f64) -> Self {
+        Self {
+            x: 0.0,
+            y: 0.0,
+            width,
+            height,
+        }
+    }
+}
+
 impl CHandle {
     pub fn set_base_url(&self, url: &str) {
         let state = self.load_state.borrow();
@@ -816,8 +830,8 @@ impl CHandle {
             .get_geometry_sub(id)
             .and_then(|(ink_r, _)| {
                 // Keep these in sync with tests/src/reference.rs
-                let width = checked_i32(ink_r.width().round())?;
-                let height = checked_i32(ink_r.height().round())?;
+                let width = checked_i32(ink_r.width.round())?;
+                let height = checked_i32(ink_r.height.round())?;
 
                 Ok((ink_r, width, height))
             })
@@ -827,8 +841,8 @@ impl CHandle {
                 RsvgDimensionData {
                     width: w,
                     height: h,
-                    em: ink_r.width(),
-                    ex: ink_r.height(),
+                    em: ink_r.width,
+                    ex: ink_r.height,
                 }
             });
 
@@ -846,8 +860,8 @@ impl CHandle {
 
         self.get_geometry_sub(id)
             .and_then(|(ink_r, _)| {
-                let width = checked_i32(ink_r.width().round())?;
-                let height = checked_i32(ink_r.height().round())?;
+                let width = checked_i32(ink_r.width.round())?;
+                let height = checked_i32(ink_r.height.round())?;
 
                 Ok((ink_r, width, height))
             })
@@ -855,18 +869,70 @@ impl CHandle {
                 inner.size_callback.call(width, height);
 
                 Ok(RsvgPositionData {
-                    x: checked_i32(ink_r.x0)?,
-                    y: checked_i32(ink_r.y0)?,
+                    x: checked_i32(ink_r.x)?,
+                    y: checked_i32(ink_r.y)?,
                 })
             })
             .map_err(warn_on_invalid_id)
     }
 
-    pub fn get_geometry_sub(&self, id: Option<&str>) -> Result<(Rect, Rect), RenderingError> {
+    pub fn get_geometry_sub(
+        &self,
+        id: Option<&str>,
+    ) -> Result<(cairo::Rectangle, cairo::Rectangle), RenderingError> {
         let handle = self.get_handle_ref()?;
         let inner = self.inner.borrow();
 
-        handle.get_geometry_sub(id, inner.dpi.into(), inner.is_testing)
+        match id {
+            Some(id) => handle.get_geometry_for_layer(
+                Some(id),
+                &unit_rectangle(),
+                inner.dpi.into(),
+                inner.is_testing,
+            ),
+
+            None => self.document_size_in_pixels(),
+        }
+    }
+
+    fn document_size_in_pixels(
+        &self,
+    ) -> Result<(cairo::Rectangle, cairo::Rectangle), RenderingError> {
+        let handle = self.get_handle_ref()?;
+        let inner = self.inner.borrow();
+
+        if let Some((width, height)) = self.get_svg_size()? {
+            let rect = cairo::Rectangle::from_size(width, height);
+            Ok((rect, rect))
+        } else {
+            handle.get_geometry_for_layer(
+                None,
+                &unit_rectangle(),
+                inner.dpi.into(),
+                inner.is_testing,
+            )
+        }
+    }
+
+    // Returns the SVG's size suitable for the legacy C API, or None
+    // if it must be computed by hand.
+    //
+    // 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; 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(&self) -> Result<Option<(f64, f64)>, RenderingError> {
+        let handle = self.get_handle_ref()?;
+        let inner = self.inner.borrow();
+
+        Ok(handle
+            .get_intrinsic_size_in_pixels(inner.dpi.into())
+            .or_else(|| {
+                size_in_pixels_from_percentage_width_and_height(&handle.get_intrinsic_dimensions())
+            }))
     }
 
     fn set_stylesheet(&self, css: &str) -> Result<(), LoadingError> {
@@ -1007,6 +1073,57 @@ impl CHandle {
     }
 }
 
+/// If the width and height 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_percentage_width_and_height(
+    dim: &IntrinsicDimensions,
+) -> Option<(f64, f64)> {
+    let IntrinsicDimensions {
+        width,
+        height,
+        vbox,
+    } = *dim;
+
+    use rsvg_internals::LengthUnit::*;
+
+    // If both width and height are 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,
+    }
+}
+
+fn unit_rectangle() -> cairo::Rectangle {
+    cairo::Rectangle::from_size(1.0, 1.0)
+}
+
 fn is_rsvg_handle(obj: *const RsvgHandle) -> bool {
     unsafe { instance_of::<CHandle>(obj as *const _) }
 }
diff --git a/librsvg/pixbuf_utils.rs b/librsvg/pixbuf_utils.rs
index fe15812e..566e6354 100644
--- a/librsvg/pixbuf_utils.rs
+++ b/librsvg/pixbuf_utils.rs
@@ -216,7 +216,7 @@ fn pixbuf_from_file_with_size_mode(
         handle
             .get_geometry_sub(None)
             .and_then(|(ink_r, _)| {
-                let (document_width, document_height) = (ink_r.width(), ink_r.height());
+                let (document_width, document_height) = (ink_r.width, ink_r.height);
                 let (desired_width, desired_height) =
                     get_final_size(document_width, document_height, size_mode);
 
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index dfdf1543..3463eb88 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -2,15 +2,12 @@
 //!
 //! 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};
 use crate::dpi::Dpi;
 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::rect::Rect;
 use crate::structure::IntrinsicDimensions;
@@ -109,27 +106,35 @@ impl Handle {
         }
     }
 
-    /// Returns (ink_rect, logical_rect)
-    pub fn get_geometry_sub(
-        &self,
-        id: Option<&str>,
-        dpi: Dpi,
-        is_testing: bool,
-    ) -> Result<(Rect, Rect), RenderingError> {
-        let node = self.get_node_or_root(id)?;
-        let root = self.document.root();
-        let is_root = node == root;
+    /// If the intrinsic dimensions are in physical units, computes their pixel size, 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.
+    pub fn get_intrinsic_size_in_pixels(&self, dpi: Dpi) -> Option<(f64, f64)> {
+        let dimensions = self.get_intrinsic_dimensions();
+
+        if dimensions.width.is_none() || dimensions.height.is_none() {
+            // If either of width/height don't exist, the spec says they should default to 100%,
+            // which is a percentage-based unit - which we can't resolve here.
+            return None;
+        }
 
-        if is_root {
-            let cascaded = CascadedValues::new_from_node(&node);
+        let w = dimensions.width.unwrap();
+        let h = dimensions.height.unwrap();
 
-            if let Some((w, h)) = get_svg_size(&self.get_intrinsic_dimensions(), &cascaded, dpi) {
-                let rect = Rect::from_size(w, h);
-                return Ok((rect, rect));
-            }
+        use crate::length::LengthUnit::*;
+
+        if w.unit == Percent || h.unit == Percent {
+            return None;
         }
 
-        self.geometry_for_layer(node, unit_rectangle(), dpi, is_testing)
+        let params = ViewParams::new(dpi, 0.0, 0.0);
+        let root = self.document.root();
+        let cascaded = CascadedValues::new_from_node(&root);
+        let values = cascaded.get();
+
+        Some((w.normalize(values, &params), h.normalize(values, &params)))
     }
 
     fn get_node_or_root(&self, id: Option<&str>) -> Result<Node, RenderingError> {
@@ -381,96 +386,3 @@ fn check_cairo_context(cr: &cairo::Context) -> Result<(), RenderingError> {
 fn unit_rectangle() -> Rect {
     Rect::from_size(1.0, 1.0)
 }
-
-/// Returns the SVG's size suitable for the legacy C API, or None
-/// if it must be computed by hand.
-///
-/// 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; 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)> {
-    size_from_intrinsic_dimensions(dimensions, cascaded, dpi)
-        .or_else(|| size_in_pixels_from_vbox(dimensions))
-}
-
-/// 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;
-    }
-
-    let w = dimensions.width.unwrap();
-    let h = dimensions.height.unwrap();
-
-    use crate::length::LengthUnit::*;
-
-    if w.unit == Percent || h.unit == Percent {
-        return None;
-    }
-
-    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,
-    }
-}