From 561e9c81f1faca671b475d00507691fb343775ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 6 May 2018 18:45:16 +0200 Subject: [PATCH] style: More useful logging for transition-related stuff. Transitions are still broken, but I found these messages more helpful than the previous ones when diagnosing problems. --- components/layout/animation.rs | 2 ++ components/style/animation.rs | 22 +++++++++---------- components/style/matching.rs | 17 +++----------- .../helpers/animated_properties.mako.rs | 15 ++++++++----- .../font-style-interpolation.html.ini | 2 +- ...nsition-property-all-before-value.html.ini | 3 +++ ...ue-before-transition-property-all.html.ini | 3 +++ 7 files changed, 32 insertions(+), 32 deletions(-) create mode 100644 tests/wpt/metadata/css/css-variables/variable-transitions-transition-property-all-before-value.html.ini create mode 100644 tests/wpt/metadata/css/css-variables/variable-transitions-value-before-transition-property-all.html.ini diff --git a/components/layout/animation.rs b/components/layout/animation.rs index 4c4865cc11a..6b78d8cb8b8 100644 --- a/components/layout/animation.rs +++ b/components/layout/animation.rs @@ -89,6 +89,8 @@ pub fn update_animation_state( }, }; + debug!("update_animation_state({:?}): {:?}", still_running, running_animation); + if still_running { animations_still_running.push(running_animation); continue; diff --git a/components/style/animation.rs b/components/style/animation.rs index 07d45e8cf5b..17e8b0981fb 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -438,9 +438,16 @@ pub fn start_transitions_if_applicable( .iter() .any(|animation| animation.has_the_same_end_value_as(&property_animation)) { + debug!( + "Not initiating transition for {}, other transition \ + found with the same end value", + property_animation.property_name() + ); continue; } + debug!("Kicking off transition of {:?}", property_animation); + // Kick off the animation. let box_style = new_style.get_box(); let now = timer.seconds(); @@ -451,7 +458,7 @@ pub fn start_transitions_if_applicable( start_time, AnimationFrame { duration: box_style.transition_duration_mod(i).seconds() as f64, - property_animation: property_animation, + property_animation, }, )).unwrap(); @@ -657,12 +664,11 @@ pub fn update_style_for_animation( where E: TElement, { - debug!("update_style_for_animation: entering"); + debug!("update_style_for_animation: {:?}", animation); debug_assert!(!animation.is_expired()); match *animation { Animation::Transition(_, start_time, ref frame) => { - debug!("update_style_for_animation: transition found"); let now = context.timer.seconds(); let mut new_style = (*style).clone(); let updated_style = @@ -678,10 +684,6 @@ where AnimationUpdate::Regular }, Animation::Keyframes(_, ref animation, ref name, ref state) => { - debug!( - "update_style_for_animation: animation found: \"{}\", {:?}", - name, state - ); let duration = state.duration; let started_at = state.started_at; @@ -716,11 +718,6 @@ where total_progress = 1.; } - debug!( - "update_style_for_animation: anim \"{}\", steps: {:?}, state: {:?}, progress: {}", - name, animation.steps, state, total_progress - ); - // Get the target and the last keyframe position. let last_keyframe_position; let target_keyframe_position; @@ -865,6 +862,7 @@ pub fn complete_expired_transitions( had_animations_to_expire = animations_to_expire.is_some(); if let Some(ref animations) = animations_to_expire { for animation in *animations { + debug!("Updating expired animation {:?}", animation); // TODO: support animation-fill-mode if let Animation::Transition(_, _, ref frame) = *animation { frame.property_animation.update(Arc::make_mut(style), 1.0); diff --git a/components/style/matching.rs b/components/style/matching.rs index f9d0b7e595d..24c9e9bbf11 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -602,17 +602,8 @@ trait PrivateMatchMethods: TElement { let mut all_running_animations = context.running_animations.write(); for mut running_animation in all_running_animations.get_mut(&this_opaque).unwrap() { - // This shouldn't happen frequently, but under some circumstances - // mainly huge load or debug builds, the constellation might be - // delayed in sending the `TickAllAnimations` message to layout. - // - // Thus, we can't assume all the animations have been already - // updated by layout, because other restyle due to script might be - // triggered by layout before the animation tick. - // - // See #12171 and the associated PR for an example where this - // happened while debugging other release panic. - if running_animation.is_expired() { + if let Animation::Transition(_, _, ref frame) = *running_animation { + possibly_expired_animations.push(frame.property_animation.clone()); continue; } @@ -624,9 +615,7 @@ trait PrivateMatchMethods: TElement { ); match *running_animation { - Animation::Transition(_, _, ref frame) => { - possibly_expired_animations.push(frame.property_animation.clone()) - } + Animation::Transition(..) => unreachable!(), Animation::Keyframes(_, _, _, ref mut state) => { match update { AnimationUpdate::Regular => {}, diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 437232d386a..98a74854b8d 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -127,17 +127,22 @@ pub enum AnimatedProperty { } impl AnimatedProperty { - /// Get the name of this property. - pub fn name(&self) -> &'static str { + /// Get the id of the property we're animating. + pub fn id(&self) -> LonghandId { match *self { % for prop in data.longhands: - % if prop.animatable and not prop.logical: - AnimatedProperty::${prop.camel_case}(..) => "${prop.name}", - % endif + % if prop.animatable and not prop.logical: + AnimatedProperty::${prop.camel_case}(..) => LonghandId::${prop.camel_case}, + % endif % endfor } } + /// Get the name of this property. + pub fn name(&self) -> &'static str { + self.id().name() + } + /// Whether this interpolation does animate, that is, whether the start and /// end values are different. pub fn does_animate(&self) -> bool { diff --git a/tests/wpt/metadata/css/css-fonts/variations/font-style-interpolation.html.ini b/tests/wpt/metadata/css/css-fonts/variations/font-style-interpolation.html.ini index 21acb456b9f..2e7944f8327 100644 --- a/tests/wpt/metadata/css/css-fonts/variations/font-style-interpolation.html.ini +++ b/tests/wpt/metadata/css/css-fonts/variations/font-style-interpolation.html.ini @@ -1,6 +1,6 @@ [font-style-interpolation.html] bug: https://github.com/servo/servo/issues/21570 expected: TIMEOUT - [font-style animation] + [font-style transition] expected: TIMEOUT diff --git a/tests/wpt/metadata/css/css-variables/variable-transitions-transition-property-all-before-value.html.ini b/tests/wpt/metadata/css/css-variables/variable-transitions-transition-property-all-before-value.html.ini new file mode 100644 index 00000000000..282cefe525a --- /dev/null +++ b/tests/wpt/metadata/css/css-variables/variable-transitions-transition-property-all-before-value.html.ini @@ -0,0 +1,3 @@ +[variable-transitions-transition-property-all-before-value.html] + [Verify substituted color value after transition] + expected: FAIL diff --git a/tests/wpt/metadata/css/css-variables/variable-transitions-value-before-transition-property-all.html.ini b/tests/wpt/metadata/css/css-variables/variable-transitions-value-before-transition-property-all.html.ini new file mode 100644 index 00000000000..e1fc8c9c968 --- /dev/null +++ b/tests/wpt/metadata/css/css-variables/variable-transitions-value-before-transition-property-all.html.ini @@ -0,0 +1,3 @@ +[variable-transitions-value-before-transition-property-all.html] + [Verify substituted color value after transition] + expected: FAIL