From bf034a83e84ea8eafdba892ad01aca514f98360a Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 11 Sep 2017 16:48:20 +0200 Subject: [PATCH 1/5] Do not use CVAS for FontSettings --- components/style/properties/helpers.mako.rs | 2 +- components/style/properties/longhand/font.mako.rs | 5 ----- components/style/values/computed/mod.rs | 1 + components/style/values/generics/mod.rs | 11 ++++++----- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index ab290abefdc..fa296a3046a 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -316,7 +316,7 @@ % if not property.derived_from: match value { - DeclaredValue::Value(ref specified_value) => { + DeclaredValue::Value(specified_value) => { % if property.ident in SYSTEM_FONT_LONGHANDS and product == "gecko": if let Some(sf) = specified_value.get_system() { longhands::system_font::resolve_system_font(sf, context); diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs index d9adc9610d8..d56bea05dc7 100644 --- a/components/style/properties/longhand/font.mako.rs +++ b/components/style/properties/longhand/font.mako.rs @@ -2014,15 +2014,10 @@ https://drafts.csswg.org/css-fonts-4/#low-level-font-variation-settings-control- animation_value_type="ComputedValue" flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER" spec="${variation_spec}"> - use values::computed::ComputedValueAsSpecified; use values::generics::FontSettings; - impl ComputedValueAsSpecified for SpecifiedValue {} - pub type SpecifiedValue = computed_value::T; - - pub mod computed_value { use values::generics::{FontSettings, FontSettingTagFloat}; pub type T = FontSettings; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 22852114eab..af6d416a20d 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -338,6 +338,7 @@ trivial_to_computed_value!(f32); trivial_to_computed_value!(i32); trivial_to_computed_value!(u8); trivial_to_computed_value!(u16); +trivial_to_computed_value!(u32); trivial_to_computed_value!(Atom); trivial_to_computed_value!(BorderStyle); trivial_to_computed_value!(Cursor); diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index 0f419da0383..34feefb04a7 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -129,8 +129,8 @@ impl Parse for CounterStyleOrNone { /// /// For font-feature-settings, this is a tag and an integer, /// for font-variation-settings this is a tag and a float -#[derive(Clone, Debug, Eq, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Clone, Debug, Eq, PartialEq, ToComputedValue)] pub struct FontSettingTag { /// A four-character tag, packed into a u32 (one byte per character) pub tag: u32, @@ -187,7 +187,7 @@ impl Parse for FontSettingTag { /// A font settings value for font-variation-settings or font-feature-settings #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -#[derive(Clone, Debug, Eq, PartialEq, ToCss)] +#[derive(Clone, Debug, Eq, PartialEq, ToComputedValue, ToCss)] pub enum FontSettings { /// No settings (default) Normal, @@ -210,16 +210,17 @@ impl Parse for FontSettings { /// /// Do not use this type anywhere except within FontSettings /// because it serializes with the preceding space -#[derive(Clone, Copy, Debug, Eq, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Clone, Copy, Debug, Eq, PartialEq, ToComputedValue)] pub struct FontSettingTagInt(pub u32); + /// A number value to be used for font-variation-settings /// /// Do not use this type anywhere except within FontSettings /// because it serializes with the preceding space -#[derive(Clone, Debug, PartialEq)] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[cfg_attr(feature = "gecko", derive(Animate, ComputeSquaredDistance))] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Clone, Debug, PartialEq, ToComputedValue)] pub struct FontSettingTagFloat(pub f32); impl ToCss for FontSettingTagInt { From 891789109ba9dd5e6a1371875322e0ff2832ca02 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 11 Sep 2017 16:35:11 +0200 Subject: [PATCH 2/5] Do not use CVAS for the list-style-image property --- components/style/properties/longhand/list.mako.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/style/properties/longhand/list.mako.rs b/components/style/properties/longhand/list.mako.rs index 10d81fc4691..92167fdf91d 100644 --- a/components/style/properties/longhand/list.mako.rs +++ b/components/style/properties/longhand/list.mako.rs @@ -98,7 +98,6 @@ ${helpers.single_keyword("list-style-position", "outside inside", animation_valu <%helpers:longhand name="list-style-image" animation_value_type="discrete" boxed="${product == 'gecko'}" spec="https://drafts.csswg.org/css-lists/#propdef-list-style-image"> - use values::computed::ComputedValueAsSpecified; use values::specified::UrlOrNone; pub use self::computed_value::T as SpecifiedValue; @@ -108,11 +107,12 @@ ${helpers.single_keyword("list-style-position", "outside inside", animation_valu #[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[derive(Clone, Debug, PartialEq, ToCss)] pub struct T(pub UrlOrNone); + + // FIXME(nox): This is wrong, there are different types for specified + // and computed URLs in Servo. + trivial_to_computed_value!(T); } - - impl ComputedValueAsSpecified for SpecifiedValue {} - #[inline] pub fn get_initial_value() -> computed_value::T { computed_value::T(Either::Second(None_)) From 4e64ccde603bb172bee3df5b5d9bbd05ccdb22fe Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 11 Sep 2017 16:49:16 +0200 Subject: [PATCH 3/5] =?UTF-8?q?Kill=20ComputedValueAsSpecified=20?= =?UTF-8?q?=F0=9F=94=AB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- components/style/values/computed/mod.rs | 20 -------------------- components/style/values/generics/image.rs | 6 +----- 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index af6d416a20d..94cc1706c4a 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -312,26 +312,6 @@ impl ToComputedValue for Box<[T]> } } -/// A marker trait to represent that the specified value is also the computed -/// value. -pub trait ComputedValueAsSpecified {} - -impl ToComputedValue for T - where T: ComputedValueAsSpecified + Clone, -{ - type ComputedValue = T; - - #[inline] - fn to_computed_value(&self, _context: &Context) -> T { - self.clone() - } - - #[inline] - fn from_computed_value(computed: &T) -> Self { - computed.clone() - } -} - trivial_to_computed_value!(()); trivial_to_computed_value!(bool); trivial_to_computed_value!(f32); diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index 874511330d4..a0b2cb1b281 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -34,11 +34,7 @@ pub enum Image { PaintWorklet(PaintWorklet), } -// Can't just use derive(ToComputedValue) on Image, because when trying to do -// "impl ToComputedValue for Box" the Rust compiler complains that -// "impl ToComputedValue for T where T: ComputedValueAsSpecified + Clone" -// aleady implements ToComputedValue for std::boxed::Box<_> and hence we have -// conflicting implementations. +// FIXME(nox): Implement TCV for Box and derive this. impl ToComputedValue for Image { From 7c31202dda934e559e3a85b8b14960dd00bf9143 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 11 Sep 2017 17:20:41 +0200 Subject: [PATCH 4/5] Implement ToComputedValue for Box --- components/style/properties/helpers.mako.rs | 4 ++++ .../helpers/animated_properties.mako.rs | 4 ++++ components/style/values/computed/length.rs | 6 +++--- components/style/values/computed/mod.rs | 16 ++++++++++++++++ 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index fa296a3046a..3e24a08103b 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -341,7 +341,11 @@ } context.builder.put_${data.current_style_struct.name_lower}(s); % else: + % if property.boxed: + let computed = (**specified_value).to_computed_value(context); + % else: let computed = specified_value.to_computed_value(context); + % endif % if property.ident == "font_size": longhands::font_size::cascade_specified_font_size( context, diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index bb0fd657b5d..278dcf2138d 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -544,7 +544,11 @@ impl AnimationValue { longhands::system_font::resolve_system_font(sf, context); } % endif + % if prop.boxed: + let computed = (**val).to_computed_value(context); + % else: let computed = val.to_computed_value(context); + % endif Some(AnimationValue::${prop.camel_case}( % if prop.is_animatable_with_computed_value: computed diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 7df0886ccd5..a0ebfda3171 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -408,7 +408,7 @@ impl ToComputedValue for specified::LengthOrPercentage { LengthOrPercentage::Percentage(value) } specified::LengthOrPercentage::Calc(ref calc) => { - LengthOrPercentage::Calc(calc.to_computed_value(context)) + LengthOrPercentage::Calc((**calc).to_computed_value(context)) } } } @@ -502,7 +502,7 @@ impl ToComputedValue for specified::LengthOrPercentageOrAuto { LengthOrPercentageOrAuto::Auto } specified::LengthOrPercentageOrAuto::Calc(ref calc) => { - LengthOrPercentageOrAuto::Calc(calc.to_computed_value(context)) + LengthOrPercentageOrAuto::Calc((**calc).to_computed_value(context)) } } } @@ -591,7 +591,7 @@ impl ToComputedValue for specified::LengthOrPercentageOrNone { LengthOrPercentageOrNone::Percentage(value) } specified::LengthOrPercentageOrNone::Calc(ref calc) => { - LengthOrPercentageOrNone::Calc(calc.to_computed_value(context)) + LengthOrPercentageOrNone::Calc((**calc).to_computed_value(context)) } specified::LengthOrPercentageOrNone::None => { LengthOrPercentageOrNone::None diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 94cc1706c4a..aedb8ca31f2 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -296,6 +296,22 @@ impl ToComputedValue for Vec } } +impl ToComputedValue for Box + where T: ToComputedValue +{ + type ComputedValue = Box<::ComputedValue>; + + #[inline] + fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { + Box::new(T::to_computed_value(self, context)) + } + + #[inline] + fn from_computed_value(computed: &Self::ComputedValue) -> Self { + Box::new(T::from_computed_value(computed)) + } +} + impl ToComputedValue for Box<[T]> where T: ToComputedValue { From 95f2d93960932e4231cc41eb5d75a4b5591fdb54 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 11 Sep 2017 17:21:04 +0200 Subject: [PATCH 5/5] Derive ToComputedValue for Image<..> --- components/style/values/generics/image.rs | 40 +---------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index a0b2cb1b281..578eb1cf055 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -11,13 +11,12 @@ use cssparser::serialize_identifier; use custom_properties; use std::fmt; use style_traits::ToCss; -use values::computed::{Context, ToComputedValue}; /// An [image]. /// /// [image]: https://drafts.csswg.org/css-images/#image-values -#[derive(Clone, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Clone, PartialEq, ToComputedValue)] pub enum Image { /// A `` image. Url(ImageUrl), @@ -34,43 +33,6 @@ pub enum Image { PaintWorklet(PaintWorklet), } -// FIXME(nox): Implement TCV for Box and derive this. -impl ToComputedValue for Image { - type ComputedValue = Image<::ComputedValue, - ::ComputedValue, - ::ComputedValue>; - - #[inline] - fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { - match *self { - Image::Url(ref url) => Image::Url(url.to_computed_value(context)), - Image::Gradient(ref gradient) => - Image::Gradient(Box::new(gradient.to_computed_value(context))), - Image::Rect(ref rect) => Image::Rect(Box::new(rect.to_computed_value(context))), - Image::Element(ref atom) => Image::Element(atom.to_computed_value(context)), - #[cfg(feature = "servo")] - Image::PaintWorklet(ref worklet) => Image::PaintWorklet(worklet.to_computed_value(context)), - } - } - - #[inline] - fn from_computed_value(computed: &Self::ComputedValue) -> Self { - match *computed { - Image::Url(ref url) => Image::Url(ImageUrl::from_computed_value(url)), - Image::Gradient(ref boxed_gradient) => - Image::Gradient(Box::new(Gradient::from_computed_value(&*boxed_gradient))), - Image::Rect(ref boxed_rect) => - Image::Rect(Box::new(MozImageRect::from_computed_value(&*boxed_rect))), - Image::Element(ref atom) => Image::Element(Atom::from_computed_value(atom)), - #[cfg(feature = "servo")] - Image::PaintWorklet(ref worklet) => - Image::PaintWorklet(PaintWorklet::from_computed_value(worklet)), - } - } -} - /// A CSS gradient. /// https://drafts.csswg.org/css-images/#gradients #[cfg_attr(feature = "servo", derive(HeapSizeOf))]