From 4070f5a7e03a4bd0b59b00a9dd3a07409d475441 Mon Sep 17 00:00:00 2001 From: zac Date: Mon, 28 Jul 2025 06:23:53 +1000 Subject: [PATCH] LibWeb: Prevent hit testing from transforming position more than once The transform of each paintable was being applied multiple times due to the recursive nature of the hit testing methods. Previously it used combined_css_transform to transform the position, and then it would pass that position to children, which would then apply combined_css_transform again, and so on. PaintableBoxes are also not hit tested anymore when having a stacking context. A similar check is done in PaintableWithLines, but it was missing from PaintableBox. Without this check some elements can get returned multiple times from a hit test. StackingContexts with zero opacity will now also get hit tested, as it should have been before. --- Libraries/LibWeb/Painting/PaintableBox.cpp | 39 +++-- Libraries/LibWeb/Painting/StackingContext.cpp | 37 +++-- .../css/CSS2/floats/hit-test-floats-003.txt | 4 +- .../css-transforms/transform-hit-testing.txt | 10 ++ .../css-transforms/transform-hit-testing.html | 153 ++++++++++++++++++ 5 files changed, 210 insertions(+), 33 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/wpt-import/css/css-transforms/transform-hit-testing.txt create mode 100644 Tests/LibWeb/Text/input/wpt-import/css/css-transforms/transform-hit-testing.html diff --git a/Libraries/LibWeb/Painting/PaintableBox.cpp b/Libraries/LibWeb/Painting/PaintableBox.cpp index 20952e09517..ac23acb8ec1 100644 --- a/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -1174,12 +1174,15 @@ TraversalDecision PaintableBox::hit_test(CSSPixelPoint position, HitTestType typ if (clip_rect_for_hit_testing().has_value() && !clip_rect_for_hit_testing()->contains(position)) return TraversalDecision::Continue; - auto position_adjusted_by_scroll_offset = adjust_position_for_cumulative_scroll_offset(position); - if (computed_values().visibility() != CSS::Visibility::Visible) return TraversalDecision::Continue; - if (hit_test_scrollbars(position_adjusted_by_scroll_offset, callback) == TraversalDecision::Break) + auto const inverse_transform = Gfx::extract_2d_affine_transform(transform()).inverse().value_or({}); + // NOTE: This CSSPixels -> Float -> CSSPixels conversion is because we can't AffineTransform::map() a CSSPixelPoint. + auto const offset_position = position.translated(-transform_origin()).to_type(); + auto const transformed_position = inverse_transform.map(offset_position).to_type() + transform_origin(); + + if (hit_test_scrollbars(transformed_position, callback) == TraversalDecision::Break) return TraversalDecision::Break; if (is_viewport()) { @@ -1187,16 +1190,21 @@ TraversalDecision PaintableBox::hit_test(CSSPixelPoint position, HitTestType typ viewport_paintable.build_stacking_context_tree_if_needed(); viewport_paintable.document().update_paint_and_hit_testing_properties_if_needed(); viewport_paintable.refresh_scroll_state(); - return stacking_context()->hit_test(position, type, callback); + return stacking_context()->hit_test(transformed_position, type, callback); } - if (hit_test_children(position, type, callback) == TraversalDecision::Break) + if (stacking_context()) + return TraversalDecision::Continue; + + if (hit_test_children(transformed_position, type, callback) == TraversalDecision::Break) return TraversalDecision::Break; if (!visible_for_hit_testing()) return TraversalDecision::Continue; - if (!absolute_border_box_rect().contains(position_adjusted_by_scroll_offset)) + auto const offset_position_adjusted_by_scroll_offset = adjust_position_for_cumulative_scroll_offset(position).translated(-transform_origin()).to_type(); + auto const transformed_position_adjusted_by_scroll_offset = inverse_transform.map(offset_position_adjusted_by_scroll_offset).to_type() + transform_origin(); + if (!absolute_border_box_rect().contains(transformed_position_adjusted_by_scroll_offset)) return TraversalDecision::Continue; if (hit_test_continuation(callback) == TraversalDecision::Break) @@ -1256,8 +1264,6 @@ TraversalDecision PaintableWithLines::hit_test(CSSPixelPoint position, HitTestTy if (clip_rect_for_hit_testing().has_value() && !clip_rect_for_hit_testing()->contains(position)) return TraversalDecision::Continue; - auto position_adjusted_by_scroll_offset = adjust_position_for_cumulative_scroll_offset(position); - // TextCursor hit testing mode should be able to place cursor in contenteditable elements even if they are empty if (m_fragments.is_empty() && !has_children() @@ -1277,25 +1283,30 @@ TraversalDecision PaintableWithLines::hit_test(CSSPixelPoint position, HitTestTy if (!layout_node().children_are_inline()) return PaintableBox::hit_test(position, type, callback); + auto const inverse_transform = Gfx::extract_2d_affine_transform(transform()).inverse().value_or({}); // NOTE: This CSSPixels -> Float -> CSSPixels conversion is because we can't AffineTransform::map() a CSSPixelPoint. - auto offset_position = position_adjusted_by_scroll_offset.translated(-transform_origin()).to_type(); - auto transformed_position_adjusted_by_scroll_offset = combined_css_transform().inverse().value_or({}).map(offset_position).to_type() + transform_origin(); + auto const offset_position = position.translated(-transform_origin()).to_type(); + auto const transformed_position = inverse_transform.map(offset_position).to_type() + transform_origin(); - if (hit_test_scrollbars(transformed_position_adjusted_by_scroll_offset, callback) == TraversalDecision::Break) + if (hit_test_scrollbars(transformed_position, callback) == TraversalDecision::Break) return TraversalDecision::Break; - if (hit_test_children(position, type, callback) == TraversalDecision::Break) + if (hit_test_children(transformed_position, type, callback) == TraversalDecision::Break) return TraversalDecision::Break; if (!visible_for_hit_testing()) return TraversalDecision::Continue; + // NOTE: This CSSPixels -> Float -> CSSPixels conversion is because we can't AffineTransform::map() a CSSPixelPoint. + auto const offset_position_adjusted_by_scroll_offset = adjust_position_for_cumulative_scroll_offset(position).translated(-transform_origin()).to_type(); + auto const transformed_position_adjusted_by_scroll_offset = inverse_transform.map(offset_position_adjusted_by_scroll_offset).to_type() + transform_origin(); + for (auto const& fragment : fragments()) { if (fragment.paintable().has_stacking_context() || !fragment.paintable().visible_for_hit_testing()) continue; auto fragment_absolute_rect = fragment.absolute_rect(); if (fragment_absolute_rect.contains(transformed_position_adjusted_by_scroll_offset)) { - if (fragment.paintable().hit_test(transformed_position_adjusted_by_scroll_offset, type, callback) == TraversalDecision::Break) + if (fragment.paintable().hit_test(transformed_position, type, callback) == TraversalDecision::Break) return TraversalDecision::Break; HitTestResult hit_test_result { const_cast(fragment.paintable()), fragment.index_in_node_for_point(transformed_position_adjusted_by_scroll_offset), 0, 0 }; if (callback(hit_test_result) == TraversalDecision::Break) @@ -1354,7 +1365,7 @@ TraversalDecision PaintableWithLines::hit_test(CSSPixelPoint position, HitTestTy } if (!stacking_context() && is_visible() && (!layout_node().is_anonymous() || layout_node().is_positioned()) - && absolute_border_box_rect().contains(position_adjusted_by_scroll_offset)) { + && absolute_border_box_rect().contains(transformed_position_adjusted_by_scroll_offset)) { if (callback(HitTestResult { const_cast(*this) }) == TraversalDecision::Break) return TraversalDecision::Break; } diff --git a/Libraries/LibWeb/Painting/StackingContext.cpp b/Libraries/LibWeb/Painting/StackingContext.cpp index 6915f905b6a..512c2c15005 100644 --- a/Libraries/LibWeb/Painting/StackingContext.cpp +++ b/Libraries/LibWeb/Painting/StackingContext.cpp @@ -384,13 +384,14 @@ void StackingContext::paint(DisplayListRecordingContext& context) const TraversalDecision StackingContext::hit_test(CSSPixelPoint position, HitTestType type, Function const& callback) const { - if (!paintable_box().is_visible()) + if (!paintable_box().visible_for_hit_testing()) return TraversalDecision::Continue; - CSSPixelPoint transform_origin = paintable_box().transform_origin(); + auto const inverse_transform = affine_transform_matrix().inverse().value_or({}); + auto const transform_origin = paintable_box().transform_origin(); // NOTE: This CSSPixels -> Float -> CSSPixels conversion is because we can't AffineTransform::map() a CSSPixelPoint. - auto offset_position = position.translated(-transform_origin).to_type(); - auto transformed_position = affine_transform_matrix().inverse().value_or({}).map(offset_position).to_type() + transform_origin; + auto const offset_position = position.translated(-transform_origin).to_type(); + auto const transformed_position = inverse_transform.map(offset_position).to_type() + transform_origin; // NOTE: Hit testing basically happens in reverse painting order. // https://www.w3.org/TR/CSS22/visuren.html#z-index @@ -406,29 +407,29 @@ TraversalDecision StackingContext::hit_test(CSSPixelPoint position, HitTestType } // 6. the child stacking contexts with stack level 0 and the positioned descendants with stack level 0. - for (auto const& paintable : m_positioned_descendants_and_stacking_contexts_with_stack_level_0.in_reverse()) { - if (paintable->stacking_context()) { - if (paintable->stacking_context()->hit_test(transformed_position, type, callback) == TraversalDecision::Break) + for (auto const& paintable_box : m_positioned_descendants_and_stacking_contexts_with_stack_level_0.in_reverse()) { + if (paintable_box->stacking_context()) { + if (paintable_box->stacking_context()->hit_test(transformed_position, type, callback) == TraversalDecision::Break) return TraversalDecision::Break; } else { - if (paintable->hit_test(transformed_position, type, callback) == TraversalDecision::Break) + if (paintable_box->hit_test(transformed_position, type, callback) == TraversalDecision::Break) return TraversalDecision::Break; } } // 5. the in-flow, inline-level, non-positioned descendants, including inline tables and inline blocks. if (paintable_box().layout_node().children_are_inline() && is(paintable_box().layout_node())) { - for (auto const* child = paintable_box().last_child(); child; child = child->previous_sibling()) { - if (child->is_inline() && !child->is_absolutely_positioned() && !child->has_stacking_context()) { - if (child->hit_test(transformed_position, type, callback) == TraversalDecision::Break) + for (auto const* paintable = paintable_box().last_child(); paintable; paintable = paintable->previous_sibling()) { + if (paintable->is_inline() && !paintable->is_absolutely_positioned() && !paintable->has_stacking_context()) { + if (paintable->hit_test(transformed_position, type, callback) == TraversalDecision::Break) return TraversalDecision::Break; } } } // 4. the non-positioned floats. - for (auto const& paintable : m_non_positioned_floating_descendants.in_reverse()) { - if (paintable->hit_test(transformed_position, type, callback) == TraversalDecision::Break) + for (auto const& paintable_box : m_non_positioned_floating_descendants.in_reverse()) { + if (paintable_box->hit_test(transformed_position, type, callback) == TraversalDecision::Break) return TraversalDecision::Break; } @@ -456,13 +457,15 @@ TraversalDecision StackingContext::hit_test(CSSPixelPoint position, HitTestType return TraversalDecision::Break; } - CSSPixelPoint enclosing_scroll_offset = paintable_box().cumulative_offset_of_enclosing_scroll_frame(); - - auto position_adjusted_by_scroll_offset = transformed_position.translated(-enclosing_scroll_offset); + auto const enclosing_scroll_offset = paintable_box().cumulative_offset_of_enclosing_scroll_frame(); + auto const raw_position_adjusted_by_scroll_offset = position.translated(-enclosing_scroll_offset); + // NOTE: This CSSPixels -> Float -> CSSPixels conversion is because we can't AffineTransform::map() a CSSPixelPoint. + auto const offset_position_adjusted_by_scroll_offset = raw_position_adjusted_by_scroll_offset.translated(-transform_origin).to_type(); + auto const transformed_position_adjusted_by_scroll_offset = inverse_transform.map(offset_position_adjusted_by_scroll_offset).to_type() + transform_origin; // 1. the background and borders of the element forming the stacking context. if (paintable_box().visible_for_hit_testing() - && paintable_box().absolute_border_box_rect().contains(position_adjusted_by_scroll_offset)) { + && paintable_box().absolute_border_box_rect().contains(transformed_position_adjusted_by_scroll_offset)) { if (callback({ const_cast(paintable_box()) }) == TraversalDecision::Break) return TraversalDecision::Break; } diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/CSS2/floats/hit-test-floats-003.txt b/Tests/LibWeb/Text/expected/wpt-import/css/CSS2/floats/hit-test-floats-003.txt index 7a92019a1b2..d46dbfedbc7 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/CSS2/floats/hit-test-floats-003.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/CSS2/floats/hit-test-floats-003.txt @@ -2,5 +2,5 @@ Harness status: OK Found 1 tests -1 Fail -Fail Miss float below something else \ No newline at end of file +1 Pass +Pass Miss float below something else \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-transforms/transform-hit-testing.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-transforms/transform-hit-testing.txt new file mode 100644 index 00000000000..794fef3c5da --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-transforms/transform-hit-testing.txt @@ -0,0 +1,10 @@ +Harness status: OK + +Found 5 tests + +5 Pass +Pass hit testing of rectangle with 'translate' and 'rotate' +Pass hit testing of rectangle with 'transform' +Pass hit testing of rectangle with 'translate' and 'rotate' and 'scale' and 'transform' +Pass hit testing of square with 'rotate' +Pass hit testing of square with 'scale' \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/wpt-import/css/css-transforms/transform-hit-testing.html b/Tests/LibWeb/Text/input/wpt-import/css/css-transforms/transform-hit-testing.html new file mode 100644 index 00000000000..94064fcd849 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/css/css-transforms/transform-hit-testing.html @@ -0,0 +1,153 @@ + +CSS Test (Transforms): Hit Testing + + + + + + + + + + + +