From 5aeb6fec680a12011df443e01e5594e154a574f5 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 3 Nov 2022 19:08:07 +0100 Subject: [PATCH] LibWeb: Make hit testing traverse positioned descendants in right order We were doing a forward traversal in hit testing which led to sometimes incorrect results when multiple boxes were occupying the same X and Y coordinate. --- .../Libraries/LibWeb/Painting/Paintable.cpp | 16 +++++ .../Libraries/LibWeb/Painting/Paintable.h | 4 +- .../LibWeb/Painting/StackingContext.cpp | 59 ++++++++++++++++--- 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.cpp b/Userland/Libraries/LibWeb/Painting/Paintable.cpp index d53563cc0a..ca931544bc 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/Paintable.cpp @@ -62,4 +62,20 @@ Paintable const* Paintable::next_sibling() const return layout_node ? layout_node->paintable() : nullptr; } +Paintable const* Paintable::last_child() const +{ + auto* layout_child = m_layout_node.last_child(); + for (; layout_child && !layout_child->paintable(); layout_child = layout_child->previous_sibling()) + ; + return layout_child ? layout_child->paintable() : nullptr; +} + +Paintable const* Paintable::previous_sibling() const +{ + auto* layout_node = m_layout_node.previous_sibling(); + for (; layout_node && !layout_node->paintable(); layout_node = layout_node->previous_sibling()) + ; + return layout_node ? layout_node->paintable() : nullptr; +} + } diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.h b/Userland/Libraries/LibWeb/Painting/Paintable.h index cb931abe39..c9e2b964aa 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.h +++ b/Userland/Libraries/LibWeb/Painting/Paintable.h @@ -56,13 +56,15 @@ public: virtual ~Paintable() = default; Paintable const* first_child() const; + Paintable const* last_child() const; Paintable const* next_sibling() const; + Paintable const* previous_sibling() const; template TraversalDecision for_each_in_inclusive_subtree_of_type(Callback callback) const { if (is(*this)) { - if (auto decision = callback(static_cast(*this)); decision != TraversalDecision::Continue) + if (auto decision = callback(static_cast(*this)); decision != TraversalDecision::Continue) return decision; } for (auto* child = first_child(); child; child = child->next_sibling()) { diff --git a/Userland/Libraries/LibWeb/Painting/StackingContext.cpp b/Userland/Libraries/LibWeb/Painting/StackingContext.cpp index df9b1ca068..2e994815ad 100644 --- a/Userland/Libraries/LibWeb/Painting/StackingContext.cpp +++ b/Userland/Libraries/LibWeb/Painting/StackingContext.cpp @@ -407,6 +407,33 @@ Gfx::FloatPoint StackingContext::compute_transform_origin() const return { x, y }; } +template +static TraversalDecision for_each_in_inclusive_subtree_of_type_within_same_stacking_context_in_reverse(Paintable const& paintable, Callback callback) +{ + if (is(paintable) && static_cast(paintable).stacking_context()) + return TraversalDecision::SkipChildrenAndContinue; + + for (auto* child = paintable.last_child(); child; child = child->previous_sibling()) { + if (for_each_in_inclusive_subtree_of_type_within_same_stacking_context_in_reverse(*child, callback) == TraversalDecision::Break) + return TraversalDecision::Break; + } + if (is(paintable)) { + if (auto decision = callback(static_cast(paintable)); decision != TraversalDecision::Continue) + return decision; + } + return TraversalDecision::Continue; +} + +template +static TraversalDecision for_each_in_subtree_of_type_within_same_stacking_context_in_reverse(Paintable const& paintable, Callback callback) +{ + for (auto* child = paintable.last_child(); child; child = child->previous_sibling()) { + if (for_each_in_inclusive_subtree_of_type_within_same_stacking_context_in_reverse(*child, callback) == TraversalDecision::Break) + return TraversalDecision::Break; + } + return TraversalDecision::Continue; +} + Optional StackingContext::hit_test(Gfx::FloatPoint const& position, HitTestType type) const { if (!m_box.is_visible()) @@ -428,7 +455,7 @@ Optional StackingContext::hit_test(Gfx::FloatPoint const& positio // NOTE: Hit testing follows reverse painting order, that's why the conditions here are reversed. for (ssize_t i = m_children.size() - 1; i >= 0; --i) { auto const& child = *m_children[i]; - if (child.m_box.computed_values().z_index().value_or(0) < 0) + if (child.m_box.computed_values().z_index().value_or(0) <= 0) break; auto result = child.hit_test(transformed_position, type); if (result.has_value() && result->paintable->visible_for_hit_testing()) @@ -437,17 +464,31 @@ Optional StackingContext::hit_test(Gfx::FloatPoint const& positio Optional result; // 6. the child stacking contexts with stack level 0 and the positioned descendants with stack level 0. - paintable().for_each_in_subtree_of_type([&](auto& paint_box) { + + for_each_in_subtree_of_type_within_same_stacking_context_in_reverse(paintable(), [&](PaintableBox const& paint_box) { // FIXME: Support more overflow variations. if (paint_box.computed_values().overflow_x() == CSS::Overflow::Hidden && paint_box.computed_values().overflow_y() == CSS::Overflow::Hidden) { if (!paint_box.absolute_border_box_rect().contains(transformed_position.x(), transformed_position.y())) return TraversalDecision::SkipChildrenAndContinue; } + if (paint_box.stacking_context()) { + auto const& z_index = paint_box.computed_values().z_index(); + if (!z_index.has_value() || z_index.value() == 0) { + auto candidate = paint_box.stacking_context()->hit_test(transformed_position, type); + if (candidate.has_value() && candidate->paintable->visible_for_hit_testing()) { + result = move(candidate); + return TraversalDecision::Break; + } + } + } + auto& layout_box = paint_box.layout_box(); if (layout_box.is_positioned() && !paint_box.stacking_context()) { - if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) + if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) { result = move(candidate); + return TraversalDecision::Break; + } } return TraversalDecision::Continue; }); @@ -462,7 +503,7 @@ Optional StackingContext::hit_test(Gfx::FloatPoint const& positio } // 4. the non-positioned floats. - paintable().for_each_in_subtree_of_type([&](auto const& paint_box) { + for_each_in_subtree_of_type_within_same_stacking_context_in_reverse(paintable(), [&](auto const& paint_box) { // FIXME: Support more overflow variations. if (paint_box.computed_values().overflow_x() == CSS::Overflow::Hidden && paint_box.computed_values().overflow_y() == CSS::Overflow::Hidden) { if (!paint_box.absolute_border_box_rect().contains(transformed_position.x(), transformed_position.y())) @@ -471,8 +512,10 @@ Optional StackingContext::hit_test(Gfx::FloatPoint const& positio auto& layout_box = paint_box.layout_box(); if (layout_box.is_floating()) { - if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) + if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) { result = move(candidate); + return TraversalDecision::Break; + } } return TraversalDecision::Continue; }); @@ -481,7 +524,7 @@ Optional StackingContext::hit_test(Gfx::FloatPoint const& positio // 3. the in-flow, non-inline-level, non-positioned descendants. if (!m_box.children_are_inline()) { - paintable().for_each_in_subtree_of_type([&](auto const& paint_box) { + for_each_in_subtree_of_type_within_same_stacking_context_in_reverse(paintable(), [&](auto const& paint_box) { // FIXME: Support more overflow variations. if (paint_box.computed_values().overflow_x() == CSS::Overflow::Hidden && paint_box.computed_values().overflow_y() == CSS::Overflow::Hidden) { if (!paint_box.absolute_border_box_rect().contains(transformed_position.x(), transformed_position.y())) @@ -490,8 +533,10 @@ Optional StackingContext::hit_test(Gfx::FloatPoint const& positio auto& layout_box = paint_box.layout_box(); if (!layout_box.is_absolutely_positioned() && !layout_box.is_floating()) { - if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) + if (auto candidate = paint_box.hit_test(transformed_position, type); candidate.has_value()) { result = move(candidate); + return TraversalDecision::Break; + } } return TraversalDecision::Continue; });