From 79775541f28ee976c69ac79aedc4b83680d396fa Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 9 Feb 2018 22:34:59 +0100 Subject: [PATCH 1/8] Make AnimationValue have the same variants as PropertyDeclaration By making AnimationValue have the same representation as PropertyDeclaration and Void variants for non-animatable properties, we know by constructions that all properties have the same discriminant in both. --- Cargo.lock | 2 + components/malloc_size_of/Cargo.toml | 1 + components/malloc_size_of/lib.rs | 9 +++ components/style/Cargo.toml | 1 + components/style/lib.rs | 1 + .../helpers/animated_properties.mako.rs | 63 +++++++++++-------- .../style/properties/properties.mako.rs | 16 ++--- 7 files changed, 58 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9b07d56bd6a..7cad7427505 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1562,6 +1562,7 @@ dependencies = [ "smallvec 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", "webrender_api 0.57.0 (git+https://github.com/servo/webrender)", "xml5ever 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -2926,6 +2927,7 @@ dependencies = [ "uluru 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "unicode-bidi 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "unicode-segmentation 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", "walkdir 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/components/malloc_size_of/Cargo.toml b/components/malloc_size_of/Cargo.toml index 9ff69d28945..2a4ab316f4b 100644 --- a/components/malloc_size_of/Cargo.toml +++ b/components/malloc_size_of/Cargo.toml @@ -26,3 +26,4 @@ string_cache = { version = "0.7", optional = true } url = { version = "1.2", optional = true } webrender_api = { git = "https://github.com/servo/webrender", features = ["ipc"], optional = true } xml5ever = { version = "0.12", optional = true } +void = "1.0.2" diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index 074ebd6b675..3801669ab5d 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -60,6 +60,7 @@ extern crate string_cache; extern crate url; #[cfg(feature = "webrender_api")] extern crate webrender_api; +extern crate void; #[cfg(feature = "servo")] extern crate xml5ever; @@ -67,6 +68,7 @@ use std::hash::{BuildHasher, Hash}; use std::mem::size_of; use std::ops::Range; use std::os::raw::c_void; +use void::Void; /// A C function that takes a pointer to a heap allocation and returns its size. type VoidPtrToSizeFn = unsafe extern "C" fn(ptr: *const c_void) -> usize; @@ -746,6 +748,13 @@ impl MallocSizeOf for selectors::parser::AncestorHashes { } } +impl MallocSizeOf for Void { + #[inline] + fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { + void::unreachable(*self) + } +} + #[cfg(feature = "servo")] impl MallocSizeOf for string_cache::Atom { fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index ac7d71f6478..915308030b6 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -70,6 +70,7 @@ time = "0.1" uluru = "0.2" unicode-bidi = "0.3" unicode-segmentation = "1.0" +void = "1.0.2" [target.'cfg(windows)'.dependencies] kernel32-sys = "0.2" diff --git a/components/style/lib.rs b/components/style/lib.rs index cbe8a12ee02..b7d062b3bdb 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -78,6 +78,7 @@ extern crate uluru; extern crate unicode_bidi; #[allow(unused_extern_crates)] extern crate unicode_segmentation; +extern crate void; #[macro_use] mod macros; diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index af32c77a64e..703b6be9fe5 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -58,6 +58,7 @@ use values::generics::position as generic_position; use values::generics::svg::{SVGLength, SvgLengthOrPercentageOrNumber, SVGPaint}; use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity}; use values::specified::font::FontTag; +use void::{self, Void}; /// pub trait RepeatableListAnimatable: Animate {} @@ -343,16 +344,20 @@ unsafe impl HasSimpleFFI for AnimationValueMap {} /// this (is a similar path to that of PropertyDeclaration). #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] +#[repr(u16)] pub enum AnimationValue { % for prop in data.longhands: - % if prop.animatable: - /// ${prop.name} - % if prop.is_animatable_with_computed_value: - ${prop.camel_case}(longhands::${prop.ident}::computed_value::T), - % else: - ${prop.camel_case}(::AnimatedValue), - % endif - % endif + % if prop.animatable: + /// `${prop.name}` + % if prop.is_animatable_with_computed_value: + ${prop.camel_case}(longhands::${prop.ident}::computed_value::T), + % else: + ${prop.camel_case}(::AnimatedValue), + % endif + % else: + /// `${prop.name}` (not animatable) + ${prop.camel_case}(Void), + % endif % endfor } @@ -362,7 +367,9 @@ impl AnimationValue { match *self { % for prop in data.longhands: % if prop.animatable: - AnimationValue::${prop.camel_case}(..) => LonghandId::${prop.camel_case}, + AnimationValue::${prop.camel_case}(..) => LonghandId::${prop.camel_case}, + % else: + AnimationValue::${prop.camel_case}(void) => void::unreachable(void), % endif % endfor } @@ -374,24 +381,26 @@ impl AnimationValue { use properties::longhands; match *self { % for prop in data.longhands: - % if prop.animatable: - AnimationValue::${prop.camel_case}(ref from) => { - PropertyDeclaration::${prop.camel_case}( - % if prop.boxed: - Box::new( - % endif - longhands::${prop.ident}::SpecifiedValue::from_computed_value( - % if prop.is_animatable_with_computed_value: - from - % else: - &ToAnimatedValue::from_animated_value(from.clone()) - % endif - )) - % if prop.boxed: - ) - % endif - } - % endif + % if prop.animatable: + AnimationValue::${prop.camel_case}(ref from) => { + PropertyDeclaration::${prop.camel_case}( + % if prop.boxed: + Box::new( + % endif + longhands::${prop.ident}::SpecifiedValue::from_computed_value( + % if prop.is_animatable_with_computed_value: + from + % else: + &ToAnimatedValue::from_animated_value(from.clone()) + % endif + )) + % if prop.boxed: + ) + % endif + } + % else: + AnimationValue::${prop.camel_case}(void) => void::unreachable(void), + % endif % endfor } } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 01d32eb0e49..254bfc254c1 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -195,14 +195,6 @@ pub mod shorthands { %> } -/// A module with all the code related to animated properties. -/// -/// This needs to be "included" by mako at least after all longhand modules, -/// given they populate the global data. -pub mod animated_properties { - <%include file="/helpers/animated_properties.mako.rs" /> -} - <% from itertools import groupby @@ -413,6 +405,14 @@ impl PropertyDeclaration { } } +/// A module with all the code related to animated properties. +/// +/// This needs to be "included" by mako at least after all longhand modules, +/// given they populate the global data. +pub mod animated_properties { + <%include file="/helpers/animated_properties.mako.rs" /> +} + /// A longhand or shorthand porperty #[derive(Clone, Copy, Debug)] pub struct NonCustomPropertyId(usize); From 921d1aeebada477a8ebe35b75e44e9dbecae8842 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 9 Feb 2018 22:54:33 +0100 Subject: [PATCH 2/8] =?UTF-8?q?Make=20AnimationValue::id=20be=20just=20a?= =?UTF-8?q?=20pointer=20cast=20=F0=9F=90=89=F0=9F=90=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that PropertyDeclaration and AnimationValue have the same discriminants, that means the trick found in PropertyDeclaration::id can be done in AnimationValue::id. --- components/malloc_size_of/lib.rs | 2 +- .../style/properties/helpers/animated_properties.mako.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index 3801669ab5d..5e97a2709dc 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -58,9 +58,9 @@ extern crate smallvec; extern crate string_cache; #[cfg(feature = "url")] extern crate url; +extern crate void; #[cfg(feature = "webrender_api")] extern crate webrender_api; -extern crate void; #[cfg(feature = "servo")] extern crate xml5ever; diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 703b6be9fe5..a5f1abd935e 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -364,7 +364,8 @@ pub enum AnimationValue { impl AnimationValue { /// Returns the longhand id this animated value corresponds to. pub fn id(&self) -> LonghandId { - match *self { + let id = unsafe { *(self as *const _ as *const LonghandId) }; + debug_assert_eq!(id, match *self { % for prop in data.longhands: % if prop.animatable: AnimationValue::${prop.camel_case}(..) => LonghandId::${prop.camel_case}, @@ -372,7 +373,8 @@ impl AnimationValue { AnimationValue::${prop.camel_case}(void) => void::unreachable(void), % endif % endfor - } + }); + id } /// "Uncompute" this animation value in order to be used inside the CSS From 57daf06c711c5a0e58379f23f46d248e2f68b2ba Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 9 Feb 2018 23:58:33 +0100 Subject: [PATCH 3/8] Make PropertyDeclaration::id and AnimationValue::id inline --- components/style/properties/helpers/animated_properties.mako.rs | 1 + components/style/properties/properties.mako.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index a5f1abd935e..f47a0cc949d 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -363,6 +363,7 @@ pub enum AnimationValue { impl AnimationValue { /// Returns the longhand id this animated value corresponds to. + #[inline] pub fn id(&self) -> LonghandId { let id = unsafe { *(self as *const _ as *const LonghandId) }; debug_assert_eq!(id, match *self { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 254bfc254c1..fb3281348a5 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1739,6 +1739,7 @@ impl fmt::Debug for PropertyDeclaration { impl PropertyDeclaration { /// Given a property declaration, return the property declaration id. + #[inline] pub fn id(&self) -> PropertyDeclarationId { match *self { PropertyDeclaration::Custom(ref declaration) => { From a90e660eacf78daba3b30048fd7111afe4659e72 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 10 Feb 2018 01:28:38 +0100 Subject: [PATCH 4/8] Kill AnimatedValueAsComputed --- components/style/macros.rs | 6 ++-- components/style/values/animated/mod.rs | 44 ++++++++++++------------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/components/style/macros.rs b/components/style/macros.rs index 5f20e3c8d0c..b2cce740002 100644 --- a/components/style/macros.rs +++ b/components/style/macros.rs @@ -68,8 +68,8 @@ macro_rules! try_match_ident_ignore_ascii_case { macro_rules! define_keyword_type { ($name: ident, $css: expr) => { #[allow(missing_docs)] - #[derive(Animate, Clone, ComputeSquaredDistance, Copy, MallocSizeOf, PartialEq)] - #[derive(ToAnimatedZero, ToComputedValue, ToCss)] + #[derive(Animate, Clone, ComputeSquaredDistance, Copy, MallocSizeOf)] + #[derive(PartialEq, ToAnimatedValue, ToAnimatedZero, ToComputedValue, ToCss)] pub struct $name; impl fmt::Debug for $name { @@ -86,8 +86,6 @@ macro_rules! define_keyword_type { input.expect_ident_matching($css).map(|_| $name).map_err(|e| e.into()) } } - - impl $crate::values::animated::AnimatedValueAsComputed for $name {} }; } diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index f7100bf77c8..17151a51d17 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -76,9 +76,6 @@ pub trait ToAnimatedValue { fn from_animated_value(animated: Self::AnimatedValue) -> Self; } -/// Marker trait for computed values with the same representation during animations. -pub trait AnimatedValueAsComputed {} - /// Returns a value similar to `self` that represents zero. /// /// This trait is derivable with `#[derive(ToAnimatedValue)]`. If a field is @@ -243,31 +240,32 @@ where } } -impl AnimatedValueAsComputed for Au {} -impl AnimatedValueAsComputed for ComputedAngle {} -impl AnimatedValueAsComputed for SpecifiedUrl {} -#[cfg(feature = "servo")] -impl AnimatedValueAsComputed for ComputedUrl {} -impl AnimatedValueAsComputed for bool {} -impl AnimatedValueAsComputed for f32 {} +macro_rules! trivial_to_animated_value { + ($ty:ty) => { + impl $crate::values::animated::ToAnimatedValue for $ty { + type AnimatedValue = Self; -impl ToAnimatedValue for T -where - T: AnimatedValueAsComputed, -{ - type AnimatedValue = Self; + #[inline] + fn to_animated_value(self) -> Self { + self + } - #[inline] - fn to_animated_value(self) -> Self { - self - } - - #[inline] - fn from_animated_value(animated: Self::AnimatedValue) -> Self { - animated + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + animated + } + } } } +trivial_to_animated_value!(Au); +trivial_to_animated_value!(ComputedAngle); +trivial_to_animated_value!(SpecifiedUrl); +#[cfg(feature = "servo")] +trivial_to_animated_value!(ComputedUrl); +trivial_to_animated_value!(bool); +trivial_to_animated_value!(f32); + impl ToAnimatedValue for ComputedNonNegativeNumber { type AnimatedValue = Self; From aa7cc261f826f1fb2a4735ac36f50bbba2a8640e Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 10 Feb 2018 01:29:14 +0100 Subject: [PATCH 5/8] Determine animated types from specified types --- components/style/properties/data.py | 12 ++++++++++++ .../properties/helpers/animated_properties.mako.rs | 11 +++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/components/style/properties/data.py b/components/style/properties/data.py index ae25fb58e84..f87278f2478 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -229,6 +229,11 @@ class Longhand(object): def enabled_in_content(self): return self.enabled_in == "content" + def base_type(self): + if self.predefined_type and not self.is_vector: + return "::values::specified::{}".format(self.predefined_type) + return "longhands::{}::SpecifiedValue".format(self.ident) + def specified_type(self): if self.predefined_type and not self.is_vector: ty = "::values::specified::{}".format(self.predefined_type) @@ -281,6 +286,13 @@ class Longhand(object): } return bool(self.keyword) + def animated_type(self): + assert self.animatable + computed = "<{} as ToComputedValue>::ComputedValue".format(self.base_type()) + if self.is_animatable_with_computed_value: + return computed + return "<{} as ToAnimatedValue>::AnimatedValue".format(computed) + class Shorthand(object): def __init__(self, name, sub_properties, spec=None, servo_pref=None, gecko_pref=None, diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index f47a0cc949d..70bd242b00f 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -4,7 +4,10 @@ <%namespace name="helpers" file="/helpers.mako.rs" /> -<% from data import to_idl_name, SYSTEM_FONT_LONGHANDS %> +<% + from data import to_idl_name, SYSTEM_FONT_LONGHANDS + from itertools import groupby +%> use cssparser::Parser; #[cfg(feature = "gecko")] use gecko_bindings::bindings::RawServoAnimationValueMap; @@ -349,11 +352,7 @@ pub enum AnimationValue { % for prop in data.longhands: % if prop.animatable: /// `${prop.name}` - % if prop.is_animatable_with_computed_value: - ${prop.camel_case}(longhands::${prop.ident}::computed_value::T), - % else: - ${prop.camel_case}(::AnimatedValue), - % endif + ${prop.camel_case}(${prop.animated_type()}), % else: /// `${prop.name}` (not animatable) ${prop.camel_case}(Void), From 38520af970c30a7913dbfaf5abe59fe1130e2787 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 10 Feb 2018 03:17:45 +0100 Subject: [PATCH 6/8] =?UTF-8?q?Merge=20similar=20arms=20in=20AnimationValu?= =?UTF-8?q?e::uncompute=20=F0=9F=90=89=F0=9F=90=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This uses the same kind of trick as PropertyDeclaration::clone and removes 15KB off of libxul according to bloaty. --- .../helpers/animated_properties.mako.rs | 65 +++++++++++++------ 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 70bd242b00f..8ea4a414f8d 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -27,7 +27,7 @@ use properties::{LonghandId, ShorthandId}; use selectors::parser::SelectorParseErrorKind; use servo_arc::Arc; use smallvec::SmallVec; -use std::cmp; +use std::{cmp, mem, ptr}; use std::fmt::{self, Write}; #[cfg(feature = "gecko")] use hash::FnvHashMap; use style_traits::{CssWriter, ParseError, ToCss}; @@ -360,6 +360,16 @@ pub enum AnimationValue { % endfor } +<% + animated = [] + unanimated = [] + for prop in data.longhands: + if prop.animatable: + animated.append(prop) + else: + unanimated.append(prop) +%> + impl AnimationValue { /// Returns the longhand id this animated value corresponds to. #[inline] @@ -381,29 +391,42 @@ impl AnimationValue { /// cascade. pub fn uncompute(&self) -> PropertyDeclaration { use properties::longhands; + use self::AnimationValue::*; + + use super::PropertyDeclarationVariantRepr; + match *self { - % for prop in data.longhands: - % if prop.animatable: - AnimationValue::${prop.camel_case}(ref from) => { - PropertyDeclaration::${prop.camel_case}( - % if prop.boxed: - Box::new( - % endif - longhands::${prop.ident}::SpecifiedValue::from_computed_value( - % if prop.is_animatable_with_computed_value: - from - % else: - &ToAnimatedValue::from_animated_value(from.clone()) - % endif - )) - % if prop.boxed: - ) - % endif + <% keyfunc = lambda x: (x.base_type(), x.specified_type(), x.boxed, x.is_animatable_with_computed_value) %> + % for (ty, specified, boxed, computed), props in groupby(animated, key=keyfunc): + <% props = list(props) %> + ${" |\n".join("{}(ref value)".format(prop.camel_case) for prop in props)} => { + % if not computed: + let ref value = ToAnimatedValue::from_animated_value(value.clone()); + % endif + let value = ${ty}::from_computed_value(&value); + % if boxed: + let value = Box::new(value); + % endif + % if len(props) == 1: + PropertyDeclaration::${props[0].camel_case}(value) + % else: + unsafe { + let mut out = mem::uninitialized(); + ptr::write( + &mut out as *mut _ as *mut PropertyDeclarationVariantRepr<${specified}>, + PropertyDeclarationVariantRepr { + tag: *(self as *const _ as *const u16), + value, + }, + ); + out + } + % endif } - % else: - AnimationValue::${prop.camel_case}(void) => void::unreachable(void), - % endif % endfor + ${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => { + void::unreachable(void) + } } } From fc24cf34c5975a65319ea26790060b009f207b9c Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 10 Feb 2018 03:49:11 +0100 Subject: [PATCH 7/8] =?UTF-8?q?Merge=20similar=20arms=20in=20AnimationValu?= =?UTF-8?q?e::animate=20=F0=9F=90=89=F0=9F=90=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This uses the same trick as in PropertyDeclaration::eq and removes roughly 10KB off of libxul. --- .../helpers/animated_properties.mako.rs | 68 +++++++++++-------- 1 file changed, 41 insertions(+), 27 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 8ea4a414f8d..3e765a997d4 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -566,35 +566,49 @@ fn animate_discrete(this: &T, other: &T, procedure: Procedure) -> Resu impl Animate for AnimationValue { fn animate(&self, other: &Self, procedure: Procedure) -> Result { - let value = match (self, other) { - % for prop in data.longhands: - % if prop.animatable: - % if prop.animation_value_type != "discrete": - ( - &AnimationValue::${prop.camel_case}(ref this), - &AnimationValue::${prop.camel_case}(ref other), - ) => { - AnimationValue::${prop.camel_case}( - this.animate(other, procedure)?, - ) - }, - % else: - ( - &AnimationValue::${prop.camel_case}(ref this), - &AnimationValue::${prop.camel_case}(ref other), - ) => { - AnimationValue::${prop.camel_case}( - animate_discrete(this, other, procedure)? - ) - }, - % endif - % endif - % endfor - _ => { + use self::AnimationValue::*; + + Ok(unsafe { + #[repr(C)] + struct AnimationValueVariantRepr { + tag: u16, + value: T + } + + let this_tag = *(self as *const _ as *const u16); + let other_tag = *(other as *const _ as *const u16); + if this_tag != other_tag { panic!("Unexpected AnimationValue::animate call"); } - }; - Ok(value) + + match *self { + <% keyfunc = lambda x: (x.animated_type(), x.animation_value_type == "discrete") %> + % for (ty, discrete), props in groupby(animated, key=keyfunc): + ${" |\n".join("{}(ref this)".format(prop.camel_case) for prop in props)} => { + let other_repr = + &*(other as *const _ as *const AnimationValueVariantRepr<${ty}>); + % if discrete: + let value = animate_discrete(this, &other_repr.value, procedure)?; + % else: + let value = this.animate(&other_repr.value, procedure)?; + % endif + + let mut out = mem::uninitialized(); + ptr::write( + &mut out as *mut _ as *mut AnimationValueVariantRepr<${ty}>, + AnimationValueVariantRepr { + tag: this_tag, + value, + }, + ); + out + } + % endfor + ${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => { + void::unreachable(void) + } + } + }) } } From b9505ae72b7a4c1c758e0c367dcda908c15eabe2 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 10 Feb 2018 03:49:11 +0100 Subject: [PATCH 8/8] =?UTF-8?q?Merge=20similar=20arms=20in=20AnimationValu?= =?UTF-8?q?e::compute=5Fsquared=5Fdistance=20=F0=9F=90=89=F0=9F=90=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This uses the same trick as in PropertyDeclaration::eq. --- .../helpers/animated_properties.mako.rs | 64 ++++++++++--------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 3e765a997d4..ba0eb950005 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -564,16 +564,16 @@ 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 { - use self::AnimationValue::*; - Ok(unsafe { - #[repr(C)] - struct AnimationValueVariantRepr { - tag: u16, - value: T - } + use self::AnimationValue::*; let this_tag = *(self as *const _ as *const u16); let other_tag = *(other as *const _ as *const u16); @@ -612,31 +612,35 @@ impl Animate for AnimationValue { } } +<% + nondiscrete = [] + for prop in animated: + if prop.animation_value_type != "discrete": + nondiscrete.append(prop) +%> + impl ComputeSquaredDistance for AnimationValue { fn compute_squared_distance(&self, other: &Self) -> Result { - match *self { - % for i, prop in enumerate([p for p in data.longhands if p.animatable and p.animation_value_type == "discrete"]): - % if i > 0: - | - % endif - AnimationValue::${prop.camel_case}(..) - % endfor - => return Err(()), - _ => (), - } - match (self, other) { - % for prop in data.longhands: - % if prop.animatable: - % if prop.animation_value_type != "discrete": - (&AnimationValue::${prop.camel_case}(ref this), &AnimationValue::${prop.camel_case}(ref other)) => { - this.compute_squared_distance(other) - }, - % endif - % endif - % endfor - _ => { - panic!("computed values should be of the same property"); - }, + unsafe { + use self::AnimationValue::*; + + let this_tag = *(self as *const _ as *const u16); + let other_tag = *(other as *const _ as *const u16); + if this_tag != other_tag { + panic!("Unexpected AnimationValue::compute_squared_distance call"); + } + + match *self { + % for ty, props in groupby(nondiscrete, 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.compute_squared_distance(&other_repr.value) + } + % endfor + _ => Err(()), + } } } }