From d1b5f55f91e2f787fc76324b0cc84022c2552b5f Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 1 Mar 2024 15:30:44 +0100 Subject: [PATCH] LibWeb: Make Paintable::containing_block() return a PaintableBox* Every single client of this function was immediately calling paintable() on the result anyway, so there was no need to return a layout node! This automatically leverages the cached containing block pointer we already have in Paintable, which melts away a bunch of unnecessary traversal in hit testing and painting. :^) --- Userland/Libraries/LibWeb/DOM/Element.cpp | 9 ++------- Userland/Libraries/LibWeb/Page/EventHandler.cpp | 2 +- .../Libraries/LibWeb/Painting/InlinePaintable.cpp | 6 +++--- Userland/Libraries/LibWeb/Painting/Paintable.cpp | 10 ++++------ Userland/Libraries/LibWeb/Painting/Paintable.h | 13 +++++++++---- .../Libraries/LibWeb/Painting/PaintableBox.cpp | 14 +++++++------- .../LibWeb/Painting/PaintableFragment.cpp | 4 ++-- .../Libraries/LibWeb/Painting/SVGPaintable.cpp | 2 +- .../LibWeb/Painting/ViewportPaintable.cpp | 8 +++----- 9 files changed, 32 insertions(+), 36 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 886bf5e044..f098a86e6b 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -923,13 +923,8 @@ JS::NonnullGCPtr Element::get_client_rects() const } Gfx::AffineTransform transform; - for (auto const* containing_block = this->layout_node(); containing_block; containing_block = containing_block->containing_block()) { - Gfx::AffineTransform containing_block_transform; - if (containing_block->paintable() && containing_block->paintable()->is_paintable_box()) { - auto const& containing_block_paintable_box = static_cast(*containing_block->paintable()); - containing_block_transform = Gfx::extract_2d_affine_transform(containing_block_paintable_box.transform()); - } - transform = transform.multiply(containing_block_transform); + for (auto const* containing_block = this->paintable_box(); containing_block; containing_block = containing_block->containing_block()) { + transform = transform.multiply(Gfx::extract_2d_affine_transform(containing_block->transform())); } auto const* paintable = this->paintable(); diff --git a/Userland/Libraries/LibWeb/Page/EventHandler.cpp b/Userland/Libraries/LibWeb/Page/EventHandler.cpp index bb096e1c72..4eeee29e11 100644 --- a/Userland/Libraries/LibWeb/Page/EventHandler.cpp +++ b/Userland/Libraries/LibWeb/Page/EventHandler.cpp @@ -174,7 +174,7 @@ bool EventHandler::handle_mousewheel(CSSPixelPoint position, CSSPixelPoint scree if (paintable) { auto* containing_block = paintable->containing_block(); while (containing_block) { - auto handled_scroll_event = const_cast(containing_block->paintable_box())->handle_mousewheel({}, position, buttons, modifiers, wheel_delta_x, wheel_delta_y); + auto handled_scroll_event = containing_block->handle_mousewheel({}, position, buttons, modifiers, wheel_delta_x, wheel_delta_y); if (handled_scroll_event) return true; containing_block = containing_block->containing_block(); diff --git a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp index a47b3523e9..a07dc9f161 100644 --- a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp @@ -73,7 +73,7 @@ void InlinePaintable::paint(PaintContext& context, PaintPhase phase) const auto& painter = context.recording_painter(); if (phase == PaintPhase::Background) { - auto containing_block_position_in_absolute_coordinates = containing_block()->paintable_box()->absolute_position(); + auto containing_block_position_in_absolute_coordinates = containing_block()->absolute_position(); for_each_fragment([&](auto const& fragment, bool is_first_fragment, bool is_last_fragment) { CSSPixelRect absolute_fragment_rect { containing_block_position_in_absolute_coordinates.translated(fragment.offset()), fragment.size() }; @@ -118,7 +118,7 @@ void InlinePaintable::paint(PaintContext& context, PaintPhase phase) const .left = computed_values().border_left(), }; - auto containing_block_position_in_absolute_coordinates = containing_block()->paintable_box()->absolute_position(); + auto containing_block_position_in_absolute_coordinates = containing_block()->absolute_position(); for_each_fragment([&](auto const& fragment, bool is_first_fragment, bool is_last_fragment) { CSSPixelRect absolute_fragment_rect { containing_block_position_in_absolute_coordinates.translated(fragment.offset()), fragment.size() }; @@ -247,7 +247,7 @@ CSSPixelRect InlinePaintable::bounding_rect() const if (bounding_rect.is_empty()) { // FIXME: This is adhoc, and we should return rect of empty fragment instead. - auto containing_block_position_in_absolute_coordinates = containing_block()->paintable_box()->absolute_position(); + auto containing_block_position_in_absolute_coordinates = containing_block()->absolute_position(); return { containing_block_position_in_absolute_coordinates, { 0, 0 } }; } return bounding_rect; diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.cpp b/Userland/Libraries/LibWeb/Painting/Paintable.cpp index ec6344b387..65c7d21e8e 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/Paintable.cpp @@ -128,8 +128,6 @@ void Paintable::set_needs_display() const auto* containing_block = this->containing_block(); if (!containing_block) return; - if (!containing_block->paintable_box()) - return; auto navigable = this->navigable(); if (!navigable) return; @@ -140,9 +138,9 @@ void Paintable::set_needs_display() const navigable->set_needs_display(fragment.absolute_rect()); } - if (!is(*containing_block->paintable_box())) + if (!is(*containing_block)) return; - static_cast(*containing_block->paintable_box()).for_each_fragment([&](auto& fragment) { + static_cast(*containing_block).for_each_fragment([&](auto& fragment) { navigable->set_needs_display(fragment.absolute_rect()); return IterationDecision::Continue; }); @@ -162,8 +160,8 @@ CSSPixelPoint Paintable::box_type_agnostic_position() const } CSSPixelPoint position; - if (auto const* block = containing_block(); block && block->paintable() && is(*block->paintable())) { - static_cast(*block->paintable_box()).for_each_fragment([&](auto& fragment) { + if (auto const* block = containing_block(); block && is(*block)) { + static_cast(*block).for_each_fragment([&](auto& fragment) { position = fragment.absolute_rect().location(); return IterationDecision::Break; }); diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.h b/Userland/Libraries/LibWeb/Painting/Paintable.h index 5b9e7cbf28..e9cedc03d9 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.h +++ b/Userland/Libraries/LibWeb/Painting/Paintable.h @@ -181,10 +181,15 @@ public: virtual void set_needs_display() const; - Layout::Box const* containing_block() const + PaintableBox* containing_block() const { - if (!m_containing_block.has_value()) - m_containing_block = m_layout_node->containing_block(); + if (!m_containing_block.has_value()) { + auto containing_layout_box = m_layout_node->containing_block(); + if (containing_layout_box) + m_containing_block = const_cast(containing_layout_box->paintable_box()); + else + m_containing_block = nullptr; + } return *m_containing_block; } @@ -209,7 +214,7 @@ private: JS::GCPtr m_dom_node; JS::NonnullGCPtr m_layout_node; JS::NonnullGCPtr m_browsing_context; - Optional> mutable m_containing_block; + Optional> mutable m_containing_block; OwnPtr m_stacking_context; }; diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index 30d84c5ccc..8ff2a3efb9 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -117,8 +117,8 @@ CSSPixelPoint PaintableBox::offset() const CSSPixelRect PaintableBox::compute_absolute_rect() const { CSSPixelRect rect { offset(), content_size() }; - for (auto const* block = containing_block(); block && block->paintable(); block = block->paintable()->containing_block()) - rect.translate_by(block->paintable_box()->offset()); + for (auto const* block = containing_block(); block; block = block->containing_block()) + rect.translate_by(block->offset()); return rect; } @@ -126,10 +126,10 @@ CSSPixelRect PaintableBox::compute_absolute_padding_rect_with_css_transform_appl { CSSPixelRect rect { offset(), content_size() }; for (auto const* block = containing_block(); block; block = block->containing_block()) { - auto offset = block->paintable_box()->offset(); - auto affine_transform = Gfx::extract_2d_affine_transform(block->paintable_box()->transform()); + auto offset = block->offset(); + auto affine_transform = Gfx::extract_2d_affine_transform(block->transform()); offset.translate_by(affine_transform.translation().to_type()); - offset.translate_by(-block->paintable_box()->scroll_offset()); + offset.translate_by(-block->scroll_offset()); rect.translate_by(offset); } auto affine_transform = Gfx::extract_2d_affine_transform(transform()); @@ -146,8 +146,8 @@ CSSPixelRect PaintableBox::compute_absolute_padding_rect_with_css_transform_appl Gfx::AffineTransform PaintableBox::compute_combined_css_transform() const { Gfx::AffineTransform combined_transform; - for (auto const* ancestor = &this->layout_box(); ancestor; ancestor = ancestor->containing_block()) { - auto affine_transform = Gfx::extract_2d_affine_transform(ancestor->paintable_box()->transform()); + for (auto const* ancestor = this; ancestor; ancestor = ancestor->containing_block()) { + auto affine_transform = Gfx::extract_2d_affine_transform(ancestor->transform()); combined_transform = combined_transform.multiply(affine_transform); } return combined_transform; diff --git a/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp b/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp index 98638809d0..9cfe0ecf5d 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp @@ -25,8 +25,8 @@ CSSPixelRect const PaintableFragment::absolute_rect() const { CSSPixelRect rect { {}, size() }; auto const* containing_block = paintable().containing_block(); - if (containing_block && containing_block->paintable_box()) - rect.set_location(containing_block->paintable_box()->absolute_position()); + if (containing_block) + rect.set_location(containing_block->absolute_position()); rect.translate_by(offset()); return rect; } diff --git a/Userland/Libraries/LibWeb/Painting/SVGPaintable.cpp b/Userland/Libraries/LibWeb/Painting/SVGPaintable.cpp index 1030102be8..378be31a48 100644 --- a/Userland/Libraries/LibWeb/Painting/SVGPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/SVGPaintable.cpp @@ -25,7 +25,7 @@ CSSPixelRect SVGPaintable::compute_absolute_rect() const { if (auto* svg_svg_box = layout_box().first_ancestor_of_type()) { CSSPixelRect rect { offset(), content_size() }; - for (Layout::Box const* ancestor = svg_svg_box; ancestor && ancestor->paintable(); ancestor = ancestor->paintable()->containing_block()) + for (Layout::Box const* ancestor = svg_svg_box; ancestor; ancestor = ancestor->containing_block()) rect.translate_by(ancestor->paintable_box()->offset()); return rect; } diff --git a/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp b/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp index d99a74de09..b2853cbc34 100644 --- a/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp @@ -77,8 +77,7 @@ void ViewportPaintable::assign_scroll_frames() for_each_in_subtree([&](auto const& paintable) { for (auto block = paintable.containing_block(); block; block = block->containing_block()) { - auto const& block_paintable_box = *block->paintable_box(); - if (auto scroll_frame = scroll_state.get(&block_paintable_box); scroll_frame.has_value()) { + if (auto scroll_frame = scroll_state.get(block); scroll_frame.has_value()) { if (paintable.is_paintable_box()) { auto const& paintable_box = static_cast(paintable); const_cast(paintable_box).set_enclosing_scroll_frame(scroll_frame.value()); @@ -108,8 +107,7 @@ void ViewportPaintable::assign_clip_frames() for_each_in_subtree([&](auto const& paintable) { for (auto block = paintable.containing_block(); block; block = block->containing_block()) { - auto const& block_paintable_box = *block->paintable_box(); - if (auto clip_frame = clip_state.get(&block_paintable_box); clip_frame.has_value()) { + if (auto clip_frame = clip_state.get(block); clip_frame.has_value()) { if (paintable.is_paintable_box()) { auto const& paintable_box = static_cast(paintable); const_cast(paintable_box).set_enclosing_clip_frame(clip_frame.value()); @@ -251,7 +249,7 @@ void ViewportPaintable::resolve_paint_only_properties() auto const& bottom_right_border_radius = inline_paintable.computed_values().border_bottom_right_radius(); auto const& bottom_left_border_radius = inline_paintable.computed_values().border_bottom_left_radius(); - auto containing_block_position_in_absolute_coordinates = inline_paintable.containing_block()->paintable_box()->absolute_position(); + auto containing_block_position_in_absolute_coordinates = inline_paintable.containing_block()->absolute_position(); for (size_t i = 0; i < fragments.size(); ++i) { auto is_first_fragment = i == 0; auto is_last_fragment = i == fragments.size() - 1;