Fix bug

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

components/compositing/compositor.rs

@@ -257,7 +257,7 @@ impl PipelineDetails {
         }
     }
 
-    // MYNOTES: we also do this in the display list now
+    /// Install a new scroll tree, but maintaining the scroll offsets it has from the old ones.
     fn install_new_scroll_tree(&mut self, new_scroll_tree: ScrollTree) {
         let old_scroll_offsets: FnvHashMap<ExternalScrollId, LayoutVector2D> = self
             .scroll_tree
@@ -744,7 +744,7 @@ impl IOCompositor {
                 txn.set_scroll_offsets(
                     external_scroll_id,
                     vec![SampledScrollOffset {
-                        offset,
+                        offset: -offset,
                         generation: 0,
                     }],
                 );

components/compositing/webview_renderer.rs

@@ -183,7 +183,6 @@ 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));
@@ -876,6 +875,8 @@ impl WebViewRenderer {
                 combined_event.scroll_location,
             )
         });
+        // If something is scrolled, we are sending the scroll information to layout in
+        // order to sync the scroll offsets for queries and scripts.
         if let Some(scroll_result) = scroll_result {
             self.send_scroll_result_to_layout(scroll_result);
         }
@@ -894,7 +895,6 @@ 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,

components/constellation/constellation.rs

@@ -6022,14 +6022,13 @@ 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")
     )]
     fn handle_set_scroll_states(&self, pipeline_id: PipelineId, scroll_states: Vec<ScrollState>) {
         let Some(pipeline) = self.pipelines.get(&pipeline_id) else {
-            warn!("Discarding scroll offset update for unknown pipeline");
+            warn!("Discarding scroll offsets update for unknown pipeline");
             return;
         };
         if let Err(error) = pipeline
@@ -6049,7 +6048,7 @@ where
     )]
     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");
+            warn!("Discarding a scroll result update for unknown pipeline");
             return;
         };
         if let Err(error) = pipeline
@@ -6059,7 +6058,7 @@ where
                 scroll_state,
             ))
         {
-            warn!("Could not send scroll offsets to pipeline: {pipeline_id:?}: {error:?}");
+            warn!("Could not send scroll result update to pipeline: {pipeline_id:?}: {error:?}");
         }
     }
 

components/layout/display_list/stacking_context.rs

