Skip to content

Commit

Permalink
Fix flakiness in animation tests
Browse files Browse the repository at this point in the history
1. When updating the animation timeline, ensure that nodes that are
   animating are marked dirty, if necessary, so any style queries will
   force an layout flush.
2. Disable the problematic transition test suites, as they are in Gecko.
   These suites often fail when Servo is so overloaded that it cannot
   deliver frames fast enough to get more than two samples during the
   animation lifecycle.

Fixes servo#28334.
Fixes servo#26435.
Fixes servo#21486.
  • Loading branch information
mrobinson committed Apr 26, 2023
1 parent b24627a commit de85265
Show file tree
Hide file tree
Showing 19 changed files with 50 additions and 4,501 deletions.
20 changes: 17 additions & 3 deletions components/script/animations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

#![deny(missing_docs)]

//! The set of animations for a document.

Expand Down Expand Up @@ -47,6 +46,11 @@ pub(crate) struct Animations {

/// A list of pending animation-related events.
pending_events: DomRefCell<Vec<TransitionOrAnimationEvent>>,

/// The timeline value at the last time all animations were marked dirty.
/// This is used to prevent marking animations dirty when the timeline
/// has not changed.
timeline_value_at_last_dirty: Cell<f64>,
}

impl Animations {
Expand All @@ -56,6 +60,7 @@ impl Animations {
has_running_animations: Cell::new(false),
rooted_nodes: Default::default(),
pending_events: Default::default(),
timeline_value_at_last_dirty: Cell::new(0.0),
}
}

Expand All @@ -65,12 +70,22 @@ impl Animations {
self.pending_events.borrow_mut().clear();
}

pub(crate) fn mark_animating_nodes_as_dirty(&self) {
// Mark all animations dirty, if they haven't been marked dirty since the
// specified `current_timeline_value`. Returns true if animations were marked
// dirty or false otherwise.
pub(crate) fn mark_animating_nodes_as_dirty(&self, current_timeline_value: f64) -> bool {
if current_timeline_value <= self.timeline_value_at_last_dirty.get() {
return false;
}
self.timeline_value_at_last_dirty.set(current_timeline_value);

let sets = self.sets.sets.read();
let rooted_nodes = self.rooted_nodes.borrow();
for node in sets.keys().filter_map(|key| rooted_nodes.get(&key.node)) {
node.dirty(NodeDamage::NodeStyleDamaged);
}

true
}

pub(crate) fn update_for_new_timeline_value(&self, window: &Window, now: f64) {
Expand All @@ -97,7 +112,6 @@ impl Animations {
}

self.unroot_unused_nodes(&sets);
//self.update_running_animations_presence(window);
}

/// Cancel animations for the given node, if any exist.
Expand Down
11 changes: 11 additions & 0 deletions components/script/dom/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3852,6 +3852,17 @@ impl Document {
.update_for_new_timeline_value(&self.window, current_timeline_value);
}

pub(crate) fn maybe_mark_animating_nodes_as_dirty(&self) {
let current_timeline_value = self.current_animation_timeline_value();
let marked_dirty = self.animations
.borrow()
.mark_animating_nodes_as_dirty(current_timeline_value);

if marked_dirty {
self.window().add_pending_reflow();
}
}

pub(crate) fn current_animation_timeline_value(&self) -> f64 {
self.animation_timeline.borrow().current_value()
}
Expand Down
7 changes: 5 additions & 2 deletions components/script/script_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,10 @@ impl ScriptThread {
document.update_animation_timeline();
}

for (_, document) in self.documents.borrow().iter() {
document.maybe_mark_animating_nodes_as_dirty();
}

for (_, document) in self.documents.borrow().iter() {
document.animations().send_pending_events(document.window());
}
Expand Down Expand Up @@ -3012,8 +3016,7 @@ impl ScriptThread {
document.run_the_animation_frame_callbacks();
}
if tick_type.contains(AnimationTickType::CSS_ANIMATIONS_AND_TRANSITIONS) {
document.animations().mark_animating_nodes_as_dirty();
document.window().add_pending_reflow();
document.maybe_mark_animating_nodes_as_dirty();
}
}

Expand Down

0 comments on commit de85265

Please sign in to comment.