Browse Source

LibWeb: Delete BlitCornerClipping display list command

Contrary to LibGfx, where corner clipping was implemented by sampling
and blitting pixels under corners into a temporary bitmap, Skia allows
us to simply apply a mask. As a result, we no longer need the
BlitCornerClipping command, which has become a no-op.

- SampleUnderCorners is renamed to AddRoundedRectClip
- The optimization that skipped unnecessary blit and sample commands has
  been removed. However, this should not result in a performance
  regression because Skia seems to perform mask rasterization lazily.
Aliaksandr Kalenik 1 year ago
parent
commit
2c0f03f5b6

+ 0 - 1
Userland/Libraries/LibWeb/HTML/Navigable.cpp

@@ -2143,7 +2143,6 @@ void Navigable::record_display_list(Painting::DisplayListRecorder& display_list_
             scroll_offsets_by_frame_id[scrollable_frame->id] = scroll_offset;
         }
         display_list_recorder.display_list().apply_scroll_offsets(scroll_offsets_by_frame_id);
-        display_list_recorder.display_list().mark_unnecessary_commands();
     }
 
     m_needs_repaint = false;

+ 3 - 4
Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.cpp

@@ -11,8 +11,6 @@ namespace Web::Painting {
 
 ScopedCornerRadiusClip::ScopedCornerRadiusClip(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii, CornerClip corner_clip)
     : m_context(context)
-    , m_id(context.allocate_corner_clipper_id())
-    , m_border_rect(border_rect)
 {
     CornerRadii const corner_radii {
         .top_left = border_radii.top_left.as_corner(context),
@@ -23,14 +21,15 @@ ScopedCornerRadiusClip::ScopedCornerRadiusClip(PaintContext& context, DevicePixe
     m_has_radius = corner_radii.has_any_radius();
     if (!m_has_radius)
         return;
-    m_context.display_list_recorder().sample_under_corners(m_id, corner_radii, border_rect.to_type<int>(), corner_clip);
+    m_context.display_list_recorder().save();
+    m_context.display_list_recorder().add_rounded_rect_clip(corner_radii, border_rect.to_type<int>(), corner_clip);
 }
 
 ScopedCornerRadiusClip::~ScopedCornerRadiusClip()
 {
     if (!m_has_radius)
         return;
-    m_context.display_list_recorder().blit_corner_clipping(m_id);
+    m_context.display_list_recorder().restore();
 }
 
 }

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

@@ -25,9 +25,7 @@ struct ScopedCornerRadiusClip {
 
 private:
     PaintContext& m_context;
-    u32 m_id;
     bool m_has_radius { false };
-    Gfx::IntRect m_border_rect;
 };
 
 }

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

@@ -345,8 +345,7 @@ struct DrawTriangleWave {
     }
 };
 
-struct SampleUnderCorners {
-    u32 id;
+struct AddRoundedRectClip {
     CornerRadii corner_radii;
     Gfx::IntRect border_rect;
     CornerClip corner_clip;
@@ -356,15 +355,6 @@ struct SampleUnderCorners {
     void translate_by(Gfx::IntPoint const& offset) { border_rect.translate_by(offset); }
 };
 
-struct BlitCornerClipping {
-    u32 id;
-    Gfx::IntRect border_rect;
-
-    [[nodiscard]] Gfx::IntRect bounding_rect() const { return border_rect; }
-
-    void translate_by(Gfx::IntPoint const& offset) { border_rect.translate_by(offset); }
-};
-
 using Command = Variant<
     DrawGlyphRun,
     FillRect,
@@ -393,7 +383,6 @@ using Command = Variant<
     ApplyBackdropFilter,
     DrawRect,
     DrawTriangleWave,
-    SampleUnderCorners,
-    BlitCornerClipping>;
+    AddRoundedRectClip>;
 
 }

+ 2 - 62
Userland/Libraries/LibWeb/Painting/DisplayList.cpp

@@ -39,77 +39,18 @@ void DisplayList::apply_scroll_offsets(Vector<Gfx::IntPoint> const& offsets_by_f
     }
 }
 
