Auto merge of #26331 - mrobinson:animation-update-on-restyle, r=jdm,emilio

Restyle should reflect animations and transitions

When doing a restyle, we should apply animations and transitions to the
new style so that it is reflected in `getComputedStyle()` and the new
style information properly cascades. This is the first part of properly
ticking animations and transitions.

This causes a couple new animations tests failures (along with many new
passes), but we currently don't have support for properly handling
animations after they have completed, so this isn't totally unexpected.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #20116

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
This commit is contained in:
bors-servo 2020-04-29 18:06:24 -04:00 committed by GitHub
commit d1279ebdc6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
63 changed files with 370 additions and 10360 deletions

View file

@ -15,6 +15,7 @@ use script_traits::UntrustedNodeAddress;
use script_traits::{
AnimationState, ConstellationControlMsg, LayoutMsg as ConstellationMsg, TransitionEventType,
};
use servo_arc::Arc;
use style::animation::{
update_style_for_animation, Animation, ElementAnimationState, PropertyAnimation,
};
@ -223,7 +224,7 @@ fn do_recalc_style_for_animations<E>(
update_style_for_animation::<E>(
&context.style_context,
animation,
&mut fragment.style,
Arc::make_mut(&mut fragment.style),
&ServoMetricsProvider,
);
let difference = RestyleDamage::compute_style_difference(&old_style, &fragment.style);

View file

@ -442,14 +442,18 @@ impl ElementAnimationState {
false
}
/// Compute before-change-style given an existing ElementAnimationState,
/// information from the StyleContext, and the values of the previous style
/// computation.
///
/// TODO(mrobinson): This is not a correct computation of before-change-style.
/// For starters it's unclear why we aren't using the running transitions to
/// transform this style into before-change-style.
pub(crate) fn compute_before_change_style<E>(
pub(crate) fn apply_completed_animations(&mut self, style: &mut Arc<ComputedValues>) {
for animation in self.finished_animations.iter() {
// TODO: Make this a bit more general and add support animation-fill-mode.
// Without support for that property though, animations that have ended should
// not affect property values. This is why we only process transitions here.
if let Animation::Transition(_, _, property_animation) = animation {
property_animation.update(Arc::make_mut(style), 1.0);
}
}
}
pub(crate) fn apply_running_animations<E>(
&mut self,
context: &SharedStyleContext,
style: &mut Arc<ComputedValues>,
@ -457,24 +461,17 @@ impl ElementAnimationState {
) where
E: TElement,
{
for animation in self.finished_animations.iter() {
debug!("Updating style for finished animation {:?}", animation);
// TODO: support animation-fill-mode
if let Animation::Transition(_, _, property_animation) = animation {
property_animation.update(Arc::make_mut(style), 1.0);
}
// Return early so that we don't unnecessarily clone the style when making it mutable.
if self.running_animations.is_empty() {
return;
}
for running_animation in self.running_animations.iter_mut() {
let update = match *running_animation {
Animation::Transition(..) => continue,
Animation::Keyframes(..) => {
update_style_for_animation::<E>(context, running_animation, style, font_metrics)
},
};
let style = Arc::make_mut(style);
for animation in self.running_animations.iter_mut() {
let update = update_style_for_animation::<E>(context, animation, style, font_metrics);
match *running_animation {
Animation::Transition(..) => unreachable!(),
match *animation {
Animation::Transition(..) => {},
Animation::Keyframes(_, _, _, ref mut state) => match update {
AnimationUpdate::Regular => {},
AnimationUpdate::AnimationCanceled => {
@ -485,6 +482,25 @@ impl ElementAnimationState {
}
}
pub(crate) fn apply_new_animations<E>(
&mut self,
context: &SharedStyleContext,
style: &mut Arc<ComputedValues>,
font_metrics: &dyn crate::font_metrics::FontMetricsProvider,
) where
E: TElement,
{
// Return early so that we don't unnecessarily clone the style when making it mutable.
if self.new_animations.is_empty() {
return;
}
let style = Arc::make_mut(style);
for animation in self.new_animations.iter_mut() {
update_style_for_animation::<E>(context, animation, style, font_metrics);
}
}
/// Whether this `ElementAnimationState` is empty, which means it doesn't
/// hold any animations in any state.
pub fn is_empty(&self) -> bool {
@ -526,8 +542,6 @@ pub fn start_transitions_if_applicable(
new_style: &mut Arc<ComputedValues>,
animation_state: &mut ElementAnimationState,
) -> LonghandIdSet {
use crate::properties::animated_properties::TransitionPropertyIteration;
// If the style of this element is display:none, then we don't start any transitions
// and we cancel any currently running transitions by returning an empty LonghandIdSet.
if new_style.get_box().clone_display().is_none() {
@ -535,12 +549,12 @@ pub fn start_transitions_if_applicable(
}
let mut properties_that_transition = LonghandIdSet::new();
let transitions: Vec<TransitionPropertyIteration> = new_style.transition_properties().collect();
for transition in &transitions {
if properties_that_transition.contains(transition.longhand_id) {
for transition in new_style.transition_properties() {
let physical_property = transition.longhand_id.to_physical(new_style.writing_mode);
if properties_that_transition.contains(physical_property) {
continue;
} else {
properties_that_transition.insert(transition.longhand_id);
properties_that_transition.insert(physical_property);
}
let property_animation = match PropertyAnimation::from_longhand(
@ -552,18 +566,12 @@ pub fn start_transitions_if_applicable(
.get_box()
.transition_duration_mod(transition.index),
old_style,
Arc::make_mut(new_style),
new_style,
) {
Some(property_animation) => property_animation,
None => continue,
};
// Set the property to the initial value.
//
// NB: get_mut is guaranteed to succeed since we called make_mut()
// above.
property_animation.update(Arc::get_mut(new_style).unwrap(), 0.0);
// Per [1], don't trigger a new transition if the end state for that
// transition is the same as that of a transition that's running or
// completed.
@ -747,7 +755,7 @@ pub enum AnimationUpdate {
pub fn update_style_for_animation<E>(
context: &SharedStyleContext,
animation: &Animation,
style: &mut Arc<ComputedValues>,
style: &mut ComputedValues,
font_metrics_provider: &dyn FontMetricsProvider,
) -> AnimationUpdate
where
@ -762,7 +770,7 @@ where
let progress = (now - start_time) / (property_animation.duration);
let progress = progress.min(1.0);
if progress >= 0.0 {
property_animation.update(Arc::make_mut(style), progress);
property_animation.update(style, progress);
}
AnimationUpdate::Regular
},
@ -867,7 +875,7 @@ where
let from_style = compute_style_for_animation_step::<E>(
context,
last_keyframe,
&**style,
style,
&state.cascade_style,
font_metrics_provider,
);
@ -911,7 +919,7 @@ where
property
);
debug!("update_style_for_animation: {:?}", property_animation);
property_animation.update(Arc::make_mut(&mut new_style), relative_progress);
property_animation.update(&mut new_style, relative_progress);
},
None => {
debug!(

View file

@ -445,7 +445,10 @@ trait PrivateMatchMethods: TElement {
let mut animation_state = animation_states.remove(&this_opaque).unwrap_or_default();
if let Some(ref mut old_values) = *old_values {
animation_state.compute_before_change_style::<Self>(
// We convert old values into `before-change-style` here.
// https://drafts.csswg.org/css-transitions/#starting
animation_state.apply_completed_animations(old_values);
animation_state.apply_running_animations::<Self>(
shared_context,
old_values,
&context.thread_local.font_metrics_provider,
@ -475,10 +478,20 @@ trait PrivateMatchMethods: TElement {
.cancel_transitions_with_nontransitioning_properties(&transitioning_properties);
}
animation_state.finished_animations.clear();
animation_state.apply_running_animations::<Self>(
shared_context,
new_values,
&context.thread_local.font_metrics_provider,
);
animation_state.apply_new_animations::<Self>(
shared_context,
new_values,
&context.thread_local.font_metrics_provider,
);
// If the ElementAnimationState is empty, don't push it to save
// memory and to avoid extra processing later.
// If the ElementAnimationState is empty, and don't store it in order to
// save memory and to avoid extra processing later.
animation_state.finished_animations.clear();
if !animation_state.is_empty() {
animation_states.insert(this_opaque, animation_state);
}