From 3bced9535ec32655057b10590916f89aededd1bd Mon Sep 17 00:00:00 2001 From: stevennovaryo Date: Wed, 4 Jun 2025 22:00:54 +0800 Subject: [PATCH] Optimize pending scroll update Signed-off-by: stevennovaryo --- components/compositing/webview_renderer.rs | 21 +++++++++++--- components/constellation/constellation.rs | 23 +++++++++++++++ components/constellation/tracing.rs | 1 + components/layout/layout_impl.rs | 29 +++++-------------- components/script/dom/window.rs | 5 ++-- components/script/messaging.rs | 1 + components/script/script_thread.rs | 12 ++++++++ components/shared/compositing/display_list.rs | 11 ++----- components/shared/constellation/lib.rs | 4 +++ components/shared/script/lib.rs | 3 ++ components/shared/script_layout/lib.rs | 10 ++----- 11 files changed, 75 insertions(+), 45 deletions(-) diff --git a/components/compositing/webview_renderer.rs b/components/compositing/webview_renderer.rs index e78e448fed3..512ead58336 100644 --- a/components/compositing/webview_renderer.rs +++ b/components/compositing/webview_renderer.rs @@ -220,6 +220,20 @@ impl WebViewRenderer { ); } + pub(crate) fn send_scroll_result_to_layout(&self, scroll_result: ScrollResult) { + let scroll_state = ScrollState { + scroll_id: scroll_result.external_scroll_id, + scroll_offset: scroll_result.offset, + }; + + let _ = self.global.borrow().constellation_sender.send( + EmbedderToConstellationMessage::UpdateScrollState( + scroll_result.pipeline_id, + scroll_state, + ), + ); + } + pub(crate) fn set_frame_tree_on_pipeline_details( &mut self, frame_tree: &SendableFrameTree, @@ -862,10 +876,9 @@ impl WebViewRenderer { combined_event.scroll_location, ) }); - // MYNOTES: change this to single update - // if let Some(scroll_result) = scroll_result { - // self.send_scroll_positions_to_layout_for_pipeline(scroll_result.pipeline_id); - // } + if let Some(scroll_result) = scroll_result { + self.send_scroll_result_to_layout(scroll_result); + } let pinch_zoom_result = match self .set_pinch_zoom_level(self.pinch_zoom_level().get() * combined_magnification) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index c95808b00ec..20e617cf17b 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -1479,6 +1479,9 @@ where EmbedderToConstellationMessage::SetScrollStates(pipeline_id, scroll_states) => { self.handle_set_scroll_states(pipeline_id, scroll_states) }, + EmbedderToConstellationMessage::UpdateScrollState(pipeline_id, scroll_state) => { + self.handle_update_scroll_state(pipeline_id, scroll_state) + }, EmbedderToConstellationMessage::PaintMetric(pipeline_id, paint_metric_event) => { self.handle_paint_metric(pipeline_id, paint_metric_event); }, @@ -6040,6 +6043,26 @@ where } } + #[cfg_attr( + feature = "tracing", + tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") + )] + fn handle_update_scroll_state(&self, pipeline_id: PipelineId, scroll_state: ScrollState) { + let Some(pipeline) = self.pipelines.get(&pipeline_id) else { + warn!("Discarding scroll offset update for unknown pipeline"); + return; + }; + if let Err(error) = pipeline + .event_loop + .send(ScriptThreadMessage::UpdateScrollState( + pipeline_id, + scroll_state, + )) + { + warn!("Could not send scroll offsets to pipeline: {pipeline_id:?}: {error:?}"); + } + } + #[cfg_attr( feature = "tracing", tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace") diff --git a/components/constellation/tracing.rs b/components/constellation/tracing.rs index 99551bd58fc..2aa7af996c4 100644 --- a/components/constellation/tracing.rs +++ b/components/constellation/tracing.rs @@ -76,6 +76,7 @@ mod from_compositor { Self::MediaSessionAction(_) => target!("MediaSessionAction"), Self::SetWebViewThrottled(_, _) => target!("SetWebViewThrottled"), Self::SetScrollStates(..) => target!("SetScrollStates"), + Self::UpdateScrollState(..) => target!("UpdateScrollState"), Self::PaintMetric(..) => target!("PaintMetric"), Self::EvaluateJavaScript(..) => target!("EvaluateJavaScript"), Self::CreateMemoryReport(..) => target!("CreateMemoryReport"), diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index 5d22ae93e03..c842732f60d 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -412,27 +412,6 @@ impl Layout for LayoutThread { .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") @@ -528,6 +507,14 @@ impl Layout for LayoutThread { } } } + + // + fn update_scroll_offset(&self, scroll_id: ExternalScrollId, scroll_offset: LayoutVector2D) { + if let Some(mut scroll_tree) = self.cached_scroll_tree_mut() { + scroll_tree + .set_scroll_offsets_for_node_with_external_scroll_id(&scroll_id, scroll_offset); + } + } } impl LayoutThread { diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index de7335720a5..32c6607554c 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -2547,13 +2547,12 @@ impl Window { // webrender has a chance to update the offsets. 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(), ); + self.layout() + .update_scroll_offset(scroll_id, Vector2D::new(x, y)); // Step 6 // > Perform a scroll of box to position, element as the associated diff --git a/components/script/messaging.rs b/components/script/messaging.rs index 41465796505..c9141249b33 100644 --- a/components/script/messaging.rs +++ b/components/script/messaging.rs @@ -91,6 +91,7 @@ impl MixedMessage { #[cfg(feature = "webgpu")] ScriptThreadMessage::SetWebGPUPort(..) => None, ScriptThreadMessage::SetScrollStates(id, ..) => Some(*id), + ScriptThreadMessage::UpdateScrollState(id, ..) => Some(*id), ScriptThreadMessage::EvaluateJavaScript(id, _, _) => Some(*id), }, MixedMessage::FromScript(inner_msg) => match inner_msg { diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 1de03799af7..aa2bfae45cd 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1909,6 +1909,9 @@ impl ScriptThread { ScriptThreadMessage::SetScrollStates(pipeline_id, scroll_states) => { self.handle_set_scroll_states(pipeline_id, scroll_states) }, + ScriptThreadMessage::UpdateScrollState(pipeline_id, scroll_state) => { + self.handle_update_scroll_state(pipeline_id, scroll_state) + }, ScriptThreadMessage::EvaluateJavaScript(pipeline_id, evaluation_id, script) => { self.handle_evaluate_javascript(pipeline_id, evaluation_id, script, can_gc); }, @@ -1940,6 +1943,15 @@ impl ScriptThread { ) } + fn handle_update_scroll_state(&self, pipeline_id: PipelineId, scroll_state: ScrollState) { + let Some(window) = self.documents.borrow().find_window(pipeline_id) else { + warn!("Received scroll states for closed pipeline {pipeline_id}"); + return; + }; + + window.layout_mut().update_scroll_offset(scroll_state.scroll_id, scroll_state.scroll_offset); + } + #[cfg(feature = "webgpu")] fn handle_msg_from_webgpu_server(&self, msg: WebGPUMsg, can_gc: CanGc) { match msg { diff --git a/components/shared/compositing/display_list.rs b/components/shared/compositing/display_list.rs index c4b9e686c45..d684a058531 100644 --- a/components/shared/compositing/display_list.rs +++ b/components/shared/compositing/display_list.rs @@ -157,6 +157,7 @@ impl ScrollTreeNode { /// Set the offset for this node, returns false if this was a /// non-scrolling node for which you cannot set the offset. + // MYNOTES: should i make this &mut so it will modify the new_offset pub fn set_offset(&mut self, new_offset: LayoutVector2D) -> bool { match self.info { SpatialTreeNodeInfo::Scroll(ref mut info) => { @@ -359,15 +360,7 @@ impl ScrollTree { offset: LayoutVector2D, ) -> bool { 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 => - { - scroll_info.offset = offset; - return true; - }, - _ => {}, - } + node.set_offset(offset); } false } diff --git a/components/shared/constellation/lib.rs b/components/shared/constellation/lib.rs index 53e924c5ad5..bc23a6f5ba1 100644 --- a/components/shared/constellation/lib.rs +++ b/components/shared/constellation/lib.rs @@ -90,7 +90,11 @@ pub enum EmbedderToConstellationMessage { SetWebViewThrottled(WebViewId, bool), /// The Servo renderer scrolled and is updating the scroll states of the nodes in the /// given pipeline via the constellation. + // MYNOTES: Do we still need this? SetScrollStates(PipelineId, Vec), + /// The Servo renderer scrolled and is updating the scroll states of the nodes in the + /// given pipeline via the constellation. + UpdateScrollState(PipelineId, ScrollState), /// Notify the constellation that a particular paint metric event has happened for the given pipeline. PaintMetric(PipelineId, PaintMetricEvent), /// Evaluate a JavaScript string in the context of a `WebView`. When execution is complete or an diff --git a/components/shared/script/lib.rs b/components/shared/script/lib.rs index a8188d16ccd..287642946ad 100644 --- a/components/shared/script/lib.rs +++ b/components/shared/script/lib.rs @@ -247,6 +247,9 @@ pub enum ScriptThreadMessage { /// The compositor scrolled and is updating the scroll states of the nodes in the given /// pipeline via the Constellation. SetScrollStates(PipelineId, Vec), + /// The compositor scrolled and is updating the scroll state of a node in the given + /// pipeline via the Constellation. + UpdateScrollState(PipelineId, ScrollState), /// Evaluate the given JavaScript and return a result via a corresponding message /// to the Constellation. EvaluateJavaScript(PipelineId, JavaScriptEvaluationId, String), diff --git a/components/shared/script_layout/lib.rs b/components/shared/script_layout/lib.rs index 8ddf11b2343..a3c6735cea9 100644 --- a/components/shared/script_layout/lib.rs +++ b/components/shared/script_layout/lib.rs @@ -48,7 +48,7 @@ use style::properties::PropertyId; use style::properties::style_structs::Font; use style::selector_parser::{PseudoElement, RestyleDamage, Snapshot}; use style::stylesheets::Stylesheet; -use webrender_api::ImageKey; +use webrender_api::{ExternalScrollId, ImageKey}; use webrender_api::units::{DeviceIntSize, LayoutVector2D}; pub trait GenericLayoutDataTrait: Any + MallocSizeOfTrait { @@ -250,13 +250,7 @@ pub trait Layout { /// 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 update_scroll_offset(&self, scroll_id: ExternalScrollId, scroll_offset: LayoutVector2D); fn query_content_box(&self, node: TrustedNodeAddress) -> Option>; fn query_content_boxes(&self, node: TrustedNodeAddress) -> Vec>;