From 8a7cd8055fd8e53fdb834d95f04fb81fb8cc7362 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Mon, 10 Jun 2024 14:18:32 +0300 Subject: [PATCH] LibWeb: Add save and restore commands in recording painter This change is a preparation before introducing Skia painter in an upcoming change. It's needed because Skia does not have an API to implement ClearClipRect command. It only allows to return previous clip rect by popping from its internal state stack. A bit more context: initially we had save and restore commands, but their frequent use led to many reallocations of vector during painting commands recording. To resolve this, we switched to SegmentedVector to store commands list, which allows fast appends. Now, having many save and restore commands no longer causes noticeable performance issue. --- .../Painting/AffineCommandExecutorCPU.cpp | 16 ++++++++++----- .../Painting/AffineCommandExecutorCPU.h | 5 +++-- Userland/Libraries/LibWeb/Painting/Command.h | 12 ++++++----- .../LibWeb/Painting/CommandExecutorCPU.cpp | 16 +++++++++------ .../LibWeb/Painting/CommandExecutorCPU.h | 5 +++-- .../LibWeb/Painting/CommandExecutorGPU.cpp | 14 +++++++++---- .../LibWeb/Painting/CommandExecutorGPU.h | 5 +++-- .../Libraries/LibWeb/Painting/CommandList.cpp | 9 +++++---- .../Libraries/LibWeb/Painting/CommandList.h | 5 +++-- .../LibWeb/Painting/RecordingPainter.cpp | 20 +++---------------- 10 files changed, 58 insertions(+), 49 deletions(-) diff --git a/Userland/Libraries/LibWeb/Painting/AffineCommandExecutorCPU.cpp b/Userland/Libraries/LibWeb/Painting/AffineCommandExecutorCPU.cpp index 9f397575fdb..0adc2979a3b 100644 --- a/Userland/Libraries/LibWeb/Painting/AffineCommandExecutorCPU.cpp +++ b/Userland/Libraries/LibWeb/Painting/AffineCommandExecutorCPU.cpp @@ -64,16 +64,22 @@ CommandResult AffineCommandExecutorCPU::draw_scaled_immutable_bitmap(DrawScaledI return CommandResult::Continue; } -CommandResult AffineCommandExecutorCPU::set_clip_rect(SetClipRect const&) +CommandResult AffineCommandExecutorCPU::save(Save const&) { - // FIXME: Implement. The plan here is to implement https://en.wikipedia.org/wiki/Sutherland%E2%80%93Hodgman_algorithm - // within the rasterizer (which should work as the clip quadrilateral will always be convex). + m_painter.save(); return CommandResult::Continue; } -CommandResult AffineCommandExecutorCPU::clear_clip_rect(ClearClipRect const&) +CommandResult AffineCommandExecutorCPU::restore(Restore const&) { - // FIXME: Implement. + m_painter.restore(); + return CommandResult::Continue; +} + +CommandResult AffineCommandExecutorCPU::add_clip_rect(AddClipRect const&) +{ + // FIXME: Implement. The plan here is to implement https://en.wikipedia.org/wiki/Sutherland%E2%80%93Hodgman_algorithm + // within the rasterizer (which should work as the clip quadrilateral will always be convex). return CommandResult::Continue; } diff --git a/Userland/Libraries/LibWeb/Painting/AffineCommandExecutorCPU.h b/Userland/Libraries/LibWeb/Painting/AffineCommandExecutorCPU.h index ad39d71be68..22ce6f1e203 100644 --- a/Userland/Libraries/LibWeb/Painting/AffineCommandExecutorCPU.h +++ b/Userland/Libraries/LibWeb/Painting/AffineCommandExecutorCPU.h @@ -19,8 +19,9 @@ public: CommandResult fill_rect(FillRect const&) override; CommandResult draw_scaled_bitmap(DrawScaledBitmap const&) override; CommandResult draw_scaled_immutable_bitmap(DrawScaledImmutableBitmap const&) override; - CommandResult set_clip_rect(SetClipRect const&) override; - CommandResult clear_clip_rect(ClearClipRect const&) override; + CommandResult save(Save const&) override; + CommandResult restore(Restore const&) override; + CommandResult add_clip_rect(AddClipRect const&) override; CommandResult push_stacking_context(PushStackingContext const&) override; CommandResult pop_stacking_context(PopStackingContext const&) override; CommandResult paint_linear_gradient(PaintLinearGradient const&) override; diff --git a/Userland/Libraries/LibWeb/Painting/Command.h b/Userland/Libraries/LibWeb/Painting/Command.h index 40a9ebcf328..6407a24bad2 100644 --- a/Userland/Libraries/LibWeb/Painting/Command.h +++ b/Userland/Libraries/LibWeb/Painting/Command.h @@ -92,12 +92,13 @@ struct DrawScaledImmutableBitmap { void translate_by(Gfx::IntPoint const& offset) { dst_rect.translate_by(offset); } }; -struct SetClipRect { +struct Save { }; +struct Restore { }; + +struct AddClipRect { Gfx::IntRect rect; }; -struct ClearClipRect { }; - struct StackingContextTransform { Gfx::FloatPoint origin; Gfx::FloatMatrix4x4 matrix; @@ -372,8 +373,9 @@ using Command = Variant< FillRect, DrawScaledBitmap, DrawScaledImmutableBitmap, - SetClipRect, - ClearClipRect, + Save, + Restore, + AddClipRect, PushStackingContext, PopStackingContext, PaintLinearGradient, diff --git a/Userland/Libraries/LibWeb/Painting/CommandExecutorCPU.cpp b/Userland/Libraries/LibWeb/Painting/CommandExecutorCPU.cpp index 850eea18f4c..f04c191ae61 100644 --- a/Userland/Libraries/LibWeb/Painting/CommandExecutorCPU.cpp +++ b/Userland/Libraries/LibWeb/Painting/CommandExecutorCPU.cpp @@ -107,17 +107,21 @@ CommandResult CommandExecutorCPU::draw_scaled_immutable_bitmap(DrawScaledImmutab return CommandResult::Continue; } -CommandResult CommandExecutorCPU::set_clip_rect(SetClipRect const& command) +CommandResult CommandExecutorCPU::save(Save const&) { - auto& painter = this->painter(); - painter.clear_clip_rect(); - painter.add_clip_rect(command.rect); + painter().save(); return CommandResult::Continue; } -CommandResult CommandExecutorCPU::clear_clip_rect(ClearClipRect const&) +CommandResult CommandExecutorCPU::restore(Restore const&) { - painter().clear_clip_rect(); + painter().restore(); + return CommandResult::Continue; +} + +CommandResult CommandExecutorCPU::add_clip_rect(AddClipRect const& command) +{ + painter().add_clip_rect(command.rect); return CommandResult::Continue; } diff --git a/Userland/Libraries/LibWeb/Painting/CommandExecutorCPU.h b/Userland/Libraries/LibWeb/Painting/CommandExecutorCPU.h index 9609faeb3e7..67f2d90c449 100644 --- a/Userland/Libraries/LibWeb/Painting/CommandExecutorCPU.h +++ b/Userland/Libraries/LibWeb/Painting/CommandExecutorCPU.h @@ -20,8 +20,9 @@ public: CommandResult fill_rect(FillRect const&) override; CommandResult draw_scaled_bitmap(DrawScaledBitmap const&) override; CommandResult draw_scaled_immutable_bitmap(DrawScaledImmutableBitmap const&) override; - CommandResult set_clip_rect(SetClipRect const&) override; - CommandResult clear_clip_rect(ClearClipRect const&) override; + CommandResult save(Save const&) override; + CommandResult restore(Restore const&) override; + CommandResult add_clip_rect(AddClipRect const&) override; CommandResult push_stacking_context(PushStackingContext const&) override; CommandResult pop_stacking_context(PopStackingContext const&) override; CommandResult paint_linear_gradient(PaintLinearGradient const&) override; diff --git a/Userland/Libraries/LibWeb/Painting/CommandExecutorGPU.cpp b/Userland/Libraries/LibWeb/Painting/CommandExecutorGPU.cpp index 2071c918f9a..adb22f2e021 100644 --- a/Userland/Libraries/LibWeb/Painting/CommandExecutorGPU.cpp +++ b/Userland/Libraries/LibWeb/Painting/CommandExecutorGPU.cpp @@ -89,15 +89,21 @@ CommandResult CommandExecutorGPU::draw_scaled_immutable_bitmap(DrawScaledImmutab return CommandResult::Continue; } -CommandResult CommandExecutorGPU::set_clip_rect(SetClipRect const& command) +CommandResult CommandExecutorGPU::save(Save const&) { - painter().set_clip_rect(command.rect); + painter().save(); return CommandResult::Continue; } -CommandResult CommandExecutorGPU::clear_clip_rect(ClearClipRect const&) +CommandResult CommandExecutorGPU::restore(Restore const&) { - painter().clear_clip_rect(); + painter().restore(); + return CommandResult::Continue; +} + +CommandResult CommandExecutorGPU::add_clip_rect(AddClipRect const& command) +{ + painter().set_clip_rect(command.rect); return CommandResult::Continue; } diff --git a/Userland/Libraries/LibWeb/Painting/CommandExecutorGPU.h b/Userland/Libraries/LibWeb/Painting/CommandExecutorGPU.h index f92218deb9c..9bf4323114e 100644 --- a/Userland/Libraries/LibWeb/Painting/CommandExecutorGPU.h +++ b/Userland/Libraries/LibWeb/Painting/CommandExecutorGPU.h @@ -19,8 +19,9 @@ public: CommandResult fill_rect(FillRect const&) override; CommandResult draw_scaled_bitmap(DrawScaledBitmap const&) override; CommandResult draw_scaled_immutable_bitmap(DrawScaledImmutableBitmap const&) override; - CommandResult set_clip_rect(SetClipRect const&) override; - CommandResult clear_clip_rect(ClearClipRect const&) override; + CommandResult save(Save const&) override; + CommandResult restore(Restore const&) override; + CommandResult add_clip_rect(AddClipRect const&) override; CommandResult push_stacking_context(PushStackingContext const&) override; CommandResult pop_stacking_context(PopStackingContext const&) override; CommandResult paint_linear_gradient(PaintLinearGradient const&) override; diff --git a/Userland/Libraries/LibWeb/Painting/CommandList.cpp b/Userland/Libraries/LibWeb/Painting/CommandList.cpp index 7b982a7a98b..66974a373ba 100644 --- a/Userland/Libraries/LibWeb/Painting/CommandList.cpp +++ b/Userland/Libraries/LibWeb/Painting/CommandList.cpp @@ -63,8 +63,8 @@ void CommandList::mark_unnecessary_commands() m_commands[command_index].skip = true; } } else { - // SetClipRect and ClearClipRect commands do not produce visible output - auto update_clip_command = command.has() || command.has(); + // Save, Restore and AddClipRect commands do not produce visible output + auto update_clip_command = command.has() || command.has() || command.has(); 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. @@ -154,8 +154,9 @@ void CommandList::execute(CommandExecutor& executor) else HANDLE_COMMAND(FillRect, fill_rect) else HANDLE_COMMAND(DrawScaledBitmap, draw_scaled_bitmap) else HANDLE_COMMAND(DrawScaledImmutableBitmap, draw_scaled_immutable_bitmap) - else HANDLE_COMMAND(SetClipRect, set_clip_rect) - else HANDLE_COMMAND(ClearClipRect, clear_clip_rect) + else HANDLE_COMMAND(AddClipRect, add_clip_rect) + else HANDLE_COMMAND(Save, save) + else HANDLE_COMMAND(Restore, restore) else HANDLE_COMMAND(PushStackingContext, push_stacking_context) else HANDLE_COMMAND(PopStackingContext, pop_stacking_context) else HANDLE_COMMAND(PaintLinearGradient, paint_linear_gradient) diff --git a/Userland/Libraries/LibWeb/Painting/CommandList.h b/Userland/Libraries/LibWeb/Painting/CommandList.h index 77e8d1893ad..8b642c1d76a 100644 --- a/Userland/Libraries/LibWeb/Painting/CommandList.h +++ b/Userland/Libraries/LibWeb/Painting/CommandList.h @@ -52,8 +52,9 @@ public: virtual CommandResult fill_rect(FillRect const&) = 0; virtual CommandResult draw_scaled_bitmap(DrawScaledBitmap const&) = 0; virtual CommandResult draw_scaled_immutable_bitmap(DrawScaledImmutableBitmap const&) = 0; - virtual CommandResult set_clip_rect(SetClipRect const&) = 0; - virtual CommandResult clear_clip_rect(ClearClipRect const&) = 0; + virtual CommandResult save(Save const&) = 0; + virtual CommandResult restore(Restore const&) = 0; + virtual CommandResult add_clip_rect(AddClipRect const&) = 0; virtual CommandResult push_stacking_context(PushStackingContext const&) = 0; virtual CommandResult pop_stacking_context(PopStackingContext const&) = 0; virtual CommandResult paint_linear_gradient(PaintLinearGradient const&) = 0; diff --git a/Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp b/Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp index b7f573557e9..8a47223a9b8 100644 --- a/Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp +++ b/Userland/Libraries/LibWeb/Painting/RecordingPainter.cpp @@ -253,15 +253,7 @@ void RecordingPainter::draw_text_run(Gfx::IntPoint baseline_start, Gfx::GlyphRun void RecordingPainter::add_clip_rect(Gfx::IntRect const& rect) { - auto prev_clip_rect = state().clip_rect; - if (!state().clip_rect.has_value()) { - state().clip_rect = state().translation.map(rect); - } else { - state().clip_rect->intersect(state().translation.map(rect)); - } - - if (prev_clip_rect != state().clip_rect) - append(SetClipRect { .rect = *state().clip_rect }); + append(AddClipRect { .rect = state().translation.map(rect) }); } void RecordingPainter::translate(int dx, int dy) @@ -276,22 +268,16 @@ void RecordingPainter::translate(Gfx::IntPoint delta) void RecordingPainter::save() { + append(Save {}); m_state_stack.append(m_state_stack.last()); } void RecordingPainter::restore() { - auto prev_clip_rect = state().clip_rect; + append(Restore {}); VERIFY(m_state_stack.size() > 1); m_state_stack.take_last(); - - if (state().clip_rect != prev_clip_rect) { - if (state().clip_rect.has_value()) - append(SetClipRect { .rect = *state().clip_rect }); - else - append(ClearClipRect {}); - } } void RecordingPainter::push_stacking_context(PushStackingContextParams params)