From 279a50fb748da051a3c16950787ac63b36a2089c Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 22 Feb 2017 18:48:46 +0900 Subject: [PATCH 1/3] Make PropertyFiledBit usable for TransitionProperty. --- .../style/properties/properties.mako.rs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index be76f0cf1a3..e0623fcf780 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -181,8 +181,10 @@ pub mod animated_properties { // TODO(SimonSapin): Convert this to a syntax extension rather than a Mako template. // Maybe submit for inclusion in libstd? -mod property_bit_field { +#[allow(missing_docs)] +pub mod property_bit_field { use logical_geometry::WritingMode; + use properties::animated_properties::TransitionProperty; /// A bitfield for all longhand properties, in order to quickly test whether /// we've seen one of them. @@ -213,7 +215,7 @@ mod property_bit_field { pub fn get_${property.ident}(&self) -> bool { self.get(${i}) } - #[allow(non_snake_case)] + #[allow(non_snake_case, missing_docs)] #[inline] pub fn set_${property.ident}(&mut self) { self.set(${i}) @@ -238,6 +240,32 @@ mod property_bit_field { } % endif % endfor + + /// Set the corresponding bit of TransitionProperty. + /// This function will panic if TransitionProperty::All is given. + pub fn set_transition_property_bit(&mut self, property: &TransitionProperty) { + match *property { + % for i, prop in enumerate(data.longhands): + % if prop.animatable: + TransitionProperty::${prop.camel_case} => self.set(${i}), + % endif + % endfor + TransitionProperty::All => unreachable!("Tried to set TransitionProperty::All in a PropertyBitfield"), + } + } + + /// Return true if the corresponding bit of TransitionProperty is set. + /// This function will panic if TransitionProperty::All is given. + pub fn has_transition_property_bit(&self, property: &TransitionProperty) -> bool { + match *property { + % for i, prop in enumerate(data.longhands): + % if prop.animatable: + TransitionProperty::${prop.camel_case} => self.get(${i}), + % endif + % endfor + TransitionProperty::All => unreachable!("Tried to get TransitionProperty::All in a PropertyBitfield"), + } + } } } From 501edfdbdb9ef7d2749a6579aef94b146b6ca48b Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 22 Feb 2017 18:50:01 +0900 Subject: [PATCH 2/3] Get all animated properties in *all* keyframes. @keyframes anim { from { transform: none; } to { opacity: 0; transform: none; } } In above case, we have to add opacity property and value in the 'from' keyframe. --- components/style/keyframes.rs | 27 ++--- tests/unit/style/keyframes.rs | 179 +++++++++++++++++++++++++++++++++- 2 files changed, 193 insertions(+), 13 deletions(-) diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index 07de56158ea..fcb05c8dc34 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -15,6 +15,7 @@ use properties::{PropertyDeclarationId, LonghandId, DeclaredValue}; use properties::PropertyDeclarationParseResult; use properties::animated_properties::TransitionProperty; use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction; +use properties::property_bit_field::PropertyBitField; use std::fmt; use std::sync::Arc; use style_traits::ToCss; @@ -244,21 +245,23 @@ pub struct KeyframesAnimation { pub properties_changed: Vec, } -/// Get all the animated properties in a keyframes animation. Note that it's not -/// defined what happens when a property is not on a keyframe, so we only peek -/// the props of the first one. -/// -/// In practice, browsers seem to try to do their best job at it, so we might -/// want to go through all the actual keyframes and deduplicate properties. -fn get_animated_properties(keyframe: &Keyframe) -> Vec { +/// Get all the animated properties in a keyframes animation. +fn get_animated_properties(keyframes: &[Arc>]) -> Vec { let mut ret = vec![]; + let mut seen = PropertyBitField::new(); // NB: declarations are already deduplicated, so we don't have to check for // it here. - for &(ref declaration, importance) in keyframe.block.read().declarations.iter() { - assert!(!importance.important()); + for keyframe in keyframes { + let keyframe = keyframe.read(); + for &(ref declaration, importance) in keyframe.block.read().declarations.iter() { + assert!(!importance.important()); - if let Some(property) = TransitionProperty::from_declaration(declaration) { - ret.push(property); + if let Some(property) = TransitionProperty::from_declaration(declaration) { + if !seen.has_transition_property_bit(&property) { + ret.push(property); + seen.set_transition_property_bit(&property); + } + } } } @@ -284,7 +287,7 @@ impl KeyframesAnimation { return result; } - result.properties_changed = get_animated_properties(&keyframes[0].read()); + result.properties_changed = get_animated_properties(keyframes); if result.properties_changed.is_empty() { return result; } diff --git a/tests/unit/style/keyframes.rs b/tests/unit/style/keyframes.rs index 14c79bfa1bf..d01dbcab2d0 100644 --- a/tests/unit/style/keyframes.rs +++ b/tests/unit/style/keyframes.rs @@ -5,7 +5,10 @@ use parking_lot::RwLock; use std::sync::Arc; use style::keyframes::{Keyframe, KeyframesAnimation, KeyframePercentage, KeyframeSelector}; -use style::properties::PropertyDeclarationBlock; +use style::keyframes::{KeyframesStep, KeyframesStepValue}; +use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock, Importance}; +use style::properties::animated_properties::TransitionProperty; +use style::values::specified::{LengthOrPercentageOrAuto, NoCalcLength}; #[test] fn test_empty_keyframe() { @@ -38,3 +41,177 @@ fn test_no_property_in_keyframe() { assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected)); } + +#[test] +fn test_missing_property_in_initial_keyframe() { + let declarations_on_initial_keyframe = + Arc::new(RwLock::new(PropertyDeclarationBlock { + declarations: vec![ + (PropertyDeclaration::Width( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal), + ], + important_count: 0, + })); + + let declarations_on_final_keyframe = + Arc::new(RwLock::new(PropertyDeclarationBlock { + declarations: vec![ + (PropertyDeclaration::Width( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal), + + (PropertyDeclaration::Height( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal), + ], + important_count: 0, + })); + + let keyframes = vec![ + Arc::new(RwLock::new(Keyframe { + selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]), + block: declarations_on_initial_keyframe.clone(), + })), + + Arc::new(RwLock::new(Keyframe { + selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]), + block: declarations_on_final_keyframe.clone(), + })), + ]; + let animation = KeyframesAnimation::from_keyframes(&keyframes); + let expected = KeyframesAnimation { + steps: vec![ + KeyframesStep { + start_percentage: KeyframePercentage(0.), + value: KeyframesStepValue::Declarations { block: declarations_on_initial_keyframe }, + declared_timing_function: false, + }, + KeyframesStep { + start_percentage: KeyframePercentage(1.), + value: KeyframesStepValue::Declarations { block: declarations_on_final_keyframe }, + declared_timing_function: false, + }, + ], + properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height], + }; + + assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected)); +} + +#[test] +fn test_missing_property_in_final_keyframe() { + let declarations_on_initial_keyframe = + Arc::new(RwLock::new(PropertyDeclarationBlock { + declarations: vec![ + (PropertyDeclaration::Width( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal), + + (PropertyDeclaration::Height( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal), + ], + important_count: 0, + })); + + let declarations_on_final_keyframe = + Arc::new(RwLock::new(PropertyDeclarationBlock { + declarations: vec![ + (PropertyDeclaration::Height( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal), + ], + important_count: 0, + })); + + let keyframes = vec![ + Arc::new(RwLock::new(Keyframe { + selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]), + block: declarations_on_initial_keyframe.clone(), + })), + + Arc::new(RwLock::new(Keyframe { + selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]), + block: declarations_on_final_keyframe.clone(), + })), + ]; + let animation = KeyframesAnimation::from_keyframes(&keyframes); + let expected = KeyframesAnimation { + steps: vec![ + KeyframesStep { + start_percentage: KeyframePercentage(0.), + value: KeyframesStepValue::Declarations { block: declarations_on_initial_keyframe }, + declared_timing_function: false, + }, + KeyframesStep { + start_percentage: KeyframePercentage(1.), + value: KeyframesStepValue::Declarations { block: declarations_on_final_keyframe }, + declared_timing_function: false, + }, + ], + properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height], + }; + + assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected)); +} + +#[test] +fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() { + let declarations = + Arc::new(RwLock::new(PropertyDeclarationBlock { + declarations: vec![ + (PropertyDeclaration::Width( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal), + + (PropertyDeclaration::Height( + DeclaredValue::Value(LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(20f32)))), + Importance::Normal), + ], + important_count: 0, + })); + + let keyframes = vec![ + Arc::new(RwLock::new(Keyframe { + selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]), + block: Arc::new(RwLock::new(PropertyDeclarationBlock { + declarations: vec![], + important_count: 0, + })) + })), + Arc::new(RwLock::new(Keyframe { + selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.5)]), + block: declarations.clone(), + })), + ]; + let animation = KeyframesAnimation::from_keyframes(&keyframes); + let expected = KeyframesAnimation { + steps: vec![ + KeyframesStep { + start_percentage: KeyframePercentage(0.), + value: KeyframesStepValue::Declarations { + block: Arc::new(RwLock::new(PropertyDeclarationBlock { + // XXX: Should we use ComputedValues in this case? + declarations: vec![], + important_count: 0, + })) + }, + declared_timing_function: false, + }, + KeyframesStep { + start_percentage: KeyframePercentage(0.5), + value: KeyframesStepValue::Declarations { block: declarations }, + declared_timing_function: false, + }, + KeyframesStep { + start_percentage: KeyframePercentage(1.), + value: KeyframesStepValue::ComputedValues, + declared_timing_function: false, + } + ], + properties_changed: vec![TransitionProperty::Width, TransitionProperty::Height], + }; + + assert_eq!(format!("{:#?}", animation), format!("{:#?}", expected)); +} From 396db281e39d097a25b9e5d72a4ace0d4365692a Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 22 Feb 2017 18:50:35 +0900 Subject: [PATCH 3/3] Fill computed property values where the propery is missing in initial and final keyframes. @keyframes anim { from { opacity: 0; transform: none; } to { transform: none; } } In above case, we have to add opacity property and value in the 'to' keyframe. --- .../helpers/animated_properties.mako.rs | 2 +- ports/geckolib/glue.rs | 47 +++++++++++++++---- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 3c05fc06e31..8001e9457a6 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -43,7 +43,7 @@ use values::specified::Angle as SpecifiedAngle; /// property. // NB: This needs to be here because it needs all the longhands generated // beforehand. -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum TransitionProperty { /// All, any animatable property changing should generate a transition. diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 54aac9b4694..c7e6ede116f 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1435,6 +1435,9 @@ pub extern "C" fn Servo_StyleSet_FillKeyframesForName(raw_data: RawServoStyleSet timing_function: *const nsTimingFunction, style: ServoComputedValuesBorrowed, keyframes: RawGeckoKeyframeListBorrowedMut) -> bool { + use style::gecko_bindings::structs::Keyframe; + use style::properties::property_bit_field::PropertyBitField; + let data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let name = unsafe { Atom::from(name.as_ref().unwrap().as_str_unchecked()) }; let style_timing_function = unsafe { timing_function.as_ref().unwrap() }; @@ -1455,16 +1458,25 @@ pub extern "C" fn Servo_StyleSet_FillKeyframesForName(raw_data: RawServoStyleSet &timing_function) }; + fn add_computed_property_value(keyframe: *mut Keyframe, + index: usize, + style: &ComputedValues, + property: &TransitionProperty) { + let block = style.to_declaration_block(property.clone().into()); + unsafe { + (*keyframe).mPropertyValues.set_len((index + 1) as u32); + (*keyframe).mPropertyValues[index].mProperty = property.clone().into(); + // FIXME. Do not set computed values once we handles missing keyframes + // with additive composition. + (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky( + Arc::new(RwLock::new(block))); + } + } + match step.value { KeyframesStepValue::ComputedValues => { for (index, property) in animation.properties_changed.iter().enumerate() { - let block = style.to_declaration_block(property.clone().into()); - unsafe { - (*keyframe).mPropertyValues.set_len((index + 1) as u32); - (*keyframe).mPropertyValues[index].mProperty = property.clone().into(); - (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky( - Arc::new(RwLock::new(block))); - } + add_computed_property_value(keyframe, index, style, property); } }, KeyframesStepValue::Declarations { ref block } => { @@ -1476,16 +1488,33 @@ pub extern "C" fn Servo_StyleSet_FillKeyframesForName(raw_data: RawServoStyleSet .filter(|&&(ref declaration, _)| { declaration.is_animatable() }); + + let mut seen = PropertyBitField::new(); + for (index, &(ref declaration, _)) in animatable.enumerate() { unsafe { + let property = TransitionProperty::from_declaration(declaration).unwrap(); (*keyframe).mPropertyValues.set_len((index + 1) as u32); - (*keyframe).mPropertyValues[index].mProperty = - TransitionProperty::from_declaration(declaration).unwrap().into(); + (*keyframe).mPropertyValues[index].mProperty = property.into(); (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky( Arc::new(RwLock::new( PropertyDeclarationBlock { declarations: vec![ (declaration.clone(), Importance::Normal) ], important_count: 0 }))); + if step.start_percentage.0 == 0. || + step.start_percentage.0 == 1. { + seen.set_transition_property_bit(&property); + } + } + } + + // Append missing property values in the initial or the finial keyframes. + if step.start_percentage.0 == 0. || + step.start_percentage.0 == 1. { + for (index, property) in animation.properties_changed.iter().enumerate() { + if !seen.has_transition_property_bit(&property) { + add_computed_property_value(keyframe, index, style, property); + } } } },