@@ -118,7 +118,7 @@ pub(crate) struct StackingContextTree {
 impl StackingContextTree {
     /// Create a new [DisplayList] given the dimensions of the layout and the WebRender
     /// pipeline id.
-    // MYNOTES: fix this
+    // FIXME(stevennovaryo): refactor the stacking context parameters
     #[allow(clippy::too_many_arguments)]
     pub fn new(
         fragment_tree: &FragmentTree,
@@ -979,6 +979,8 @@ impl BoxFragment {
         StackingContextSection::DescendantBackgroundsAndBorders
     }
 
+    // FIXME(stevennovaryo): refactor the stacking context parameters
+    #[allow(clippy::too_many_arguments)]
     fn build_stacking_context_tree(
         &self,
         fragment: Fragment,
@@ -1000,6 +1002,8 @@ impl BoxFragment {
         );
     }
 
+    // FIXME(stevennovaryo): refactor the stacking context parameters
+    #[allow(clippy::too_many_arguments)]
     fn build_stacking_context_tree_maybe_creating_reference_frame(
         &self,
         fragment: Fragment,
@@ -1076,7 +1080,7 @@ impl BoxFragment {
         );
     }
 
-    // MYNOTES: fix this
+    // FIXME(stevennovaryo): refactor the stacking context parameters
     #[allow(clippy::too_many_arguments)]
     fn build_stacking_context_tree_maybe_creating_stacking_context(
         &self,
@@ -1172,7 +1176,7 @@ impl BoxFragment {
             .append(&mut stolen_children);
     }
 
-    // MYNOTES: fix this
+    // FIXME(stevennovaryo): refactor the stacking context parameters
     #[allow(clippy::too_many_arguments)]
     fn build_stacking_context_tree_for_children(
         &self,

components/layout/layout_impl.rs

@@ -496,6 +496,9 @@ impl Layout for LayoutThread {
     ) {
     }
 
+    /// Sets the scroll offsets of the cached [`ScrollTree`] based on the scroll states.
+    ///
+    /// This is done whenever the embedder ask layout to do so.
     fn set_scroll_offsets(&mut self, scroll_states: &[ScrollState]) {
         if let Some(mut tree) = self.cached_scroll_tree_mut() {
             for ScrollState {
@@ -508,7 +511,9 @@ impl Layout for LayoutThread {
         }
     }
 
-    //
+    /// Update a node's scroll offsets in the cached [`ScrollTree`].
+    ///
+    /// This is done whenever the embedder ask layout to do so.
     fn update_scroll_offset(&self, scroll_id: ExternalScrollId, scroll_offset: LayoutVector2D) {
         if let Some(mut scroll_tree) = self.cached_scroll_tree_mut() {
             scroll_tree
@@ -773,10 +778,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,
@@ -848,7 +853,8 @@ impl LayoutThread {
         *self.fragment_tree.borrow_mut() = Some(fragment_tree);
 
         // The FragmentTree has been updated, so any existing StackingContext tree that layout
-        // had is now out of date and should be rebuilt.
+        // had is now out of date and should be rebuilt. We could still uses some information
+        // from the previous StackingContext tree though.
         let old_stacking_context_tree = self.stacking_context_tree.borrow_mut().take();
 
         if self.debug.dump_style_tree {

components/script/dom/window.rs

@@ -77,7 +77,6 @@ use servo_arc::Arc as ServoArc;
 use servo_config::{opts, pref};
 use servo_geometry::{DeviceIndependentIntRect, MaxRect, f32_rect_to_au_rect};
 use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl};
-use style::dom::OpaqueNode;
 use style::error_reporting::{ContextualParseError, ParseErrorReporter};
 use style::properties::PropertyId;
 use style::properties::style_structs::Font;
@@ -2552,7 +2551,7 @@ impl Window {
             self.pipeline_id().into(),
         );
         self.layout()
-            .update_scroll_offset(scroll_id, Vector2D::new(x, y));
+            .update_scroll_offset(scroll_id, Vector2D::new(-x, -y));
 
         // Step 6
         // > Perform a scroll of box to position, element as the associated

components/script/script_thread.rs

@@ -83,16 +83,13 @@ use percent_encoding::percent_decode;
 use profile_traits::mem::{ProcessReports, ReportsChan, perform_memory_report};
 use profile_traits::time::ProfilerCategory;
 use profile_traits::time_profile;
-use script_layout_interface::{
-    LayoutConfig, LayoutFactory, ReflowGoal, ScriptThreadFactory, node_id_from_scroll_id,
-};
+use script_layout_interface::{LayoutConfig, LayoutFactory, ReflowGoal, ScriptThreadFactory};
 use script_traits::{
     ConstellationInputEvent, DiscardBrowsingContext, DocumentActivity, InitialScriptState,
     NewLayoutInfo, Painter, ProgressiveWebMetricType, ScriptThreadMessage, UpdatePipelineIdReason,
 };
 use servo_config::opts;
 use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl};
-use style::dom::OpaqueNode;
 use style::thread_state::{self, ThreadState};
 use stylo_atoms::Atom;
 use timers::{TimerEventRequest, TimerScheduler};
@@ -1928,7 +1925,6 @@ impl ScriptThread {
             ScriptThreadEventCategory::SetScrollState,
             Some(pipeline_id),
             || {
-                // MYNOTES: FIXME
                 window.layout_mut().set_scroll_offsets(&scroll_states);
 
                 for scroll_state in scroll_states.into_iter() {
@@ -1949,7 +1945,15 @@ impl ScriptThread {
             return;
         };
 
-        window.layout_mut().update_scroll_offset(scroll_state.scroll_id, scroll_state.scroll_offset);
+        if scroll_state.scroll_id.is_root() {
+            window.update_viewport_for_scroll(
+                -scroll_state.scroll_offset.x,
+                -scroll_state.scroll_offset.y,
+            );
+        }
+        window
+            .layout_mut()
+            .update_scroll_offset(scroll_state.scroll_id, scroll_state.scroll_offset);
     }
 
     #[cfg(feature = "webgpu")]

components/shared/compositing/display_list.rs

@@ -157,7 +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
+    // TODO(stevennovaryo): We should consider writing mode as well.
     pub fn set_offset(&mut self, new_offset: LayoutVector2D) -> bool {
         match self.info {
             SpatialTreeNodeInfo::Scroll(ref mut info) => {
@@ -361,12 +361,11 @@ impl ScrollTree {
     ) -> bool {
         if let Some(node) = self.get_node_by_external_scroll_id_mut(external_scroll_id) {
             node.set_offset(offset);
+            true
+        } else {
+            false
         }
-        false
     }
-
-    // Update this stacking context
-    pub fn persevere_old_stacking_context_tree_scroll() {}
 }
 
 /// A data structure which stores compositor-side information about

components/shared/constellation/lib.rs

@@ -90,7 +90,6 @@ 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<ScrollState>),
     /// The Servo renderer scrolled and is updating the scroll states of the nodes in the
     /// given pipeline via the constellation.

components/shared/script_layout/lib.rs

@@ -48,8 +48,8 @@ use style::properties::PropertyId;
 use style::properties::style_structs::Font;
 use style::selector_parser::{PseudoElement, RestyleDamage, Snapshot};
 use style::stylesheets::Stylesheet;
-use webrender_api::{ExternalScrollId, ImageKey};
 use webrender_api::units::{DeviceIntSize, LayoutVector2D};
+use webrender_api::{ExternalScrollId, ImageKey};
 
 pub trait GenericLayoutDataTrait: Any + MallocSizeOfTrait {
     fn as_any(&self) -> &dyn Any;