From 48eff19ff31702ecfb8f1bd676d9ec3bd371c098 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 7 Feb 2020 00:40:55 +0000 Subject: [PATCH] style: Don't use a binary tree representation for calc() sums / products. Keep a flat list of sum members. Simpify product and division ASAP. I want to preserve the tree for a bit longer to implement min / max / clamp. This doesn't do anything for it that we weren't doing already, but it helps to eventually keep this specified representation and the equivalent computed representation for values. Enable the tests for the comparison functions too, to prevent regressions. Differential Revision: https://phabricator.services.mozilla.com/D61514 --- components/style/values/specified/calc.rs | 218 +++++++++++++--------- 1 file changed, 130 insertions(+), 88 deletions(-) diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index 4fb3c009e04..6120c809b12 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -13,6 +13,7 @@ use crate::values::specified::length::{AbsoluteLength, FontRelativeLength, NoCal use crate::values::specified::{Angle, Time}; use crate::values::{CSSFloat, CSSInteger}; use cssparser::{AngleOrNumber, CowRcStr, NumberOrPercentage, Parser, Token}; +use smallvec::SmallVec; use std::fmt::{self, Write}; use style_traits::values::specified::AllowedNumericType; use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, StyleParseErrorKind, ToCss}; @@ -30,6 +31,15 @@ pub enum MathFunction { Clamp, } +/// Whether we're a `min` or `max` function. +#[derive(Copy, Clone, Debug)] +pub enum MinMaxOp { + /// `min()` + Min, + /// `max()` + Max, +} + /// A node inside a `Calc` expression's AST. #[derive(Clone, Debug)] pub enum CalcNode { @@ -43,18 +53,11 @@ pub enum CalcNode { Percentage(CSSFloat), /// `` Number(CSSFloat), - /// An expression of the form `x + y` - Sum(Box, Box), - /// An expression of the form `x - y` - Sub(Box, Box), - /// An expression of the form `x * y` - Mul(Box, Box), - /// An expression of the form `x / y` - Div(Box, Box), - /// A `min()` function. - Min(Box<[CalcNode]>), - /// A `max()` function. - Max(Box<[CalcNode]>), + /// An expression of the form `x + y + ...`. Subtraction is represented by + /// the negated expression of the right hand side. + Sum(Box<[CalcNode]>), + /// A `min()` / `max()` function. + MinMax(Box<[CalcNode]>, MinMaxOp), /// A `clamp()` function. Clamp { /// The minimum value. @@ -183,26 +186,12 @@ macro_rules! impl_generic_to_type { } Ok(match *$self { - Self::Sub(ref a, ref b) => $from_float(a.$to_self()?.$to_float() - b.$to_self()?.$to_float()), - Self::Sum(ref a, ref b) => $from_float(a.$to_self()?.$to_float() + b.$to_self()?.$to_float()), - Self::Mul(ref a, ref b) => match a.$to_self() { - Ok(lhs) => { - let rhs = b.to_number()?; - $from_float(lhs.$to_float() * rhs) - }, - Err(..) => { - let lhs = a.to_number()?; - let rhs = b.$to_self()?; - $from_float(lhs * rhs.$to_float()) - }, - }, - Self::Div(ref a, ref b) => { - let lhs = a.$to_self()?; - let rhs = b.to_number()?; - if rhs == 0. { - return Err(()); + Self::Sum(ref expressions) => { + let mut sum = 0.; + for sub in &**expressions { + sum += sub.$to_self()?.$to_float(); } - $from_float(lhs.$to_float() / rhs) + $from_float(sum) }, Self::Clamp { ref min, ref center, ref max } => { let min = min.$to_self()?; @@ -221,25 +210,21 @@ macro_rules! impl_generic_to_type { } result }, - Self::Min(ref nodes) => { - let mut min = nodes[0].$to_self()?; + Self::MinMax(ref nodes, op) => { + let mut result = nodes[0].$to_self()?; for node in nodes.iter().skip(1) { let candidate = node.$to_self()?; - if candidate.$to_float() < min.$to_float() { - min = candidate; + let candidate_float = candidate.$to_float(); + let result_float = result.$to_float(); + let candidate_wins = match op { + MinMaxOp::Min => candidate_float < result_float, + MinMaxOp::Max => candidate_float > result_float, + }; + if candidate_wins { + result = candidate; } } - min - }, - Self::Max(ref nodes) => { - let mut max = nodes[0].$to_self()?; - for node in nodes.iter().skip(1) { - let candidate = node.$to_self()?; - if candidate.$to_float() > max.$to_float() { - max = candidate; - } - } - max + result }, Self::Length(..) | Self::Angle(..) | @@ -251,6 +236,62 @@ macro_rules! impl_generic_to_type { } impl CalcNode { + fn negate(&mut self) { + self.mul_by(-1.); + } + + fn mul_by(&mut self, scalar: f32) { + match *self { + Self::Length(ref mut l) => { + // FIXME: For consistency this should probably convert absolute + // lengths into pixels. + *l = *l * scalar; + }, + Self::Number(ref mut n) => { + *n *= scalar; + }, + Self::Angle(ref mut a) => { + *a = Angle::from_calc(a.degrees() * scalar); + }, + Self::Time(ref mut t) => { + *t = Time::from_calc(t.seconds() * scalar); + }, + Self::Percentage(ref mut p) => { + *p *= scalar; + }, + // Multiplication is distributive across this. + Self::Sum(ref mut children) => { + for node in &mut **children { + node.mul_by(scalar); + } + }, + // This one is a bit trickier. + Self::MinMax(ref mut children, ref mut op) => { + for node in &mut **children { + node.mul_by(scalar); + } + + // For negatives we need to invert the operation. + if scalar < 0. { + *op = match *op { + MinMaxOp::Min => MinMaxOp::Max, + MinMaxOp::Max => MinMaxOp::Min, + } + } + }, + // Multiplication is distributive across these. + Self::Clamp { ref mut min, ref mut center, ref mut max } => { + min.mul_by(scalar); + center.mul_by(scalar); + max.mul_by(scalar); + // For negatives we need to swap min / max. + if scalar < 0. { + std::mem::swap(min, max); + } + }, + } + } + /// Tries to parse a single element in the expression, that is, a /// ``, ``, `