Browse Source

LibWeb: Make AddClipRect display list item account for scroll offset

Before this change AddClipRect was a "special" because it didn't respect
scroll frame offset and was meant to be recorded using viewport-relative
coordinates. The motivation behind this was to record a "final" clip
rectangle computed by intersecting all clip rectangles used by a clip
frame. The disadvantage of this approach is that it blocks us from
implementing an optimisation to reuse display list if the only change is
in the scroll offset, because any scroll offset change leads to
invalidating all AddClipRect items within a list.

This change aligns AddClipRect with the rest of display list items by
making it account for scroll frame offset. It required discontinuing
the recording of the intersection of all clip rectangles within a clip
frame and instead producing an AddClipRect for each of them.

A nice side effect is the removal of code that shifts clip rectangle by
`enclosing_scroll_offset()` in a bunch of places, because now it happens
automatically in `DisplayList::apply_scroll_offsets()`.
Aliaksandr Kalenik 11 months ago
parent
commit
d0da377767

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

@@ -554,6 +554,7 @@ set(SOURCES
     Painting/CanvasPaintable.cpp
     Painting/Command.cpp
     Painting/CheckBoxPaintable.cpp
+    Painting/ClipFrame.cpp
     Painting/ClippableAndScrollable.cpp
     Painting/DisplayList.cpp
     Painting/DisplayListPlayerSkia.cpp

+ 1 - 10
Userland/Libraries/LibWeb/Painting/BackgroundPainting.cpp

@@ -162,15 +162,6 @@ void paint_background(PaintContext& context, Layout::NodeWithStyleAndBoxModelMet
         clip_shrink.right = context.rounded_device_pixels(border_right.width);
     }
 
-    CSSPixelPoint enclosing_scroll_offset;
-    if (is<PaintableBox>(layout_node.paintable())) {
-        auto const& paintable_box = static_cast<PaintableBox const&>(*layout_node.paintable());
-        enclosing_scroll_offset = paintable_box.enclosing_scroll_frame_offset();
-    } else if (is<InlinePaintable>(layout_node.paintable())) {
-        auto const& inline_paintable = static_cast<InlinePaintable const&>(*layout_node.paintable());
-        enclosing_scroll_offset = inline_paintable.enclosing_scroll_frame_offset();
-    }
-
     // Note: Background layers are ordered front-to-back, so we paint them in reverse
     for (auto& layer : resolved_background.layers.in_reverse()) {
         DisplayListRecorderStateSaver state { display_list_recorder };
@@ -179,7 +170,7 @@ void paint_background(PaintContext& context, Layout::NodeWithStyleAndBoxModelMet
         auto clip_box = get_box(layer.clip, border_box, layout_node);
 
         CSSPixelRect const& css_clip_rect = clip_box.rect;
-        auto clip_rect = context.rounded_device_rect(css_clip_rect.translated(enclosing_scroll_offset));
+        auto clip_rect = context.rounded_device_rect(css_clip_rect);
         display_list_recorder.add_clip_rect(clip_rect.to_type<int>());
         ScopedCornerRadiusClip corner_clip { context, context.rounded_device_rect(css_clip_rect), clip_box.radii };
 

+ 44 - 0
Userland/Libraries/LibWeb/Painting/ClipFrame.cpp

@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibWeb/Painting/ClipFrame.h>
+
+namespace Web::Painting {
+
+void ClipFrame::add_clip_rect(CSSPixelRect rect, BorderRadiiData radii, RefPtr<ScrollFrame const> enclosing_scroll_frame)
+{
+    for (auto& existing_clip : m_clip_rects) {
+        if (rect == existing_clip.rect && enclosing_scroll_frame == existing_clip.enclosing_scroll_frame) {
+            existing_clip.corner_radii.union_max_radii(radii);
+            return;
+        }
+    }
+    m_clip_rects.append({ rect, radii, move(enclosing_scroll_frame) });
+}
+
+void ClipFrame::clear_rects()
+{
+    m_clip_rects.clear_with_capacity();
+}
+
+CSSPixelRect ClipFrame::clip_rect_for_hit_testing() const
+{
+    VERIFY(!m_clip_rects.is_empty());
+    auto rect = m_clip_rects[0].rect;
+    if (m_clip_rects[0].enclosing_scroll_frame) {
+        rect.translate_by(m_clip_rects[0].enclosing_scroll_frame->offset);
+    }
+    for (size_t i = 1; i < m_clip_rects.size(); ++i) {
+        auto clip_rect = m_clip_rects[i].rect;
+        if (m_clip_rects[i].enclosing_scroll_frame) {
+            clip_rect.translate_by(m_clip_rects[i].enclosing_scroll_frame->offset);
+        }
+        rect.intersect(clip_rect);
+    }
+    return rect;
+}
+
+}

+ 10 - 19
Userland/Libraries/LibWeb/Painting/ClipFrame.h

@@ -7,35 +7,26 @@
 #pragma once
 
 #include <LibWeb/Painting/BorderRadiiData.h>
+#include <LibWeb/Painting/ScrollFrame.h>
 #include <LibWeb/PixelUnits.h>
 
 namespace Web::Painting {
 
-struct BorderRadiiClip {
+struct ClipRectWithScrollFrame {
     CSSPixelRect rect;
-    BorderRadiiData radii;
+    BorderRadiiData corner_radii;
+    RefPtr<ScrollFrame const> enclosing_scroll_frame;
 };
 
 struct ClipFrame : public RefCounted<ClipFrame> {
-    Vector<BorderRadiiClip> const& border_radii_clips() const { return m_border_radii_clips; }
-    void add_border_radii_clip(BorderRadiiClip border_radii_clip)
-    {
-        for (auto& existing_clip : m_border_radii_clips) {
-            if (border_radii_clip.rect == existing_clip.rect) {
-                existing_clip.radii.union_max_radii(border_radii_clip.radii);
-                return;
-            }
-        }
-        m_border_radii_clips.append(border_radii_clip);
-    }
-    void clear_border_radii_clips() { m_border_radii_clips.clear(); }
-
-    CSSPixelRect rect() const { return m_rect; }
-    void set_rect(CSSPixelRect rect) { m_rect = rect; }
+    Vector<ClipRectWithScrollFrame> const& clip_rects() const { return m_clip_rects; }
+    void add_clip_rect(CSSPixelRect rect, BorderRadiiData radii, RefPtr<ScrollFrame const> enclosing_scroll_frame);
+    void clear_rects();
+
+    CSSPixelRect clip_rect_for_hit_testing() const;
 
 private:
-    CSSPixelRect m_rect;
-    Vector<BorderRadiiClip> m_border_radii_clips;
+    Vector<ClipRectWithScrollFrame> m_clip_rects;
 };
 
 }

+ 38 - 6
Userland/Libraries/LibWeb/Painting/ClippableAndScrollable.cpp

@@ -5,6 +5,7 @@
  */
 
 #include <LibWeb/Painting/ClippableAndScrollable.h>
+#include <LibWeb/Painting/PaintContext.h>
 
 namespace Web::Painting {
 
@@ -22,18 +23,49 @@ CSSPixelPoint ClippableAndScrollable::enclosing_scroll_frame_offset() const
     return {};
 }
 
-Optional<CSSPixelRect> ClippableAndScrollable::clip_rect() const
+Optional<CSSPixelRect> ClippableAndScrollable::clip_rect_for_hit_testing() const
 {
     if (m_enclosing_clip_frame)
-        return m_enclosing_clip_frame->rect();
+        return m_enclosing_clip_frame->clip_rect_for_hit_testing();
     return {};
 }
 
-Span<BorderRadiiClip const> ClippableAndScrollable::border_radii_clips() const
+void ClippableAndScrollable::apply_clip(PaintContext& context) const
 {
-    if (m_enclosing_clip_frame)
-        return m_enclosing_clip_frame->border_radii_clips();
-    return {};
+    if (!m_enclosing_clip_frame)
+        return;
+    auto const& clip_rects = m_enclosing_clip_frame->clip_rects();
+    if (clip_rects.is_empty())
+        return;
+
+    auto& display_list_recorder = context.display_list_recorder();
+    display_list_recorder.save();
+    auto saved_scroll_frame_id = display_list_recorder.scroll_frame_id();
+    for (auto const& clip_rect : clip_rects) {
+        Optional<i32> clip_scroll_frame_id;
+        if (clip_rect.enclosing_scroll_frame)
+            clip_scroll_frame_id = clip_rect.enclosing_scroll_frame->id;
+        display_list_recorder.set_scroll_frame_id(clip_scroll_frame_id);
+        auto rect = context.rounded_device_rect(clip_rect.rect).to_type<int>();
+        auto corner_radii = clip_rect.corner_radii.as_corners(context);
+        if (corner_radii.has_any_radius()) {
+            display_list_recorder.add_rounded_rect_clip(corner_radii, rect, CornerClip::Outside);
+        } else {
+            display_list_recorder.add_clip_rect(rect);
+        }
+    }
+    display_list_recorder.set_scroll_frame_id(saved_scroll_frame_id);
+}
+
+void ClippableAndScrollable::restore_clip(PaintContext& context) const
+{
+    if (!m_enclosing_clip_frame)
+        return;
+    auto const& clip_rects = m_enclosing_clip_frame->clip_rects();
+    if (clip_rects.is_empty())
+        return;
+
+    context.display_list_recorder().restore();
 }
 
 }

+ 6 - 7
Userland/Libraries/LibWeb/Painting/ClippableAndScrollable.h

@@ -7,14 +7,10 @@
 #pragma once
 
 #include <LibWeb/Painting/ClipFrame.h>
+#include <LibWeb/Painting/ScrollFrame.h>
 
 namespace Web::Painting {
 
-struct ScrollFrame : public RefCounted<ScrollFrame> {
-    i32 id { -1 };
-    CSSPixelPoint offset;
-};
-
 class ClippableAndScrollable {
 public:
     virtual ~ClippableAndScrollable() = default;
@@ -22,10 +18,13 @@ public:
     void set_enclosing_scroll_frame(RefPtr<ScrollFrame> scroll_frame) { m_enclosing_scroll_frame = scroll_frame; }
     void set_enclosing_clip_frame(RefPtr<ClipFrame> clip_frame) { m_enclosing_clip_frame = clip_frame; }
 
+    [[nodiscard]] RefPtr<ScrollFrame const> enclosing_scroll_frame() const { return m_enclosing_scroll_frame; }
     [[nodiscard]] Optional<int> scroll_frame_id() const;
     [[nodiscard]] CSSPixelPoint enclosing_scroll_frame_offset() const;
-    [[nodiscard]] Optional<CSSPixelRect> clip_rect() const;
-    [[nodiscard]] Span<BorderRadiiClip const> border_radii_clips() const;
+    [[nodiscard]] Optional<CSSPixelRect> clip_rect_for_hit_testing() const;
+
+    void apply_clip(PaintContext&) const;
+    void restore_clip(PaintContext&) const;
 
     Gfx::AffineTransform const& combined_css_transform() const { return m_combined_css_transform; }
     void set_combined_css_transform(Gfx::AffineTransform const& transform) { m_combined_css_transform = transform; }

+ 2 - 0
Userland/Libraries/LibWeb/Painting/Command.h

@@ -95,6 +95,8 @@ struct Restore { };
 
 struct AddClipRect {
     Gfx::IntRect rect;
+
+    void translate_by(Gfx::IntPoint const& offset) { rect.translate_by(offset); }
 };
 
 struct StackingContextTransform {

+ 6 - 1
Userland/Libraries/LibWeb/Painting/DisplayListRecorder.h

@@ -103,11 +103,16 @@ public:
     void translate(int dx, int dy);
     void translate(Gfx::IntPoint delta);
 
-    void set_scroll_frame_id(i32 id)
+    void set_scroll_frame_id(Optional<i32> id)
     {
         state().scroll_frame_id = id;
     }
 
+    Optional<i32> scroll_frame_id() const
+    {
+        return state().scroll_frame_id;
+    }
+
     void save();
     void restore();
 

+ 5 - 7
Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp

@@ -32,22 +32,20 @@ Layout::InlineNode const& InlinePaintable::layout_node() const
 
 void InlinePaintable::before_paint(PaintContext& context, PaintPhase) const
 {
+    apply_clip(context);
+
     if (scroll_frame_id().has_value()) {
         context.display_list_recorder().save();
         context.display_list_recorder().set_scroll_frame_id(scroll_frame_id().value());
     }
-    if (clip_rect().has_value()) {
-        context.display_list_recorder().save();
-        context.display_list_recorder().add_clip_rect(context.enclosing_device_rect(*clip_rect()).to_type<int>());
-    }
 }
 
 void InlinePaintable::after_paint(PaintContext& context, PaintPhase) const
 {
-    if (clip_rect().has_value())
-        context.display_list_recorder().restore();
     if (scroll_frame_id().has_value())
         context.display_list_recorder().restore();
+
+    restore_clip(context);
 }
 
 void InlinePaintable::paint(PaintContext& context, PaintPhase phase) const
@@ -189,7 +187,7 @@ void InlinePaintable::for_each_fragment(Callback callback) const
 
 TraversalDecision InlinePaintable::hit_test(CSSPixelPoint position, HitTestType type, Function<TraversalDecision(HitTestResult)> const& callback) const
 {
-    if (clip_rect().has_value() && !clip_rect().value().contains(position))
+    if (clip_rect_for_hit_testing().has_value() && !clip_rect_for_hit_testing().value().contains(position))
         return TraversalDecision::Continue;
 
     auto position_adjusted_by_scroll_offset = position;

+ 1 - 1
Userland/Libraries/LibWeb/Painting/NestedBrowsingContextPaintable.cpp

@@ -40,7 +40,7 @@ void NestedBrowsingContextPaintable::paint(PaintContext& context, PaintPhase pha
 
     if (phase == PaintPhase::Foreground) {
         auto absolute_rect = this->absolute_rect();
-        auto clip_rect = context.rounded_device_rect(absolute_rect.translated(enclosing_scroll_frame_offset()));
+        auto clip_rect = context.rounded_device_rect(absolute_rect);
         ScopedCornerRadiusClip corner_clip { context, clip_rect, normalized_border_radii_data(ShrinkRadiiForBorders::Yes) };
 
         auto const* hosted_document = layout_box().dom_node().content_document_without_origin_check();

+ 5 - 34
Userland/Libraries/LibWeb/Painting/PaintableBox.cpp

@@ -158,19 +158,6 @@ CSSPixelRect PaintableBox::compute_absolute_rect() const
     return rect;
 }
 
-CSSPixelRect PaintableBox::compute_absolute_padding_rect_with_scroll_offset_applied() const
-{
-    auto rect = absolute_rect();
-    rect.translate_by(enclosing_scroll_frame_offset());
-
-    CSSPixelRect padding_rect;
-    padding_rect.set_x(rect.x() - box_model().padding.left);
-    padding_rect.set_width(content_width() + box_model().padding.left + box_model().padding.right);
-    padding_rect.set_y(rect.y() - box_model().padding.top);
-    padding_rect.set_height(content_height() + box_model().padding.top + box_model().padding.bottom);
-    return padding_rect;
-}
-
 CSSPixelRect PaintableBox::absolute_rect() const
 {
     if (!m_absolute_rect.has_value())
@@ -503,19 +490,7 @@ void PaintableBox::apply_clip_overflow_rect(PaintContext& context, PaintPhase ph
     if (!AK::first_is_one_of(phase, PaintPhase::Background, PaintPhase::Border, PaintPhase::TableCollapsedBorder, PaintPhase::Foreground, PaintPhase::Outline))
         return;
 
-    if (clip_rect().has_value()) {
-        auto overflow_clip_rect = clip_rect().value();
-        context.display_list_recorder().save();
-        context.display_list_recorder().add_clip_rect(context.enclosing_device_rect(overflow_clip_rect).to_type<int>());
-        auto const& border_radii_clips = this->border_radii_clips();
-        for (size_t corner_clip_index = 0; corner_clip_index < border_radii_clips.size(); ++corner_clip_index) {
-            auto const& corner_clip = border_radii_clips[corner_clip_index];
-            auto corners = corner_clip.radii.as_corners(context);
-            if (!corners.has_any_radius())
-                continue;
-            context.display_list_recorder().add_rounded_rect_clip(corner_clip.radii.as_corners(context), context.rounded_device_rect(corner_clip.rect).to_type<int>(), CornerClip::Outside);
-        }
-    }
+    apply_clip(context);
 }
 
 void PaintableBox::clear_clip_overflow_rect(PaintContext& context, PaintPhase phase) const
@@ -523,9 +498,7 @@ void PaintableBox::clear_clip_overflow_rect(PaintContext& context, PaintPhase ph
     if (!AK::first_is_one_of(phase, PaintPhase::Background, PaintPhase::Border, PaintPhase::TableCollapsedBorder, PaintPhase::Foreground, PaintPhase::Outline))
         return;
 
-    if (clip_rect().has_value()) {
-        context.display_list_recorder().restore();
-    }
+    restore_clip(context);
 }
 
 void paint_cursor_if_needed(PaintContext& context, TextPaintable const& paintable, PaintableFragment const& fragment)
@@ -694,9 +667,7 @@ void PaintableWithLines::paint(PaintContext& context, PaintPhase phase) const
     if (should_clip_overflow) {
         context.display_list_recorder().save();
         // FIXME: Handle overflow-x and overflow-y being different values.
-        auto clip_box_with_enclosing_scroll_frame_offset = clip_box;
-        clip_box_with_enclosing_scroll_frame_offset.translate_by(enclosing_scroll_frame_offset());
-        context.display_list_recorder().add_clip_rect(context.rounded_device_rect(clip_box_with_enclosing_scroll_frame_offset).to_type<int>());
+        context.display_list_recorder().add_clip_rect(context.rounded_device_rect(clip_box).to_type<int>());
 
         auto border_radii = normalized_border_radii_data(ShrinkRadiiForBorders::Yes);
         CornerRadii corner_radii {
@@ -833,7 +804,7 @@ TraversalDecision PaintableBox::hit_test_scrollbars(CSSPixelPoint position, Func
 
 TraversalDecision PaintableBox::hit_test(CSSPixelPoint position, HitTestType type, Function<TraversalDecision(HitTestResult)> const& callback) const
 {
-    if (clip_rect().has_value() && !clip_rect()->contains(position))
+    if (clip_rect_for_hit_testing().has_value() && !clip_rect_for_hit_testing()->contains(position))
         return TraversalDecision::Continue;
 
     auto position_adjusted_by_scroll_offset = position;
@@ -886,7 +857,7 @@ Optional<HitTestResult> PaintableBox::hit_test(CSSPixelPoint position, HitTestTy
 
 TraversalDecision PaintableWithLines::hit_test(CSSPixelPoint position, HitTestType type, Function<TraversalDecision(HitTestResult)> const& callback) const
 {
-    if (clip_rect().has_value() && !clip_rect()->contains(position))
+    if (clip_rect_for_hit_testing().has_value() && !clip_rect_for_hit_testing()->contains(position))
         return TraversalDecision::Continue;
 
     auto position_adjusted_by_scroll_offset = position;

+ 0 - 2
Userland/Libraries/LibWeb/Painting/PaintableBox.h

@@ -202,8 +202,6 @@ public:
     void set_outline_offset(CSSPixels outline_offset) { m_outline_offset = outline_offset; }
     CSSPixels outline_offset() const { return m_outline_offset; }
 
-    CSSPixelRect compute_absolute_padding_rect_with_scroll_offset_applied() const;
-
     Optional<CSSPixelRect> get_clip_rect() const;
 
     bool is_viewport() const { return layout_box().is_viewport(); }

+ 18 - 0
Userland/Libraries/LibWeb/Painting/ScrollFrame.h

@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) 2024, Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <LibWeb/PixelUnits.h>
+
+namespace Web::Painting {
+
+struct ScrollFrame : public RefCounted<ScrollFrame> {
+    i32 id { -1 };
+    CSSPixelPoint offset;
+};
+
+}

+ 13 - 19
Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp

@@ -165,21 +165,18 @@ void ViewportPaintable::refresh_clip_state()
     for (auto& it : clip_state) {
         auto const& paintable_box = *it.key;
         auto& clip_frame = *it.value;
+
+        clip_frame.clear_rects();
+
         auto overflow_x = paintable_box.computed_values().overflow_x();
         auto overflow_y = paintable_box.computed_values().overflow_y();
-        // Start from CSS clip property if it exists.
-        Optional<CSSPixelRect> clip_rect = paintable_box.get_clip_rect();
+        if (auto clip_rect = paintable_box.get_clip_rect(); clip_rect.has_value()) {
+            clip_frame.add_clip_rect(clip_rect.value(), {}, paintable_box.enclosing_scroll_frame());
+        }
 
-        auto add_border_radii_clip = [&](auto rect, auto border_radii_data) {
-            if (border_radii_data.has_any_radius()) {
-                BorderRadiiClip border_radii_clip { .rect = rect, .radii = border_radii_data };
-                clip_frame.add_border_radii_clip(border_radii_clip);
-            }
-        };
-        clip_frame.clear_border_radii_clips();
         if (overflow_x != CSS::Overflow::Visible && overflow_y != CSS::Overflow::Visible) {
-            auto overflow_clip_rect = paintable_box.compute_absolute_padding_rect_with_scroll_offset_applied();
-            add_border_radii_clip(overflow_clip_rect, paintable_box.normalized_border_radii_data(ShrinkRadiiForBorders::Yes));
+            auto overflow_clip_rect = paintable_box.absolute_padding_box_rect();
+            clip_frame.add_clip_rect(overflow_clip_rect, paintable_box.normalized_border_radii_data(ShrinkRadiiForBorders::Yes), paintable_box.enclosing_scroll_frame());
             for (auto const* block = &paintable_box.layout_box(); !block->is_viewport(); block = block->containing_block()) {
                 if (block->has_css_transform()) {
                     break;
@@ -188,17 +185,14 @@ void ViewportPaintable::refresh_clip_state()
                 auto block_overflow_x = block_paintable_box.computed_values().overflow_x();
                 auto block_overflow_y = block_paintable_box.computed_values().overflow_y();
                 if (block_overflow_x != CSS::Overflow::Visible && block_overflow_y != CSS::Overflow::Visible) {
-                    auto rect = block_paintable_box.compute_absolute_padding_rect_with_scroll_offset_applied();
-                    overflow_clip_rect.intersect(rect);
-                    add_border_radii_clip(rect, block_paintable_box.normalized_border_radii_data(ShrinkRadiiForBorders::Yes));
+                    auto rect = block_paintable_box.absolute_padding_box_rect();
+                    clip_frame.add_clip_rect(rect, block_paintable_box.normalized_border_radii_data(ShrinkRadiiForBorders::Yes), block_paintable_box.enclosing_scroll_frame());
+                }
+                if (auto css_clip_property_rect = block->paintable_box()->get_clip_rect(); css_clip_property_rect.has_value()) {
+                    clip_frame.add_clip_rect(css_clip_property_rect.value(), {}, block_paintable_box.enclosing_scroll_frame());
                 }
-                if (auto css_clip_property_rect = block->paintable_box()->get_clip_rect(); css_clip_property_rect.has_value())
-                    overflow_clip_rect.intersect(css_clip_property_rect.value());
             }
-            clip_rect = overflow_clip_rect;
         }
-
-        clip_frame.set_rect(*clip_rect);
     }
 }