Auto merge of #26076 - jdm:client-rect-cache, r=asajeffrey

Reduce unnecessary layout queries in babylon.js content

Every frame, Babylon.js compares the width property of the webgl canvas element to its clientWidth. This incurs two layout operations every frame - one to get the dimensions of the element, and at the end of the frame to construct a display list and send that to webrender.

These changes introduce the concept of cached layout values which are constructed from the result of layout queries. These cached values are automatically invalidated when a new layout operation takes place, but as long as only the query operations that stored a cached value are used and the DOM is not otherwise dirtied, the cached values will remain valid and no further layout operations will take place.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix (part of) #26019
- [x] These changes do not require tests because we have no infrastructure to test whether or not reflow occurred.

5 files changed, 125 insertions, 22 deletions

diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index 1e60b3b79fc..d7ba280cdf8 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -588,16 +588,28 @@ impl Document {
         self.needs_paint.get()
     }
 
-    pub fn needs_reflow(&self) -> bool {
+    pub fn needs_reflow(&self) -> Option<ReflowTriggerCondition> {
         // FIXME: This should check the dirty bit on the document,
         // not the document element. Needs some layout changes to make
         // that workable.
-        self.stylesheets.borrow().has_changed() ||
-            self.GetDocumentElement().map_or(false, |root| {
-                root.upcast::<Node>().has_dirty_descendants() ||
-                    !self.pending_restyles.borrow().is_empty() ||
-                    self.needs_paint()
-            })
+        if self.stylesheets.borrow().has_changed() {
+            return Some(ReflowTriggerCondition::StylesheetsChanged);
+        }
+
+        let root = self.GetDocumentElement()?;
+        if root.upcast::<Node>().has_dirty_descendants() {
+            return Some(ReflowTriggerCondition::DirtyDescendants);
+        }
+
+        if !self.pending_restyles.borrow().is_empty() {
+            return Some(ReflowTriggerCondition::PendingRestyles);
+        }
+
+        if self.needs_paint() {
+            return Some(ReflowTriggerCondition::PaintPostponed);
+        }
+
+        None
     }
 
     /// Returns the first `base` element in the DOM that has an `href` attribute.
@@ -1683,7 +1695,7 @@ impl Document {
             // is considered spurious, we need to ensure that the layout
             // and compositor *do* tick the animation.
             self.window
-                .force_reflow(ReflowGoal::Full, ReflowReason::RequestAnimationFrame);
+                .force_reflow(ReflowGoal::Full, ReflowReason::RequestAnimationFrame, None);
         }
 
         // Only send the animation change state message after running any callbacks.
@@ -4954,3 +4966,11 @@ impl PendingScript {
             .map(|result| (DomRoot::from_ref(&*self.element), result))
     }
 }
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum ReflowTriggerCondition {
+    StylesheetsChanged,
+    DirtyDescendants,
+    PendingRestyles,
+    PaintPostponed,
+}
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 7d167bf2574..fa9e0f82cbc 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -85,6 +85,7 @@ use crate::stylesheet_loader::StylesheetOwner;
 use crate::task::TaskOnce;
 use devtools_traits::AttrInfo;
 use dom_struct::dom_struct;
+use euclid::default::Rect;
 use html5ever::serialize;
 use html5ever::serialize::SerializeOpts;
 use html5ever::serialize::TraversalScope;