-void DisplayList::mark_unnecessary_commands()
-{
-    // The pair sample_under_corners and blit_corner_clipping commands is not needed if there are no painting commands
-    // in between them that produce visible output.
-    struct SampleCornersBlitCornersRange {
-        u32 sample_command_index;
-        bool has_painting_commands_in_between { false };
-    };
-    // Stack of sample_under_corners commands that have not been matched with a blit_corner_clipping command yet.
-    Vector<SampleCornersBlitCornersRange> sample_blit_ranges;
-    for (u32 command_index = 0; command_index < m_commands.size(); ++command_index) {
-        auto const& command = m_commands[command_index].command;
-        if (command.has<SampleUnderCorners>()) {
-            sample_blit_ranges.append({
-                .sample_command_index = command_index,
-                .has_painting_commands_in_between = false,
-            });
-        } else if (command.has<BlitCornerClipping>()) {
-            auto range = sample_blit_ranges.take_last();
-            if (!range.has_painting_commands_in_between) {
-                m_commands[range.sample_command_index].skip = true;
-                m_commands[command_index].skip = true;
-            }
-        } else {
-            // Save, Restore and AddClipRect commands do not produce visible output
-            auto update_clip_command = command.has<Save>() || command.has<Restore>() || command.has<AddClipRect>();
-            if (sample_blit_ranges.size() > 0 && !update_clip_command) {
-                // If painting command is found for sample_under_corners command on top of the stack, then all
-                // sample_under_corners commands below should also not be skipped.
-                for (auto& sample_blit_range : sample_blit_ranges)
-                    sample_blit_range.has_painting_commands_in_between = true;
-            }
-        }
-    }
-    VERIFY(sample_blit_ranges.is_empty());
-}
-
 void DisplayListPlayer::execute(DisplayList& display_list)
 {
     auto const& commands = display_list.commands();
 
-    HashTable<u32> skipped_sample_corner_commands;
     size_t next_command_index = 0;
     while (next_command_index < commands.size()) {
-        if (commands[next_command_index].skip) {
-            next_command_index++;
-            continue;
-        }
-
-        auto& command = commands[next_command_index++].command;
+        auto const& command = commands[next_command_index++].command;
         auto bounding_rect = command_bounding_rectangle(command);
         if (bounding_rect.has_value() && (bounding_rect->is_empty() || would_be_fully_clipped_by_painter(*bounding_rect))) {
-            if (command.has<SampleUnderCorners>()) {
-                auto const& sample_under_corners = command.get<SampleUnderCorners>();
-                skipped_sample_corner_commands.set(sample_under_corners.id);
-            }
             continue;
         }
 
-        if (command.has<BlitCornerClipping>()) {
-            auto const& blit_corner_clipping = command.get<BlitCornerClipping>();
-            // FIXME: If a sampling command falls outside the viewport and is not executed, the associated blit
-            //        should also be skipped if it is within the viewport. In a properly generated list of
-            //        painting commands, sample and blit commands should have matching rectangles, preventing
-            //        this discrepancy.
-            if (skipped_sample_corner_commands.contains(blit_corner_clipping.id)) {
-                dbgln("Skipping blit_corner_clipping command because the sample_under_corners command was skipped.");
-                continue;
-            }
-        }
-
 #define HANDLE_COMMAND(command_type, executor_method) \
     if (command.has<command_type>()) {                \
         executor_method(command.get<command_type>()); \
@@ -143,8 +84,7 @@ void DisplayListPlayer::execute(DisplayList& display_list)
         else HANDLE_COMMAND(ApplyBackdropFilter, apply_backdrop_filter)
         else HANDLE_COMMAND(DrawRect, draw_rect)
         else HANDLE_COMMAND(DrawTriangleWave, draw_triangle_wave)
-        else HANDLE_COMMAND(SampleUnderCorners, sample_under_corners)
-        else HANDLE_COMMAND(BlitCornerClipping, blit_corner_clipping)
+        else HANDLE_COMMAND(AddRoundedRectClip, add_rounded_rect_clip)
         else VERIFY_NOT_REACHED();
         // clang-format on
     }

+ 1 - 4
Userland/Libraries/LibWeb/Painting/DisplayList.h

@@ -68,8 +68,7 @@ private:
     virtual void apply_backdrop_filter(ApplyBackdropFilter const&) = 0;
     virtual void draw_rect(DrawRect const&) = 0;
     virtual void draw_triangle_wave(DrawTriangleWave const&) = 0;
-    virtual void sample_under_corners(SampleUnderCorners const&) = 0;
-    virtual void blit_corner_clipping(BlitCornerClipping const&) = 0;
+    virtual void add_rounded_rect_clip(AddRoundedRectClip const&) = 0;
     virtual bool would_be_fully_clipped_by_painter(Gfx::IntRect) const = 0;
 };
 
@@ -83,12 +82,10 @@ public:
     void append(Command&& command, Optional<i32> scroll_frame_id);
 
     void apply_scroll_offsets(Vector<Gfx::IntPoint> const& offsets_by_frame_id);
-    void mark_unnecessary_commands();
 
     struct CommandListItem {
         Optional<i32> scroll_frame_id;
         Command command;
-        bool skip { false };
     };
 
     AK::SegmentedVector<CommandListItem, 512> const& commands() const { return m_commands; }

+ 1 - 8
Userland/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp

@@ -1294,21 +1294,14 @@ void DisplayListPlayerSkia::draw_triangle_wave(DrawTriangleWave const&)
 {
 }
 
-void DisplayListPlayerSkia::sample_under_corners(SampleUnderCorners const& command)
+void DisplayListPlayerSkia::add_rounded_rect_clip(AddRoundedRectClip const& command)
 {
     auto rounded_rect = to_skia_rrect(command.border_rect, command.corner_radii);
     auto& canvas = surface().canvas();
-    canvas.save();
     auto clip_op = command.corner_clip == CornerClip::Inside ? SkClipOp::kDifference : SkClipOp::kIntersect;
     canvas.clipRRect(rounded_rect, clip_op, true);
 }
 
-void DisplayListPlayerSkia::blit_corner_clipping(BlitCornerClipping const&)
-{
-    auto& canvas = surface().canvas();
-    canvas.restore();
-}
-
 bool DisplayListPlayerSkia::would_be_fully_clipped_by_painter(Gfx::IntRect rect) const
 {
     return surface().canvas().quickReject(to_skia_rect(rect));

+ 1 - 2
Userland/Libraries/LibWeb/Painting/DisplayListPlayerSkia.h

@@ -75,8 +75,7 @@ private:
     void paint_radial_gradient(PaintRadialGradient const&) override;
     void paint_conic_gradient(PaintConicGradient const&) override;
     void draw_triangle_wave(DrawTriangleWave const&) override;
-    void sample_under_corners(SampleUnderCorners const&) override;
-    void blit_corner_clipping(BlitCornerClipping const&) override;
+    void add_rounded_rect_clip(AddRoundedRectClip const&) override;
 
     bool would_be_fully_clipped_by_painter(Gfx::IntRect) const override;
 

+ 3 - 15
Userland/Libraries/LibWeb/Painting/DisplayListRecorder.cpp

@@ -15,33 +15,21 @@ DisplayListRecorder::DisplayListRecorder(DisplayList& command_list)
     m_state_stack.append(State());
 }
 
-DisplayListRecorder::~DisplayListRecorder()
-{
-    VERIFY(m_corner_clip_state_stack.is_empty());
-}
+DisplayListRecorder::~DisplayListRecorder() = default;
 
 void DisplayListRecorder::append(Command&& command)
 {
     m_command_list.append(move(command), state().scroll_frame_id);
 }
 
-void DisplayListRecorder::sample_under_corners(u32 id, CornerRadii corner_radii, Gfx::IntRect border_rect, CornerClip corner_clip)
+void DisplayListRecorder::add_rounded_rect_clip(CornerRadii corner_radii, Gfx::IntRect border_rect, CornerClip corner_clip)
 {
-    m_corner_clip_state_stack.append({ id, border_rect });
-    append(SampleUnderCorners {
-        id,
+    append(AddRoundedRectClip {
         corner_radii,
         border_rect = state().translation.map(border_rect),
         corner_clip });
 }
 
-void DisplayListRecorder::blit_corner_clipping(u32 id)
-{
-    auto clip_state = m_corner_clip_state_stack.take_last();
-    VERIFY(clip_state.id == id);
-    append(BlitCornerClipping { id, state().translation.map(clip_state.rect) });
-}
-
 void DisplayListRecorder::fill_rect(Gfx::IntRect const& rect, Color color, RefPtr<DisplayList> text_clip)
 {
     if (rect.is_empty())

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

@@ -122,8 +122,7 @@ public:
     void push_stacking_context(PushStackingContextParams params);
     void pop_stacking_context();
 
-    void sample_under_corners(u32 id, CornerRadii corner_radii, Gfx::IntRect border_rect, CornerClip corner_clip);
-    void blit_corner_clipping(u32 id);
+    void add_rounded_rect_clip(CornerRadii corner_radii, Gfx::IntRect border_rect, CornerClip corner_clip);
 
     void apply_backdrop_filter(Gfx::IntRect const& backdrop_region, BorderRadiiData const& border_radii_data, CSS::ResolvedBackdropFilter const& backdrop_filter);
 
@@ -153,12 +152,6 @@ private:
     State& state() { return m_state_stack.last(); }
     State const& state() const { return m_state_stack.last(); }
 
-    struct CornerClipState {
-        u32 id;
-        Gfx::IntRect rect;
-    };
-    Vector<CornerClipState> m_corner_clip_state_stack;
-
     Vector<State> m_state_stack;
     DisplayList& m_command_list;
 };

+ 0 - 3
Userland/Libraries/LibWeb/Painting/PaintContext.h

@@ -82,8 +82,6 @@ public:
 
     double device_pixels_per_css_pixel() const { return m_device_pixels_per_css_pixel; }
 
-    u32 allocate_corner_clipper_id() { return m_next_corner_clipper_id++; }
-
     u64 paint_generation_id() const { return m_paint_generation_id; }
 
 private:
@@ -96,7 +94,6 @@ private:
     bool m_focus { false };
     bool m_draw_svg_geometry_for_clip_path { false };
     Gfx::AffineTransform m_svg_transform;
-    u32 m_next_corner_clipper_id { 0 };
     u64 m_paint_generation_id { 0 };
 };
 

+ 2 - 20
Userland/Libraries/LibWeb/Painting/PaintableBox.cpp

@@ -528,17 +528,14 @@ void PaintableBox::apply_clip_overflow_rect(PaintContext& context, PaintPhase ph
         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();
-        m_corner_clipper_ids.resize(border_radii_clips.size());
         auto const& combined_transform = combined_css_transform();
         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;
-            auto corner_clipper_id = context.allocate_corner_clipper_id();
-            m_corner_clipper_ids[corner_clip_index] = corner_clipper_id;
             auto rect = corner_clip.rect.translated(-combined_transform.translation().to_type<CSSPixels>());
-            context.display_list_recorder().sample_under_corners(corner_clipper_id, corner_clip.radii.as_corners(context), context.rounded_device_rect(rect).to_type<int>(), CornerClip::Outside);
+            context.display_list_recorder().add_rounded_rect_clip(corner_clip.radii.as_corners(context), context.rounded_device_rect(rect).to_type<int>(), CornerClip::Outside);
         }
     }
 }
