From 25b27b2040ebbe2fe1628cd191f3e704ab158c4d Mon Sep 17 00:00:00 2001 From: stevennovaryo Date: Wed, 4 Jun 2025 20:28:27 +0800 Subject: [PATCH] Remove scroll_offset hashmap from script and layout thread Signed-off-by: stevennovaryo --- components/compositing/compositor.rs | 9 +- components/compositing/webview_renderer.rs | 2 + components/constellation/constellation.rs | 1 + components/layout/layout_impl.rs | 115 ++++++++++++++---- components/script/dom/window.rs | 43 ++----- components/script/script_thread.rs | 10 +- components/shared/compositing/display_list.rs | 53 ++++++-- components/shared/compositing/lib.rs | 8 +- components/shared/script_layout/lib.rs | 13 +- 9 files changed, 171 insertions(+), 83 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 0acbbec977a..23bba8da3c9 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -257,6 +257,7 @@ impl PipelineDetails { } } + // MYNOTES: move this to display list fn install_new_scroll_tree(&mut self, new_scroll_tree: ScrollTree) { let old_scroll_offsets: FnvHashMap = self .scroll_tree @@ -717,7 +718,7 @@ impl IOCompositor { self.global.borrow_mut().send_transaction(txn); }, - CompositorMsg::SendScrollNode(webview_id, pipeline_id, point, external_scroll_id) => { + CompositorMsg::SendScrollNode(webview_id, pipeline_id, offset, external_scroll_id) => { let Some(webview_renderer) = self.webview_renderers.get_mut(webview_id) else { return; }; @@ -728,13 +729,9 @@ impl IOCompositor { return; }; - let offset = LayoutVector2D::new(point.x, point.y); if !pipeline_details .scroll_tree - .set_scroll_offsets_for_node_with_external_scroll_id( - external_scroll_id, - -offset, - ) + .set_scroll_offsets_for_node_with_external_scroll_id(&external_scroll_id, offset) { warn!("Could not scroll not with id: {external_scroll_id:?}"); return; diff --git a/components/compositing/webview_renderer.rs b/components/compositing/webview_renderer.rs index 84d5fd877e6..40538beff42 100644 --- a/components/compositing/webview_renderer.rs +++ b/components/compositing/webview_renderer.rs @@ -183,6 +183,7 @@ impl WebViewRenderer { self.pipelines.remove(&pipeline_id); } + // MYNOTES: I suppose this is not needed now? If changing composition's frame tree does trigger reflow? pub(crate) fn set_frame_tree(&mut self, frame_tree: &SendableFrameTree) { let pipeline_id = frame_tree.pipeline.id; let old_pipeline_id = std::mem::replace(&mut self.root_pipeline_id, Some(pipeline_id)); @@ -879,6 +880,7 @@ impl WebViewRenderer { /// scrolling to the applicable scroll node under that point. If a scroll was /// performed, returns the [`PipelineId`] of the node scrolled, the id, and the final /// scroll delta. + // MYNOTES: this could be changed to we only store the diff fn scroll_node_at_device_point( &mut self, cursor: DevicePoint, diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 5b4d244e746..c95808b00ec 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -6019,6 +6019,7 @@ where } } + // MYNOTES: this shouldn't be necessary anymore since, we have scroll offsets tree at layout impl #[cfg_attr( feature = "tracing", tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index 0fbc8395c35..6007d34c477 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -4,8 +4,7 @@ #![allow(unsafe_code)] -use std::cell::{Cell, RefCell}; -use std::collections::HashMap; +use std::cell::{Cell, Ref, RefCell, RefMut}; use std::fmt::Debug; use std::process; use std::rc::Rc; @@ -15,10 +14,11 @@ use app_units::Au; use base::Epoch; use base::id::{PipelineId, WebViewId}; use compositing_traits::CrossProcessCompositorApi; +use compositing_traits::display_list::{ScrollTree, SpatialTreeNodeInfo}; use constellation_traits::ScrollState; use embedder_traits::{Theme, UntrustedNodeAddress, ViewportDetails}; use euclid::default::{Point2D as UntypedPoint2D, Rect as UntypedRect}; -use euclid::{Point2D, Scale, Size2D, Vector2D}; +use euclid::{Point2D, Scale, Size2D}; use fnv::FnvHashMap; use fonts::{FontContext, FontContextWebFontMethods}; use fonts_traits::StylesheetWebFontLoadFinishedCallback; @@ -36,8 +36,9 @@ use profile_traits::{path, time_profile}; use rayon::ThreadPool; use script::layout_dom::{ServoLayoutDocument, ServoLayoutElement, ServoLayoutNode}; use script_layout_interface::{ - Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, OffsetParentResponse, ReflowGoal, - ReflowRequest, ReflowResult, TrustedNodeAddress, + FragmentType, Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, + OffsetParentResponse, ReflowGoal, ReflowRequest, ReflowResult, TrustedNodeAddress, + combine_id_with_fragment_type, }; use script_traits::{DrawAPaintImageResult, PaintWorkletError, Painter, ScriptThreadMessage}; use servo_arc::Arc as ServoArc; @@ -74,7 +75,7 @@ use style::{Zero, driver}; use style_traits::{CSSPixel, SpeculativePainter}; use stylo_atoms::Atom; use url::Url; -use webrender_api::units::{DevicePixel, DevicePoint, LayoutPixel, LayoutPoint, LayoutSize}; +use webrender_api::units::{DevicePixel, DevicePoint, LayoutSize, LayoutVector2D}; use webrender_api::{ExternalScrollId, HitTestFlags}; use crate::context::{CachedImageOrError, LayoutContext}; @@ -151,9 +152,6 @@ pub struct LayoutThread { /// A counter for epoch messages epoch: Cell, - /// Scroll offsets of nodes that scroll. - scroll_offsets: RefCell>>, - // A cache that maps image resources specified in CSS (e.g as the `url()` value // for `background-image` or `content` properties) to either the final resolved // image data, or an error if the image cache failed to load/decode the image. @@ -391,6 +389,45 @@ impl Layout for LayoutThread { process_node_scroll_area_request(node, self.fragment_tree.borrow().clone()) } + // #[cfg_attr( + // feature = "tracing", + // tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") + // )] + fn query_scroll_offset(&self, node: OpaqueNode) -> LayoutVector2D { + let scroll_id = ExternalScrollId( + combine_id_with_fragment_type(node.id(), FragmentType::FragmentBody), + self.id.into(), + ); + self.cached_scroll_tree().and_then(|tree| { + tree.get_node_by_external_scroll_id(&scroll_id) + .map(|scroll_node| match &scroll_node.info { + SpatialTreeNodeInfo::Scroll(spatial_scroll_node) => spatial_scroll_node.offset, + _ => Default::default(), + }) + }).unwrap_or_default() + } + + /// Step 1-4 of + /// Additionally, we are updating the scroll states to be processed by + fn process_scroll_an_element_position( + &self, + node: OpaqueNode, + scroll_offset: LayoutVector2D, + ) -> LayoutVector2D { + // TODO(stevennovaryo): handle step 1-4 properly here + let scroll_id = ExternalScrollId( + combine_id_with_fragment_type(node.id(), FragmentType::FragmentBody), + self.id.into(), + ); + let scroll_state = ScrollState { + scroll_id, + scroll_offset, + }; + self.update_scroll_node_state(&scroll_state); + + scroll_offset + } + #[cfg_attr( feature = "tracing", tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") @@ -476,10 +513,11 @@ impl Layout for LayoutThread { } fn set_scroll_offsets(&mut self, scroll_states: &[ScrollState]) { - *self.scroll_offsets.borrow_mut() = scroll_states - .iter() - .map(|scroll_state| (scroll_state.scroll_id, scroll_state.scroll_offset)) - .collect(); + if let Some(mut tree) = self.cached_scroll_tree_mut() { + for ScrollState {scroll_id, scroll_offset} in scroll_states { + tree.set_scroll_offsets_for_node_with_external_scroll_id(scroll_id, *scroll_offset); + } + } } } @@ -527,7 +565,6 @@ impl LayoutThread { // Epoch starts at 1 because of the initial display list for epoch 0 that we send to WR epoch: Cell::new(Epoch(1)), compositor_api: config.compositor_api, - scroll_offsets: Default::default(), stylist: Stylist::new(device, QuirksMode::NoQuirks), resolved_images_cache: Default::default(), debug: opts::get().debug.clone(), @@ -933,17 +970,47 @@ impl LayoutThread { .remove_unused_font_resources(keys, instance_keys) } + fn cached_scroll_tree(&self) -> Option> { + let stacking_context_tree = self.stacking_context_tree.borrow(); + if stacking_context_tree.is_none() { + return None; + } + Some(Ref::map(stacking_context_tree, |stacking_context_tree| { + &stacking_context_tree + .as_ref() + .expect("Uninitialized stacking context tree") + .compositor_info + .scroll_tree + })) + } + + fn cached_scroll_tree_mut(&self) -> Option> { + let stacking_context_tree = self.stacking_context_tree.borrow_mut(); + if stacking_context_tree.is_none() { + return None; + } + Some(RefMut::map(stacking_context_tree, |stacking_context_tree| { + &mut stacking_context_tree + .as_mut() + .expect("Uninitialized stacking context tree") + .compositor_info + .scroll_tree + })) + } + fn update_scroll_node_state(&self, state: &ScrollState) { - self.scroll_offsets - .borrow_mut() - .insert(state.scroll_id, state.scroll_offset); - let point = Point2D::new(-state.scroll_offset.x, -state.scroll_offset.y); - self.compositor_api.send_scroll_node( - self.webview_id, - self.id.into(), - LayoutPoint::from_untyped(point), - state.scroll_id, - ); + if let Some(mut tree) = self.cached_scroll_tree_mut() { + tree.set_scroll_offsets_for_node_with_external_scroll_id( + &state.scroll_id, + state.scroll_offset, + ); + self.compositor_api.send_scroll_node( + self.webview_id, + self.id.into(), + state.scroll_offset, + state.scroll_id, + ); + } } /// Returns profiling information which is passed to the time profiler. diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index dcaa025c778..de7335720a5 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -319,10 +319,6 @@ pub(crate) struct Window { error_reporter: CSSErrorReporter, - /// A list of scroll offsets for each scrollable element. - #[no_trace] - scroll_offsets: DomRefCell>>, - /// All the MediaQueryLists we need to update media_query_lists: DOMTracker, @@ -543,16 +539,6 @@ impl Window { Some(&self.error_reporter) } - /// Sets a new list of scroll offsets. - /// - /// This is called when layout gives us new ones and WebRender is in use. - pub(crate) fn set_scroll_offsets( - &self, - offsets: HashMap>, - ) { - *self.scroll_offsets.borrow_mut() = offsets - } - pub(crate) fn current_viewport(&self) -> UntypedRect { self.current_viewport.clone().get() } @@ -2544,13 +2530,10 @@ impl Window { } pub(crate) fn scroll_offset_query(&self, node: &Node) -> Vector2D { - if let Some(scroll_offset) = self.scroll_offsets.borrow().get(&node.to_opaque()) { - return *scroll_offset; - } - Vector2D::new(0.0, 0.0) + self.layout.borrow().query_scroll_offset(node.to_opaque()) } - // https://drafts.csswg.org/cssom-view/#element-scrolling-members + /// pub(crate) fn scroll_node( &self, node: &Node, @@ -2562,23 +2545,20 @@ impl Window { // The scroll offsets are immediatly updated since later calls // to topScroll and others may access the properties before // webrender has a chance to update the offsets. - self.scroll_offsets - .borrow_mut() - .insert(node.to_opaque(), Vector2D::new(x_ as f32, y_ as f32)); + let x = x_.to_f32().unwrap_or(0.0f32); + let y = y_.to_f32().unwrap_or(0.0f32); + self.layout() + .process_scroll_an_element_position(node.to_opaque(), Vector2D::new(x, y)); + let scroll_id = ExternalScrollId( combine_id_with_fragment_type(node.to_opaque().id(), FragmentType::FragmentBody), self.pipeline_id().into(), ); - // Step 12 - self.perform_a_scroll( - x_.to_f32().unwrap_or(0.0f32), - y_.to_f32().unwrap_or(0.0f32), - scroll_id, - behavior, - None, - can_gc, - ); + // Step 6 + // > Perform a scroll of box to position, element as the associated + // > element and behavior as the scroll behavior. + self.perform_a_scroll(x, y, scroll_id, behavior, None, can_gc); } pub(crate) fn resolved_style_query( @@ -3143,7 +3123,6 @@ impl Window { devtools_markers: Default::default(), webdriver_script_chan: Default::default(), error_reporter, - scroll_offsets: Default::default(), media_query_lists: DOMTracker::new(), #[cfg(feature = "bluetooth")] test_runner: Default::default(), diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index e7f60e23772..eacf84ba357 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1925,20 +1925,14 @@ impl ScriptThread { ScriptThreadEventCategory::SetScrollState, Some(pipeline_id), || { + // MYNOTES: FIXME window.layout_mut().set_scroll_offsets(&scroll_states); - let mut scroll_offsets = HashMap::new(); for scroll_state in scroll_states.into_iter() { - let scroll_offset = scroll_state.scroll_offset; if scroll_state.scroll_id.is_root() { - window.update_viewport_for_scroll(-scroll_offset.x, -scroll_offset.y); - } else if let Some(node_id) = - node_id_from_scroll_id(scroll_state.scroll_id.0 as usize) - { - scroll_offsets.insert(OpaqueNode(node_id), -scroll_offset); + window.update_viewport_for_scroll(-scroll_state.scroll_offset.x, -scroll_state.scroll_offset.y); } } - window.set_scroll_offsets(scroll_offsets) }, ) } diff --git a/components/shared/compositing/display_list.rs b/components/shared/compositing/display_list.rs index 4fb0d5e94db..2a1120ee3ea 100644 --- a/components/shared/compositing/display_list.rs +++ b/components/shared/compositing/display_list.rs @@ -4,6 +4,8 @@ //! Defines data structures which are consumed by the Compositor. +use std::collections::HashMap; + use base::id::ScrollTreeNodeId; use embedder_traits::Cursor; use euclid::SideOffsets2D; @@ -130,7 +132,7 @@ pub struct ScrollTreeNode { } impl ScrollTreeNode { - /// Get the WebRender [`SpatialId`] for the given [`ScrollNodeId`]. This will + /// Get the WebRender [`SpatialId`] for the given [`ScrollTreeNodeId`]. This will /// panic if [`ScrollTree::build_display_list`] has not been called yet. pub fn webrender_id(&self) -> SpatialId { self.webrender_id @@ -249,6 +251,10 @@ pub struct ScrollTree { /// of WebRender spatial nodes, used by the compositor to scroll the /// contents of the display list. pub nodes: Vec, + + /// Mapping of [ExternalScrollId] to [ScrollTreeNodeId], this is used for quick + /// access of scrolling nodes operation and querying. + pub external_scroll_id_to_node_id: HashMap, } impl ScrollTree { @@ -258,14 +264,19 @@ impl ScrollTree { parent: Option<&ScrollTreeNodeId>, info: SpatialTreeNodeInfo, ) -> ScrollTreeNodeId { + let new_scroll_id = ScrollTreeNodeId { + index: self.nodes.len(), + }; + if let SpatialTreeNodeInfo::Scroll(spatial_scroll_node) = &info { + self.external_scroll_id_to_node_id.insert(spatial_scroll_node.external_id, new_scroll_id); + } + self.nodes.push(ScrollTreeNode { parent: parent.cloned(), webrender_id: None, info, }); - ScrollTreeNodeId { - index: self.nodes.len() - 1, - } + new_scroll_id } /// Once WebRender display list construction is complete for this [`ScrollTree`], update @@ -286,7 +297,33 @@ impl ScrollTree { &self.nodes[id.index] } - /// Get the WebRender [`SpatialId`] for the given [`ScrollNodeId`]. This will + /// Get a mutable reference to the node with the given external scroll id, returning [None] if an + /// [ExternalScrollId] does not have a respective [ScrollTreeNode] (e.g. a fragment doesn't have a scroll node). + /// Same as [Self::get_node_mut], it will panic if the given id is not registered. + pub fn get_node_by_external_scroll_id_mut( + &mut self, + external_scroll_id: &ExternalScrollId, + ) -> Option<&mut ScrollTreeNode> { + self.external_scroll_id_to_node_id + .get(external_scroll_id) + .cloned() + .map(|scroll_id| self.get_node_mut(&scroll_id)) + } + + /// Get an immutable reference to the node with the given external scroll id, returning [None] if an + /// [ExternalScrollId] does not have a respective [ScrollTreeNode] (e.g. a fragment doesn't have a scroll node). + /// Same as [Self::get_node_mut], it will panic if the given id is not registered. + pub fn get_node_by_external_scroll_id( + &self, + external_scroll_id: &ExternalScrollId, + ) -> Option<&ScrollTreeNode> { + self.external_scroll_id_to_node_id + .get(external_scroll_id) + .cloned() + .map(|scroll_id| self.get_node(&scroll_id)) + } + + /// Get the WebRender [`SpatialId`] for the given [`ScrollTreeNodeId`]. This will /// panic if [`ScrollTree::build_display_list`] has not been called yet. pub fn webrender_id(&self, id: &ScrollTreeNodeId) -> SpatialId { self.get_node(id).webrender_id() @@ -317,13 +354,13 @@ impl ScrollTree { /// with the given id. pub fn set_scroll_offsets_for_node_with_external_scroll_id( &mut self, - external_scroll_id: ExternalScrollId, + external_scroll_id: &ExternalScrollId, offset: LayoutVector2D, ) -> bool { - for node in self.nodes.iter_mut() { + if let Some(node) = self.get_node_by_external_scroll_id_mut(external_scroll_id) { match node.info { SpatialTreeNodeInfo::Scroll(ref mut scroll_info) - if scroll_info.external_id == external_scroll_id => + if &scroll_info.external_id == external_scroll_id => { scroll_info.offset = offset; return true; diff --git a/components/shared/compositing/lib.rs b/components/shared/compositing/lib.rs index 67ff0046885..5ee2c722405 100644 --- a/components/shared/compositing/lib.rs +++ b/components/shared/compositing/lib.rs @@ -34,7 +34,7 @@ use ipc_channel::ipc::{self, IpcSharedMemory}; use profile_traits::mem::{OpaqueSender, ReportsChan}; use serde::{Deserialize, Serialize}; use servo_geometry::{DeviceIndependentIntRect, DeviceIndependentIntSize}; -use webrender_api::units::{DevicePoint, LayoutPoint, TexelRect}; +use webrender_api::units::{DevicePoint, LayoutVector2D, TexelRect}; use webrender_api::{ BuiltDisplayList, BuiltDisplayListDescriptor, ExternalImage, ExternalImageData, ExternalImageHandler, ExternalImageId, ExternalImageSource, ExternalScrollId, @@ -121,7 +121,7 @@ pub enum CompositorMsg { SendScrollNode( WebViewId, WebRenderPipelineId, - LayoutPoint, + LayoutVector2D, ExternalScrollId, ), /// Inform WebRender of a new display list for the given pipeline. @@ -227,13 +227,13 @@ impl CrossProcessCompositorApi { &self, webview_id: WebViewId, pipeline_id: WebRenderPipelineId, - point: LayoutPoint, + offset: LayoutVector2D, scroll_id: ExternalScrollId, ) { if let Err(e) = self.0.send(CompositorMsg::SendScrollNode( webview_id, pipeline_id, - point, + offset, scroll_id, )) { warn!("Error sending scroll node: {}", e); diff --git a/components/shared/script_layout/lib.rs b/components/shared/script_layout/lib.rs index e82ee16e24b..8ddf11b2343 100644 --- a/components/shared/script_layout/lib.rs +++ b/components/shared/script_layout/lib.rs @@ -49,7 +49,7 @@ use style::properties::style_structs::Font; use style::selector_parser::{PseudoElement, RestyleDamage, Snapshot}; use style::stylesheets::Stylesheet; use webrender_api::ImageKey; -use webrender_api::units::DeviceIntSize; +use webrender_api::units::{DeviceIntSize, LayoutVector2D}; pub trait GenericLayoutDataTrait: Any + MallocSizeOfTrait { fn as_any(&self) -> &dyn Any; @@ -247,6 +247,17 @@ pub trait Layout { /// Set the scroll states of this layout after a compositor scroll. fn set_scroll_offsets(&mut self, scroll_states: &[ScrollState]); + /// Query scroll offset for any scroll offset query through the layout. + fn query_scroll_offset(&self, node: OpaqueNode) -> LayoutVector2D; + + /// Process the step 1-4 of scroll an element algorithm. + /// + fn process_scroll_an_element_position( + &self, + node: OpaqueNode, + scroll_offset: LayoutVector2D, + ) -> LayoutVector2D; + fn query_content_box(&self, node: TrustedNodeAddress) -> Option>; fn query_content_boxes(&self, node: TrustedNodeAddress) -> Vec>; fn query_client_rect(&self, node: TrustedNodeAddress) -> Rect;