From 5af6abfb784c57ea7d0a81563a98fed8e46f3461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 4 Nov 2018 15:48:08 +0100 Subject: [PATCH] style: Simplify the SVG animation code. It's overly generic for no good reason. Differential Revision: https://phabricator.services.mozilla.com/D10844 --- components/style/properties/gecko.mako.rs | 23 ++-- .../longhands/inherited_svg.mako.rs | 7 +- components/style/values/animated/svg.rs | 112 +++++------------- components/style/values/computed/svg.rs | 20 ++-- components/style/values/generics/svg.rs | 57 +-------- 5 files changed, 63 insertions(+), 156 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 59beaea0a80..d51a43166b9 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -561,17 +561,24 @@ def set_gecko_property(ffi_name, expr): return SVGLength::ContextValue; } let length = match self.gecko.${gecko_ffi_name}.as_value() { - CoordDataValue::Factor(number) => - SvgLengthOrPercentageOrNumber::Number(number), - CoordDataValue::Coord(coord) => + CoordDataValue::Factor(number) => { + SvgLengthOrPercentageOrNumber::Number(number) + }, + CoordDataValue::Coord(coord) => { SvgLengthOrPercentageOrNumber::LengthOrPercentage( - LengthOrPercentage::Length(Au(coord).into())), - CoordDataValue::Percent(p) => + LengthOrPercentage::Length(Au(coord).into()) + ) + }, + CoordDataValue::Percent(p) => { SvgLengthOrPercentageOrNumber::LengthOrPercentage( - LengthOrPercentage::Percentage(Percentage(p))), - CoordDataValue::Calc(calc) => + LengthOrPercentage::Percentage(Percentage(p)) + ) + }, + CoordDataValue::Calc(calc) => { SvgLengthOrPercentageOrNumber::LengthOrPercentage( - LengthOrPercentage::Calc(calc.into())), + LengthOrPercentage::Calc(calc.into()) + ) + }, _ => unreachable!("Unexpected coordinate in ${ident}"), }; SVGLength::Length(length.into()) diff --git a/components/style/properties/longhands/inherited_svg.mako.rs b/components/style/properties/longhands/inherited_svg.mako.rs index 3cdc5e76569..4ecad6e39fd 100644 --- a/components/style/properties/longhands/inherited_svg.mako.rs +++ b/components/style/properties/longhands/inherited_svg.mako.rs @@ -85,7 +85,7 @@ ${helpers.predefined_type( ${helpers.predefined_type( "stroke-width", "SVGWidth", - "::values::computed::NonNegativeLength::new(1.).into()", + "computed::SVGWidth::one()", products="gecko", animation_value_type="::values::computed::SVGWidth", spec="https://www.w3.org/TR/SVG2/painting.html#StrokeWidth", @@ -135,8 +135,9 @@ ${helpers.predefined_type( )} ${helpers.predefined_type( - "stroke-dashoffset", "SVGLength", - "Au(0).into()", + "stroke-dashoffset", + "SVGLength", + "computed::SVGLength::zero()", products="gecko", animation_value_type="ComputedValue", spec="https://www.w3.org/TR/SVG2/painting.html#StrokeDashing", diff --git a/components/style/values/animated/svg.rs b/components/style/values/animated/svg.rs index 49641a8e376..0c4e3be7166 100644 --- a/components/style/values/animated/svg.rs +++ b/components/style/values/animated/svg.rs @@ -6,12 +6,11 @@ use properties::animated_properties::ListAnimation; use values::animated::color::Color as AnimatedColor; -use values::computed::{NonNegativeNumber, Number, NumberOrPercentage, LengthOrPercentage}; +use values::computed::{Number, NumberOrPercentage, LengthOrPercentage}; use values::computed::url::ComputedUrl; -use values::computed::length::NonNegativeLengthOrPercentage; use values::distance::{ComputeSquaredDistance, SquaredDistance}; use values::generics::svg::{SVGLength, SvgLengthOrPercentageOrNumber, SVGPaint}; -use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity}; +use values::generics::svg::{SVGStrokeDashArray, SVGOpacity}; use super::{Animate, Procedure, ToAnimatedZero}; /// Animated SVGPaint. @@ -27,102 +26,44 @@ impl ToAnimatedZero for IntermediateSVGPaint { } } -impl From for NumberOrPercentage { - fn from(lop: NonNegativeLengthOrPercentage) -> NumberOrPercentage { - lop.0.into() - } -} - -impl From for NumberOrPercentage { - fn from(num: NonNegativeNumber) -> NumberOrPercentage { - num.0.into() - } -} - -impl From for NumberOrPercentage { - fn from(lop: LengthOrPercentage) -> NumberOrPercentage { - match lop { - LengthOrPercentage::Length(len) => NumberOrPercentage::Number(len.px()), - LengthOrPercentage::Percentage(p) => NumberOrPercentage::Percentage(p), - LengthOrPercentage::Calc(_) => { - panic!("We dont't expected calc interpolation for SvgLengthOrPercentageOrNumber"); - }, +// FIXME: We need to handle calc here properly, see +// https://bugzilla.mozilla.org/show_bug.cgi?id=1386967 +fn to_number_or_percentage( + value: &SvgLengthOrPercentageOrNumber, +) -> Result { + Ok(match *value { + SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref l) => { + match *l { + LengthOrPercentage::Length(ref l) => NumberOrPercentage::Number(l.px()), + LengthOrPercentage::Percentage(ref p) => NumberOrPercentage::Percentage(*p), + LengthOrPercentage::Calc(..) => return Err(()), + } } - } + SvgLengthOrPercentageOrNumber::Number(ref n) => NumberOrPercentage::Number(*n), + }) } -impl From for NumberOrPercentage { - fn from(num: Number) -> NumberOrPercentage { - NumberOrPercentage::Number(num) - } -} - -fn convert_to_number_or_percentage( - from: SvgLengthOrPercentageOrNumber, -) -> NumberOrPercentage -where - LengthOrPercentageType: Into, - NumberType: Into, -{ - match from { - SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop) => { - lop.into() - } - SvgLengthOrPercentageOrNumber::Number(num) => { - num.into() - } - } -} - -fn convert_from_number_or_percentage( - from: NumberOrPercentage, -) -> SvgLengthOrPercentageOrNumber -where - LengthOrPercentageType: From, - NumberType: From, -{ - match from { - NumberOrPercentage::Number(num) => - SvgLengthOrPercentageOrNumber::Number(num.into()), - NumberOrPercentage::Percentage(p) => - SvgLengthOrPercentageOrNumber::LengthOrPercentage( - (LengthOrPercentage::Percentage(p)).into()) - } -} - -impl Animate for SvgLengthOrPercentageOrNumber -where - L: Animate + From + Into + Copy, - N: Animate + From + Into, - LengthOrPercentage: From, - Self: Copy, -{ +impl Animate for SvgLengthOrPercentageOrNumber { #[inline] fn animate(&self, other: &Self, procedure: Procedure) -> Result { - if self.has_calc() || other.has_calc() { - // TODO: We need to treat calc value. - // https://bugzilla.mozilla.org/show_bug.cgi?id=1386967 - return Err(()); - } - - let this = convert_to_number_or_percentage(*self); - let other = convert_to_number_or_percentage(*other); + let this = to_number_or_percentage(self)?; + let other = to_number_or_percentage(other)?; match (this, other) { ( NumberOrPercentage::Number(ref this), NumberOrPercentage::Number(ref other), ) => { - Ok(convert_from_number_or_percentage( - NumberOrPercentage::Number(this.animate(other, procedure)?) + Ok(SvgLengthOrPercentageOrNumber::Number( + this.animate(other, procedure)? )) }, ( NumberOrPercentage::Percentage(ref this), NumberOrPercentage::Percentage(ref other), ) => { - Ok(convert_from_number_or_percentage( - NumberOrPercentage::Percentage(this.animate(other, procedure)?) + Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage( + LengthOrPercentage::Percentage(this.animate(other, procedure)?) )) }, _ => Err(()), @@ -130,6 +71,13 @@ where } } +impl ComputeSquaredDistance for SvgLengthOrPercentageOrNumber { + fn compute_squared_distance(&self, other: &Self) -> Result { + to_number_or_percentage(self)? + .compute_squared_distance(&to_number_or_percentage(other)?) + } +} + impl Animate for SVGLength where L: Animate + Clone, diff --git a/components/style/values/computed/svg.rs b/components/style/values/computed/svg.rs index ab9bbd18b4e..11917b01779 100644 --- a/components/style/values/computed/svg.rs +++ b/components/style/values/computed/svg.rs @@ -4,9 +4,8 @@ //! Computed types for SVG properties. -use app_units::Au; use values::RGBA; -use values::computed::{LengthOrPercentage, NonNegativeLength}; +use values::computed::LengthOrPercentage; use values::computed::{NonNegativeLengthOrPercentage, NonNegativeNumber, Number}; use values::computed::Opacity; use values::computed::color::Color; @@ -50,10 +49,11 @@ pub type SvgLengthOrPercentageOrNumber = /// | | | context-value pub type SVGLength = generic::SVGLength; -impl From for SVGLength { - fn from(length: Au) -> Self { +impl SVGLength { + /// `0px` + pub fn zero() -> Self { generic::SVGLength::Length(generic::SvgLengthOrPercentageOrNumber::LengthOrPercentage( - length.into(), + LengthOrPercentage::zero() )) } } @@ -63,6 +63,8 @@ impl From for SVGLength { pub type NonNegativeSvgLengthOrPercentageOrNumber = generic::SvgLengthOrPercentageOrNumber; +// FIXME(emilio): This is really hacky, and can go away with a bit of work on +// the clone_stroke_width code in gecko.mako.rs. impl Into for SvgLengthOrPercentageOrNumber { fn into(self) -> NonNegativeSvgLengthOrPercentageOrNumber { match self { @@ -79,10 +81,12 @@ impl Into for SvgLengthOrPercentageOrN /// An non-negative wrapper of SVGLength. pub type SVGWidth = generic::SVGLength; -impl From for SVGWidth { - fn from(length: NonNegativeLength) -> Self { +impl SVGWidth { + /// `1px`. + pub fn one() -> Self { + use values::generics::NonNegative; generic::SVGLength::Length(generic::SvgLengthOrPercentageOrNumber::LengthOrPercentage( - length.into(), + NonNegative(LengthOrPercentage::one()) )) } } diff --git a/components/style/values/generics/svg.rs b/components/style/values/generics/svg.rs index 8b8fa786e8f..2dd81da5eda 100644 --- a/components/style/values/generics/svg.rs +++ b/components/style/values/generics/svg.rs @@ -8,9 +8,6 @@ use cssparser::Parser; use parser::{Parse, ParserContext}; use style_traits::{ParseError, StyleParseErrorKind}; use values::{Either, None_}; -use values::computed::NumberOrPercentage; -use values::computed::length::LengthOrPercentage; -use values::distance::{ComputeSquaredDistance, SquaredDistance}; /// An SVG paint value /// @@ -152,54 +149,6 @@ pub enum SvgLengthOrPercentageOrNumber { Number(Number), } -impl ComputeSquaredDistance for SvgLengthOrPercentageOrNumber -where - L: ComputeSquaredDistance + Copy + Into, - N: ComputeSquaredDistance + Copy + Into, -{ - #[inline] - fn compute_squared_distance(&self, other: &Self) -> Result { - match (self, other) { - ( - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref from), - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref to), - ) => from.compute_squared_distance(to), - ( - &SvgLengthOrPercentageOrNumber::Number(ref from), - &SvgLengthOrPercentageOrNumber::Number(ref to), - ) => from.compute_squared_distance(to), - ( - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(from), - &SvgLengthOrPercentageOrNumber::Number(to), - ) => from.into().compute_squared_distance(&to.into()), - ( - &SvgLengthOrPercentageOrNumber::Number(from), - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(to), - ) => from.into().compute_squared_distance(&to.into()), - } - } -} - -impl - SvgLengthOrPercentageOrNumber -where - LengthOrPercentage: From, - LengthOrPercentageType: Copy, -{ - /// return true if this struct has calc value. - pub fn has_calc(&self) -> bool { - match self { - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop) => { - match LengthOrPercentage::from(lop) { - LengthOrPercentage::Calc(_) => true, - _ => false, - } - }, - _ => false, - } - } -} - /// Parsing the SvgLengthOrPercentageOrNumber. At first, we need to parse number /// since prevent converting to the length. impl Parse @@ -213,10 +162,8 @@ impl Parse return Ok(SvgLengthOrPercentageOrNumber::Number(num)); } - if let Ok(lop) = input.try(|i| LengthOrPercentageType::parse(context, i)) { - return Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop)); - } - Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + let lop = LengthOrPercentageType::parse(context, input)?; + Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop)) } }