From 3e856cbf11d64dd0dee4ae996bb858771d910e24 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 5 Aug 2025 10:39:42 +0200 Subject: [PATCH] layout: Introduce `ReflowPhasesRun` (#38467) There were various booleans on `ReflowResults` that represented various actions that might have been taken during a reflow request. Replace those with a bitflags that better represents what reflow phases have actually been run. Update variable names to reflect what they mean. In addition, run some post-layout tasks unconditionally. They are already contingent on the results returned from layout. This simplifies and clarifies the code a good deal. Testing: This should not change observable behavior and thus is covered by existing WPT tests. Signed-off-by: Martin Robinson --- components/layout/layout_impl.rs | 75 +++++++++++++++----------- components/script/dom/document.rs | 11 ++-- components/script/dom/window.rs | 84 ++++++++++-------------------- components/script/script_thread.rs | 15 ++++-- components/shared/layout/lib.rs | 28 ++++------ 5 files changed, 100 insertions(+), 113 deletions(-) diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index 15820ad574b..7340127765c 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -27,8 +27,8 @@ use fxhash::FxHashMap; use ipc_channel::ipc::IpcSender; use layout_api::{ IFrameSizes, Layout, LayoutConfig, LayoutDamage, LayoutFactory, NodesFromPointQueryType, - OffsetParentResponse, QueryMsg, ReflowGoal, ReflowRequest, ReflowRequestRestyle, ReflowResult, - TrustedNodeAddress, + OffsetParentResponse, QueryMsg, ReflowGoal, ReflowPhasesRun, ReflowRequest, + ReflowRequestRestyle, ReflowResult, TrustedNodeAddress, }; use log::{debug, error, warn}; use malloc_size_of::{MallocConditionalSizeOf, MallocSizeOf, MallocSizeOfOps}; @@ -672,13 +672,13 @@ impl LayoutThread { self.maybe_print_reflow_event(&reflow_request); if self.can_skip_reflow_request_entirely(&reflow_request) { - // We could skip the layout, but we might need to update the scroll node. - let update_scroll_reflow_target_scrolled = - self.handle_update_scroll_node_request(&reflow_request); - - return Some(ReflowResult::new_without_relayout( - update_scroll_reflow_target_scrolled, - )); + // We can skip layout, but we might need to update a scroll node. + return self + .handle_update_scroll_node_request(&reflow_request) + .then(|| ReflowResult { + reflow_phases_run: ReflowPhasesRun::UpdatedScrollNodeOffset, + ..Default::default() + }); } let document = unsafe { ServoLayoutNode::new(&reflow_request.document) }; @@ -698,30 +698,34 @@ impl LayoutThread { animation_timeline_value: reflow_request.animation_timeline_value, }); - let (damage, iframe_sizes) = self.restyle_and_build_trees( + let (mut reflow_phases_run, damage, iframe_sizes) = self.restyle_and_build_trees( &mut reflow_request, document, root_element, &image_resolver, ); - self.calculate_overflow(damage); - self.build_stacking_context_tree(&reflow_request, damage); - let built_display_list = self.build_display_list(&reflow_request, damage, &image_resolver); - - let update_scroll_reflow_target_scrolled = - self.handle_update_scroll_node_request(&reflow_request); + if self.calculate_overflow(damage) { + reflow_phases_run.insert(ReflowPhasesRun::CalculatedOverflow); + } + if self.build_stacking_context_tree(&reflow_request, damage) { + reflow_phases_run.insert(ReflowPhasesRun::BuiltStackingContextTree); + } + if self.build_display_list(&reflow_request, damage, &image_resolver) { + reflow_phases_run.insert(ReflowPhasesRun::BuiltDisplayList); + } + if self.handle_update_scroll_node_request(&reflow_request) { + reflow_phases_run.insert(ReflowPhasesRun::UpdatedScrollNodeOffset); + } let pending_images = std::mem::take(&mut *image_resolver.pending_images.lock()); let pending_rasterization_images = std::mem::take(&mut *image_resolver.pending_rasterization_images.lock()); Some(ReflowResult { - built_display_list, + reflow_phases_run, pending_images, pending_rasterization_images, iframe_sizes: Some(iframe_sizes), - update_scroll_reflow_target_scrolled, - processed_relayout: true, }) } @@ -793,11 +797,11 @@ impl LayoutThread { document: ServoLayoutDocument<'_>, root_element: ServoLayoutElement<'_>, image_resolver: &Arc, - ) -> (RestyleDamage, IFrameSizes) { + ) -> (ReflowPhasesRun, RestyleDamage, IFrameSizes) { let mut snapshot_map = SnapshotMap::new(); let _snapshot_setter = match reflow_request.restyle.as_mut() { Some(restyle) => SnapshotSetter::new(restyle, &mut snapshot_map), - None => return (RestyleDamage::empty(), IFrameSizes::default()), + None => return Default::default(), }; let document_shared_lock = document.style_shared_lock(); @@ -877,7 +881,7 @@ impl LayoutThread { if !token.should_traverse() { layout_context.style_context.stylist.rule_tree().maybe_gc(); - return (RestyleDamage::empty(), IFrameSizes::default()); + return Default::default(); } dirty_root = driver::traverse_dom(&recalc_style_traversal, token, rayon_pool).as_node(); @@ -895,7 +899,7 @@ impl LayoutThread { if !damage.contains(RestyleDamage::RELAYOUT) { layout_context.style_context.stylist.rule_tree().maybe_gc(); - return (damage, IFrameSizes::default()); + return (ReflowPhasesRun::empty(), damage, IFrameSizes::default()); } let mut box_tree = self.box_tree.borrow_mut(); @@ -956,13 +960,17 @@ impl LayoutThread { layout_context.style_context.stylist.rule_tree().maybe_gc(); let mut iframe_sizes = layout_context.iframe_sizes.lock(); - (damage, std::mem::take(&mut *iframe_sizes)) + ( + ReflowPhasesRun::RanLayout, + damage, + std::mem::take(&mut *iframe_sizes), + ) } #[servo_tracing::instrument(name = "Overflow Calculation", skip_all)] - fn calculate_overflow(&self, damage: RestyleDamage) { + fn calculate_overflow(&self, damage: RestyleDamage) -> bool { if !damage.contains(RestyleDamage::RECALCULATE_OVERFLOW) { - return; + return false; } if let Some(fragment_tree) = &*self.fragment_tree.borrow() { @@ -976,22 +984,27 @@ impl LayoutThread { // display list the next time one is requested. self.need_new_display_list.set(true); self.need_new_stacking_context_tree.set(true); + true } #[servo_tracing::instrument(name = "Stacking Context Tree Construction", skip_all)] - fn build_stacking_context_tree(&self, reflow_request: &ReflowRequest, damage: RestyleDamage) { + fn build_stacking_context_tree( + &self, + reflow_request: &ReflowRequest, + damage: RestyleDamage, + ) -> bool { if !ReflowPhases::necessary(&reflow_request.reflow_goal) .contains(ReflowPhases::StackingContextTreeConstruction) { - return; + return false; } let Some(fragment_tree) = &*self.fragment_tree.borrow() else { - return; + return false; }; if !damage.contains(RestyleDamage::REBUILD_STACKING_CONTEXT) && !self.need_new_stacking_context_tree.get() { - return; + return false; } let mut stacking_context_tree = self.stacking_context_tree.borrow_mut(); @@ -1041,6 +1054,8 @@ impl LayoutThread { ); } } + + true } /// Build the display list for the current layout and send it to the renderer. If no display diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index afb392161e2..c61cd649fd6 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -43,7 +43,8 @@ use ipc_channel::ipc; use js::rust::{HandleObject, HandleValue, MutableHandleValue}; use keyboard_types::{Code, Key, KeyState, Modifiers, NamedKey}; use layout_api::{ - PendingRestyle, ReflowGoal, RestyleReason, TrustedNodeAddress, node_id_from_scroll_id, + PendingRestyle, ReflowGoal, ReflowPhasesRun, RestyleReason, TrustedNodeAddress, + node_id_from_scroll_id, }; use metrics::{InteractiveFlag, InteractiveWindow, ProgressiveWebMetrics}; use net_traits::CookieSource::NonHTTP; @@ -3694,8 +3695,8 @@ impl Document { // > Step 22: For each doc of docs, update the rendering or user interface of // > doc and its node navigable to reflect the current state. // - // Returns true if a reflow occured. - pub(crate) fn update_the_rendering(&self) -> bool { + // Returns the set of reflow phases run as a [`ReflowPhasesRun`]. + pub(crate) fn update_the_rendering(&self) -> ReflowPhasesRun { self.update_animating_images(); // All dirty canvases are flushed before updating the rendering. @@ -3730,9 +3731,7 @@ impl Document { receiver.recv().unwrap(); } - self.window() - .reflow(ReflowGoal::UpdateTheRendering) - .reflow_issued + self.window().reflow(ReflowGoal::UpdateTheRendering) } /// From : diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 0f52f3fd22e..b9af64afcf5 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -52,8 +52,8 @@ use js::rust::{ }; use layout_api::{ FragmentType, Layout, PendingImage, PendingImageState, PendingRasterizationImage, QueryMsg, - ReflowGoal, ReflowRequest, ReflowRequestRestyle, RestyleReason, TrustedNodeAddress, - combine_id_with_fragment_type, + ReflowGoal, ReflowPhasesRun, ReflowRequest, ReflowRequestRestyle, RestyleReason, + TrustedNodeAddress, combine_id_with_fragment_type, }; use malloc_size_of::MallocSizeOf; use media::WindowGLContext; @@ -225,25 +225,6 @@ impl LayoutBlocker { type PendingImageRasterizationKey = (PendingImageId, DeviceIntSize); -/// Feedbacks of the reflow that is required by the one who is initiating the reflow. -pub(crate) struct WindowReflowResult { - /// Whether the reflow actually happened and it sends a new display list to the embedder. - pub reflow_issued: bool, - /// Whether the reflow is for [ReflowGoal::UpdateScrollNode] and the target is scrolled. - /// Specifically, a node is scrolled whenever the scroll position of it changes. Note - /// that reflow that is cancalled would not scroll the target. - pub update_scroll_reflow_target_scrolled: bool, -} - -impl WindowReflowResult { - fn new_empty() -> Self { - WindowReflowResult { - reflow_issued: false, - update_scroll_reflow_target_scrolled: false, - } - } -} - #[dom_struct] pub(crate) struct Window { globalscope: GlobalScope, @@ -2165,16 +2146,14 @@ impl Window { // TODO Step 1 // TODO(mrobinson, #18709): Add smooth scrolling support to WebRender so that we can // properly process ScrollBehavior here. - let WindowReflowResult { - update_scroll_reflow_target_scrolled, - .. - } = self.reflow(ReflowGoal::UpdateScrollNode(scroll_id, Vector2D::new(x, y))); + let reflow_phases_run = + self.reflow(ReflowGoal::UpdateScrollNode(scroll_id, Vector2D::new(x, y))); // > If the scroll position did not change as a result of the user interaction or programmatic // > invocation, where no translations were applied as a result, then no scrollend event fires // > because no scrolling occurred. // Even though the note mention the scrollend, it is relevant to the scroll as well. - if update_scroll_reflow_target_scrolled { + if reflow_phases_run.contains(ReflowPhasesRun::UpdatedScrollNodeOffset) { match element { Some(el) => self.Document().handle_element_scroll_event(el), None => self.Document().handle_viewport_scroll_event(), @@ -2210,25 +2189,25 @@ impl Window { /// /// NOTE: This method should almost never be called directly! Layout and rendering updates should /// happen as part of the HTML event loop via *update the rendering*. - pub(crate) fn reflow(&self, reflow_goal: ReflowGoal) -> WindowReflowResult { + pub(crate) fn reflow(&self, reflow_goal: ReflowGoal) -> ReflowPhasesRun { let document = self.Document(); // Never reflow inactive Documents. if !document.is_fully_active() { - return WindowReflowResult::new_empty(); + return ReflowPhasesRun::empty(); } self.Document().ensure_safe_to_run_script_or_layout(); // If layouts are blocked, we block all layouts that are for display only. Other // layouts (for queries and scrolling) are not blocked, as they do not display - // anything and script excpects the layout to be up-to-date after they run. + // anything and script expects the layout to be up-to-date after they run. let pipeline_id = self.pipeline_id(); if reflow_goal == ReflowGoal::UpdateTheRendering && self.layout_blocker.get().layout_blocked() { debug!("Suppressing pre-load-event reflow pipeline {pipeline_id}"); - return WindowReflowResult::new_empty(); + return ReflowPhasesRun::empty(); } debug!("script: performing reflow for goal {reflow_goal:?}"); @@ -2279,37 +2258,30 @@ impl Window { highlighted_dom_node: document.highlighted_dom_node().map(|node| node.to_opaque()), }; - let Some(results) = self.layout.borrow_mut().reflow(reflow) else { - return WindowReflowResult::new_empty(); + let Some(reflow_result) = self.layout.borrow_mut().reflow(reflow) else { + return ReflowPhasesRun::empty(); }; - // We are maintaining the previous behavior of layout where we are skipping these behavior if we are not - // doing layout calculation. - if results.processed_relayout { - debug!("script: layout complete"); - if let Some(marker) = marker { - self.emit_timeline_marker(marker.end()); - } - - self.handle_pending_images_post_reflow( - results.pending_images, - results.pending_rasterization_images, - ); - if let Some(iframe_sizes) = results.iframe_sizes { - document - .iframes_mut() - .handle_new_iframe_sizes_after_layout(self, iframe_sizes); - } - document.update_animations_post_reflow(); - self.update_constellation_epoch(); - } else { - debug!("script: layout-side reflow finished without relayout"); + debug!("script: layout complete"); + if let Some(marker) = marker { + self.emit_timeline_marker(marker.end()); } - WindowReflowResult { - reflow_issued: results.built_display_list, - update_scroll_reflow_target_scrolled: results.update_scroll_reflow_target_scrolled, + self.handle_pending_images_post_reflow( + reflow_result.pending_images, + reflow_result.pending_rasterization_images, + ); + + if let Some(iframe_sizes) = reflow_result.iframe_sizes { + document + .iframes_mut() + .handle_new_iframe_sizes_after_layout(self, iframe_sizes); } + + document.update_animations_post_reflow(); + self.update_constellation_epoch(); + + reflow_result.reflow_phases_run } pub(crate) fn maybe_send_idle_document_state_to_constellation(&self) { diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index cd115f5d3d6..a5e077347a2 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -68,7 +68,9 @@ use js::jsapi::{ }; use js::jsval::UndefinedValue; use js::rust::ParentRuntime; -use layout_api::{LayoutConfig, LayoutFactory, RestyleReason, ScriptThreadFactory}; +use layout_api::{ + LayoutConfig, LayoutFactory, ReflowPhasesRun, RestyleReason, ScriptThreadFactory, +}; use media::WindowGLContext; use metrics::MAX_TASK_NS; use net_traits::image_cache::{ImageCache, ImageCacheResponseMessage}; @@ -1238,6 +1240,8 @@ impl ScriptThread { /// /// Attempt to update the rendering and then do a microtask checkpoint if rendering was actually /// updated. + /// + /// Returns true if any reflows produced a new display list. pub(crate) fn update_the_rendering(&self, can_gc: CanGc) -> bool { self.last_render_opportunity_time.set(Some(Instant::now())); self.cancel_scheduled_update_the_rendering(); @@ -1275,7 +1279,7 @@ impl ScriptThread { // steps per doc in docs. Currently `