From 51555dea7c1fa0ac1f426a609482d8fd19bf0ead Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 23 Jan 2023 14:59:51 +0100 Subject: [PATCH] LibWeb: Make Layout::Node::containing_block() return a Layout::Box Containing blocks can be formed by boxes that aren't block containers, so let's make this return a Box and work towards type correctness here. --- Userland/Libraries/LibWeb/Layout/Node.cpp | 42 ++++++++++++------- Userland/Libraries/LibWeb/Layout/Node.h | 4 +- .../LibWeb/Painting/InlinePaintable.cpp | 2 +- .../Libraries/LibWeb/Painting/Paintable.cpp | 9 ++-- .../Libraries/LibWeb/Painting/Paintable.h | 4 +- .../LibWeb/Painting/PaintableBox.cpp | 11 ++--- 6 files changed, 42 insertions(+), 30 deletions(-) diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index 8eaa2a28ed..fba5dd4ca4 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -62,26 +62,36 @@ bool Node::can_contain_boxes_with_position_absolute() const return computed_values().position() != CSS::Position::Static || is(*this); } -BlockContainer const* Node::containing_block() const +static Box const* nearest_ancestor_capable_of_forming_a_containing_block(Node const& node) +{ + for (auto const* ancestor = node.parent(); ancestor; ancestor = ancestor->parent()) { + if (ancestor->is_block_container()) + return verify_cast(ancestor); + } + return nullptr; +} + +Box const* Node::containing_block() const { if (is(*this)) - return first_ancestor_of_type(); + return nearest_ancestor_capable_of_forming_a_containing_block(*this); auto position = computed_values().position(); + // https://drafts.csswg.org/css-position-3/#absolute-cb if (position == CSS::Position::Absolute) { - auto* ancestor = parent(); + auto const* ancestor = parent(); while (ancestor && !ancestor->can_contain_boxes_with_position_absolute()) ancestor = ancestor->parent(); - while (ancestor && (!is(*ancestor) || ancestor->is_anonymous())) - ancestor = ancestor->containing_block(); + while (ancestor && ancestor->is_anonymous()) + ancestor = nearest_ancestor_capable_of_forming_a_containing_block(*ancestor); return static_cast(ancestor); } if (position == CSS::Position::Fixed) return &root(); - return first_ancestor_of_type(); + return nearest_ancestor_capable_of_forming_a_containing_block(*this); } // https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context @@ -158,7 +168,9 @@ void Node::set_needs_display() return; if (!containing_block->paint_box()) return; - containing_block->paint_box()->for_each_fragment([&](auto& fragment) { + if (!is(*containing_block->paint_box())) + return; + static_cast(*containing_block->paint_box()).for_each_fragment([&](auto& fragment) { if (&fragment.layout_node() == this || is_ancestor_of(fragment.layout_node())) { browsing_context().set_needs_display(fragment.absolute_rect()); } @@ -173,13 +185,15 @@ CSSPixelPoint Node::box_type_agnostic_position() const VERIFY(is_inline()); CSSPixelPoint position; if (auto* block = containing_block()) { - block->paint_box()->for_each_fragment([&](auto& fragment) { - if (&fragment.layout_node() == this || is_ancestor_of(fragment.layout_node())) { - position = fragment.absolute_rect().location(); - return IterationDecision::Break; - } - return IterationDecision::Continue; - }); + if (is(*block)) { + static_cast(*block->paint_box()).for_each_fragment([&](auto& fragment) { + if (&fragment.layout_node() == this || is_ancestor_of(fragment.layout_node())) { + position = fragment.absolute_rect().location(); + return IterationDecision::Break; + } + return IterationDecision::Continue; + }); + } } return position; } diff --git a/Userland/Libraries/LibWeb/Layout/Node.h b/Userland/Libraries/LibWeb/Layout/Node.h index 7a9a391b38..cfaaa97c50 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.h +++ b/Userland/Libraries/LibWeb/Layout/Node.h @@ -104,8 +104,8 @@ public: bool is_flex_item() const { return m_is_flex_item; } void set_flex_item(bool b) { m_is_flex_item = b; } - BlockContainer const* containing_block() const; - BlockContainer* containing_block() { return const_cast(const_cast(this)->containing_block()); } + Box const* containing_block() const; + Box* containing_block() { return const_cast(const_cast(this)->containing_block()); } bool establishes_stacking_context() const; diff --git a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp index 2350e841d0..3bbec40096 100644 --- a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp @@ -132,7 +132,7 @@ void InlinePaintable::for_each_fragment(Callback callback) const { // FIXME: This will be slow if the containing block has a lot of fragments! Vector fragments; - containing_block()->paint_box()->for_each_fragment([&](auto& fragment) { + verify_cast(*containing_block()->paint_box()).for_each_fragment([&](auto& fragment) { if (layout_node().is_inclusive_ancestor_of(fragment.layout_node())) fragments.append(fragment); return IterationDecision::Continue; diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.cpp b/Userland/Libraries/LibWeb/Painting/Paintable.cpp index 59ececf2f8..930114303f 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/Paintable.cpp @@ -36,13 +36,16 @@ Paintable::DispatchEventOfSameName Paintable::handle_mousemove(Badge, CSSPixelPoint, unsigned, unsigned, int wheel_delta_x, int wheel_delta_y) { if (auto* containing_block = this->containing_block()) { - if (!containing_block->is_scrollable()) + if (!containing_block->is_block_container()) return false; - auto new_offset = containing_block->scroll_offset(); + auto* scroll_container = static_cast(containing_block); + if (!scroll_container->is_scrollable()) + return false; + auto new_offset = scroll_container->scroll_offset(); new_offset.translate_by(wheel_delta_x, wheel_delta_y); // FIXME: This const_cast is gross. // FIXME: Scroll offset shouldn't live in the layout tree. - const_cast(containing_block)->set_scroll_offset(new_offset); + const_cast(scroll_container)->set_scroll_offset(new_offset); return true; } diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.h b/Userland/Libraries/LibWeb/Painting/Paintable.h index 026b77d27d..10269ed234 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.h +++ b/Userland/Libraries/LibWeb/Painting/Paintable.h @@ -121,7 +121,7 @@ public: void set_needs_display() const { const_cast(*m_layout_node).set_needs_display(); } - Layout::BlockContainer const* containing_block() const + Layout::Box const* containing_block() const { if (!m_containing_block.has_value()) m_containing_block = m_layout_node->containing_block(); @@ -141,7 +141,7 @@ protected: private: JS::NonnullGCPtr m_layout_node; - Optional> mutable m_containing_block; + Optional> mutable m_containing_block; }; inline DOM::Node* HitTestResult::dom_node() diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index 4d15d45847..9e9c9bf866 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -75,14 +75,9 @@ void PaintableBox::set_content_size(CSSPixelSize size) CSSPixelPoint PaintableBox::effective_offset() const { CSSPixelPoint offset; - if (m_containing_line_box_fragment.has_value()) { - - // FIXME: This is a hack to deal with situations where the layout tree has been garbage collected. - // We could avoid this by making the paintable tree garbage collected as well. - if (!containing_block() || !containing_block()->paint_box()) - return offset; - - auto const& fragment = containing_block()->paint_box()->line_boxes()[m_containing_line_box_fragment->line_box_index].fragments()[m_containing_line_box_fragment->fragment_index]; + if (containing_block() && m_containing_line_box_fragment.has_value()) { + auto& paintable_with_lines = *verify_cast(containing_block()->paint_box()); + auto const& fragment = paintable_with_lines.line_boxes()[m_containing_line_box_fragment->line_box_index].fragments()[m_containing_line_box_fragment->fragment_index]; offset = fragment.offset(); } else { offset = m_offset;