@@ -550,16 +547,6 @@ void PaintableBox::clear_clip_overflow_rect(PaintContext& context, PaintPhase ph
 
     if (m_clipping_overflow) {
         m_clipping_overflow = false;
-        auto const& border_radii_clips = this->border_radii_clips();
-        for (int corner_clip_index = border_radii_clips.size() - 1; corner_clip_index >= 0; --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;
-            auto corner_clipper_id = m_corner_clipper_ids[corner_clip_index];
-            m_corner_clipper_ids[corner_clip_index] = corner_clipper_id;
-            context.display_list_recorder().blit_corner_clipping(corner_clipper_id);
-        }
         context.display_list_recorder().restore();
     }
 }
@@ -742,8 +729,7 @@ void PaintableWithLines::paint(PaintContext& context, PaintPhase phase) const
             .bottom_left = border_radii.bottom_left.as_corner(context)
         };
         if (corner_radii.has_any_radius()) {
-            corner_clip_id = context.allocate_corner_clipper_id();
-            context.display_list_recorder().sample_under_corners(*corner_clip_id, corner_radii, context.rounded_device_rect(clip_box).to_type<int>(), CornerClip::Outside);
+            context.display_list_recorder().add_rounded_rect_clip(corner_radii, context.rounded_device_rect(clip_box).to_type<int>(), CornerClip::Outside);
         }
 
         context.display_list_recorder().save();
@@ -776,10 +762,6 @@ void PaintableWithLines::paint(PaintContext& context, PaintPhase phase) const
 
     if (should_clip_overflow) {
         context.display_list_recorder().restore();
-        if (corner_clip_id.has_value()) {
-            context.display_list_recorder().blit_corner_clipping(*corner_clip_id);
-            corner_clip_id = {};
-        }
         context.display_list_recorder().restore();
     }
 }

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

@@ -245,7 +245,6 @@ private:
     Optional<CSSPixelRect> mutable m_absolute_paint_rect;
 
     mutable bool m_clipping_overflow { false };
-    mutable Vector<u32> m_corner_clipper_ids;
 
     RefPtr<ScrollFrame const> m_enclosing_scroll_frame;
     RefPtr<ClipFrame const> m_enclosing_clip_frame;