@@ -2438,22 +2439,22 @@ impl ElementMethods for Element {
 
     // https://drafts.csswg.org/cssom-view/#dom-element-clienttop
     fn ClientTop(&self) -> i32 {
-        self.upcast::<Node>().client_rect().origin.y
+        self.client_rect().origin.y
     }
 
     // https://drafts.csswg.org/cssom-view/#dom-element-clientleft
     fn ClientLeft(&self) -> i32 {
-        self.upcast::<Node>().client_rect().origin.x
+        self.client_rect().origin.x
     }
 
     // https://drafts.csswg.org/cssom-view/#dom-element-clientwidth
     fn ClientWidth(&self) -> i32 {
-        self.upcast::<Node>().client_rect().size.width
+        self.client_rect().size.width
     }
 
     // https://drafts.csswg.org/cssom-view/#dom-element-clientheight
     fn ClientHeight(&self) -> i32 {
-        self.upcast::<Node>().client_rect().size.height
+        self.client_rect().size.height
     }
 
     /// <https://w3c.github.io/DOM-Parsing/#widl-Element-innerHTML>
@@ -3206,6 +3207,20 @@ impl<'a> SelectorsElement for DomRoot<Element> {
 }
 
 impl Element {
+    fn client_rect(&self) -> Rect<i32> {
+        if let Some(rect) = self
+            .rare_data()
+            .as_ref()
+            .and_then(|data| data.client_rect.as_ref())
+            .and_then(|rect| rect.get().ok())
+        {
+            return rect;
+        }
+        let rect = self.upcast::<Node>().client_rect();
+        self.ensure_rare_data().client_rect = Some(window_from_node(self).cache_layout_value(rect));
+        rect
+    }
+
     pub fn as_maybe_activatable(&self) -> Option<&dyn Activatable> {
         let element = match self.upcast::<Node>().type_id() {
             NodeTypeId::Element(ElementTypeId::HTMLElement(
diff --git a/components/script/dom/raredata.rs b/components/script/dom/raredata.rs
index 198b6bfe720..9d4a82d63d6 100644
--- a/components/script/dom/raredata.rs
+++ b/components/script/dom/raredata.rs
@@ -9,6 +9,8 @@ use crate::dom::customelementregistry::{
 use crate::dom::mutationobserver::RegisteredObserver;
 use crate::dom::node::UniqueId;
 use crate::dom::shadowroot::ShadowRoot;
+use crate::dom::window::LayoutValue;
+use euclid::default::Rect;
 use servo_atoms::Atom;
 use std::rc::Rc;
 
@@ -46,4 +48,6 @@ pub struct ElementRareData {
     /// The "name" content attribute; not used as frequently as id, but used
     /// in named getter loops so it's worth looking up quickly when present
     pub name_attribute: Option<Atom>,
+    /// The client rect reported by layout.
+    pub client_rect: Option<LayoutValue<Rect<i32>>>,
 }
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index 688ced4fda7..89a49de8d9c 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -23,14 +23,14 @@ use crate::dom::bindings::reflector::DomObject;
 use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom};
 use crate::dom::bindings::str::{DOMString, USVString};
 use crate::dom::bindings::structuredclone;
-use crate::dom::bindings::trace::RootedTraceableBox;
+use crate::dom::bindings::trace::{JSTraceable, RootedTraceableBox};
 use crate::dom::bindings::utils::{GlobalStaticData, WindowProxyHandler};
 use crate::dom::bindings::weakref::DOMTracker;
 use crate::dom::bluetooth::BluetoothExtraPermissionData;
 use crate::dom::crypto::Crypto;
 use crate::dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSStyleOwner};
 use crate::dom::customelementregistry::CustomElementRegistry;
-use crate::dom::document::{AnimationFrameCallback, Document};
+use crate::dom::document::{AnimationFrameCallback, Document, ReflowTriggerCondition};
 use crate::dom::element::Element;
 use crate::dom::event::{Event, EventStatus};
 use crate::dom::eventtarget::EventTarget;
@@ -55,6 +55,7 @@ use crate::dom::worklet::Worklet;
 use crate::dom::workletglobalscope::WorkletGlobalScopeType;
 use crate::fetch;
 use crate::layout_image::fetch_image_for_layout;
+use crate::malloc_size_of::MallocSizeOf;
 use crate::microtask::MicrotaskQueue;
 use crate::realms::InRealm;
 use crate::script_runtime::{
@@ -334,6 +335,12 @@ pub struct Window {
     event_loop_waker: Option<Box<dyn EventLoopWaker>>,
 
     visible: Cell<bool>,
+
+    /// A shared marker for the validity of any cached layout values. A value of true
+    /// indicates that any such values remain valid; any new layout that invalidates
+    /// those values will cause the marker to be set to false.
+    #[ignore_malloc_size_of = "Rc is hard"]
+    layout_marker: DomRefCell<Rc<Cell<bool>>>,
 }
 
 impl Window {
@@ -1561,7 +1568,12 @@ impl Window {
     ///
     /// Returns true if layout actually happened, false otherwise.
     #[allow(unsafe_code)]
-    pub fn force_reflow(&self, reflow_goal: ReflowGoal, reason: ReflowReason) -> bool {
+    pub fn force_reflow(
+        &self,
+        reflow_goal: ReflowGoal,
+        reason: ReflowReason,
+        condition: Option<ReflowTriggerCondition>,
+    ) -> bool {
         self.Document().ensure_safe_to_run_script_or_layout();
         // Check if we need to unsuppress reflow. Note that this needs to be
         // *before* any early bailouts, or reflow might never be unsuppresed!
@@ -1580,6 +1592,19 @@ impl Window {
             return false;
         }
 
+        if condition != Some(ReflowTriggerCondition::PaintPostponed) {
+            debug!(
+                "Invalidating layout cache due to reflow condition {:?}",
+                condition
+            );
+            // Invalidate any existing cached layout values.
+            self.layout_marker.borrow().set(false);
+            // Create a new layout caching token.
+            *self.layout_marker.borrow_mut() = Rc::new(Cell::new(true));
+        } else {
+            debug!("Not invalidating cached layout values for paint-only reflow.");
+        }
+
         debug!("script: performing reflow for reason {:?}", reason);
 
         let marker = if self.need_emit_timeline_marker(TimelineMarkerType::Reflow) {
@@ -1714,16 +1739,18 @@ impl Window {
         let for_display = reflow_goal == ReflowGoal::Full;
 
         let mut issued_reflow = false;
-        if !for_display || self.Document().needs_reflow() {
-            issued_reflow = self.force_reflow(reflow_goal, reason);
+        let condition = self.Document().needs_reflow();
+        if !for_display || condition.is_some() {
+            issued_reflow = self.force_reflow(reflow_goal, reason, condition);
 
             // We shouldn't need a reflow immediately after a
             // reflow, except if we're waiting for a deferred paint.
-            assert!(
-                !self.Document().needs_reflow() ||
-                    (!for_display && self.Document().needs_paint()) ||
+            assert!({
+                let condition = self.Document().needs_reflow();
+                condition.is_none() ||
+                    (!for_display && condition == Some(ReflowTriggerCondition::PaintPostponed)) ||
                     self.suppress_reflow.get()
-            );
+            });
         } else {
             debug!(
                 "Document doesn't need reflow - skipping it (reason {:?})",
@@ -2348,6 +2375,7 @@ impl Window {
             player_context,
             event_loop_waker,
             visible: Cell::new(true),
+            layout_marker: DomRefCell::new(Rc::new(Cell::new(true))),
         });
 
         unsafe { WindowBinding::Wrap(JSContext::from_ptr(runtime.cx()), win) }
@@ -2356,6 +2384,42 @@ impl Window {
     pub fn pipeline_id(&self) -> PipelineId {
         self.upcast::<GlobalScope>().pipeline_id()
     }
+
+    /// Create a new cached instance of the given value.
+    pub fn cache_layout_value<T>(&self, value: T) -> LayoutValue<T>
+    where
+        T: Copy + JSTraceable + MallocSizeOf,
+    {
+        LayoutValue::new(self.layout_marker.borrow().clone(), value)
+    }
+}
+
+/// An instance of a value associated with a particular snapshot of layout. This stored
+/// value can only be read as long as the associated layout marker that is considered
+/// valid. It will automatically become unavailable when the next layout operation is
+/// performed.
+#[derive(JSTraceable, MallocSizeOf)]
+pub struct LayoutValue<T: JSTraceable + MallocSizeOf> {
+    #[ignore_malloc_size_of = "Rc is hard"]
+    is_valid: Rc<Cell<bool>>,
+    value: T,
+}
+
+impl<T: Copy + JSTraceable + MallocSizeOf> LayoutValue<T> {
+    fn new(marker: Rc<Cell<bool>>, value: T) -> Self {
+        LayoutValue {
+            is_valid: marker,
+            value,
+        }
+    }
+
+    /// Retrieve the stored value if it is still valid.
+    pub fn get(&self) -> Result<T, ()> {
+        if self.is_valid.get() {
+            return Ok(self.value);
+        }
+        Err(())
+    }
 }
 
 fn should_move_clip_rect(clip_rect: UntypedRect<Au>, new_viewport: UntypedRect<f32>) -> bool {
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index 94aac4841da..714c6857d9c 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -3709,7 +3709,7 @@ impl ScriptThread {
             new_size
         );
         window.set_window_size(new_size);
-        window.force_reflow(ReflowGoal::Full, ReflowReason::WindowResize);
+        window.force_reflow(ReflowGoal::Full, ReflowReason::WindowResize, None);
 
         // http://dev.w3.org/csswg/cssom-view/#resizing-viewports
         if size_type == WindowSizeType::Resize {