From 270bbf43ab0eab556991b1a810ceab4409062159 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Thu, 25 Jan 2024 15:29:12 +0100 Subject: [PATCH] LibWeb: Remove is check for fragments in hit testing This change introduces a method for direct access to the paintable of a PaintableFragment. This method is intended to replace the usage of the layout node pointer. Currently, we are only eliminating the use of `layout_node()` in hit testing. Additionally, we no longer check if a fragment's layout node is a `BlockContainer` before recursing into its children during hit testing. This check was likely relevant when all fragments were owned by `PaintableWithLines`, but now it should be safe to remove this check. --- .../LibWeb/Painting/InlinePaintable.cpp | 8 ++++---- .../Libraries/LibWeb/Painting/PaintableBox.cpp | 16 ++++++++-------- .../LibWeb/Painting/PaintableFragment.h | 1 + 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp index 0b05535493..c30cef76e6 100644 --- a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp @@ -163,13 +163,13 @@ void InlinePaintable::for_each_fragment(Callback callback) const Optional InlinePaintable::hit_test(CSSPixelPoint position, HitTestType type) const { for (auto& fragment : m_fragments) { - if (is(fragment.layout_node()) && static_cast(fragment.layout_node()).paintable_box()->stacking_context()) + if (fragment.paintable().stacking_context()) continue; auto fragment_absolute_rect = fragment.absolute_rect(); if (fragment_absolute_rect.contains(position)) { - if (is(fragment.layout_node()) && fragment.layout_node().paintable()) - return fragment.layout_node().paintable()->hit_test(position, type); - return HitTestResult { const_cast(const_cast(*fragment.layout_node().paintable())), + if (auto result = fragment.paintable().hit_test(position, type); result.has_value()) + return result; + return HitTestResult { const_cast(fragment.paintable()), fragment.text_index_at(position.x()) }; } } diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index aba7806b84..cb679cdb52 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -752,17 +752,17 @@ Optional PaintableWithLines::hit_test(CSSPixelPoint position, Hit Optional last_good_candidate; for (auto const& fragment : fragments()) { - if (is(fragment.layout_node()) && static_cast(fragment.layout_node()).paintable_box()->stacking_context()) + if (fragment.paintable().stacking_context()) continue; - if (!fragment.layout_node().containing_block()) { + if (!fragment.paintable().containing_block()) { dbgln("FIXME: PaintableWithLines::hit_test(): Missing containing block on {}", fragment.layout_node().debug_description()); continue; } auto fragment_absolute_rect = fragment.absolute_rect(); if (fragment_absolute_rect.contains(position)) { - if (is(fragment.layout_node()) && fragment.layout_node().paintable()) - return fragment.layout_node().paintable()->hit_test(position, type); - return HitTestResult { const_cast(const_cast(*fragment.layout_node().paintable())), fragment.text_index_at(position.x()) }; + if (auto result = fragment.paintable().hit_test(position, type); result.has_value()) + return result; + return HitTestResult { const_cast(fragment.paintable()), fragment.text_index_at(position.x()) }; } // If we reached this point, the position is not within the fragment. However, the fragment start or end might be the place to place the cursor. @@ -771,14 +771,14 @@ Optional PaintableWithLines::hit_test(CSSPixelPoint position, Hit // We arbitrarily choose to consider the end of the line above and ignore the beginning of the line below. // If we knew the direction of selection, we could make a better choice. if (fragment_absolute_rect.bottom() - 1 <= position.y()) { // fully below the fragment - last_good_candidate = HitTestResult { const_cast(*fragment.layout_node().paintable()), fragment.start() + fragment.length() }; + last_good_candidate = HitTestResult { const_cast(fragment.paintable()), fragment.start() + fragment.length() }; } else if (fragment_absolute_rect.top() <= position.y()) { // vertically within the fragment if (position.x() < fragment_absolute_rect.left()) { // left of the fragment if (!last_good_candidate.has_value()) { // first fragment of the line - last_good_candidate = HitTestResult { const_cast(*fragment.layout_node().paintable()), fragment.start() }; + last_good_candidate = HitTestResult { const_cast(fragment.paintable()), fragment.start() }; } } else { // right of the fragment - last_good_candidate = HitTestResult { const_cast(*fragment.layout_node().paintable()), fragment.start() + fragment.length() }; + last_good_candidate = HitTestResult { const_cast(fragment.paintable()), fragment.start() + fragment.length() }; } } } diff --git a/Userland/Libraries/LibWeb/Painting/PaintableFragment.h b/Userland/Libraries/LibWeb/Painting/PaintableFragment.h index c378bbe4ea..dcad8dca84 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableFragment.h +++ b/Userland/Libraries/LibWeb/Painting/PaintableFragment.h @@ -20,6 +20,7 @@ public: explicit PaintableFragment(Layout::LineBoxFragment const&); Layout::Node const& layout_node() const { return m_layout_node; } + Paintable const& paintable() const { return *m_layout_node->paintable(); } int start() const { return m_start; } int length() const { return m_length; }