From 241e6e256fc4e048bcb1e8a972475541f60d9680 Mon Sep 17 00:00:00 2001 From: Tiaan Louw Date: Mon, 27 Mar 2023 08:05:47 +0000 Subject: [PATCH] style: Refactor mul_by fn into map fn Refactor the mul_by function on leafs into a more generic map function that can be used for more operations like abs, signum and mul. Differential Revision: https://phabricator.services.mozilla.com/D172936 --- .../values/computed/length_percentage.rs | 14 ++-- components/style/values/generics/calc.rs | 55 +++++++++++-- components/style/values/specified/calc.rs | 38 +++------ components/style/values/specified/length.rs | 77 +++++++++++++++++++ 4 files changed, 146 insertions(+), 38 deletions(-) diff --git a/components/style/values/computed/length_percentage.rs b/components/style/values/computed/length_percentage.rs index cb0fe727015..a8f5868b997 100644 --- a/components/style/values/computed/length_percentage.rs +++ b/components/style/values/computed/length_percentage.rs @@ -241,7 +241,7 @@ impl LengthPercentage { // TODO: This could in theory take ownership of the calc node in `v` if // possible instead of cloning. let mut node = v.to_calc_node().into_owned(); - node.negate(); + node.map(std::ops::Neg::neg); let new_node = CalcNode::Sum( vec![ @@ -679,10 +679,14 @@ impl calc::CalcNodeLeaf for CalcLengthPercentageLeaf { } } - fn mul_by(&mut self, scalar: f32) { - match *self { - Self::Length(ref mut l) => *l = *l * scalar, - Self::Percentage(ref mut p) => p.0 *= scalar, + fn map(&mut self, mut op: impl FnMut(f32) -> f32) { + match self { + CalcLengthPercentageLeaf::Length(value) => { + *value = Length::new(op(value.px())); + }, + CalcLengthPercentageLeaf::Percentage(value) => { + *value = Percentage(op(value.0)); + }, } } diff --git a/components/style/values/generics/calc.rs b/components/style/values/generics/calc.rs index 1ceefa0ef8d..213595b9140 100644 --- a/components/style/values/generics/calc.rs +++ b/components/style/values/generics/calc.rs @@ -232,12 +232,12 @@ pub trait CalcNodeLeaf: Clone + Sized + PartialOrd + PartialEq + ToCss { where O: Fn(f32, f32) -> f32; - /// Multiplies the leaf by a given scalar number. - fn mul_by(&mut self, scalar: f32); + /// Map the value of this node with the given operation. + fn map(&mut self, op: impl FnMut(f32) -> f32); /// Negates the leaf. fn negate(&mut self) { - self.mul_by(-1.); + self.map(std::ops::Neg::neg); } /// Canonicalizes the expression if necessary. @@ -248,9 +248,9 @@ pub trait CalcNodeLeaf: Clone + Sized + PartialOrd + PartialEq + ToCss { } impl CalcNode { - /// Negates the node. - pub fn negate(&mut self) { - self.mul_by(-1.); + /// Negates the leaf. + fn negate(&mut self) { + self.map(std::ops::Neg::neg); } fn sort_key(&self) -> SortKey { @@ -291,6 +291,47 @@ impl CalcNode { } } + /// Map the value of this node with the given operation. + pub fn map(&mut self, mut op: impl FnMut(f32) -> f32) { + fn map_internal(node: &mut CalcNode, op: &mut impl FnMut(f32) -> f32) { + match node { + CalcNode::Leaf(l) => l.map(op), + CalcNode::Sum(children) => { + for node in &mut **children { + map_internal(node, op); + } + }, + CalcNode::MinMax(children, _) => { + for node in &mut **children { + map_internal(node, op); + } + }, + CalcNode::Clamp { min, center, max } => { + map_internal(min, op); + map_internal(center, op); + map_internal(max, op); + }, + CalcNode::Round { value, step, .. } => { + map_internal(value, op); + map_internal(step, op); + }, + CalcNode::ModRem { + dividend, divisor, .. + } => { + map_internal(dividend, op); + map_internal(divisor, op); + }, + CalcNode::Hypot(children) => { + for node in &mut **children { + map_internal(node, op); + } + }, + } + } + + map_internal(self, &mut op); + } + /// Convert this `CalcNode` into a `CalcNode` with a different leaf kind. pub fn map_leaves(&self, mut map: F) -> CalcNode where @@ -573,7 +614,7 @@ impl CalcNode { /// Multiplies the node by a scalar. pub fn mul_by(&mut self, scalar: f32) { match *self { - Self::Leaf(ref mut l) => l.mul_by(scalar), + Self::Leaf(ref mut l) => l.map(|v| v * scalar), // Multiplication is distributive across this. Self::Sum(ref mut children) => { for node in &mut **children { diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index a4a9319a679..ff6380380a0 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -238,31 +238,6 @@ impl generic::CalcNodeLeaf for Leaf { } } - 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::Resolution(ref mut r) => { - *r = Resolution::from_dppx(r.dppx() * scalar); - }, - Self::Time(ref mut t) => { - *t = Time::from_seconds(t.seconds() * scalar); - }, - Self::Percentage(ref mut p) => { - *p *= scalar; - }, - } - } - fn sort_key(&self) -> SortKey { match *self { Self::Number(..) => SortKey::Number, @@ -416,6 +391,17 @@ impl generic::CalcNodeLeaf for Leaf { }, } } + + fn map(&mut self, mut op: impl FnMut(f32) -> f32) { + match self { + Leaf::Length(one) => *one = one.map(op), + Leaf::Angle(one) => *one = specified::Angle::from_calc(op(one.degrees())), + Leaf::Time(one) => *one = specified::Time::from_seconds(op(one.seconds())), + Leaf::Resolution(one) => *one = specified::Resolution::from_dppx(op(one.dppx())), + Leaf::Percentage(one) => *one = op(*one), + Leaf::Number(one) => *one = op(*one), + } + } } /// A calc node representation for specified values. @@ -738,7 +724,7 @@ impl CalcNode { }, Token::Delim('-') => { let mut rhs = Self::parse_product(context, input, allowed_units)?; - rhs.negate(); + rhs.mul_by(-1.0); sum.push(rhs); }, _ => { diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 85a328e629c..9512ca78054 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -139,6 +139,17 @@ impl FontRelativeLength { }) } + fn map(&self, mut op: impl FnMut(f32) -> f32) -> Self { + match self { + FontRelativeLength::Em(x) => FontRelativeLength::Em(op(*x)), + FontRelativeLength::Ex(x) => FontRelativeLength::Ex(op(*x)), + FontRelativeLength::Ch(x) => FontRelativeLength::Ch(op(*x)), + FontRelativeLength::Cap(x) => FontRelativeLength::Cap(op(*x)), + FontRelativeLength::Ic(x) => FontRelativeLength::Ic(op(*x)), + FontRelativeLength::Rem(x) => FontRelativeLength::Rem(op(*x)), + } + } + /// Computes the font-relative length. pub fn to_computed_value( &self, @@ -513,6 +524,35 @@ impl ViewportPercentageLength { }) } + fn map(&self, mut op: impl FnMut(f32) -> f32) -> Self { + match self { + ViewportPercentageLength::Vw(x) => ViewportPercentageLength::Vw(op(*x)), + ViewportPercentageLength::Svw(x) => ViewportPercentageLength::Svw(op(*x)), + ViewportPercentageLength::Lvw(x) => ViewportPercentageLength::Lvw(op(*x)), + ViewportPercentageLength::Dvw(x) => ViewportPercentageLength::Dvw(op(*x)), + ViewportPercentageLength::Vh(x) => ViewportPercentageLength::Vh(op(*x)), + ViewportPercentageLength::Svh(x) => ViewportPercentageLength::Svh(op(*x)), + ViewportPercentageLength::Lvh(x) => ViewportPercentageLength::Lvh(op(*x)), + ViewportPercentageLength::Dvh(x) => ViewportPercentageLength::Dvh(op(*x)), + ViewportPercentageLength::Vmin(x) => ViewportPercentageLength::Vmin(op(*x)), + ViewportPercentageLength::Svmin(x) => ViewportPercentageLength::Svmin(op(*x)), + ViewportPercentageLength::Lvmin(x) => ViewportPercentageLength::Lvmin(op(*x)), + ViewportPercentageLength::Dvmin(x) => ViewportPercentageLength::Dvmin(op(*x)), + ViewportPercentageLength::Vmax(x) => ViewportPercentageLength::Vmax(op(*x)), + ViewportPercentageLength::Svmax(x) => ViewportPercentageLength::Svmax(op(*x)), + ViewportPercentageLength::Lvmax(x) => ViewportPercentageLength::Lvmax(op(*x)), + ViewportPercentageLength::Dvmax(x) => ViewportPercentageLength::Dvmax(op(*x)), + ViewportPercentageLength::Vb(x) => ViewportPercentageLength::Vb(op(*x)), + ViewportPercentageLength::Svb(x) => ViewportPercentageLength::Svb(op(*x)), + ViewportPercentageLength::Lvb(x) => ViewportPercentageLength::Lvb(op(*x)), + ViewportPercentageLength::Dvb(x) => ViewportPercentageLength::Dvb(op(*x)), + ViewportPercentageLength::Vi(x) => ViewportPercentageLength::Vi(op(*x)), + ViewportPercentageLength::Svi(x) => ViewportPercentageLength::Svi(op(*x)), + ViewportPercentageLength::Lvi(x) => ViewportPercentageLength::Lvi(op(*x)), + ViewportPercentageLength::Dvi(x) => ViewportPercentageLength::Dvi(op(*x)), + } + } + /// Computes the given viewport-relative length for the given viewport size. pub fn to_computed_value(&self, context: &Context) -> CSSPixelLength { let (variant, unit, factor) = self.unpack(); @@ -633,6 +673,18 @@ impl AbsoluteLength { { Ok(Self::Px(op(self.to_px(), other.to_px()))) } + + fn map(&self, mut op: impl FnMut(f32) -> f32) -> Self { + match self { + AbsoluteLength::Px(x) => AbsoluteLength::Px(op(*x)), + AbsoluteLength::In(x) => AbsoluteLength::In(op(*x)), + AbsoluteLength::Cm(x) => AbsoluteLength::Cm(op(*x)), + AbsoluteLength::Mm(x) => AbsoluteLength::Mm(op(*x)), + AbsoluteLength::Q(x) => AbsoluteLength::Q(op(*x)), + AbsoluteLength::Pt(x) => AbsoluteLength::Pt(op(*x)), + AbsoluteLength::Pc(x) => AbsoluteLength::Pc(op(*x)), + } + } } impl ToComputedValue for AbsoluteLength { @@ -741,6 +793,17 @@ impl ContainerRelativeLength { }) } + pub(crate) fn map(&self, mut op: impl FnMut(f32) -> f32) -> Self { + match self { + ContainerRelativeLength::Cqw(x) => ContainerRelativeLength::Cqw(op(*x)), + ContainerRelativeLength::Cqh(x) => ContainerRelativeLength::Cqh(op(*x)), + ContainerRelativeLength::Cqi(x) => ContainerRelativeLength::Cqi(op(*x)), + ContainerRelativeLength::Cqb(x) => ContainerRelativeLength::Cqb(op(*x)), + ContainerRelativeLength::Cqmin(x) => ContainerRelativeLength::Cqmin(op(*x)), + ContainerRelativeLength::Cqmax(x) => ContainerRelativeLength::Cqmax(op(*x)), + } + } + /// Computes the given container-relative length. pub fn to_computed_value(&self, context: &Context) -> CSSPixelLength { let size = context.get_container_size_query(); @@ -1037,6 +1100,20 @@ impl NoCalcLength { }) } + pub(crate) fn map(&self, mut op: impl FnMut(f32) -> f32) -> Self { + use self::NoCalcLength::*; + + match self { + Absolute(ref one) => Absolute(one.map(op)), + FontRelative(ref one) => FontRelative(one.map(op)), + ViewportPercentage(ref one) => ViewportPercentage(one.map(op)), + ContainerRelative(ref one) => ContainerRelative(one.map(op)), + ServoCharacterWidth(ref one) => { + ServoCharacterWidth(CharacterWidth(op(one.0 as f32) as i32)) + }, + } + } + /// Get a px value without context. #[inline] pub fn to_computed_pixel_length_without_context(&self) -> Result {