Update new scroll tree offset based on the old one

Signed-off-by: stevennovaryo <steven.novaryo@gmail.com>

components/compositing/compositor.rs

@@ -257,7 +257,7 @@ impl PipelineDetails {
         }
     }
 
-    // MYNOTES: move this to display list
+    // MYNOTES: we also do this in the display list now
     fn install_new_scroll_tree(&mut self, new_scroll_tree: ScrollTree) {
         let old_scroll_offsets: FnvHashMap<ExternalScrollId, LayoutVector2D> = self
             .scroll_tree
@@ -731,7 +731,10 @@ impl IOCompositor {
 
                 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;

components/compositing/webview_renderer.rs

@@ -862,9 +862,10 @@ impl WebViewRenderer {
                 combined_event.scroll_location,
             )
         });
-        if let Some(scroll_result) = scroll_result {
-            self.send_scroll_positions_to_layout_for_pipeline(scroll_result.pipeline_id);
-        }
+        // 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);
+        // }
 
         let pinch_zoom_result = match self
             .set_pinch_zoom_level(self.pinch_zoom_level().get() * combined_magnification)

components/layout/display_list/stacking_context.rs

@@ -118,6 +118,8 @@ pub(crate) struct StackingContextTree {
 impl StackingContextTree {
     /// Create a new [DisplayList] given the dimensions of the layout and the WebRender
     /// pipeline id.
+    // MYNOTES: fix this
+    #[allow(clippy::too_many_arguments)]
     pub fn new(
         fragment_tree: &FragmentTree,
         viewport_size: LayoutSize,
@@ -126,6 +128,7 @@ impl StackingContextTree {
         viewport_scroll_sensitivity: AxesScrollSensitivity,
         first_reflow: bool,
         debug: &DebugOptions,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
     ) -> Self {
         let compositor_info = CompositorDisplayListInfo::new(
             viewport_size,
@@ -179,6 +182,7 @@ impl StackingContextTree {
                 &mut root_stacking_context,
                 StackingContextBuildMode::SkipHoisted,
                 &text_decorations,
+                maybe_old_stacking_context_tree,
             );
         }
         root_stacking_context.sort();
@@ -211,7 +215,7 @@ impl StackingContextTree {
         )
     }
 
-    fn define_scroll_frame(
+    fn define_scroll_frame_with_zero_offset(
         &mut self,
         parent_scroll_node_id: &ScrollTreeNodeId,
         external_id: wr::ExternalScrollId,
@@ -231,6 +235,39 @@ impl StackingContextTree {
         )
     }
 
+    /// Define a new scroll frame, but we are considering the old offset
+    /// from the previous scroll tree.
+    fn define_scroll_frame(
+        &mut self,
+        parent_scroll_node_id: &ScrollTreeNodeId,
+        external_id: wr::ExternalScrollId,
+        content_rect: LayoutRect,
+        clip_rect: LayoutRect,
+        scroll_sensitivity: AxesScrollSensitivity,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
+    ) -> ScrollTreeNodeId {
+        let scroll_id = self.define_scroll_frame_with_zero_offset(
+            parent_scroll_node_id,
+            external_id,
+            content_rect,
+            clip_rect,
+            scroll_sensitivity,
+        );
+        if let Some(old_stacking_context_tree) = maybe_old_stacking_context_tree.as_ref() {
+            if let Some(old_scroll_node) = old_stacking_context_tree
+                .compositor_info
+                .scroll_tree
+                .get_node_by_external_scroll_id(&external_id)
+            {
+                self.compositor_info
+                    .scroll_tree
+                    .get_node_mut(&scroll_id)
+                    .set_offset(old_scroll_node.offset().unwrap());
+            }
+        }
+        scroll_id
+    }
+
     fn define_sticky_frame(
         &mut self,
         parent_scroll_node_id: &ScrollTreeNodeId,
@@ -817,6 +854,7 @@ impl Fragment {
         stacking_context: &mut StackingContext,
         mode: StackingContextBuildMode,
         text_decorations: &Arc<Vec<FragmentTextDecoration>>,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
     ) {
         let containing_block = containing_block_info.get_containing_block_for_fragment(self);
         let fragment_clone = self.clone();
@@ -841,6 +879,7 @@ impl Fragment {
                     containing_block_info,
                     stacking_context,
                     text_decorations,
+                    maybe_old_stacking_context_tree,
                 );
             },
             Fragment::AbsoluteOrFixedPositioned(fragment) => {
@@ -856,6 +895,7 @@ impl Fragment {
                     stacking_context,
                     StackingContextBuildMode::IncludeHoisted,
                     &Default::default(),
+                    maybe_old_stacking_context_tree,
                 );
             },
             Fragment::Positioning(fragment) => {
@@ -866,6 +906,7 @@ impl Fragment {
                     containing_block_info,
                     stacking_context,
                     text_decorations,
+                    maybe_old_stacking_context_tree,
                 );
             },
             Fragment::Text(_) | Fragment::Image(_) | Fragment::IFrame(_) => {
@@ -946,6 +987,7 @@ impl BoxFragment {
         containing_block_info: &ContainingBlockInfo,
         parent_stacking_context: &mut StackingContext,
         text_decorations: &Arc<Vec<FragmentTextDecoration>>,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
     ) {
         self.build_stacking_context_tree_maybe_creating_reference_frame(
             fragment,
@@ -954,6 +996,7 @@ impl BoxFragment {
             containing_block_info,
             parent_stacking_context,
             text_decorations,
+            maybe_old_stacking_context_tree,
         );
     }
 
@@ -965,6 +1008,7 @@ impl BoxFragment {
         containing_block_info: &ContainingBlockInfo,
         parent_stacking_context: &mut StackingContext,
         text_decorations: &Arc<Vec<FragmentTextDecoration>>,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
     ) {
         let reference_frame_data =
             match self.reference_frame_data_if_necessary(&containing_block.rect) {
@@ -977,6 +1021,7 @@ impl BoxFragment {
                         containing_block_info,
                         parent_stacking_context,
                         text_decorations,
+                        maybe_old_stacking_context_tree,
                     );
                 },
             };
@@ -1027,9 +1072,12 @@ impl BoxFragment {
             &new_containing_block_info,
             parent_stacking_context,
             text_decorations,
+            maybe_old_stacking_context_tree,
         );
     }
 
+    // MYNOTES: fix this
+    #[allow(clippy::too_many_arguments)]
     fn build_stacking_context_tree_maybe_creating_stacking_context(
         &self,
         fragment: Fragment,
@@ -1038,6 +1086,7 @@ impl BoxFragment {
         containing_block_info: &ContainingBlockInfo,
         parent_stacking_context: &mut StackingContext,
         text_decorations: &Arc<Vec<FragmentTextDecoration>>,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
     ) {
         let context_type = match self.get_stacking_context_type() {
             Some(context_type) => context_type,
@@ -1049,6 +1098,7 @@ impl BoxFragment {
                     containing_block_info,
                     parent_stacking_context,
                     text_decorations,
+                    maybe_old_stacking_context_tree,
                 );
                 return;
             },
@@ -1103,6 +1153,7 @@ impl BoxFragment {
             containing_block_info,
             &mut child_stacking_context,
             text_decorations,
+            maybe_old_stacking_context_tree,
         );
 
         let mut stolen_children = vec![];
@@ -1121,6 +1172,8 @@ impl BoxFragment {
             .append(&mut stolen_children);
     }
 
+    // MYNOTES: fix this
+    #[allow(clippy::too_many_arguments)]
     fn build_stacking_context_tree_for_children(
         &self,
         fragment: Fragment,
@@ -1129,6 +1182,7 @@ impl BoxFragment {
         containing_block_info: &ContainingBlockInfo,
         stacking_context: &mut StackingContext,
         text_decorations: &Arc<Vec<FragmentTextDecoration>>,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
     ) {
         let mut new_scroll_node_id = containing_block.scroll_node_id;
         let mut new_clip_id = containing_block.clip_id;
@@ -1213,6 +1267,7 @@ impl BoxFragment {
             &new_scroll_node_id,
             new_clip_id,
             &containing_block.rect,
+            maybe_old_stacking_context_tree,
         ) {
             new_clip_id = overflow_frame_data.clip_id;
             if let Some(scroll_frame_data) = overflow_frame_data.scroll_frame_data {
@@ -1313,6 +1368,7 @@ impl BoxFragment {
                 stacking_context,
                 StackingContextBuildMode::SkipHoisted,
                 text_decorations,
+                maybe_old_stacking_context_tree,
             );
         }
 
@@ -1375,6 +1431,7 @@ impl BoxFragment {
         parent_scroll_node_id: &ScrollTreeNodeId,
         parent_clip_id: ClipId,
         containing_block_rect: &PhysicalRect<Au>,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
     ) -> Option<OverflowFrameData> {
         let overflow = self.style.effective_overflow(self.base.flags);
 
@@ -1476,6 +1533,7 @@ impl BoxFragment {
             content_rect,
             scroll_frame_rect,
             sensitivity,
+            maybe_old_stacking_context_tree,
         );
 
         Some(OverflowFrameData {
@@ -1702,6 +1760,7 @@ impl PositioningFragment {
         containing_block_info: &ContainingBlockInfo,
         stacking_context: &mut StackingContext,
         text_decorations: &Arc<Vec<FragmentTextDecoration>>,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
     ) {
         let rect = self
             .rect
@@ -1717,6 +1776,7 @@ impl PositioningFragment {
                 stacking_context,
                 StackingContextBuildMode::SkipHoisted,
                 text_decorations,
+                maybe_old_stacking_context_tree,
             );
         }
     }

components/layout/layout_impl.rs

@@ -389,22 +389,27 @@ 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")
-    // )]
+    #[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,
+        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()
+            .unwrap_or_default()
     }
 
     /// Step 1-4 of <https://drafts.csswg.org/cssom-view/#element-scrolling-members>
@@ -514,7 +519,11 @@ impl Layout for LayoutThread {
 
     fn set_scroll_offsets(&mut self, scroll_states: &[ScrollState]) {
         if let Some(mut tree) = self.cached_scroll_tree_mut() {
-            for ScrollState {scroll_id, scroll_offset} in scroll_states {
+            for ScrollState {
+                scroll_id,
+                scroll_offset,
+            } in scroll_states
+            {
                 tree.set_scroll_offsets_for_node_with_external_scroll_id(scroll_id, *scroll_offset);
             }
         }
@@ -687,7 +696,7 @@ impl LayoutThread {
             highlighted_dom_node: reflow_request.highlighted_dom_node,
         };
 
-        let damage = self.restyle_and_build_trees(
+        let (damage, maybe_old_stacking_context_tree) = self.restyle_and_build_trees(
             &reflow_request,
             root_element,
             rayon_pool,
@@ -695,7 +704,7 @@ impl LayoutThread {
             viewport_changed,
         );
         self.calculate_overflow(damage);
-        self.build_stacking_context_tree(&reflow_request, damage);
+        self.build_stacking_context_tree(&reflow_request, damage, &maybe_old_stacking_context_tree);
         self.build_display_list(&reflow_request, &mut layout_context);
 
         self.first_reflow.set(false);
@@ -777,10 +786,10 @@ impl LayoutThread {
             .flush(guards, Some(root_element), Some(snapshot_map));
     }
 
-    #[cfg_attr(
-        feature = "tracing",
-        tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace")
-    )]
+    // #[cfg_attr(
+    //     feature = "tracing",
+    //     tracing::instrument(skip_all, fields(servo_profiling = true), level = "trace")
+    // )]
     fn restyle_and_build_trees(
         &self,
         reflow_request: &ReflowRequest,
@@ -788,7 +797,7 @@ impl LayoutThread {
         rayon_pool: Option<&ThreadPool>,
         layout_context: &mut LayoutContext<'_>,
         viewport_changed: bool,
-    ) -> RestyleDamage {
+    ) -> (RestyleDamage, Option<Box<StackingContextTree>>) {
         let dirty_root = unsafe {
             ServoLayoutNode::new(&reflow_request.dirty_root.unwrap())
                 .as_element()
@@ -804,7 +813,7 @@ impl LayoutThread {
 
         if !token.should_traverse() {
             layout_context.style_context.stylist.rule_tree().maybe_gc();
-            return RestyleDamage::empty();
+            return (RestyleDamage::empty(), None);
         }
 
         let dirty_root: ServoLayoutNode =
@@ -817,7 +826,7 @@ impl LayoutThread {
             damage = RestyleDamage::REBUILD_BOX;
         } else if !damage.contains(RestyleDamage::REBUILD_BOX) {
             layout_context.style_context.stylist.rule_tree().maybe_gc();
-            return damage;
+            return (damage, None);
         }
 
         let mut box_tree = self.box_tree.borrow_mut();
@@ -853,7 +862,7 @@ impl LayoutThread {
 
         // The FragmentTree has been updated, so any existing StackingContext tree that layout
         // had is now out of date and should be rebuilt.
-        *self.stacking_context_tree.borrow_mut() = None;
+        let old_stacking_context_tree = self.stacking_context_tree.borrow_mut().take();
 
         if self.debug.dump_style_tree {
             println!(
@@ -872,7 +881,7 @@ impl LayoutThread {
 
         // GC the rule tree if some heuristics are met.
         layout_context.style_context.stylist.rule_tree().maybe_gc();
-        damage
+        (damage, old_stacking_context_tree.map(Box::new))
     }
 
     fn calculate_overflow(&self, damage: RestyleDamage) {
@@ -888,7 +897,12 @@ impl LayoutThread {
         }
     }
 
-    fn build_stacking_context_tree(&self, reflow_request: &ReflowRequest, damage: RestyleDamage) {
+    fn build_stacking_context_tree(
+        &self,
+        reflow_request: &ReflowRequest,
+        damage: RestyleDamage,
+        maybe_old_stacking_context_tree: &Option<Box<StackingContextTree>>,
+    ) {
         if !reflow_request.reflow_goal.needs_display_list() &&
             !reflow_request.reflow_goal.needs_display()
         {
@@ -926,6 +940,7 @@ impl LayoutThread {
             fragment_tree.viewport_scroll_sensitivity,
             self.first_reflow.get(),
             &self.debug,
+            maybe_old_stacking_context_tree,
         ));
     }
 
@@ -957,6 +972,7 @@ impl LayoutThread {
             fragment_tree,
             &self.debug,
         );
+
         self.compositor_api.send_display_list(
             self.webview_id,
             &stacking_context_tree.compositor_info,
@@ -989,13 +1005,16 @@ impl LayoutThread {
         if stacking_context_tree.is_none() {
             return None;
         }
-        Some(RefMut::map(stacking_context_tree, |stacking_context_tree| {
+        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) {

components/script/script_thread.rs

@@ -1930,7 +1930,10 @@ impl ScriptThread {
 
                 for scroll_state in scroll_states.into_iter() {
                     if scroll_state.scroll_id.is_root() {
-                        window.update_viewport_for_scroll(-scroll_state.scroll_offset.x, -scroll_state.scroll_offset.y);
+                        window.update_viewport_for_scroll(
+                            -scroll_state.scroll_offset.x,
+                            -scroll_state.scroll_offset.y,
+                        );
                     }
                 }
             },

components/shared/compositing/display_list.rs

@@ -268,7 +268,8 @@ impl ScrollTree {
             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.external_scroll_id_to_node_id
+                .insert(spatial_scroll_node.external_id, new_scroll_id);
         }
 
         self.nodes.push(ScrollTreeNode {
@@ -370,6 +371,9 @@ impl ScrollTree {
         }
         false
     }
+
+    // Update this stacking context
+    pub fn persevere_old_stacking_context_tree_scroll() {}
 }
 
 /// A data structure which stores compositor-side information about