Browse Source

LibWeb: Introduce a new class responsible for scroll state

It's good to have a wrapper for scroll and sticky state instead of
directly mutating vectors with frames.

No behavior change.
Aliaksandr Kalenik 10 months ago
parent
commit
d07643b122

+ 1 - 0
Userland/Libraries/LibWeb/CMakeLists.txt

@@ -606,6 +606,7 @@ set(SOURCES
     Painting/SVGClipPaintable.cpp
     Painting/SVGPaintable.cpp
     Painting/SVGSVGPaintable.cpp
+    Painting/ScrollFrame.cpp
     Painting/ShadowPainting.cpp
     Painting/StackingContext.cpp
     Painting/TableBordersPainting.cpp

+ 5 - 5
Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -5584,12 +5584,12 @@ RefPtr<Painting::DisplayList> Document::record_display_list(PaintConfig config)
     display_list->set_device_pixels_per_css_pixel(page().client().device_pixels_per_css_pixel());
 
     Vector<RefPtr<Painting::ScrollFrame>> scroll_state;
-    scroll_state.resize(viewport_paintable.scroll_state.size() + viewport_paintable.sticky_state.size());
-    for (auto& [_, scrollable_frame] : viewport_paintable.scroll_state) {
-        scroll_state[scrollable_frame->id()] = scrollable_frame;
+    scroll_state.resize(viewport_paintable.scroll_state().scroll_frames().size() + viewport_paintable.scroll_state().sticky_frames().size());
+    for (auto const& scroll_frame : viewport_paintable.scroll_state().scroll_frames()) {
+        scroll_state[scroll_frame->id()] = scroll_frame;
     }
-    for (auto& [_, scrollable_frame] : viewport_paintable.sticky_state) {
-        scroll_state[scrollable_frame->id()] = scrollable_frame;
+    for (auto const& scroll_frame : viewport_paintable.scroll_state().sticky_frames()) {
+        scroll_state[scroll_frame->id()] = scroll_frame;
     }
 
     display_list->set_scroll_state(move(scroll_state));

+ 19 - 0
Userland/Libraries/LibWeb/Painting/ScrollFrame.cpp

@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibWeb/Painting/PaintableBox.h>
+#include <LibWeb/Painting/ScrollFrame.h>
+
+namespace Web::Painting {
+
+ScrollFrame::ScrollFrame(PaintableBox const& paintable_box, size_t id, RefPtr<ScrollFrame const> parent)
+    : m_paintable_box(paintable_box)
+    , m_id(id)
+    , m_parent(move(parent))
+{
+}
+
+}

+ 7 - 7
Userland/Libraries/LibWeb/Painting/ScrollFrame.h

@@ -6,19 +6,18 @@
 
 #pragma once
 
+#include <LibWeb/Forward.h>
 #include <LibWeb/PixelUnits.h>
 
 namespace Web::Painting {
 
 class ScrollFrame : public RefCounted<ScrollFrame> {
 public:
-    ScrollFrame(i32 id, RefPtr<ScrollFrame const> parent)
-        : m_id(id)
-        , m_parent(move(parent))
-    {
-    }
+    ScrollFrame(PaintableBox const& paintable_box, size_t id, RefPtr<ScrollFrame const> parent);
+
+    PaintableBox const& paintable_box() const { return *m_paintable_box; }
 
-    i32 id() const { return m_id; }
+    size_t id() const { return m_id; }
 
     CSSPixelPoint cumulative_offset() const
     {
@@ -39,7 +38,8 @@ public:
     }
 
 private:
-    i32 m_id { -1 };
+    WeakPtr<PaintableBox> m_paintable_box;
+    size_t m_id { 0 };
     RefPtr<ScrollFrame const> m_parent;
     CSSPixelPoint m_own_offset;
 

+ 43 - 0
Userland/Libraries/LibWeb/Painting/ScrollState.h

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <LibWeb/Painting/ScrollFrame.h>
+
+namespace Web::Painting {
+
+class ScrollState {
+public:
+    NonnullRefPtr<ScrollFrame> create_scroll_frame_for(PaintableBox const& paintable, RefPtr<ScrollFrame const> parent)
+    {
+        auto scroll_frame = adopt_ref(*new ScrollFrame(paintable, m_next_id++, parent));
+        m_scroll_frames.append(scroll_frame);
+        return scroll_frame;
+    }
+
+    NonnullRefPtr<ScrollFrame> create_sticky_frame_for(PaintableBox const& paintable, RefPtr<ScrollFrame const> parent)
+    {
+        auto scroll_frame = adopt_ref(*new ScrollFrame(paintable, m_next_id++, parent));
+        m_sticky_frames.append(scroll_frame);
+        return scroll_frame;
+    }
+
+    void clear()
+    {
+        m_scroll_frames.clear();
+    }
+
+    Vector<NonnullRefPtr<ScrollFrame>> const& scroll_frames() const { return m_scroll_frames; }
+    Vector<NonnullRefPtr<ScrollFrame>> const& sticky_frames() const { return m_sticky_frames; }
+
+private:
+    size_t m_next_id { 0 };
+    Vector<NonnullRefPtr<ScrollFrame>> m_scroll_frames;
+    Vector<NonnullRefPtr<ScrollFrame>> m_sticky_frames;
+};
+
+}

+ 7 - 15
Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp

@@ -66,7 +66,6 @@ void ViewportPaintable::paint_all_phases(PaintContext& context)
 
 void ViewportPaintable::assign_scroll_frames()
 {
-    int next_id = 0;
     for_each_in_inclusive_subtree_of_type<PaintableBox>([&](auto& paintable_box) {
         RefPtr<ScrollFrame> sticky_scroll_frame;
         if (paintable_box.is_sticky_position()) {
@@ -75,11 +74,10 @@ void ViewportPaintable::assign_scroll_frames()
             if (nearest_scrollable_ancestor) {
                 parent_scroll_frame = nearest_scrollable_ancestor->nearest_scroll_frame();
             }
-            sticky_scroll_frame = adopt_ref(*new ScrollFrame(next_id++, parent_scroll_frame));
+            sticky_scroll_frame = m_scroll_state.create_sticky_frame_for(paintable_box, parent_scroll_frame);
 
             const_cast<PaintableBox&>(paintable_box).set_enclosing_scroll_frame(sticky_scroll_frame);
             const_cast<PaintableBox&>(paintable_box).set_own_scroll_frame(sticky_scroll_frame);
-            sticky_state.set(paintable_box, sticky_scroll_frame);
         }
 
         if (paintable_box.has_scrollable_overflow() || is<ViewportPaintable>(paintable_box)) {
@@ -89,9 +87,8 @@ void ViewportPaintable::assign_scroll_frames()
             } else {
                 parent_scroll_frame = paintable_box.nearest_scroll_frame();
             }
-            auto scroll_frame = adopt_ref(*new ScrollFrame(next_id++, parent_scroll_frame));
+            auto scroll_frame = m_scroll_state.create_scroll_frame_for(paintable_box, parent_scroll_frame);
             paintable_box.set_own_scroll_frame(scroll_frame);
-            scroll_state.set(paintable_box, move(scroll_frame));
         }
 
         return TraversalDecision::Continue;
@@ -182,9 +179,8 @@ void ViewportPaintable::refresh_scroll_state()
         return;
     m_needs_to_refresh_scroll_state = false;
 
-    for (auto& it : sticky_state) {
-        auto const& sticky_box = *it.key;
-        auto& scroll_frame = *it.value;
+    for (auto const& scroll_frame : m_scroll_state.sticky_frames()) {
+        auto const& sticky_box = scroll_frame->paintable_box();
         auto const& sticky_insets = sticky_box.sticky_insets();
 
         auto const* nearest_scrollable_ancestor = sticky_box.nearest_scrollable_ancestor();
@@ -253,13 +249,11 @@ void ViewportPaintable::refresh_scroll_state()
             }
         }
 
-        scroll_frame.set_own_offset(sticky_offset);
+        scroll_frame->set_own_offset(sticky_offset);
     }
 
-    for (auto& it : scroll_state) {
-        auto const& paintable_box = *it.key;
-        auto& scroll_frame = *it.value;
-        scroll_frame.set_own_offset(-paintable_box.scroll_offset());
+    for (auto const& scroll_frame : m_scroll_state.scroll_frames()) {
+        scroll_frame->set_own_offset(-scroll_frame->paintable_box().scroll_offset());
     }
 }
 
@@ -368,8 +362,6 @@ bool ViewportPaintable::handle_mousewheel(Badge<EventHandler>, CSSPixelPoint, un
 void ViewportPaintable::visit_edges(Visitor& visitor)
 {
     Base::visit_edges(visitor);
-    visitor.visit(scroll_state);
-    visitor.visit(sticky_state);
     visitor.visit(clip_state);
 }
 

+ 4 - 2
Userland/Libraries/LibWeb/Painting/ViewportPaintable.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <LibWeb/Painting/PaintableBox.h>
+#include <LibWeb/Painting/ScrollState.h>
 
 namespace Web::Painting {
 
@@ -21,8 +22,6 @@ public:
     void paint_all_phases(PaintContext&);
     void build_stacking_context_tree_if_needed();
 
-    HashMap<JS::GCPtr<PaintableBox const>, RefPtr<ScrollFrame>> scroll_state;
-    HashMap<JS::GCPtr<PaintableBox const>, RefPtr<ScrollFrame>> sticky_state;
     void assign_scroll_frames();
     void refresh_scroll_state();
 
@@ -39,6 +38,8 @@ public:
 
     void set_needs_to_refresh_scroll_state(bool value) { m_needs_to_refresh_scroll_state = value; }
 
+    ScrollState const& scroll_state() const { return m_scroll_state; }
+
 private:
     void build_stacking_context_tree();
 
@@ -46,6 +47,7 @@ private:
 
     virtual void visit_edges(Visitor&) override;
 
+    ScrollState m_scroll_state;
     bool m_needs_to_refresh_scroll_state { true };
 };