From b95b6c66f3d2e477452884dd3ac5f3313bc56ad5 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 5 Feb 2018 23:51:45 +0100 Subject: [PATCH 1/3] =?UTF-8?q?Implement=20PartialEq=20for=20AnimationValu?= =?UTF-8?q?e=20by=20hand=20=F0=9F=90=89=F0=9F=90=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We use the same trick as in PropertyDeclaration::eq. --- .../helpers/animated_properties.mako.rs | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index ba0eb950005..c3a80eda360 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -345,8 +345,8 @@ unsafe impl HasSimpleFFI for AnimationValueMap {} /// /// FIXME: We need to add a path for custom properties, but that's trivial after /// this (is a similar path to that of PropertyDeclaration). -#[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] +#[derive(Clone, Debug)] #[repr(u16)] pub enum AnimationValue { % for prop in data.longhands: @@ -370,6 +370,40 @@ pub enum AnimationValue { unanimated.append(prop) %> +#[repr(C)] +struct AnimationValueVariantRepr { + tag: u16, + value: T +} + +impl PartialEq for AnimationValue { + #[inline] + fn eq(&self, other: &Self) -> bool { + use self::AnimationValue::*; + + unsafe { + let this_tag = *(self as *const _ as *const u16); + let other_tag = *(other as *const _ as *const u16); + if this_tag != other_tag { + return false; + } + + match *self { + % for ty, props in groupby(animated, key=lambda x: x.animated_type()): + ${" |\n".join("{}(ref this)".format(prop.camel_case) for prop in props)} => { + let other_repr = + &*(other as *const _ as *const AnimationValueVariantRepr<${ty}>); + *this == other_repr.value + } + % endfor + ${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => { + void::unreachable(void) + } + } + } + } +} + impl AnimationValue { /// Returns the longhand id this animated value corresponds to. #[inline] @@ -564,12 +598,6 @@ fn animate_discrete(this: &T, other: &T, procedure: Procedure) -> Resu } } -#[repr(C)] -struct AnimationValueVariantRepr { - tag: u16, - value: T -} - impl Animate for AnimationValue { fn animate(&self, other: &Self, procedure: Procedure) -> Result { Ok(unsafe { From c92d0a8902054ed067e567ce78751d0dcbc08fd8 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 5 Feb 2018 23:51:45 +0100 Subject: [PATCH 2/3] =?UTF-8?q?Implement=20Clone=20for=20AnimationValue=20?= =?UTF-8?q?by=20hand=20=F0=9F=90=89=F0=9F=90=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We use the same trick as in PropertyDeclaration::clone. --- .../helpers/animated_properties.mako.rs | 60 ++++++++++++++++++- components/style/values/computed/border.rs | 2 +- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index c3a80eda360..ad64c3e7224 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -27,8 +27,9 @@ use properties::{LonghandId, ShorthandId}; use selectors::parser::SelectorParseErrorKind; use servo_arc::Arc; use smallvec::SmallVec; -use std::{cmp, mem, ptr}; +use std::{cmp, ptr}; use std::fmt::{self, Write}; +use std::mem::{self, ManuallyDrop}; #[cfg(feature = "gecko")] use hash::FnvHashMap; use style_traits::{CssWriter, ParseError, ToCss}; use super::ComputedValues; @@ -346,7 +347,7 @@ unsafe impl HasSimpleFFI for AnimationValueMap {} /// FIXME: We need to add a path for custom properties, but that's trivial after /// this (is a similar path to that of PropertyDeclaration). #[cfg_attr(feature = "servo", derive(MallocSizeOf))] -#[derive(Clone, Debug)] +#[derive(Debug)] #[repr(u16)] pub enum AnimationValue { % for prop in data.longhands: @@ -376,6 +377,61 @@ struct AnimationValueVariantRepr { value: T } +impl Clone for AnimationValue { + #[inline] + fn clone(&self) -> Self { + use self::AnimationValue::*; + + <% + [copy, others] = [list(g) for _, g in groupby(animated, key=lambda x: not x.specified_is_copy())] + %> + + let self_tag = unsafe { *(self as *const _ as *const u16) }; + if self_tag <= LonghandId::${copy[-1].camel_case} as u16 { + #[derive(Clone, Copy)] + #[repr(u16)] + enum CopyVariants { + % for prop in copy: + _${prop.camel_case}(${prop.animated_type()}), + % endfor + } + + unsafe { + let mut out = mem::uninitialized(); + ptr::write( + &mut out as *mut _ as *mut CopyVariants, + *(self as *const _ as *const CopyVariants), + ); + return out; + } + } + + match *self { + % for ty, props in groupby(others, key=lambda x: x.animated_type()): + <% props = list(props) %> + ${" |\n".join("{}(ref value)".format(prop.camel_case) for prop in props)} => { + % if len(props) == 1: + ${props[0].camel_case}(value.clone()) + % else: + unsafe { + let mut out = ManuallyDrop::new(mem::uninitialized()); + ptr::write( + &mut out as *mut _ as *mut AnimationValueVariantRepr<${ty}>, + AnimationValueVariantRepr { + tag: *(self as *const _ as *const u16), + value: value.clone(), + }, + ); + ManuallyDrop::into_inner(out) + } + % endif + } + % endfor + _ => unsafe { debug_unreachable!() } + } + } +} + impl PartialEq for AnimationValue { #[inline] fn eq(&self, other: &Self) -> bool { diff --git a/components/style/values/computed/border.rs b/components/style/values/computed/border.rs index bb9b71edeb6..cfef89a62e1 100644 --- a/components/style/values/computed/border.rs +++ b/components/style/values/computed/border.rs @@ -88,7 +88,7 @@ impl ToAnimatedZero for BorderCornerRadius { /// The computed value of the `border-image-repeat` property: /// /// https://drafts.csswg.org/css-backgrounds/#the-border-image-repeat -#[derive(Clone, Debug, MallocSizeOf, PartialEq)] +#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq)] pub struct BorderImageRepeat(pub BorderImageRepeatKeyword, pub BorderImageRepeatKeyword); impl BorderImageRepeat { From e2c674994a3cbf32e9046df06e354f7ce3cf0111 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 11 Feb 2018 15:54:31 +0100 Subject: [PATCH 3/3] Use ascii_case_insensitive_phf_map in TransitionProperty::parse This divides the size of this method by 30. --- .../helpers/animated_properties.mako.rs | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index ad64c3e7224..281d9c53fb3 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -24,7 +24,6 @@ use properties::longhands::visibility::computed_value::T as Visibility; #[cfg(feature = "gecko")] use properties::PropertyId; use properties::{LonghandId, ShorthandId}; -use selectors::parser::SelectorParseErrorKind; use servo_arc::Arc; use smallvec::SmallVec; use std::{cmp, ptr}; @@ -142,21 +141,42 @@ impl TransitionProperty { /// Parse a transition-property value. pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result> { + // FIXME(https://github.com/rust-lang/rust/issues/33156): remove this + // enum and use PropertyId when stable Rust allows destructors in + // statics. + // + // FIXME: This should handle aliases too. + pub enum StaticId { + All, + Longhand(LonghandId), + Shorthand(ShorthandId), + } + ascii_case_insensitive_phf_map! { + static_id -> StaticId = { + "all" => StaticId::All, + % for prop in data.shorthands_except_all(): + "${prop.name}" => StaticId::Shorthand(ShorthandId::${prop.camel_case}), + % endfor + % for prop in data.longhands: + "${prop.name}" => StaticId::Longhand(LonghandId::${prop.camel_case}), + % endfor + } + } + let location = input.current_source_location(); let ident = input.expect_ident()?; - match_ignore_ascii_case! { &ident, - "all" => Ok(TransitionProperty::All), - % for prop in data.shorthands_except_all(): - "${prop.name}" => Ok(TransitionProperty::Shorthand(ShorthandId::${prop.camel_case})), - % endfor - % for prop in data.longhands: - "${prop.name}" => Ok(TransitionProperty::Longhand(LonghandId::${prop.camel_case})), - % endfor - "none" => Err(location.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(ident.clone()))), - _ => CustomIdent::from_ident(location, ident, &[]).map(TransitionProperty::Unsupported), - } - } + Ok(match static_id(&ident) { + Some(&StaticId::All) => TransitionProperty::All, + Some(&StaticId::Longhand(id)) => TransitionProperty::Longhand(id), + Some(&StaticId::Shorthand(id)) => TransitionProperty::Shorthand(id), + None => { + TransitionProperty::Unsupported( + CustomIdent::from_ident(location, ident, &["none"])?, + ) + }, + }) + } /// Convert TransitionProperty to nsCSSPropertyID. #[cfg(feature = "gecko")]