diff --git a/Cargo.lock b/Cargo.lock index 4bbb902a20b..4f8eb0aada9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1358,7 +1358,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "117725a109d387c937a1533ce01b450cbde6b88abceea8473c4d7a85853cda3c" dependencies = [ "lazy_static", - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] @@ -2384,7 +2384,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "778e2ac28f6c47af28e4907f13ffd1e1ddbd400980a9abd7c8df189bf578a5ad" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -3003,7 +3003,7 @@ dependencies = [ "gobject-sys", "libc", "system-deps", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -4507,7 +4507,7 @@ checksum = "e04d7f318608d35d4b61ddd75cbdaee86b023ebe2bd5a66ee0915f0bf93095a9" dependencies = [ "hermit-abi 0.5.0", "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -4735,6 +4735,7 @@ dependencies = [ "libc", "malloc_size_of_derive", "net_traits", + "parking_lot", "pixels", "profile_traits", "range", @@ -4803,7 +4804,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "07033963ba89ebaf1584d767badaa2e8fcec21aedea6b8c0346d487d49c28667" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] @@ -6898,7 +6899,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -7078,6 +7079,7 @@ dependencies = [ "nom", "num-traits", "num_cpus", + "parking_lot", "percent-encoding", "phf", "pixels", @@ -8289,7 +8291,7 @@ dependencies = [ "getrandom 0.2.16", "once_cell", "rustix 0.38.44", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -9732,7 +9734,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/components/layout/context.rs b/components/layout/context.rs index a1aac631fbc..39b24f92025 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -55,7 +55,10 @@ pub struct LayoutContext<'a> { pub resolved_images_cache: Arc>>, - pub node_image_animation_map: Arc>>, + /// A shared reference to script's map of DOM nodes with animated images. This is used + /// to manage image animations in script and inform the script about newly animating + /// nodes. + pub node_to_animating_image_map: Arc>>, /// The DOM node that is highlighted by the devtools inspector, if any pub highlighted_dom_node: Option, @@ -153,31 +156,24 @@ impl LayoutContext<'_> { } pub fn handle_animated_image(&self, node: OpaqueNode, image: Arc) { - let mut store = self.node_image_animation_map.write(); + let mut map = self.node_to_animating_image_map.write(); + if !image.should_animate() { + map.remove(&node); + return; + } + let new_image_animation_state = || { + ImageAnimationState::new( + image.clone(), + self.shared_context().current_time_for_animations, + ) + }; - // 1. first check whether node previously being track for animated image. - if let Some(image_state) = store.get(&node) { - // a. if the node is not containing the same image as before. - if image_state.image_key() != image.id { - if image.should_animate() { - // i. Register/Replace tracking item in image_animation_manager. - store.insert( - node, - ImageAnimationState::new( - image, - self.shared_context().current_time_for_animations, - ), - ); - } else { - // ii. Cancel Action if the node's image is no longer animated. - store.remove(&node); - } - } - } else if image.should_animate() { - store.insert( - node, - ImageAnimationState::new(image, self.shared_context().current_time_for_animations), - ); + let entry = map.entry(node).or_insert_with(new_image_animation_state); + + // If the entry exists, but it is for a different image id, replace it as the image + // has changed during this layout. + if entry.image.id != image.id { + *entry = new_image_animation_state(); } } diff --git a/components/layout/fragment_tree/fragment_tree.rs b/components/layout/fragment_tree/fragment_tree.rs index bab28ccc4de..295b3e1a699 100644 --- a/components/layout/fragment_tree/fragment_tree.rs +++ b/components/layout/fragment_tree/fragment_tree.rs @@ -58,7 +58,10 @@ impl FragmentTree { // 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 image_animations = layout_context.node_image_animation_map.write().to_owned(); + let mut image_animations = layout_context + .node_to_animating_image_map + .write() + .to_owned(); let mut invalid_image_animating_nodes: FxHashSet<_> = image_animations .keys() .cloned() diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index 8730902549f..528e544e2b8 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -644,9 +644,7 @@ impl LayoutThread { resolved_images_cache: self.resolved_images_cache.clone(), pending_images: Mutex::default(), pending_rasterization_images: Mutex::default(), - node_image_animation_map: Arc::new(RwLock::new(std::mem::take( - &mut reflow_request.node_to_image_animation_map, - ))), + node_to_animating_image_map: reflow_request.node_to_animating_image_map.clone(), iframe_sizes: Mutex::default(), use_rayon: rayon_pool.is_some(), highlighted_dom_node: reflow_request.highlighted_dom_node, @@ -687,15 +685,12 @@ impl LayoutThread { let pending_rasterization_images = std::mem::take(&mut *layout_context.pending_rasterization_images.lock()); let iframe_sizes = std::mem::take(&mut *layout_context.iframe_sizes.lock()); - let node_to_image_animation_map = - std::mem::take(&mut *layout_context.node_image_animation_map.write()); Some(ReflowResult { built_display_list, pending_images, pending_rasterization_images, iframe_sizes, - node_to_image_animation_map, }) } diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index ae1a74bd1a4..127cf672845 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -95,6 +95,7 @@ net_traits = { workspace = true } nom = "7.1.3" num-traits = { workspace = true } num_cpus = { workspace = true } +parking_lot = { workspace = true } percent-encoding = { workspace = true } phf = "0.11" pixels = { path = "../pixels" } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 62419e75984..d2b61734007 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -4189,7 +4189,7 @@ impl Document { DomRefCell::new(AnimationTimeline::new()) }, animations: DomRefCell::new(Animations::new()), - image_animation_manager: DomRefCell::new(ImageAnimationManager::new()), + image_animation_manager: DomRefCell::new(ImageAnimationManager::default()), dirty_root: Default::default(), declarative_refresh: Default::default(), pending_input_events: Default::default(), @@ -4960,6 +4960,7 @@ impl Document { self.animations .borrow() .do_post_reflow_update(&self.window, self.current_animation_timeline_value()); + self.image_animation_manager().update_rooted_dom_nodes(); } pub(crate) fn cancel_animations_for_node(&self, node: &Node) { @@ -4998,12 +4999,9 @@ impl Document { pub(crate) fn image_animation_manager(&self) -> Ref { self.image_animation_manager.borrow() } - pub(crate) fn image_animation_manager_mut(&self) -> RefMut { - self.image_animation_manager.borrow_mut() - } pub(crate) fn update_animating_images(&self) { - let mut image_animation_manager = self.image_animation_manager.borrow_mut(); + let image_animation_manager = self.image_animation_manager.borrow(); if !image_animation_manager.image_animations_present() { return; } @@ -5011,8 +5009,8 @@ impl Document { .update_active_frames(&self.window, self.current_animation_timeline_value()); if !self.animations().animations_present() { - let next_scheduled_time = - image_animation_manager.next_schedule_time(self.current_animation_timeline_value()); + let next_scheduled_time = image_animation_manager + .next_scheduled_time(self.current_animation_timeline_value()); // TODO: Once we have refresh signal from the compositor, // we should get rid of timer for animated image update. if let Some(next_scheduled_time) = next_scheduled_time { diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 9ce257244f2..89c15e73692 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -2218,9 +2218,7 @@ impl Window { pending_restyles, animation_timeline_value: document.current_animation_timeline_value(), animations: document.animations().sets.clone(), - node_to_image_animation_map: document - .image_animation_manager_mut() - .take_image_animate_set(), + node_to_animating_image_map: document.image_animation_manager().node_to_image_map(), theme: self.theme.get(), highlighted_dom_node, }; @@ -2297,9 +2295,6 @@ impl Window { if !size_messages.is_empty() { self.send_to_constellation(ScriptToConstellationMessage::IFrameSizes(size_messages)); } - document - .image_animation_manager_mut() - .restore_image_animate_set(results.node_to_image_animation_map); document.update_animations_post_reflow(); self.update_constellation_epoch(); diff --git a/components/script/image_animation.rs b/components/script/image_animation.rs index d7e14c110d8..eace2f6e3a4 100644 --- a/components/script/image_animation.rs +++ b/components/script/image_animation.rs @@ -2,12 +2,16 @@ * 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/. */ +use std::sync::Arc; + use compositing_traits::{ImageUpdate, SerializableImageData}; use embedder_traits::UntrustedNodeAddress; -use fxhash::{FxHashMap, FxHashSet}; +use fxhash::FxHashMap; use ipc_channel::ipc::IpcSharedMemory; use layout_api::ImageAnimationState; use libc::c_void; +use malloc_size_of::MallocSizeOf; +use parking_lot::RwLock; use script_bindings::root::Dom; use style::dom::OpaqueNode; use webrender_api::units::DeviceIntSize; @@ -18,97 +22,90 @@ use crate::dom::bindings::trace::NoTrace; use crate::dom::node::{Node, from_untrusted_node_address}; use crate::dom::window::Window; -#[derive(Clone, Debug, Default, JSTraceable, MallocSizeOf)] +#[derive(Clone, Debug, Default, JSTraceable)] #[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] pub struct ImageAnimationManager { #[no_trace] - pub node_to_image_map: FxHashMap, + node_to_image_map: Arc>>, /// A list of nodes with in-progress image animations. + /// + /// TODO(mrobinson): This does not properly handle animating images that are in pseudo-elements. rooted_nodes: DomRefCell, Dom>>, } +impl MallocSizeOf for ImageAnimationManager { + fn size_of(&self, ops: &mut malloc_size_of::MallocSizeOfOps) -> usize { + (*self.node_to_image_map.read()).size_of(ops) + self.rooted_nodes.size_of(ops) + } +} + impl ImageAnimationManager { - pub fn new() -> Self { - ImageAnimationManager { - node_to_image_map: Default::default(), - rooted_nodes: DomRefCell::new(FxHashMap::default()), - } + pub(crate) fn node_to_image_map( + &self, + ) -> Arc>> { + self.node_to_image_map.clone() } - pub fn take_image_animate_set(&mut self) -> FxHashMap { - std::mem::take(&mut self.node_to_image_map) - } - - pub fn restore_image_animate_set(&mut self, map: FxHashMap) { - let _ = std::mem::replace(&mut self.node_to_image_map, map); - self.root_newly_animating_dom_nodes(); - self.unroot_unused_nodes(); - } - - pub fn next_schedule_time(&self, now: f64) -> Option { + pub(crate) fn next_scheduled_time(&self, now: f64) -> Option { self.node_to_image_map + .read() .values() .map(|state| state.time_to_next_frame(now)) .min_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal)) } - pub fn image_animations_present(&self) -> bool { - !self.node_to_image_map.is_empty() + pub(crate) fn image_animations_present(&self) -> bool { + !self.node_to_image_map.read().is_empty() } - pub fn update_active_frames(&mut self, window: &Window, now: f64) { + pub(crate) fn update_active_frames(&self, window: &Window, now: f64) { let rooted_nodes = self.rooted_nodes.borrow(); - let updates: Vec = self + let updates = self .node_to_image_map + .write() .iter_mut() .filter_map(|(node, state)| { - if state.update_frame_for_animation_timeline_value(now) { - let image = &state.image; - let frame = image - .frames() - .nth(state.active_frame) - .expect("active_frame should within range of frames"); - - if let Some(node) = rooted_nodes.get(&NoTrace(*node)) { - node.dirty(crate::dom::node::NodeDamage::Other); - } - Some(ImageUpdate::UpdateImage( - image.id.unwrap(), - ImageDescriptor { - format: ImageFormat::BGRA8, - size: DeviceIntSize::new( - image.metadata.width as i32, - image.metadata.height as i32, - ), - stride: None, - offset: 0, - flags: ImageDescriptorFlags::ALLOW_MIPMAPS, - }, - SerializableImageData::Raw(IpcSharedMemory::from_bytes(frame.bytes)), - )) - } else { - None + if !state.update_frame_for_animation_timeline_value(now) { + return None; } + + let image = &state.image; + let frame = image + .frames() + .nth(state.active_frame) + .expect("active_frame should within range of frames"); + + if let Some(node) = rooted_nodes.get(&NoTrace(*node)) { + node.dirty(crate::dom::node::NodeDamage::Other); + } + Some(ImageUpdate::UpdateImage( + image.id.unwrap(), + ImageDescriptor { + format: ImageFormat::BGRA8, + size: DeviceIntSize::new( + image.metadata.width as i32, + image.metadata.height as i32, + ), + stride: None, + offset: 0, + flags: ImageDescriptorFlags::ALLOW_MIPMAPS, + }, + SerializableImageData::Raw(IpcSharedMemory::from_bytes(frame.bytes)), + )) }) .collect(); window.compositor_api().update_images(updates); } - - // Unroot any nodes that we have rooted but no longer have animating images. - fn unroot_unused_nodes(&self) { - let nodes: FxHashSet<&OpaqueNode> = self.node_to_image_map.keys().collect(); - self.rooted_nodes - .borrow_mut() - .retain(|node, _| nodes.contains(&node.0)); - } - - /// Ensure that all nodes with Image animations are rooted. This should be called - /// immediately after a restyle, to ensure that these addresses are still valid. + /// Ensure that all nodes with animating images are rooted and unroots any nodes that + /// no longer have an animating image. This should be called immediately after a + /// restyle, to ensure that these addresses are still valid. #[allow(unsafe_code)] - fn root_newly_animating_dom_nodes(&self) { + pub(crate) fn update_rooted_dom_nodes(&self) { let mut rooted_nodes = self.rooted_nodes.borrow_mut(); - for opaque_node in self.node_to_image_map.keys() { + let node_to_image_map = self.node_to_image_map.read(); + + for opaque_node in node_to_image_map.keys() { let opaque_node = *opaque_node; if rooted_nodes.contains_key(&NoTrace(opaque_node)) { continue; @@ -121,5 +118,7 @@ impl ImageAnimationManager { ) }; } + + rooted_nodes.retain(|node, _| node_to_image_map.contains_key(&node.0)); } } diff --git a/components/shared/layout/Cargo.toml b/components/shared/layout/Cargo.toml index 3874757066e..3e97d206aac 100644 --- a/components/shared/layout/Cargo.toml +++ b/components/shared/layout/Cargo.toml @@ -29,6 +29,7 @@ libc = { workspace = true } malloc_size_of = { workspace = true } malloc_size_of_derive = { workspace = true } net_traits = { workspace = true } +parking_lot = { workspace = true } pixels = { path = "../../pixels" } profile_traits = { workspace = true } range = { path = "../../range" } diff --git a/components/shared/layout/lib.rs b/components/shared/layout/lib.rs index ed4178647d8..9aa363598da 100644 --- a/components/shared/layout/lib.rs +++ b/components/shared/layout/lib.rs @@ -31,6 +31,7 @@ use libc::c_void; use malloc_size_of::{MallocSizeOf as MallocSizeOfTrait, MallocSizeOfOps}; use malloc_size_of_derive::MallocSizeOf; use net_traits::image_cache::{ImageCache, PendingImageId}; +use parking_lot::RwLock; use pixels::RasterImage; use profile_traits::mem::Report; use profile_traits::time; @@ -412,8 +413,6 @@ pub struct ReflowResult { /// to communicate them with the Constellation and also the `Window` /// element of their content pages. pub iframe_sizes: IFrameSizes, - /// The mapping of node to animated image, need to be returned to ImageAnimationManager - pub node_to_image_animation_map: FxHashMap, } /// Information needed for a script-initiated reflow. @@ -440,7 +439,7 @@ pub struct ReflowRequest { /// The set of animations for this document. pub animations: DocumentAnimationSet, /// The set of image animations. - pub node_to_image_animation_map: FxHashMap, + pub node_to_animating_image_map: Arc>>, /// The theme for the window pub theme: Theme, /// The node highlighted by the devtools, if any