diff --git a/components/style/animation.rs b/components/style/animation.rs index 91146571589..5d63938c768 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -10,8 +10,8 @@ use bezier::Bezier; use context::SharedStyleContext; use dom::OpaqueNode; use font_metrics::FontMetricsProvider; -use properties::{self, CascadeFlags, ComputedValues}; -use properties::animated_properties::{AnimatableLonghand, AnimatedProperty, TransitionProperty}; +use properties::{self, CascadeFlags, ComputedValues, LonghandId}; +use properties::animated_properties::{AnimatedProperty, TransitionProperty}; use properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection; use properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount; use properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState; @@ -271,87 +271,80 @@ impl PropertyAnimation { /// and new styles. Any number of animations may be returned, from zero (if /// the property did not animate) to one (for a single transition property) /// to arbitrarily many (for `all`). - pub fn from_transition(transition_index: usize, - old_style: &ComputedValues, - new_style: &mut ComputedValues) - -> Vec { + pub fn from_transition( + transition_index: usize, + old_style: &ComputedValues, + new_style: &mut ComputedValues, + ) -> Vec { let mut result = vec![]; let box_style = new_style.get_box(); let transition_property = box_style.transition_property_at(transition_index); let timing_function = box_style.transition_timing_function_mod(transition_index); let duration = box_style.transition_duration_mod(transition_index); - if let TransitionProperty::Unsupported(_) = transition_property { - return result - } - - if transition_property.is_shorthand() { - return transition_property.longhands().iter().filter_map(|transition_property| { - PropertyAnimation::from_transition_property(transition_property, - timing_function, - duration, - old_style, - new_style) - }).collect(); - } - - if transition_property != TransitionProperty::All { - if let Some(property_animation) = - PropertyAnimation::from_transition_property(&transition_property, - timing_function, - duration, - old_style, - new_style) { - result.push(property_animation) + match transition_property { + TransitionProperty::Unsupported(_) => result, + TransitionProperty::Shorthand(ref shorthand_id) => { + shorthand_id.longhands().iter().filter_map(|longhand| { + PropertyAnimation::from_longhand( + &longhand, + timing_function, + duration, + old_style, + new_style, + ) + }).collect() } - return result - } + TransitionProperty::Longhand(ref longhand_id) => { + let animation = PropertyAnimation::from_longhand( + longhand_id, + timing_function, + duration, + old_style, + new_style, + ); - TransitionProperty::each(|transition_property| { - if let Some(property_animation) = - PropertyAnimation::from_transition_property(&transition_property, - timing_function, - duration, - old_style, - new_style) { - result.push(property_animation) + if let Some(animation) = animation { + result.push(animation); + } + result } - }); + TransitionProperty::All => { + TransitionProperty::each(|longhand_id| { + let animation = PropertyAnimation::from_longhand( + longhand_id, + timing_function, + duration, + old_style, + new_style, + ); - result + if let Some(animation) = animation { + result.push(animation); + } + }); + result + } + } } - fn from_transition_property(transition_property: &TransitionProperty, - timing_function: TimingFunction, - duration: Time, - old_style: &ComputedValues, - new_style: &ComputedValues) - -> Option { - debug_assert!(!transition_property.is_shorthand() && - transition_property != &TransitionProperty::All); + fn from_longhand( + longhand: &LonghandId, + timing_function: TimingFunction, + duration: Time, + old_style: &ComputedValues, + new_style: &ComputedValues, + ) -> Option { + let animated_property = AnimatedProperty::from_longhand( + longhand, + old_style, + new_style, + ); - // We're not expecting |transition_property| to be a shorthand (including 'all') and - // all other transitionable properties should be animatable longhands (since transitionable - // is a subset of animatable). - let animatable_longhand = - AnimatableLonghand::from_transition_property(transition_property).unwrap(); - - PropertyAnimation::from_animatable_longhand(&animatable_longhand, - timing_function, - duration, - old_style, - new_style) - } - - fn from_animatable_longhand(animatable_longhand: &AnimatableLonghand, - timing_function: TimingFunction, - duration: Time, - old_style: &ComputedValues, - new_style: &ComputedValues) - -> Option { - let animated_property = AnimatedProperty::from_animatable_longhand(animatable_longhand, - old_style, - new_style); + let animated_property = match animated_property { + Some(p) => p, + None => return None, + }; let property_animation = PropertyAnimation { property: animated_property, @@ -758,14 +751,18 @@ pub fn update_style_for_animation(context: &SharedStyleContext, let mut new_style = (*style).clone(); - for property in &animation.properties_changed { + for property in animation.properties_changed.iter() { debug!("update_style_for_animation: scanning prop {:?} for animation \"{}\"", property, name); - match PropertyAnimation::from_animatable_longhand(property, - timing_function, - Time::from_seconds(relative_duration as f32), - &from_style, - &target_style) { + let animation = PropertyAnimation::from_longhand( + &property, + timing_function, + Time::from_seconds(relative_duration as f32), + &from_style, + &target_style + ); + + match animation { Some(property_animation) => { debug!("update_style_for_animation: got property animation for prop {:?}", property); debug!("update_style_for_animation: {:?}", property_animation); diff --git a/components/style/dom.rs b/components/style/dom.rs index 2b3ce58c74f..c749862a76b 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -17,6 +17,7 @@ use element_state::ElementState; use font_metrics::FontMetricsProvider; use media_queries::Device; use properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock}; +#[cfg(feature = "gecko")] use properties::LonghandId; #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValue; #[cfg(feature = "gecko")] use properties::animated_properties::TransitionProperty; use rule_tree::CascadeLevel; @@ -679,7 +680,7 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + #[cfg(feature = "gecko")] fn needs_transitions_update_per_property( &self, - property: &TransitionProperty, + property: &LonghandId, combined_duration: f32, before_change_style: &ComputedValues, after_change_style: &ComputedValues, diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index e659d585e85..5c709814dc6 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -69,9 +69,9 @@ use gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI}; use hash::HashMap; use logical_geometry::WritingMode; use media_queries::Device; -use properties::{ComputedValues, parse_style_attribute}; +use properties::{ComputedValues, LonghandId, parse_style_attribute}; use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; -use properties::animated_properties::{AnimatableLonghand, AnimationValue, AnimationValueMap}; +use properties::animated_properties::{AnimationValue, AnimationValueMap}; use properties::animated_properties::TransitionProperty; use properties::style_structs::Font; use rule_tree::CascadeLevel as ServoCascadeLevel; @@ -1357,12 +1357,13 @@ impl<'le> TElement for GeckoElement<'le> { // update. // // https://drafts.csswg.org/css-transitions/#starting - fn needs_transitions_update(&self, - before_change_style: &ComputedValues, - after_change_style: &ComputedValues) - -> bool { + fn needs_transitions_update( + &self, + before_change_style: &ComputedValues, + after_change_style: &ComputedValues + ) -> bool { use gecko_bindings::structs::nsCSSPropertyID; - use hash::HashSet; + use std::collections::HashSet; debug_assert!(self.might_need_transitions_update(Some(before_change_style), after_change_style), @@ -1372,13 +1373,6 @@ impl<'le> TElement for GeckoElement<'le> { let after_change_box_style = after_change_style.get_box(); let transitions_count = after_change_box_style.transition_property_count(); let existing_transitions = self.get_css_transitions_info(); - let mut transitions_to_keep = if !existing_transitions.is_empty() && - (after_change_box_style.transition_nscsspropertyid_at(0) != - nsCSSPropertyID::eCSSPropertyExtra_all_properties) { - Some(HashSet::::with_capacity(transitions_count)) - } else { - None - }; // Check if this property is none, custom or unknown. let is_none_or_custom_property = |property: nsCSSPropertyID| -> bool { @@ -1387,6 +1381,14 @@ impl<'le> TElement for GeckoElement<'le> { property == nsCSSPropertyID::eCSSProperty_UNKNOWN; }; + let mut transitions_to_keep = if !existing_transitions.is_empty() && + (after_change_box_style.transition_nscsspropertyid_at(0) != + nsCSSPropertyID::eCSSPropertyExtra_all_properties) { + Some(HashSet::::with_capacity(transitions_count)) + } else { + None + }; + for i in 0..transitions_count { let property = after_change_box_style.transition_nscsspropertyid_at(i); let combined_duration = after_change_box_style.transition_combined_duration_at(i); @@ -1398,21 +1400,14 @@ impl<'le> TElement for GeckoElement<'le> { let transition_property: TransitionProperty = property.into(); - let mut property_check_helper = |property: &TransitionProperty| -> bool { - if self.needs_transitions_update_per_property(property, - combined_duration, - before_change_style, - after_change_style, - &existing_transitions) { - return true; - } - - if let Some(set) = transitions_to_keep.as_mut() { - // The TransitionProperty here must be animatable, so cloning it is cheap - // because it is an integer-like enum. - set.insert(property.clone()); - } - false + let property_check_helper = |property: &LonghandId| -> bool { + self.needs_transitions_update_per_property( + property, + combined_duration, + before_change_style, + after_change_style, + &existing_transitions + ) }; match transition_property { @@ -1421,22 +1416,26 @@ impl<'le> TElement for GeckoElement<'le> { return true; } }, - TransitionProperty::Unsupported(_) => { }, - ref shorthand if shorthand.is_shorthand() => { - if shorthand.longhands().iter().any(|p| property_check_helper(p)) { + TransitionProperty::Unsupported(..) => {}, + TransitionProperty::Shorthand(ref shorthand) => { + if shorthand.longhands().iter().any(property_check_helper) { return true; } }, - ref longhand => { - if property_check_helper(longhand) { + TransitionProperty::Longhand(ref longhand_id) => { + if property_check_helper(longhand_id) { return true; } }, }; + + if let Some(ref mut transitions_to_keep) = transitions_to_keep { + transitions_to_keep.insert(transition_property); + } } - // Check if we have to cancel the running transition because this is not a matching - // transition-property value. + // Check if we have to cancel the running transition because this is not + // a matching transition-property value. transitions_to_keep.map_or(false, |set| { existing_transitions.keys().any(|property| !set.contains(property)) }) @@ -1444,7 +1443,7 @@ impl<'le> TElement for GeckoElement<'le> { fn needs_transitions_update_per_property( &self, - property: &TransitionProperty, + longhand_id: &LonghandId, combined_duration: f32, before_change_style: &ComputedValues, after_change_style: &ComputedValues, @@ -1452,27 +1451,42 @@ impl<'le> TElement for GeckoElement<'le> { ) -> bool { use values::animated::{Animate, Procedure}; - // |property| should be an animatable longhand - let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap(); - - if existing_transitions.contains_key(property) { - // If there is an existing transition, update only if the end value differs. - // If the end value has not changed, we should leave the currently running - // transition as-is since we don't want to interrupt its timing function. + // If there is an existing transition, update only if the end value + // differs. + // + // If the end value has not changed, we should leave the currently + // running transition as-is since we don't want to interrupt its timing + // function. + // + // FIXME(emilio): The shorthand / longhand mapping with transitions + // looks pretty fishy! + if let Some(ref existing) = existing_transitions.get(&TransitionProperty::Longhand(*longhand_id)) { let after_value = - Arc::new(AnimationValue::from_computed_values(&animatable_longhand, - after_change_style)); - return existing_transitions.get(property).unwrap() != &after_value; + AnimationValue::from_computed_values( + &longhand_id, + after_change_style + ).unwrap(); + + return ***existing != after_value } - let from = AnimationValue::from_computed_values(&animatable_longhand, - before_change_style); - let to = AnimationValue::from_computed_values(&animatable_longhand, - after_change_style); + let from = AnimationValue::from_computed_values( + &longhand_id, + before_change_style, + ); + let to = AnimationValue::from_computed_values( + &longhand_id, + after_change_style, + ); + + debug_assert_eq!(to.is_some(), from.is_some()); combined_duration > 0.0f32 && from != to && - from.animate(&to, Procedure::Interpolate { progress: 0.5 }).is_ok() + from.unwrap().animate( + to.as_ref().unwrap(), + Procedure::Interpolate { progress: 0.5 } + ).is_ok() } #[inline] diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 3f9a54ac0c9..2adcce9fc2f 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -156,10 +156,11 @@ pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> { } impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { - fn new(declarations: &'a PropertyDeclarationBlock, - context: &'cx mut Context<'cx_a>, - default_values: &'a ComputedValues, - extra_custom_properties: &'a Option>, + fn new( + declarations: &'a PropertyDeclarationBlock, + context: &'cx mut Context<'cx_a>, + default_values: &'a ComputedValues, + extra_custom_properties: &'a Option>, ) -> AnimationValueIterator<'a, 'cx, 'cx_a> { AnimationValueIterator { iter: declarations.declaration_importance_iter(), @@ -171,32 +172,29 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { } impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> { - type Item = (AnimatableLonghand, AnimationValue); + type Item = AnimationValue; #[inline] fn next(&mut self) -> Option { loop { let next = self.iter.next(); - match next { - Some((decl, importance)) => { - if importance == Importance::Normal { - let property = AnimatableLonghand::from_declaration(decl); - let animation = AnimationValue::from_declaration( - decl, - &mut self.context, - self.extra_custom_properties, - self.default_values - ); - debug_assert!(property.is_none() == animation.is_none(), - "The failure condition of AnimatableLonghand::from_declaration \ - and AnimationValue::from_declaration should be the same"); - // Skip the property if either ::from_declaration fails. - match (property, animation) { - (Some(p), Some(a)) => return Some((p, a)), - (_, _) => {}, - } - } - }, + let (decl, importance) = match next { + Some(decl_and_importance) => decl_and_importance, None => return None, + }; + + if importance.important() { + continue; + } + + let animation = AnimationValue::from_declaration( + decl, + &mut self.context, + self.extra_custom_properties, + self.default_values, + ); + + if let Some(anim) = animation { + return Some(anim); } } } @@ -651,14 +649,14 @@ impl PropertyDeclarationBlock { let mut longhands = LonghandIdSet::new(); for (property, animation_value) in animation_value_map.iter() { - longhands.set_animatable_longhand_bit(property); + longhands.insert(*property); declarations.push(animation_value.uncompute()); } PropertyDeclarationBlock { - declarations: declarations, + declarations, + longhands, declarations_importance: SmallBitVec::from_elem(len as u32, false), - longhands: longhands, } } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index eef9fc89a59..590eb4a0b7c 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3208,7 +3208,7 @@ fn static_assert() { TransitionProperty::Unsupported(ref ident) => unsafe { Gecko_StyleTransition_SetUnsupportedProperty(gecko, ident.0.as_ptr()) }, - _ => gecko.mProperty = (&servo).into(), + _ => gecko.mProperty = servo.to_nscsspropertyid().unwrap(), } } } else { diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 6c6b07238e5..bb2f4c8a5bf 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -11,7 +11,6 @@ use cssparser::Parser; #[cfg(feature = "gecko")] use gecko_bindings::structs::RawGeckoGfxMatrix4x4; #[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSPropertyID; #[cfg(feature = "gecko")] use gecko_bindings::sugar::ownership::{HasFFI, HasSimpleFFI}; -#[cfg(feature = "gecko")] use gecko_string_cache::Atom; use itertools::{EitherOrBoth, Itertools}; use properties::{CSSWideKeyword, PropertyDeclaration}; use properties::longhands; @@ -23,8 +22,9 @@ use properties::longhands::transform::computed_value::ComputedMatrix; use properties::longhands::transform::computed_value::ComputedOperation as TransformOperation; use properties::longhands::transform::computed_value::T as TransformList; use properties::longhands::visibility::computed_value::T as Visibility; -#[cfg(feature = "gecko")] use properties::{PropertyId, PropertyDeclarationId, LonghandId}; -#[cfg(feature = "gecko")] use properties::{ShorthandId}; +#[cfg(feature = "gecko")] +use properties::PropertyId; +use properties::{LonghandId, ShorthandId}; use selectors::parser::SelectorParseError; use servo_arc::Arc; use smallvec::SmallVec; @@ -32,7 +32,7 @@ use std::borrow::Cow; use std::cmp; use std::fmt; #[cfg(feature = "gecko")] use hash::FnvHashMap; -use style_traits::ParseError; +use style_traits::{ParseError, ToCss}; use super::ComputedValues; use values::{CSSFloat, CustomIdent, Either}; use values::animated::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero}; @@ -60,137 +60,6 @@ use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity}; /// https://drafts.csswg.org/css-transitions/#animtype-repeatable-list pub trait RepeatableListAnimatable: Animate {} -/// A longhand property whose animation type is not "none". -/// -/// NOTE: This includes the 'display' property since it is animatable from SMIL even though it is -/// not animatable from CSS animations or Web Animations. CSS transitions also does not allow -/// animating 'display', but for CSS transitions we have the separate TransitionProperty type. -#[derive(Clone, Eq, Hash, PartialEq)] -#[cfg_attr(feature = "gecko", derive(MallocSizeOf))] -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] -pub enum AnimatableLonghand { - % for prop in data.longhands: - % if prop.animatable: - /// ${prop.name} - ${prop.camel_case}, - % endif - % endfor -} - -impl fmt::Debug for AnimatableLonghand { - fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - let name = match *self { - % for property in data.longhands: - % if property.animatable: - AnimatableLonghand::${property.camel_case} => "${property.camel_case}", - % endif - % endfor - }; - formatter.write_str(name) - } -} - -impl AnimatableLonghand { - /// Returns true if this AnimatableLonghand is one of the discretely animatable properties. - pub fn is_discrete(&self) -> bool { - match *self { - % for prop in data.longhands: - % if prop.animation_value_type == "discrete": - AnimatableLonghand::${prop.camel_case} => true, - % endif - % endfor - _ => false - } - } - - /// Converts from an nsCSSPropertyID. Returns None if nsCSSPropertyID is not an animatable - /// longhand in Servo. - #[cfg(feature = "gecko")] - pub fn from_nscsspropertyid(css_property: nsCSSPropertyID) -> Option { - match css_property { - % for prop in data.longhands: - % if prop.animatable: - ${helpers.to_nscsspropertyid(prop.ident)} - => Some(AnimatableLonghand::${prop.camel_case}), - % endif - % endfor - _ => None - } - } - - /// Converts from TransitionProperty. Returns None if the property is not an animatable - /// longhand. - pub fn from_transition_property(transition_property: &TransitionProperty) -> Option { - match *transition_property { - % for prop in data.longhands: - % if prop.transitionable and prop.animatable: - TransitionProperty::${prop.camel_case} - => Some(AnimatableLonghand::${prop.camel_case}), - % endif - % endfor - _ => None - } - } - - /// Get an animatable longhand property from a property declaration. - pub fn from_declaration(declaration: &PropertyDeclaration) -> Option { - use properties::LonghandId; - match *declaration { - % for prop in data.longhands: - % if prop.animatable: - PropertyDeclaration::${prop.camel_case}(..) - => Some(AnimatableLonghand::${prop.camel_case}), - % endif - % endfor - PropertyDeclaration::CSSWideKeyword(id, _) | - PropertyDeclaration::WithVariables(id, _) => { - match id { - % for prop in data.longhands: - % if prop.animatable: - LonghandId::${prop.camel_case} => - Some(AnimatableLonghand::${prop.camel_case}), - % endif - % endfor - _ => None, - } - }, - _ => None, - } - } -} - -/// Convert to nsCSSPropertyID. -#[cfg(feature = "gecko")] -#[allow(non_upper_case_globals)] -impl<'a> From< &'a AnimatableLonghand> for nsCSSPropertyID { - fn from(property: &'a AnimatableLonghand) -> nsCSSPropertyID { - match *property { - % for prop in data.longhands: - % if prop.animatable: - AnimatableLonghand::${prop.camel_case} - => ${helpers.to_nscsspropertyid(prop.ident)}, - % endif - % endfor - } - } -} - -/// Convert to PropertyDeclarationId. -#[cfg(feature = "gecko")] -#[allow(non_upper_case_globals)] -impl<'a> From for PropertyDeclarationId<'a> { - fn from(property: AnimatableLonghand) -> PropertyDeclarationId<'a> { - match property { - % for prop in data.longhands: - % if prop.animatable: - AnimatableLonghand::${prop.camel_case} - => PropertyDeclarationId::Longhand(LonghandId::${prop.camel_case}), - % endif - % endfor - } - } -} - /// Returns true if this nsCSSPropertyID is one of the animatable properties. #[cfg(feature = "gecko")] pub fn nscsspropertyid_is_animatable(property: nsCSSPropertyID) -> bool { @@ -210,56 +79,52 @@ pub fn nscsspropertyid_is_animatable(property: nsCSSPropertyID) -> bool { // beforehand. #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -#[derive(Clone, Eq, Hash, PartialEq, ToCss, ToComputedValue)] +#[derive(Clone, Debug, Eq, Hash, PartialEq)] pub enum TransitionProperty { /// All, any transitionable property changing should generate a transition. + /// + /// FIXME(emilio): Can we remove this and just use + /// Shorthand(ShorthandId::All)? All, - % for prop in data.longhands + data.shorthands_except_all(): - % if prop.transitionable: - /// ${prop.name} - ${prop.camel_case}, - % endif - % endfor + /// A shorthand. + Shorthand(ShorthandId), + /// A longhand transitionable property. + Longhand(LonghandId), /// Unrecognized property which could be any non-transitionable, custom property, or /// unknown property. - Unsupported(CustomIdent) + Unsupported(CustomIdent), } -impl fmt::Debug for TransitionProperty { - fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - let name = match *self { - % for property in data.longhands + data.shorthands_except_all(): - % if property.transitionable: - TransitionProperty::${property.camel_case} => "${property.camel_case}", - % endif - % endfor - TransitionProperty::All => "All", - TransitionProperty::Unsupported(ref ident) => { - formatter.write_str("Unsupported(")?; - ident.fmt(formatter)?; - return formatter.write_str(")"); - } - }; - formatter.write_str(name) +impl ToCss for TransitionProperty { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + match *self { + TransitionProperty::All => dest.write_str("all"), + TransitionProperty::Shorthand(ref id) => dest.write_str(id.name()), + TransitionProperty::Longhand(ref id) => dest.write_str(id.name()), + TransitionProperty::Unsupported(ref id) => id.to_css(dest), + } } } +trivial_to_computed_value!(TransitionProperty); + impl TransitionProperty { /// Iterates over each longhand property. - pub fn each ()>(mut cb: F) { + pub fn each ()>(mut cb: F) { % for prop in data.longhands: % if prop.transitionable: - cb(&TransitionProperty::${prop.camel_case}); + cb(&LonghandId::${prop.camel_case}); % endif % endfor } - /// Iterates over every longhand property that is not TransitionProperty::All, stopping and - /// returning true when the provided callback returns true for the first time. - pub fn any bool>(mut cb: F) -> bool { + /// Iterates over every longhand property that is not + /// TransitionProperty::All, stopping and returning true when the provided + /// callback returns true for the first time. + pub fn any bool>(mut cb: F) -> bool { % for prop in data.longhands: % if prop.transitionable: - if cb(&TransitionProperty::${prop.camel_case}) { + if cb(&LonghandId::${prop.camel_case}) { return true; } % endif @@ -272,67 +137,27 @@ impl TransitionProperty { let ident = input.expect_ident()?; match_ignore_ascii_case! { &ident, "all" => Ok(TransitionProperty::All), - % for prop in data.longhands + data.shorthands_except_all(): - % if prop.transitionable: - "${prop.name}" => Ok(TransitionProperty::${prop.camel_case}), - % endif + % 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(SelectorParseError::UnexpectedIdent(ident.clone()).into()), _ => CustomIdent::from_ident(ident, &[]).map(TransitionProperty::Unsupported), } } - /// Return transitionable longhands of this shorthand TransitionProperty, except for "all". - pub fn longhands(&self) -> &'static [TransitionProperty] { - % for prop in data.shorthands_except_all(): - % if prop.transitionable: - static ${prop.ident.upper()}: &'static [TransitionProperty] = &[ - % for sub in prop.sub_properties: - % if sub.transitionable: - TransitionProperty::${sub.camel_case}, - % endif - % endfor - ]; - % endif - % endfor - match *self { - % for prop in data.shorthands_except_all(): - % if prop.transitionable: - TransitionProperty::${prop.camel_case} => ${prop.ident.upper()}, - % endif - % endfor - _ => panic!("Not allowed to call longhands() for this TransitionProperty") - } - } - /// Returns true if this TransitionProperty is a shorthand. - pub fn is_shorthand(&self) -> bool { - match *self { - % for prop in data.shorthands_except_all(): - % if prop.transitionable: - TransitionProperty::${prop.camel_case} => true, - % endif - % endfor - _ => false - } - } -} - -/// Convert to nsCSSPropertyID. -#[cfg(feature = "gecko")] -#[allow(non_upper_case_globals)] -impl<'a> From< &'a TransitionProperty> for nsCSSPropertyID { - fn from(transition_property: &'a TransitionProperty) -> nsCSSPropertyID { - match *transition_property { - % for prop in data.longhands + data.shorthands_except_all(): - % if prop.transitionable: - TransitionProperty::${prop.camel_case} - => ${helpers.to_nscsspropertyid(prop.ident)}, - % endif - % endfor + /// Convert TransitionProperty to nsCSSPropertyID. + #[cfg(feature = "gecko")] + pub fn to_nscsspropertyid(&self) -> Result { + Ok(match *self { TransitionProperty::All => nsCSSPropertyID::eCSSPropertyExtra_all_properties, - _ => panic!("Unconvertable Servo transition property: {:?}", transition_property), - } + TransitionProperty::Shorthand(ref id) => id.to_nscsspropertyid(), + TransitionProperty::Longhand(ref id) => id.to_nscsspropertyid(), + TransitionProperty::Unsupported(..) => return Err(()), + }) } } @@ -341,20 +166,22 @@ impl<'a> From< &'a TransitionProperty> for nsCSSPropertyID { #[allow(non_upper_case_globals)] impl From for TransitionProperty { fn from(property: nsCSSPropertyID) -> TransitionProperty { - let unsupported = match property { - % for prop in data.longhands + data.shorthands_except_all(): - % if prop.transitionable: - ${helpers.to_nscsspropertyid(prop.ident)} - => return TransitionProperty::${prop.camel_case}, - % else: - ${helpers.to_nscsspropertyid(prop.ident)} - => "${prop.ident}", - % endif + match property { + % for prop in data.longhands: + ${helpers.to_nscsspropertyid(prop.ident)} => { + TransitionProperty::Longhand(LonghandId::${prop.camel_case}) + } % endfor - nsCSSPropertyID::eCSSPropertyExtra_all_properties => return TransitionProperty::All, - _ => panic!("Unconvertable nsCSSPropertyID: {:?}", property), - }; - TransitionProperty::Unsupported(CustomIdent(Atom::from(unsupported))) + % for prop in data.shorthands_except_all(): + ${helpers.to_nscsspropertyid(prop.ident)} => { + TransitionProperty::Shorthand(ShorthandId::${prop.camel_case}) + } + % endfor + nsCSSPropertyID::eCSSPropertyExtra_all_properties => TransitionProperty::All, + _ => { + panic!("non-convertible nsCSSPropertyID::{:?}", property) + } + } } } @@ -459,14 +286,15 @@ impl AnimatedProperty { /// Get an animatable value from a transition-property, an old style, and a /// new style. - pub fn from_animatable_longhand(property: &AnimatableLonghand, - old_style: &ComputedValues, - new_style: &ComputedValues) - -> AnimatedProperty { - match *property { + pub fn from_longhand( + property: &LonghandId, + old_style: &ComputedValues, + new_style: &ComputedValues, + ) -> Option { + Some(match *property { % for prop in data.longhands: % if prop.animatable: - AnimatableLonghand::${prop.camel_case} => { + LonghandId::${prop.camel_case} => { let old_computed = old_style.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}(); let new_computed = new_style.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}(); AnimatedProperty::${prop.camel_case}( @@ -481,7 +309,8 @@ impl AnimatedProperty { } % endif % endfor - } + _ => return None, + }) } } @@ -489,7 +318,7 @@ impl AnimatedProperty { /// This HashMap stores the values that are the last AnimationValue to be /// composed for each TransitionProperty. #[cfg(feature = "gecko")] -pub type AnimationValueMap = FnvHashMap; +pub type AnimationValueMap = FnvHashMap; #[cfg(feature = "gecko")] unsafe impl HasFFI for AnimationValueMap { type FFIType = RawServoAnimationValueMap; @@ -525,18 +354,22 @@ pub enum AnimationValue { impl fmt::Debug for AnimationValue { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - let name = match *self { - % for prop in data.longhands: - % if prop.animatable: - AnimationValue::${prop.camel_case}(..) => "${prop.camel_case}", - % endif - % endfor - }; - formatter.write_str(name) + formatter.write_str(self.id().name()) } } impl AnimationValue { + /// Returns the longhand id this animated value corresponds to. + pub fn id(&self) -> LonghandId { + match *self { + % for prop in data.longhands: + % if prop.animatable: + AnimationValue::${prop.camel_case}(..) => LonghandId::${prop.camel_case}, + % endif + % endfor + } + } + /// "Uncompute" this animation value in order to be used inside the CSS /// cascade. pub fn uncompute(&self) -> PropertyDeclaration { @@ -657,27 +490,29 @@ impl AnimationValue { } /// Get an AnimationValue for an AnimatableLonghand from a given computed values. - pub fn from_computed_values(property: &AnimatableLonghand, - computed_values: &ComputedValues) - -> Self { - match *property { + pub fn from_computed_values( + property: &LonghandId, + computed_values: &ComputedValues + ) -> Option { + Some(match *property { % for prop in data.longhands: - % if prop.animatable: - AnimatableLonghand::${prop.camel_case} => { - let computed = computed_values - .get_${prop.style_struct.ident.strip("_")}() - .clone_${prop.ident}(); - AnimationValue::${prop.camel_case}( - % if prop.is_animatable_with_computed_value: - computed - % else: - computed.to_animated_value() - % endif - ) - } + % if prop.animatable: + LonghandId::${prop.camel_case} => { + let computed = computed_values + .get_${prop.style_struct.ident.strip("_")}() + .clone_${prop.ident}(); + AnimationValue::${prop.camel_case}( + % if prop.is_animatable_with_computed_value: + computed + % else: + computed.to_animated_value() % endif + ) + } + % endif % endfor - } + _ => return None, + }) } } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index e372277901c..09664f7dbee 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -33,7 +33,6 @@ use font_metrics::FontMetricsProvider; use logical_geometry::WritingMode; use media_queries::Device; use parser::ParserContext; -use properties::animated_properties::AnimatableLonghand; #[cfg(feature = "gecko")] use properties::longhands::system_font::SystemFont; use rule_cache::{RuleCache, RuleCacheConditions}; use selector_parser::PseudoElement; @@ -253,12 +252,56 @@ static ${name}: LonghandIdSet = LonghandIdSet { /// A set of longhand properties #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] -#[derive(Clone, PartialEq)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Clone, Debug, PartialEq)] pub struct LonghandIdSet { storage: [u32; (${len(data.longhands)} - 1 + 32) / 32] } +/// An iterator over a set of longhand ids. +pub struct LonghandIdSetIterator<'a> { + longhands: &'a LonghandIdSet, + cur: usize, +} + +impl<'a> Iterator for LonghandIdSetIterator<'a> { + type Item = LonghandId; + + fn next(&mut self) -> Option { + use std::mem; + + loop { + if self.cur >= ${len(data.longhands)} { + return None; + } + + let id: LonghandId = unsafe { mem::transmute(self.cur as ${"u16" if product == "gecko" else "u8"}) }; + self.cur += 1; + + if self.longhands.contains(id) { + return Some(id); + } + } + } +} + impl LonghandIdSet { + /// Iterate over the current longhand id set. + pub fn iter(&self) -> LonghandIdSetIterator { + LonghandIdSetIterator { longhands: self, cur: 0, } + } + + /// Returns whether this set contains at least every longhand that `other` + /// also contains. + pub fn contains_all(&self, other: &Self) -> bool { + for (self_cell, other_cell) in self.storage.iter().zip(other.storage.iter()) { + if (*self_cell & *other_cell) != *other_cell { + return false; + } + } + true + } + /// Create an empty set #[inline] pub fn new() -> LonghandIdSet { @@ -294,26 +337,10 @@ impl LonghandIdSet { } } - /// Set the corresponding bit of AnimatableLonghand. - pub fn set_animatable_longhand_bit(&mut self, property: &AnimatableLonghand) { - match *property { - % for prop in data.longhands: - % if prop.animatable: - AnimatableLonghand::${prop.camel_case} => self.insert(LonghandId::${prop.camel_case}), - % endif - % endfor - } - } - - /// Return true if the corresponding bit of AnimatableLonghand is set. - pub fn has_animatable_longhand_bit(&self, property: &AnimatableLonghand) -> bool { - match *property { - % for prop in data.longhands: - % if prop.animatable: - AnimatableLonghand::${prop.camel_case} => self.contains(LonghandId::${prop.camel_case}), - % endif - % endfor - } + /// Returns whether the set is empty. + #[inline] + pub fn is_empty(&self) -> bool { + self.storage.iter().all(|c| *c == 0) } } @@ -334,6 +361,24 @@ impl PropertyDeclarationIdSet { } } + /// Returns all the longhands that this set contains. + pub fn longhands(&self) -> &LonghandIdSet { + &self.longhands + } + + /// Returns whether the set is empty. + #[inline] + pub fn is_empty(&self) -> bool { + self.longhands.is_empty() && self.custom.is_empty() + } + + /// Clears the set. + #[inline] + pub fn clear(&mut self) { + self.longhands.clear(); + self.custom.clear(); + } + /// Returns whether the given ID is in the set pub fn contains(&mut self, id: PropertyDeclarationId) -> bool { match id { @@ -421,7 +466,7 @@ bitflags! { } /// An identifier for a given longhand property. -#[derive(Clone, Copy, Eq, PartialEq)] +#[derive(Clone, Copy, Eq, Hash, PartialEq)] #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum LonghandId { @@ -515,6 +560,50 @@ impl LonghandId { } } + /// Returns whether this property is animatable. + pub fn is_animatable(self) -> bool { + match self { + % for property in data.longhands: + LonghandId::${property.camel_case} => { + ${str(property.animatable).lower()} + } + % endfor + } + } + + /// Returns whether this property is animatable in a discrete way. + pub fn is_discrete_animatable(self) -> bool { + match self { + % for property in data.longhands: + LonghandId::${property.camel_case} => { + ${str(property.animation_value_type == "discrete").lower()} + } + % endfor + } + } + + /// Converts from a LonghandId to an adequate nsCSSPropertyID. + #[cfg(feature = "gecko")] + pub fn to_nscsspropertyid(self) -> nsCSSPropertyID { + match self { + % for property in data.longhands: + LonghandId::${property.camel_case} => { + ${helpers.to_nscsspropertyid(property.ident)} + } + % endfor + } + } + + #[cfg(feature = "gecko")] + #[allow(non_upper_case_globals)] + /// Returns a longhand id from Gecko's nsCSSPropertyID. + pub fn from_nscsspropertyid(id: nsCSSPropertyID) -> Result { + match PropertyId::from_nscsspropertyid(id) { + Ok(PropertyId::Longhand(id)) => Ok(id), + _ => Err(()), + } + } + /// If this is a logical property, return the corresponding physical one in the given writing mode. /// Otherwise, return unchanged. pub fn to_physical(&self, wm: WritingMode) -> Self { @@ -642,7 +731,8 @@ impl LonghandId { /// An identifier for a given shorthand property. #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -#[derive(Clone, Copy, Debug, Eq, PartialEq, ToCss)] +#[cfg_attr(feature = "gecko", derive(MallocSizeOf))] +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, ToCss)] pub enum ShorthandId { % for property in data.shorthands: /// ${property.name} @@ -660,6 +750,18 @@ impl ShorthandId { } } + /// Converts from a ShorthandId to an adequate nsCSSPropertyID. + #[cfg(feature = "gecko")] + pub fn to_nscsspropertyid(self) -> nsCSSPropertyID { + match self { + % for property in data.shorthands: + ShorthandId::${property.camel_case} => { + ${helpers.to_nscsspropertyid(property.ident)} + } + % endfor + } + } + /// Get the longhand ids that form this shorthand. pub fn longhands(&self) -> &'static [LonghandId] { % for property in data.shorthands: @@ -1112,23 +1214,9 @@ impl PropertyId { #[cfg(feature = "gecko")] #[allow(non_upper_case_globals)] pub fn to_nscsspropertyid(&self) -> Result { - use gecko_bindings::structs::*; - match *self { - PropertyId::Longhand(id) => match id { - % for property in data.longhands: - LonghandId::${property.camel_case} => { - Ok(${helpers.to_nscsspropertyid(property.ident)}) - } - % endfor - }, - PropertyId::Shorthand(id) => match id { - % for property in data.shorthands: - ShorthandId::${property.camel_case} => { - Ok(${helpers.to_nscsspropertyid(property.ident)}) - } - % endfor - }, + PropertyId::Longhand(id) => Ok(id.to_nscsspropertyid()), + PropertyId::Shorthand(id) => Ok(id.to_nscsspropertyid()), _ => Err(()) } } @@ -1485,8 +1573,8 @@ impl PropertyDeclaration { } } - /// Returns true if this property is one of the animable properties, false - /// otherwise. + /// Returns true if this property declaration is for one of the animatable + /// properties. pub fn is_animatable(&self) -> bool { match *self { % for property in data.longhands: diff --git a/components/style/stylesheets/keyframes_rule.rs b/components/style/stylesheets/keyframes_rule.rs index bd01a64529b..8541f58d248 100644 --- a/components/style/stylesheets/keyframes_rule.rs +++ b/components/style/stylesheets/keyframes_rule.rs @@ -11,7 +11,6 @@ use parser::{ParserContext, ParserErrorContext}; use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId, PropertyParserContext}; use properties::{PropertyDeclarationId, LonghandId, SourcePropertyDeclaration}; use properties::LonghandIdSet; -use properties::animated_properties::AnimatableLonghand; use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction; use selectors::parser::SelectorParseError; use servo_arc::Arc; @@ -360,16 +359,17 @@ pub struct KeyframesAnimation { /// The difference steps of the animation. pub steps: Vec, /// The properties that change in this animation. - pub properties_changed: Vec, + pub properties_changed: LonghandIdSet, /// Vendor prefix type the @keyframes has. pub vendor_prefix: Option, } /// Get all the animated properties in a keyframes animation. -fn get_animated_properties(keyframes: &[Arc>], guard: &SharedRwLockReadGuard) - -> Vec { - let mut ret = vec![]; - let mut seen = LonghandIdSet::new(); +fn get_animated_properties( + keyframes: &[Arc>], + guard: &SharedRwLockReadGuard +) -> LonghandIdSet { + let mut ret = LonghandIdSet::new(); // NB: declarations are already deduplicated, so we don't have to check for // it here. for keyframe in keyframes { @@ -382,15 +382,20 @@ fn get_animated_properties(keyframes: &[Arc>], guard: &SharedRw // be properties with !important in keyframe rules here. // See the spec issue https://github.com/w3c/csswg-drafts/issues/1824 for declaration in block.normal_declaration_iter() { - if let Some(property) = AnimatableLonghand::from_declaration(declaration) { - // Skip the 'display' property because although it is animatable from SMIL, - // it should not be animatable from CSS Animations or Web Animations. - if property != AnimatableLonghand::Display && - !seen.has_animatable_longhand_bit(&property) { - seen.set_animatable_longhand_bit(&property); - ret.push(property); - } + let longhand_id = match declaration.id() { + PropertyDeclarationId::Longhand(id) => id, + _ => continue, + }; + + if longhand_id == LonghandId::Display { + continue; } + + if !longhand_id.is_animatable() { + continue; + } + + ret.insert(longhand_id); } } @@ -406,14 +411,15 @@ impl KeyframesAnimation { /// /// Otherwise, this will compute and sort the steps used for the animation, /// and return the animation object. - pub fn from_keyframes(keyframes: &[Arc>], - vendor_prefix: Option, - guard: &SharedRwLockReadGuard) - -> Self { + pub fn from_keyframes( + keyframes: &[Arc>], + vendor_prefix: Option, + guard: &SharedRwLockReadGuard, + ) -> Self { let mut result = KeyframesAnimation { steps: vec![], - properties_changed: vec![], - vendor_prefix: vendor_prefix, + properties_changed: LonghandIdSet::new(), + vendor_prefix, }; if keyframes.is_empty() { diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index d2193d43098..478f52a3790 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -113,10 +113,11 @@ use style::media_queries::{Device, MediaList, parse_media_query_list}; use style::parser::{ParserContext, self}; use style::properties::{CascadeFlags, ComputedValues, Importance}; use style::properties::{IS_FIELDSET_CONTENT, IS_LINK, IS_VISITED_LINK, LonghandIdSet}; -use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, PropertyId, ShorthandId}; +use style::properties::{LonghandId, PropertyDeclaration, PropertyDeclarationBlock, PropertyId}; +use style::properties::{PropertyDeclarationId, ShorthandId}; use style::properties::{SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, SourcePropertyDeclaration, StyleBuilder}; use style::properties::PROHIBIT_DISPLAY_CONTENTS; -use style::properties::animated_properties::{AnimatableLonghand, AnimationValue}; +use style::properties::animated_properties::AnimationValue; use style::properties::animated_properties::compare_property_priority; use style::properties::parse_one_declaration_into; use style::rule_cache::RuleCacheConditions; @@ -423,9 +424,9 @@ pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMa use style::gecko_bindings::bindings::Gecko_GetProgressFromComputedTiming; use style::properties::animated_properties::AnimationValueMap; - let property = match AnimatableLonghand::from_nscsspropertyid(css_property) { - Some(longhand) => longhand, - None => { return (); } + let property = match LonghandId::from_nscsspropertyid(css_property) { + Ok(longhand) if longhand.is_animatable() => longhand, + _ => return, }; let value_map = AnimationValueMap::from_ffi_mut(raw_value_map); @@ -753,16 +754,19 @@ pub extern "C" fn Servo_StyleSet_GetBaseComputedValuesForElement(raw_data: RawSe } #[no_mangle] -pub extern "C" fn Servo_ComputedValues_ExtractAnimationValue(computed_values: ServoStyleContextBorrowed, - property_id: nsCSSPropertyID) - -> RawServoAnimationValueStrong -{ - let property = match AnimatableLonghand::from_nscsspropertyid(property_id) { - Some(longhand) => longhand, - None => return Strong::null(), +pub extern "C" fn Servo_ComputedValues_ExtractAnimationValue( + computed_values: ServoStyleContextBorrowed, + property_id: nsCSSPropertyID, +) -> RawServoAnimationValueStrong { + let property = match LonghandId::from_nscsspropertyid(property_id) { + Ok(longhand) => longhand, + Err(()) => return Strong::null(), }; - Arc::new(AnimationValue::from_computed_values(&property, &computed_values)).into_strong() + match AnimationValue::from_computed_values(&property, &computed_values) { + Some(v) => Arc::new(v).into_strong(), + None => Strong::null(), + } } #[no_mangle] @@ -779,9 +783,9 @@ pub extern "C" fn Servo_Property_IsTransitionable(property: nsCSSPropertyID) -> #[no_mangle] pub extern "C" fn Servo_Property_IsDiscreteAnimatable(property: nsCSSPropertyID) -> bool { - match AnimatableLonghand::from_nscsspropertyid(property) { - Some(longhand) => longhand.is_discrete(), - None => false + match LonghandId::from_nscsspropertyid(property) { + Ok(longhand) => longhand.is_discrete_animatable(), + Err(()) => return false, } } @@ -2046,13 +2050,14 @@ pub extern "C" fn Servo_ComputedValues_EqualCustomProperties( #[no_mangle] pub extern "C" fn Servo_ComputedValues_GetStyleRuleList(values: ServoStyleContextBorrowed, rules: RawGeckoServoStyleRuleListBorrowedMut) { + use smallvec::SmallVec; + let rule_node = match values.rules { Some(ref r) => r, None => return, }; - // TODO(emilio): Will benefit from SmallVec. - let mut result = vec![]; + let mut result = SmallVec::<[_; 10]>::new(); for node in rule_node.self_and_ancestors() { let style_rule = match *node.style_source() { StyleSource::Style(ref rule) => rule, @@ -3392,16 +3397,15 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis continue; } - let mut maybe_append_animation_value = |property: AnimatableLonghand, - value: Option| { - if seen.has_animatable_longhand_bit(&property) { + let mut maybe_append_animation_value = |property: LonghandId, value: Option| { + if seen.contains(property) { return; } - seen.set_animatable_longhand_bit(&property); + seen.insert(property); // This is safe since we immediately write to the uninitialized values. unsafe { animation_values.set_len((property_index + 1) as u32) }; - animation_values[property_index].mProperty = (&property).into(); + animation_values[property_index].mProperty = property.to_nscsspropertyid(); // We only make sure we have enough space for this variable, // but didn't construct a default value for StyleAnimationValue, // so we should zero it to avoid getting undefined behaviors. @@ -3418,13 +3422,10 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis }; if property.mServoDeclarationBlock.mRawPtr.is_null() { - let animatable_longhand = - AnimatableLonghand::from_nscsspropertyid(property.mProperty); - // |keyframes.mPropertyValues| should only contain animatable - // properties, but we check the result from_nscsspropertyid - // just in case. - if let Some(property) = animatable_longhand { - maybe_append_animation_value(property, None); + let property = + LonghandId::from_nscsspropertyid(property.mProperty); + if let Ok(prop) = property { + maybe_append_animation_value(prop, None); } continue; } @@ -3438,8 +3439,9 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis &custom_properties, ); - for anim in iter { - maybe_append_animation_value(anim.0, Some(anim.1)); + for value in iter { + let id = value.id(); + maybe_append_animation_value(id, Some(value)); } } } @@ -3485,7 +3487,7 @@ pub extern "C" fn Servo_GetAnimationValues(declarations: RawServoDeclarationBloc ); for (index, anim) in iter.enumerate() { unsafe { animation_values.set_len((index + 1) as u32) }; - animation_values[index].set_arc_leaky(Arc::new(anim.1)); + animation_values[index].set_arc_leaky(Arc::new(anim)); } } @@ -3571,17 +3573,15 @@ enum Offset { One } -fn fill_in_missing_keyframe_values(all_properties: &[AnimatableLonghand], - timing_function: nsTimingFunctionBorrowed, - properties_set_at_offset: &LonghandIdSet, - offset: Offset, - keyframes: RawGeckoKeyframeListBorrowedMut) { - let needs_filling = all_properties.iter().any(|ref property| { - !properties_set_at_offset.has_animatable_longhand_bit(property) - }); - - // Return earli if all animated properties are already set. - if !needs_filling { +fn fill_in_missing_keyframe_values( + all_properties: &LonghandIdSet, + timing_function: nsTimingFunctionBorrowed, + longhands_at_offset: &LonghandIdSet, + offset: Offset, + keyframes: RawGeckoKeyframeListBorrowedMut, +) { + // Return early if all animated properties are already set. + if longhands_at_offset.contains_all(all_properties) { return; } @@ -3595,10 +3595,14 @@ fn fill_in_missing_keyframe_values(all_properties: &[AnimatableLonghand], }; // Append properties that have not been set at this offset. - for ref property in all_properties.iter() { - if !properties_set_at_offset.has_animatable_longhand_bit(property) { - unsafe { Gecko_AppendPropertyValuePair(&mut (*keyframe).mPropertyValues, - (*property).into()); } + for property in all_properties.iter() { + if !longhands_at_offset.contains(property) { + unsafe { + Gecko_AppendPropertyValuePair( + &mut (*keyframe).mPropertyValues, + property.to_nscsspropertyid() + ); + } } } } @@ -3608,8 +3612,6 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetB name: *const nsACString, inherited_timing_function: nsTimingFunctionBorrowed, keyframes: RawGeckoKeyframeListBorrowedMut) -> bool { - use smallvec::SmallVec; - debug_assert!(keyframes.len() == 0, "keyframes should be initially empty"); @@ -3664,8 +3666,10 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetB // that keyframe. for property in animation.properties_changed.iter() { unsafe { - Gecko_AppendPropertyValuePair(&mut (*keyframe).mPropertyValues, - property.into()); + Gecko_AppendPropertyValuePair( + &mut (*keyframe).mPropertyValues, + property.to_nscsspropertyid(), + ); } } if current_offset == 0.0 { @@ -3676,55 +3680,79 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetB }, KeyframesStepValue::Declarations { ref block } => { let guard = block.read_with(&guard); - // Filter out non-animatable properties and properties with !important. - let animatable = - guard.normal_declaration_iter() - .filter(|declaration| declaration.is_animatable()); - let custom_properties: SmallVec<[&PropertyDeclaration; 1]> = - guard.normal_declaration_iter() - .filter(|declaration| declaration.is_custom()) - .collect(); + let mut custom_properties = PropertyDeclarationBlock::new(); - if custom_properties.len() > 0 { - let mut pdb = PropertyDeclarationBlock::new(); - for custom in custom_properties.iter() { - pdb.push((*custom).clone(), Importance::Normal); + // Filter out non-animatable properties and properties with + // !important. + for declaration in guard.normal_declaration_iter() { + let id = declaration.id(); + + let id = match id { + PropertyDeclarationId::Longhand(id) => { + // Skip the 'display' property because although it + // is animatable from SMIL, it should not be + // animatable from CSS Animations. + if id == LonghandId::Display { + continue; + } + + if !id.is_animatable() { + continue; + } + + id + } + PropertyDeclarationId::Custom(..) => { + custom_properties.push(declaration.clone(), Importance::Normal); + continue; + } + }; + + if properties_set_at_current_offset.contains(id) { + continue; } + + let pair = unsafe { + Gecko_AppendPropertyValuePair( + &mut (*keyframe).mPropertyValues, + id.to_nscsspropertyid(), + ) + }; + unsafe { - let pair = - Gecko_AppendPropertyValuePair(&mut (*keyframe).mPropertyValues, - nsCSSPropertyID::eCSSPropertyExtra_variable); (*pair).mServoDeclarationBlock.set_arc_leaky( - Arc::new(global_style_data.shared_lock.wrap(pdb))); + Arc::new(global_style_data.shared_lock.wrap( + PropertyDeclarationBlock::with_one( + declaration.clone(), + Importance::Normal, + ) + )) + ); } + + if current_offset == 0.0 { + properties_set_at_start.insert(id); + } else if current_offset == 1.0 { + properties_set_at_end.insert(id); + } + properties_set_at_current_offset.insert(id); } - for declaration in animatable { - let property = AnimatableLonghand::from_declaration(declaration).unwrap(); - // Skip the 'display' property because although it is animatable from SMIL, - // it should not be animatable from CSS Animations. - if property != AnimatableLonghand::Display && - !properties_set_at_current_offset.has_animatable_longhand_bit(&property) { - properties_set_at_current_offset.set_animatable_longhand_bit(&property); - if current_offset == 0.0 { - properties_set_at_start.set_animatable_longhand_bit(&property); - } else if current_offset == 1.0 { - properties_set_at_end.set_animatable_longhand_bit(&property); - } + if custom_properties.any_normal() { + let pair = unsafe { + Gecko_AppendPropertyValuePair( + &mut (*keyframe).mPropertyValues, + nsCSSPropertyID::eCSSPropertyExtra_variable, + ) + }; - let property = AnimatableLonghand::from_declaration(declaration).unwrap(); - unsafe { - let pair = - Gecko_AppendPropertyValuePair(&mut (*keyframe).mPropertyValues, - (&property).into()); - (*pair).mServoDeclarationBlock.set_arc_leaky( - Arc::new(global_style_data.shared_lock.wrap( - PropertyDeclarationBlock::with_one( - declaration.clone(), Importance::Normal - )))); - } + unsafe { + (*pair).mServoDeclarationBlock.set_arc_leaky(Arc::new( + global_style_data.shared_lock.wrap(custom_properties) + )); } + } }, } @@ -3732,18 +3760,22 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetB // Append property values that are missing in the initial or the final keyframes. if !has_complete_initial_keyframe { - fill_in_missing_keyframe_values(&animation.properties_changed, - inherited_timing_function, - &properties_set_at_start, - Offset::Zero, - keyframes); + fill_in_missing_keyframe_values( + &animation.properties_changed, + inherited_timing_function, + &properties_set_at_start, + Offset::Zero, + keyframes, + ); } if !has_complete_final_keyframe { - fill_in_missing_keyframe_values(&animation.properties_changed, - inherited_timing_function, - &properties_set_at_end, - Offset::One, - keyframes); + fill_in_missing_keyframe_values( + &animation.properties_changed, + inherited_timing_function, + &properties_set_at_end, + Offset::One, + keyframes, + ); } true } diff --git a/tests/unit/style/keyframes.rs b/tests/unit/style/keyframes.rs index e5b7132b7ac..0d5642054e8 100644 --- a/tests/unit/style/keyframes.rs +++ b/tests/unit/style/keyframes.rs @@ -4,13 +4,23 @@ use cssparser::SourceLocation; use servo_arc::Arc; -use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, Importance}; -use style::properties::animated_properties::AnimatableLonghand; +use style::properties::{LonghandId, LonghandIdSet, PropertyDeclaration, PropertyDeclarationBlock, Importance}; use style::shared_lock::SharedRwLock; use style::stylesheets::keyframes_rule::{Keyframe, KeyframesAnimation, KeyframePercentage, KeyframeSelector}; use style::stylesheets::keyframes_rule::{KeyframesStep, KeyframesStepValue}; use style::values::specified::{LengthOrPercentageOrAuto, NoCalcLength}; +macro_rules! longhand_set { + ($($word:ident),+) => {{ + let mut set = LonghandIdSet::new(); + $( + set.insert(LonghandId::$word); + )+ + set + }} +} + + #[test] fn test_empty_keyframe() { let shared_lock = SharedRwLock::new(); @@ -20,7 +30,7 @@ fn test_empty_keyframe() { &shared_lock.read()); let expected = KeyframesAnimation { steps: vec![], - properties_changed: vec![], + properties_changed: LonghandIdSet::new(), vendor_prefix: None, }; @@ -43,7 +53,7 @@ fn test_no_property_in_keyframe() { &shared_lock.read()); let expected = KeyframesAnimation { steps: vec![], - properties_changed: vec![], + properties_changed: LonghandIdSet::new(), vendor_prefix: None, }; @@ -106,7 +116,7 @@ fn test_missing_property_in_initial_keyframe() { declared_timing_function: false, }, ], - properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height], + properties_changed: longhand_set!(Width, Height), vendor_prefix: None, }; @@ -169,7 +179,7 @@ fn test_missing_property_in_final_keyframe() { declared_timing_function: false, }, ], - properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height], + properties_changed: longhand_set!(Width, Height), vendor_prefix: None, }; @@ -234,7 +244,7 @@ fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() { declared_timing_function: false, } ], - properties_changed: vec![AnimatableLonghand::Width, AnimatableLonghand::Height], + properties_changed: longhand_set!(Width, Height), vendor_prefix: None, }; diff --git a/tests/unit/style/parsing/mod.rs b/tests/unit/style/parsing/mod.rs index e2ab39e238c..3a18935dcdf 100644 --- a/tests/unit/style/parsing/mod.rs +++ b/tests/unit/style/parsing/mod.rs @@ -119,6 +119,5 @@ mod selectors; mod supports; mod text_overflow; mod transition_duration; -mod transition_property; mod transition_timing_function; mod value; diff --git a/tests/unit/style/parsing/transition_property.rs b/tests/unit/style/parsing/transition_property.rs deleted file mode 100644 index 68663b3cb19..00000000000 --- a/tests/unit/style/parsing/transition_property.rs +++ /dev/null @@ -1,97 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -use parsing::parse; -use servo_atoms::Atom; -use style::properties::animated_properties::TransitionProperty; -use style::properties::longhands::transition_property; -use style::properties::shorthands::transition; -use style::values::CustomIdent; -use style_traits::ToCss; - -#[test] -fn test_longhand_properties() { - assert_roundtrip_with_context!(transition_property::parse, "margin-left"); - assert_roundtrip_with_context!(transition_property::parse, "background-color"); - assert_roundtrip_with_context!(transition_property::parse, "width"); - assert_roundtrip_with_context!(transition_property::parse, "transition-duration"); - assert_roundtrip_with_context!(transition_property::parse, "unsupported-property"); - assert_roundtrip_with_context!(transition_property::parse, "-other-unsupported-property"); - assert_roundtrip_with_context!(transition_property::parse, "--var"); - - assert_eq!( - parse_longhand!(transition_property, "margin-left, transition-delay, width, --var"), - transition_property::SpecifiedValue(vec![ - TransitionProperty::MarginLeft, - TransitionProperty::Unsupported(CustomIdent(Atom::from("transition-delay"))), - TransitionProperty::Width, - TransitionProperty::Unsupported(CustomIdent(Atom::from("--var"))), - ]) - ); - - assert!(parse(transition_property::parse, ".width").is_err()); - assert!(parse(transition_property::parse, "1width").is_err()); - assert!(parse(transition_property::parse, "- ").is_err()); -} - -#[test] -fn test_shorthand_properties() { - assert_roundtrip_with_context!(transition_property::parse, "margin"); - assert_roundtrip_with_context!(transition_property::parse, "background"); - assert_roundtrip_with_context!(transition_property::parse, "border-bottom"); - - assert_eq!(parse_longhand!(transition_property, "margin, background"), - transition_property::SpecifiedValue( - vec![TransitionProperty::Margin, - TransitionProperty::Background])); -} - -#[test] -fn test_keywords() { - assert_roundtrip_with_context!(transition_property::parse, "all"); - assert_roundtrip_with_context!(transition_property::parse, "none"); - - assert_eq!(parse_longhand!(transition_property, "all"), - transition_property::SpecifiedValue(vec![TransitionProperty::All])); - assert_eq!(parse_longhand!(transition_property, "width, all"), - transition_property::SpecifiedValue(vec![TransitionProperty::Width, - TransitionProperty::All])); - - // Using CSS Wide Keyword or none in the list will get the syntax invalid. - // Note: Only "none" alone is valid. - assert!(parse(transition_property::parse, "none").is_ok()); - assert_eq!(parse_longhand!(transition_property, "none"), - transition_property::SpecifiedValue(vec![])); - assert!(parse(transition_property::parse, "inherit").is_err()); - assert!(parse(transition_property::parse, "initial").is_err()); - assert!(parse(transition_property::parse, "unset").is_err()); - assert!(parse(transition_property::parse, "width, none").is_err()); - assert!(parse(transition_property::parse, "width, initial").is_err()); - assert!(parse(transition_property::parse, "width, inherit").is_err()); - assert!(parse(transition_property::parse, "width, unset").is_err()); -} - -#[test] -fn test_transition_shorthand() { - let result = parse(transition::parse_value, "2s margin-left, 4s background").unwrap(); - assert_eq!(result.transition_property, - parse_longhand!(transition_property, "margin-left, background")); - - let result = parse(transition::parse_value, "2s margin, 4s all").unwrap(); - assert_eq!(result.transition_property, - parse_longhand!(transition_property, "margin, all")); - - let result = parse(transition::parse_value, "2s width, 3s --var, 4s background").unwrap(); - assert_eq!(result.transition_property, - parse_longhand!(transition_property, "width, --var, background")); - - let result = parse(transition::parse_value, "none").unwrap(); - assert_eq!(result.transition_property, - parse_longhand!(transition_property, "none")); - - assert!(parse(transition::parse_value, "2s width, none").is_err()); - assert!(parse(transition::parse_value, "2s width, 2s initial").is_err()); - assert!(parse(transition::parse_value, "2s width, 3s unset").is_err()); - assert!(parse(transition::parse_value, "2s width, 4s inherit").is_err()); -}