Bug 1616691 - Simplify math function resolution. r=heycam

So as to avoid allocating an intermediate tree in Rust to resolve
`<length-percentage>` values.

Differential Revision: https://phabricator.services.mozilla.com/D63399

--HG--
extra : moz-landing-system : lando

servo/components/style/values/computed/length_percentage.rs

@@ -656,23 +656,14 @@ impl CalcLengthPercentage {
     /// Resolves the percentage.
     #[inline]
     fn resolve(&self, basis: Length) -> Length {
-        // TODO: This could be faster (without the extra allocations),
-        // potentially.
-        let mut resolved = self.node.map_leafs(|l| {
-            match l {
-                CalcLengthPercentageLeaf::Length(..) => l.clone(),
-                CalcLengthPercentageLeaf::Percentage(ref p) => {
-                    CalcLengthPercentageLeaf::Length(Length::new(basis.px() * p.0))
-                },
-            }
-        });
-
-        resolved.simplify_and_sort_children();
-
-        match resolved {
-            CalcNode::Leaf(CalcLengthPercentageLeaf::Length(l)) => l,
-            other => unreachable!("Didn't manage to resolve <length-percentage>: {:?}", other),
-        }
+        // unwrap() is fine because the conversion below is infallible.
+        let px = self.node.resolve(|l| {
+            Ok(match *l {
+                CalcLengthPercentageLeaf::Length(l) => l.px(),
+                CalcLengthPercentageLeaf::Percentage(ref p) => basis.px() * p.0,
+            })
+        }).unwrap();
+        Length::new(self.clamping_mode.clamp(px))
     }
 
     fn clamp_to_non_negative(&self) -> LengthPercentage {

servo/components/style/values/generics/calc.rs

@@ -6,9 +6,11 @@
 //!
 //! [calc]: https://drafts.csswg.org/css-values/#calc-notation
 
+use crate::Zero;
 use style_traits::{CssWriter, ToCss};
 use std::fmt::{self, Write};
 use std::{cmp, mem};
+use std::ops::Add;
 use smallvec::SmallVec;
 
 /// Whether we're a `min` or `max` function.
@@ -149,11 +151,64 @@ impl<L: CalcNodeLeaf> CalcNode<L> {
                 let center = Box::new(center.map_leaves_internal(map));
                 let max = Box::new(max.map_leaves_internal(map));
                 CalcNode::Clamp { min, center, max }
-
             }
         }
     }
 
