eliott/servo
1
0
Fork 0
mirror of https://github.com/servo/servo synced 2026-04-25 17:15:48 +02:00
Code Issues Packages Projects Releases Wiki Activity

script: Cancel animations for non-rendered nodes in script (#44299)

Browse Source 
Instead of walking the entire fragment tree to find nodes for which
animations and image animations need to be cancelled, this change moves
that logic to `script`. Now, for each animating node the animation
managers will explicitly ask `layout` if the node is being rendered (or
delegating rendering in the case of CSS animations and transitions).

The main goal here is a performance improvement, elimating roughly 1% of
layout time from the profiler when running the
`flexbox-deeply-nested-column-flow.html` test case. This will almost
certainly be an even better improvement on more complex pages as we are
no longer doing things once per fragment tree entry, but once per
animating node.

There is also a subtle behavior improvement here. Before nodes with
`display: contents` had their animations canceled, but now they are not.
For instance, this test case now works properly:

```html
<!DOCTYPE html>
<style>
@keyframes anim {
  from { color: cyan }
  to { color: magenta }
}
div {
  display: contents;
  animation: anim 1s infinite alternate linear;
}
</style>
```

The new layout query will additionally be useful for other parts of
script that need to answer the same "being rendered" or "delegates
rendering to children" question.

Testing: This change adds a new test and gets one more subtest passing.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This commit is contained in:
Martin Robinson
2026-04-17 23:59:20 +02:00
committed by GitHub
parent a908081352
commit b39d9833a4
12 changed files with 207 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
components/layout/dom.rs
View File

@@ -7,7 +7,7 @@ use std::marker::PhantomData;
use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
use layout_api::{
GenericLayoutDataTrait, LayoutDataTrait, LayoutElement, LayoutElementType, LayoutNode,
LayoutNodeType as ScriptLayoutNodeType, SVGElementData,
LayoutNodeType as ScriptLayoutNodeType, NodeRenderingType, SVGElementData,
};
use malloc_size_of_derive::MallocSizeOf;
use script::layout_dom::ServoLayoutNode;
@@ -320,6 +320,11 @@ pub(crate) trait NodeExt<'dom> {
/// Remove boxes for the element itself, and all of its pseudo-element boxes.
fn unset_all_boxes(&self);
/// Returns the [`NodeRenderingType`] for this [`LayoutNode`] which describes whether
/// the node is being rendered, delegating rendering, or not being rendered at all
/// based on whether it has a [`LayoutBox`] and what kind.
fn rendering_type(&self) -> NodeRenderingType;
fn fragments_for_pseudo(&self, pseudo_element: Option<PseudoElement>) -> Vec<Fragment>;
fn with_layout_box_base_including_pseudos(&self, callback: impl Fn(&LayoutBoxBase));
@@ -502,6 +507,17 @@ impl<'dom> NodeExt<'dom> for ServoLayoutNode<'dom> {
// for DOM descendants of elements with `display: none`.
}
fn rendering_type(&self) -> NodeRenderingType {
let Some(layout_data) = self.inner_layout_data() else {
return NodeRenderingType::NotRendered;
};
match &*layout_data.self_box.borrow() {
Some(LayoutBox::DisplayContents(..)) => NodeRenderingType::DelegatesRendering,
Some(..) => NodeRenderingType::Rendered,
None => NodeRenderingType::NotRendered,
}
}
fn with_layout_box_base_including_pseudos(&self, callback: impl Fn(&LayoutBoxBase)) {
if let Some(inner_layout_data) = self.inner_layout_data() {
inner_layout_data.with_layout_box_base_including_pseudos(callback);

1
components/layout/flow/root.rs
View File

@@ -219,7 +219,6 @@ impl BoxTree {
};
FragmentTree::new(
layout_context,
root_fragments,
physical_containing_block,
viewport_scroll_sensitivity,

41
components/layout/fragment_tree/fragment_tree.rs
View File

@@ -7,14 +7,11 @@ use std::cell::Cell;
use app_units::Au;
use malloc_size_of_derive::MallocSizeOf;
use paint_api::display_list::AxesScrollSensitivity;
use rustc_hash::FxHashSet;
use servo_base::print_tree::PrintTree;
use style::animation::AnimationSetKey;
use style::computed_values::position::T as Position;
use super::{BoxFragment, ContainingBlockManager, Fragment};
use crate::ArcRefCell;
use crate::context::LayoutContext;
use crate::geom::PhysicalRect;
#[derive(MallocSizeOf)]
@@ -42,7 +39,6 @@ pub struct FragmentTree {
impl FragmentTree {
pub(crate) fn new(
layout_context: &LayoutContext,
root_fragments: Vec<Fragment>,
initial_containing_block: PhysicalRect<Au>,
viewport_scroll_sensitivity: AxesScrollSensitivity,
@@ -54,48 +50,11 @@ impl FragmentTree {
viewport_scroll_sensitivity,
};
// As part of building the fragment tree, we want to stop animating elements and
// pseudo-elements that used to be animating or had animating images attached to
// them. Create a set of all elements that used to be animating.
let mut animations = layout_context.style_context.animations.sets.write();
let mut invalid_animating_nodes: FxHashSet<_> = animations.keys().cloned().collect();
let mut animating_images = layout_context.image_resolver.animating_images.write();
let mut invalid_image_animating_nodes: FxHashSet<_> = animating_images
.node_to_state_map
.keys()
.cloned()
.map(|node| AnimationSetKey::new(node, None))
.collect();
fragment_tree.find(|fragment, _level, containing_block| {
if let Some(tag) = fragment.tag() {
// TODO: Support animations on nested pseudo-elements.
invalid_animating_nodes.remove(&AnimationSetKey::new(
tag.node,
tag.pseudo_element_chain.primary,
));
invalid_image_animating_nodes.remove(&AnimationSetKey::new(
tag.node,
tag.pseudo_element_chain.primary,
));
}
fragment.set_containing_block(containing_block);
None::<()>
});
// Cancel animations for any elements and pseudo-elements that are no longer found
// in the fragment tree.
for node in &invalid_animating_nodes {
if let Some(state) = animations.get_mut(node) {
state.cancel_all_animations();
}
}
for node in &invalid_image_animating_nodes {
animating_images.remove(node.node);
}
fragment_tree
}

36
components/layout/layout_impl.rs
View File

@@ -22,10 +22,10 @@ use fonts_traits::StylesheetWebFontLoadFinishedCallback;
use icu_locid::subtags::Language;
use layout_api::{
AxesOverflow, BoxAreaType, CSSPixelRectIterator, DangerousStyleNode, IFrameSizes, Layout,
LayoutConfig, LayoutElement, LayoutFactory, LayoutNode, OffsetParentResponse, PhysicalSides,
QueryMsg, ReflowGoal, ReflowPhasesRun, ReflowRequest, ReflowRequestRestyle, ReflowResult,
ReflowStatistics, ScrollContainerQueryFlags, ScrollContainerResponse, TrustedNodeAddress,
with_layout_state,
LayoutConfig, LayoutElement, LayoutFactory, LayoutNode, NodeRenderingType,
OffsetParentResponse, PhysicalSides, QueryMsg, ReflowGoal, ReflowPhasesRun, ReflowRequest,
ReflowRequestRestyle, ReflowResult, ReflowStatistics, ScrollContainerQueryFlags,
ScrollContainerResponse, TrustedNodeAddress, with_layout_state,
};
use log::{debug, error, warn};
use malloc_size_of::{MallocConditionalSizeOf, MallocSizeOf, MallocSizeOfOps};
@@ -86,6 +86,7 @@ use webrender_api::units::{DevicePixel, LayoutVector2D};
use crate::accessibility_tree::AccessibilityTree;
use crate::context::{CachedImageOrError, ImageResolver, LayoutContext};
use crate::display_list::{DisplayListBuilder, HitTest, PaintTimingHandler, StackingContextTree};
use crate::dom::NodeExt;
use crate::query::{
find_character_offset_in_fragment_descendants, get_the_text_steps, process_box_area_request,
process_box_areas_request, process_client_rect_request, process_containing_block_query,
@@ -320,6 +321,33 @@ impl Layout for LayoutThread {
resolved_images_cache.remove(url);
}
fn node_rendering_type(
&self,
node: TrustedNodeAddress,
pseudo: Option<PseudoElement>,
) -> NodeRenderingType {
with_layout_state(|| {
let node = unsafe { ServoLayoutNode::new(&node) };
// Nodes that are not currently styled are never being rendered.
if node
.as_element()
.is_none_or(|element| element.style_data().is_none())
{
return NodeRenderingType::NotRendered;
}
let node = match pseudo {
Some(pseudo) => node.with_pseudo(pseudo),
None => Some(node),
};
let Some(node) = node else {
return NodeRenderingType::NotRendered;
};
node.rendering_type()
})
}
/// Return the node corresponding to the containing block of the provided node.
#[servo_tracing::instrument(skip_all)]
fn query_containing_block(&self, node: TrustedNodeAddress) -> Option<UntrustedNodeAddress> {

19
components/script/animations.rs
View File

@@ -145,11 +145,24 @@ impl Animations {
));
}
/// Processes any new animations that were discovered after reflow. Collect messages
/// that trigger events for any animations that changed state.
/// This does three things:
/// - Cancel animations for any nodes that are no longer being rendered or delegating rendering.
/// - Process any new animations that were discovered after reflow.
/// - Collect pending events for any animations that changed state.
pub(crate) fn do_post_reflow_update(&self, window: &Window, now: f64) {
let pipeline_id = window.pipeline_id();
let mut sets = self.sets.sets.write();
{
let rooted_nodes = self.rooted_nodes.borrow();
for (key, set) in sets.iter_mut() {
if rooted_nodes.get(&NoTrace(key.node)).is_some_and(|node| {
!node.is_being_rendered_or_delegates_rendering(key.pseudo_element)
}) {
set.cancel_all_animations();
}
}
}
let pipeline_id = window.pipeline_id();
self.root_newly_animating_dom_nodes(&sets);
for (key, set) in sets.iter_mut() {

12
components/script/dom/document/document.rs
View File

@@ -4326,20 +4326,18 @@ impl Document {
}
pub(crate) fn update_animations_post_reflow(&self) {
let current_timeline_value = self.current_animation_timeline_value();
self.animations
.do_post_reflow_update(&self.window, self.current_animation_timeline_value());
.do_post_reflow_update(&self.window, current_timeline_value);
self.image_animation_manager
.borrow()
.maybe_schedule_update_after_layout(
&self.window,
self.current_animation_timeline_value(),
);
.borrow_mut()
.do_post_reflow_update(&self.window, current_timeline_value);
}
pub(crate) fn cancel_animations_for_node(&self, node: &Node) {
self.animations.cancel_animations_for_node(node);
self.image_animation_manager
.borrow()
.borrow_mut()
.cancel_animations_for_node(node);
}

29
components/script/dom/node/node.rs
View File

@@ -29,8 +29,8 @@ use js::rust::HandleObject;
use keyboard_types::Modifiers;
use layout_api::{
AxesOverflow, BoxAreaType, CSSPixelRectIterator, GenericLayoutData, HTMLCanvasData,
HTMLMediaData, LayoutElementType, LayoutNodeType, PhysicalSides, SVGElementData,
SharedSelection, TrustedNodeAddress, with_layout_state,
HTMLMediaData, LayoutElementType, LayoutNodeType, NodeRenderingType, PhysicalSides,
SVGElementData, SharedSelection, TrustedNodeAddress, with_layout_state,
};
use libc::{self, c_void, uintptr_t};
use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
@@ -632,6 +632,31 @@ impl Node {
}
}
}
/// Implements the combination of:
/// - <https://html.spec.whatwg.org/multipage/#being-rendered>
/// - <https://html.spec.whatwg.org/multipage/#delegating-its-rendering-to-its-children>
pub(crate) fn is_being_rendered_or_delegates_rendering(
&self,
pseudo_element: Option<PseudoElement>,
) -> bool {
matches!(
self.owner_window()
.layout()
.node_rendering_type(self.to_trusted_node_address(), pseudo_element),
NodeRenderingType::Rendered | NodeRenderingType::DelegatesRendering
)
}
/// <https://html.spec.whatwg.org/multipage/#being-rendered>
pub(crate) fn is_being_rendered(&self, pseudo_element: Option<PseudoElement>) -> bool {
matches!(
self.owner_window()
.layout()
.node_rendering_type(self.to_trusted_node_address(), pseudo_element),
NodeRenderingType::Rendered
)
}
}
impl Node {

49
components/script/image_animation.rs
View File

@@ -3,16 +3,23 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use std::cell::Cell;
use std::ffi::c_void;
use std::sync::Arc;
use std::time::Duration;
use embedder_traits::UntrustedNodeAddress;
use layout_api::AnimatingImages;
use paint_api::ImageUpdate;
use parking_lot::RwLock;
use rustc_hash::FxHashMap;
use script_bindings::codegen::GenericBindings::WindowBinding::WindowMethods;
use script_bindings::root::Dom;
use style::dom::OpaqueNode;
use timers::{TimerEventRequest, TimerId};
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::trace::NoTrace;
use crate::dom::from_untrusted_node_address;
use crate::dom::node::Node;
use crate::dom::window::Window;
use crate::script_thread::with_script_thread;
@@ -29,6 +36,10 @@ pub struct ImageAnimationManager {
/// The [`TimerId`] of the currently scheduled animated image update callback.
#[no_trace]
callback_timer_id: Cell<Option<TimerId>>,
/// A map of nodes with in-progress image animations. This is kept outside
/// of [`Self::animating_images`] as that data structure is shared with layout.
rooted_nodes: FxHashMap<NoTrace<OpaqueNode>, Dom<Node>>,
}
impl ImageAnimationManager {
@@ -86,14 +97,40 @@ impl ImageAnimationManager {
self.maybe_schedule_update(window, now);
}
/// After doing a layout, if the set of animating images was updated in some way,
/// schedule a new animation update.
pub(crate) fn maybe_schedule_update_after_layout(&self, window: &Window, now: f64) {
/// This does three things:
/// - Root any nodes with newly animating images
/// - Schedule an image update for newly animating images
/// - Cancel animations for any nodes that no longer have layout boxes.
pub(crate) fn do_post_reflow_update(&mut self, window: &Window, now: f64) {
// Cancel animations for any images that are no longer rendering.
self.rooted_nodes.retain(|opaque_node, node| {
if node.is_being_rendered(None) {
return true;
}
self.animating_images.write().remove(opaque_node.0);
false
});
if self.animating_images().write().clear_dirty() {
self.root_nodes_with_newly_animating_images();
self.maybe_schedule_update(window, now);
}
}
fn root_nodes_with_newly_animating_images(&mut self) {
for opaque_node in self.animating_images().read().node_to_state_map.keys() {
#[expect(unsafe_code)]
self.rooted_nodes
.entry(NoTrace(*opaque_node))
.or_insert_with(|| {
// SAFETY: This should be safe as this method is run directly after layout,
// which should not remove any nodes.
let address = UntrustedNodeAddress(opaque_node.0 as *const c_void);
unsafe { Dom::from_ref(&*from_untrusted_node_address(address)) }
});
}
}
fn maybe_schedule_update(&self, window: &Window, now: f64) {
with_script_thread(|script_thread| {
if let Some(current_timer_id) = self.callback_timer_id.take() {
@@ -115,7 +152,9 @@ impl ImageAnimationManager {
})
}
pub(crate) fn cancel_animations_for_node(&self, node: &Node) {
self.animating_images().write().remove(node.to_opaque());
pub(crate) fn cancel_animations_for_node(&mut self, node: &Node) {
let opaque_node = node.to_opaque();
self.animating_images().write().remove(opaque_node);
self.rooted_nodes.remove(&NoTrace(opaque_node));
}
}

21
components/shared/layout/lib.rs
View File

@@ -339,6 +339,14 @@ pub trait Layout {
/// Marks that this layout needs to produce a new display list for rendering updates.
fn set_needs_new_display_list(&self);
/// Returns the [`NodeRenderingType`] for this node and pseudo. This is used to determine
/// if a node is being rendered, delegating its rendering, or not being rendered at all.
fn node_rendering_type(
&self,
node: TrustedNodeAddress,
pseudo: Option<PseudoElement>,
) -> NodeRenderingType;
fn query_containing_block(&self, node: TrustedNodeAddress) -> Option<UntrustedNodeAddress>;
fn query_padding(&self, node: TrustedNodeAddress) -> Option<PhysicalSides>;
fn query_box_area(
@@ -433,6 +441,19 @@ pub enum BoxAreaType {
pub type CSSPixelRectIterator = Box<dyn Iterator<Item = Rect<Au, CSSPixel>>>;
/// Whether or not this node is being rendered or delegates rendering according
/// to the HTML standard.
#[derive(Copy, Clone)]
pub enum NodeRenderingType {
/// <https://html.spec.whatwg.org/multipage/#being-rendered>
Rendered,
/// <https://html.spec.whatwg.org/multipage/#delegating-its-rendering-to-its-children>
DelegatesRendering,
/// If neither of the other two cases are true, this is. The node is effectively not
/// taking part in the final layout of the page.
NotRendered,
}
#[derive(Default)]
pub struct PhysicalSides {
pub left: Au,

7
tests/wpt/meta/MANIFEST.json vendored
View File

@@ -632285,6 +632285,13 @@
{}
]
],
"display-contents-animates.html": [
"bb9c28bcb63395d18a24adfa2036437b784da8d0",
[
null,
{}
]
],
"display-interpolation.html": [
"fb2503ac8a415566c6770ccf391690860fddec6b",
[

3
tests/wpt/meta/css/css-animations/display-none-dont-cancel.tentative.html.ini vendored
View File

@@ -10,6 +10,3 @@
[Animating from display:none to display:none with an intermediate variable should not cancel the animation.]
expected: FAIL
[Animating a variable of "none" which gets set to display elsewhere should not cancel the animation.]
expected: FAIL

38
tests/wpt/tests/css/css-animations/display-contents-animates.html vendored Normal file
View File

@@ -0,0 +1,38 @@
<!DOCTYPE html>
<link rel=author href="mailto:mrobinson@igalia.com">
<link rel=help href="https://drafts.csswg.org/css-display-4/#display-animation">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/css/css-animations/support/testcommon.js"></script>
<div id="outer" style="visibility: hidden; display: contents">
<div id="inner">hello</div>
</div>
<style>
@keyframes display1 {
/* 'visibility' is discrete, animatable, and inherited. */
0% { visibility: visible; }
100% { visibility: hidden; }
}
.animate1 {
animation: display1 1s infinite;
}
</style>
<script>
promise_test(async (t) => {
t.add_cleanup(() => outer.classList.remove('animate1'));
let numAnimationstartFired = 0;
outer.addEventListener('animationstart', () => numAnimationstartFired++);
await waitForAnimationFrames(1);
outer.classList.add('animate1');
await waitForAnimationFrames(2);
assert_equals(getComputedStyle(inner).visibility, 'visible',
'The display should be inline during the animation.');
assert_equals(numAnimationstartFired, 1,
'Only one animation should start.');
}, 'Animations inside of display:contents should work properly.');
</script>