+    /// Resolves the expression returning a value of `O`, given a function to
+    /// turn a leaf into the relevant value.
+    pub fn resolve<O>(&self, mut leaf_to_output_fn: impl FnMut(&L) -> Result<O, ()>) -> Result<O, ()>
+    where
+        O: PartialOrd + PartialEq + Add<Output = O> + Zero,
+    {
+        self.resolve_internal(&mut leaf_to_output_fn)
+    }
+
+    fn resolve_internal<O, F>(&self, leaf_to_output_fn: &mut F) -> Result<O, ()>
+    where
+        O: PartialOrd + PartialEq + Add<Output = O> + Zero,
+        F: FnMut(&L) -> Result<O, ()>,
+    {
+        Ok(match *self {
+            Self::Leaf(ref l) => return leaf_to_output_fn(l),
+            Self::Sum(ref c) => {
+                let mut result = Zero::zero();
+                for child in &**c {
+                    result = result + child.resolve_internal(leaf_to_output_fn)?;
+                }
+                result
+            },
+            Self::MinMax(ref nodes, op) => {
+                let mut result = nodes[0].resolve_internal(leaf_to_output_fn)?;
+                for node in nodes.iter().skip(1) {
+                    let candidate = node.resolve_internal(leaf_to_output_fn)?;
+                    let candidate_wins = match op {
+                        MinMaxOp::Min => candidate < result,
+                        MinMaxOp::Max => candidate > result,
+                    };
+                    if candidate_wins {
+                        result = candidate;
+                    }
+                }
+                result
+            },
+            Self::Clamp { ref min, ref center, ref max } => {
+                let min = min.resolve_internal(leaf_to_output_fn)?;
+                let center = center.resolve_internal(leaf_to_output_fn)?;
+                let max = max.resolve_internal(leaf_to_output_fn)?;
+
+                let mut result = center;
+                if result > max {
+                    result = max;
+                }
+                if result < min {
+                    result = min
+                }
+                result
+            },
+        })
+    }
+
     fn is_negative_leaf(&self) -> bool {
         match *self {
             Self::Leaf(ref l) => l.is_negative(),

servo/components/style/values/specified/calc.rs

@@ -98,72 +98,6 @@ pub struct CalcLengthPercentage {
 
 impl SpecifiedValueInfo for CalcLengthPercentage {}
 
-macro_rules! impl_generic_to_type {
-    ($self:ident, $self_variant:ident, $to_self:ident, $to_float:ident, $from_float:path) => {{
-        if let Self::Leaf(Leaf::$self_variant(ref v)) = *$self {
-            return Ok(v.clone());
-        }
-
-        Ok(match *$self {
-            Self::Sum(ref expressions) => {
-                let mut sum = 0.;
-                for sub in &**expressions {
-                    sum += sub.$to_self()?.$to_float();
-                }
-                $from_float(sum)
-            },
-            Self::Clamp {
-                ref min,
-                ref center,
-                ref max,
-            } => {
-                let min = min.$to_self()?;
-                let center = center.$to_self()?;
-                let max = max.$to_self()?;
-
-                // Equivalent to cmp::max(min, cmp::min(center, max))
-                //
-                // But preserving units when appropriate.
-                let center_float = center.$to_float();
-                let min_float = min.$to_float();
-                let max_float = max.$to_float();
-
-                let mut result = center;
-                let mut result_float = center_float;
-
-                if result_float > max_float {
-                    result = max;
-                    result_float = max_float;
-                }
-
-                if result_float < min_float {
-                    min
-                } else {
-                    result
-                }
-            },
-            Self::MinMax(ref nodes, op) => {
-                let mut result = nodes[0].$to_self()?;
-                let mut result_float = result.$to_float();
-                for node in nodes.iter().skip(1) {
-                    let candidate = node.$to_self()?;
-                    let candidate_float = candidate.$to_float();
-                    let candidate_wins = match op {
-                        MinMaxOp::Min => candidate_float < result_float,
-                        MinMaxOp::Max => candidate_float > result_float,
-                    };
-                    if candidate_wins {
-                        result = candidate;
-                        result_float = candidate_float;
-                    }
-                }
-                result
-            },
-            Self::Leaf(..) => return Err(()),
-        })
-    }};
-}
-
 impl PartialOrd for Leaf {
     fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
         use self::Leaf::*;
@@ -527,22 +461,44 @@ impl CalcNode {
 
     /// Tries to simplify this expression into a `<time>` value.
     fn to_time(&self) -> Result<Time, ()> {
-        impl_generic_to_type!(self, Time, to_time, seconds, Time::from_calc)
+        let seconds = self.resolve(|leaf| {
+            match *leaf {
+                Leaf::Time(ref t) => Ok(t.seconds()),
+                _ => Err(()),
+            }
+        })?;
+        Ok(Time::from_calc(seconds))
     }
 
     /// Tries to simplify this expression into an `Angle` value.
     fn to_angle(&self) -> Result<Angle, ()> {
-        impl_generic_to_type!(self, Angle, to_angle, degrees, Angle::from_calc)
+        let degrees = self.resolve(|leaf| {
+            match *leaf {
+                Leaf::Angle(ref angle) => Ok(angle.degrees()),
+                _ => Err(()),
+            }
+        })?;
+        Ok(Angle::from_calc(degrees))
     }
 
     /// Tries to simplify this expression into a `<number>` value.
     fn to_number(&self) -> Result<CSSFloat, ()> {
-        impl_generic_to_type!(self, Number, to_number, clone, From::from)
+        self.resolve(|leaf| {
+            match *leaf {
+                Leaf::Number(n) => Ok(n),
+                _ => Err(()),
+            }
+        })
     }
 
     /// Tries to simplify this expression into a `<percentage>` value.
     fn to_percentage(&self) -> Result<CSSFloat, ()> {
-        impl_generic_to_type!(self, Percentage, to_percentage, clone, From::from)
+        self.resolve(|leaf| {
+            match *leaf {
+                Leaf::Percentage(p) => Ok(p),
+                _ => Err(()),
+            }
+        })
     }
 
     /// Given a function name, and the location from where the token came from,