Commit a748ef4a authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

[LayoutNG] Fix offset accumulation for hit test fallback

Current LayoutNG hit test code computes wrong |accumulated_offset|
when falling back to legacy. It tries to maintain an additional
offset for legacy, but fails to do so.

This patch computes the fallback offset directly from NG offset, which
fixes legacy fallback, and also removes the legacy offset maintenance
code.

Bug: 811502
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I06b2df66dc2a36efaf32077ea26a3abb4a26eb49
Reviewed-on: https://chromium-review.googlesource.com/1089800Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565070}
parent 43164af2
...@@ -1004,7 +1004,6 @@ crbug.com/591099 svg/custom/inline-svg-use-available-width-in-stf.html [ Failure ...@@ -1004,7 +1004,6 @@ crbug.com/591099 svg/custom/inline-svg-use-available-width-in-stf.html [ Failure
crbug.com/591099 svg/custom/object-sizing-no-width-height.xhtml [ Failure ] crbug.com/591099 svg/custom/object-sizing-no-width-height.xhtml [ Failure ]
crbug.com/591099 svg/custom/stf-container-with-intrinsic-ratio-svg.html [ Failure ] crbug.com/591099 svg/custom/stf-container-with-intrinsic-ratio-svg.html [ Failure ]
crbug.com/591099 svg/custom/text-match-highlight.html [ Failure ] crbug.com/591099 svg/custom/text-match-highlight.html [ Failure ]
crbug.com/591099 svg/custom/use-event-retargeting.html [ Failure ]
crbug.com/591099 svg/filters/feTurbulence-bad-seeds.html [ Failure ] crbug.com/591099 svg/filters/feTurbulence-bad-seeds.html [ Failure ]
crbug.com/591099 svg/in-html/sizing/svg-inline.html [ Failure ] crbug.com/591099 svg/in-html/sizing/svg-inline.html [ Failure ]
crbug.com/714962 svg/text/tspan-multiple-outline.svg [ Failure ] crbug.com/714962 svg/text/tspan-multiple-outline.svg [ Failure ]
......
...@@ -908,7 +908,7 @@ bool LayoutInline::NodeAtPoint(HitTestResult& result, ...@@ -908,7 +908,7 @@ bool LayoutInline::NodeAtPoint(HitTestResult& result,
fragment->Parent()->InlineOffsetToContainerBox().ToLayoutPoint(); fragment->Parent()->InlineOffsetToContainerBox().ToLayoutPoint();
if (NGBoxFragmentPainter(*fragment).NodeAtPoint( if (NGBoxFragmentPainter(*fragment).NodeAtPoint(
result, location_in_container, adjusted_location, result, location_in_container, adjusted_location,
accumulated_offset, hit_test_action)) hit_test_action))
return true; return true;
} }
return false; return false;
......
...@@ -312,7 +312,6 @@ bool LayoutNGMixin<Base>::NodeAtPoint( ...@@ -312,7 +312,6 @@ bool LayoutNGMixin<Base>::NodeAtPoint(
return true; return true;
return NGBlockFlowPainter(*this).NodeAtPoint(result, location_in_container, return NGBlockFlowPainter(*this).NodeAtPoint(result, location_in_container,
accumulated_offset,
accumulated_offset, action); accumulated_offset, action);
} }
......
...@@ -29,12 +29,10 @@ bool NGBlockFlowPainter::NodeAtPoint( ...@@ -29,12 +29,10 @@ bool NGBlockFlowPainter::NodeAtPoint(
HitTestResult& result, HitTestResult& result,
const HitTestLocation& location_in_container, const HitTestLocation& location_in_container,
const LayoutPoint& accumulated_offset, const LayoutPoint& accumulated_offset,
const LayoutPoint& accumulated_offset_for_legacy,
HitTestAction action) { HitTestAction action) {
if (const NGPaintFragment* paint_fragment = block_.PaintFragment()) { if (const NGPaintFragment* paint_fragment = block_.PaintFragment()) {
return NGBoxFragmentPainter(*paint_fragment) return NGBoxFragmentPainter(*paint_fragment)
.NodeAtPoint(result, location_in_container, accumulated_offset, .NodeAtPoint(result, location_in_container, accumulated_offset, action);
accumulated_offset_for_legacy, action);
} }
return false; return false;
} }
......
...@@ -32,7 +32,6 @@ class NGBlockFlowPainter { ...@@ -32,7 +32,6 @@ class NGBlockFlowPainter {
bool NodeAtPoint(HitTestResult&, bool NodeAtPoint(HitTestResult&,
const HitTestLocation& location_in_container, const HitTestLocation& location_in_container,
const LayoutPoint& accumulated_offset, const LayoutPoint& accumulated_offset,
const LayoutPoint& accumulated_offset_for_legacy,
HitTestAction); HitTestAction);
private: private:
......
...@@ -55,6 +55,25 @@ bool ShouldPaintBoxFragmentBorders(const LayoutObject& object) { ...@@ -55,6 +55,25 @@ bool ShouldPaintBoxFragmentBorders(const LayoutObject& object) {
return !ToLayoutTableCell(object).Table()->ShouldCollapseBorders(); return !ToLayoutTableCell(object).Table()->ShouldCollapseBorders();
} }
// In hit testing, legacy and NG assumes different semantics for the
// |accumulated_offset| parameter:
// - Legacy: offset of the containing block of |child| (excluding |child| when
// it is block by itslef) in the paint layer.
// - NG: offset of the parent fragment in the paint layer
// This function converts the NG offset to legacy so that hit testing can fall
// back to legacy with the correct |accumulated_offset|.
LayoutPoint FallbackAccumulatedOffset(const NGPaintFragment& child,
const LayoutPoint& accumulated_offset) {
DCHECK(child.Parent());
const NGPaintFragment& parent = *child.Parent();
if (parent.PhysicalFragment().IsBlockFlow())
return accumulated_offset;
LayoutPoint parent_inline_offset =
parent.InlineOffsetToContainerBox().ToLayoutPoint();
return accumulated_offset -
LayoutSize(parent_inline_offset.X(), parent_inline_offset.Y());
}
} // anonymous namespace } // anonymous namespace
NGBoxFragmentPainter::NGBoxFragmentPainter(const NGPaintFragment& box) NGBoxFragmentPainter::NGBoxFragmentPainter(const NGPaintFragment& box)
...@@ -817,7 +836,6 @@ bool NGBoxFragmentPainter::NodeAtPoint( ...@@ -817,7 +836,6 @@ bool NGBoxFragmentPainter::NodeAtPoint(
HitTestResult& result, HitTestResult& result,
const HitTestLocation& location_in_container, const HitTestLocation& location_in_container,
const LayoutPoint& accumulated_offset, const LayoutPoint& accumulated_offset,
const LayoutPoint& accumulated_offset_for_legacy,
HitTestAction action) { HitTestAction action) {
// TODO(eae): Switch to using NG geometry types. // TODO(eae): Switch to using NG geometry types.
LayoutSize offset; LayoutSize offset;
...@@ -859,13 +877,9 @@ bool NGBoxFragmentPainter::NodeAtPoint( ...@@ -859,13 +877,9 @@ bool NGBoxFragmentPainter::NodeAtPoint(
} }
} }
// TODO(layout-dev): Accumulate |accumulated_offset_for_legacy| properly.
LayoutPoint adjusted_location_for_legacy =
accumulated_offset_for_legacy + offset;
if (!skip_children && if (!skip_children &&
HitTestChildren(result, box_fragment_.Children(), location_in_container, HitTestChildren(result, box_fragment_.Children(), location_in_container,
adjusted_location, adjusted_location_for_legacy, adjusted_location, action)) {
action)) {
return true; return true;
} }
...@@ -950,7 +964,6 @@ bool NGBoxFragmentPainter::HitTestChildren( ...@@ -950,7 +964,6 @@ bool NGBoxFragmentPainter::HitTestChildren(
const Vector<std::unique_ptr<NGPaintFragment>>& children, const Vector<std::unique_ptr<NGPaintFragment>>& children,
const HitTestLocation& location_in_container, const HitTestLocation& location_in_container,
const LayoutPoint& accumulated_offset, const LayoutPoint& accumulated_offset,
const LayoutPoint& accumulated_offset_for_legacy,
HitTestAction action) { HitTestAction action) {
for (auto iter = children.rbegin(); iter != children.rend(); iter++) { for (auto iter = children.rbegin(); iter != children.rend(); iter++) {
const std::unique_ptr<NGPaintFragment>& child = *iter; const std::unique_ptr<NGPaintFragment>& child = *iter;
...@@ -961,25 +974,22 @@ bool NGBoxFragmentPainter::HitTestChildren( ...@@ -961,25 +974,22 @@ bool NGBoxFragmentPainter::HitTestChildren(
bool stop_hit_testing = false; bool stop_hit_testing = false;
if (fragment.Type() == NGPhysicalFragment::kFragmentBox) { if (fragment.Type() == NGPhysicalFragment::kFragmentBox) {
if (FragmentRequiresLegacyFallback(fragment)) { if (FragmentRequiresLegacyFallback(fragment)) {
LayoutPoint fallback_accumulated_offset =
FallbackAccumulatedOffset(*child, accumulated_offset);
stop_hit_testing = fragment.GetLayoutObject()->NodeAtPoint( stop_hit_testing = fragment.GetLayoutObject()->NodeAtPoint(
result, location_in_container, accumulated_offset_for_legacy, result, location_in_container, fallback_accumulated_offset, action);
action);
} else { } else {
// TODO(layout-dev): Accumulate |accumulated_offset_for_legacy|
// properly.
stop_hit_testing = NGBoxFragmentPainter(*child).NodeAtPoint( stop_hit_testing = NGBoxFragmentPainter(*child).NodeAtPoint(
result, location_in_container, accumulated_offset, result, location_in_container, accumulated_offset, action);
accumulated_offset_for_legacy, action);
} }
} else if (fragment.Type() == NGPhysicalFragment::kFragmentLineBox) { } else if (fragment.Type() == NGPhysicalFragment::kFragmentLineBox) {
const LayoutSize line_box_offset(fragment.Offset().left, const LayoutSize line_box_offset(fragment.Offset().left,
fragment.Offset().top); fragment.Offset().top);
const LayoutPoint adjusted_offset = accumulated_offset + line_box_offset; const LayoutPoint adjusted_offset = accumulated_offset + line_box_offset;
// TODO(layout-dev): Accumulate |accumulated_offset_for_legacy| properly. stop_hit_testing =
stop_hit_testing = HitTestChildren(result, child->Children(), HitTestChildren(result, child->Children(), location_in_container,
location_in_container, adjusted_offset, adjusted_offset, action);
accumulated_offset_for_legacy, action);
} else if (fragment.Type() == NGPhysicalFragment::kFragmentText) { } else if (fragment.Type() == NGPhysicalFragment::kFragmentText) {
// TODO(eae): Should this hit test on the text itself or the containing // TODO(eae): Should this hit test on the text itself or the containing
......
...@@ -40,7 +40,6 @@ class NGBoxFragmentPainter : public BoxPainterBase { ...@@ -40,7 +40,6 @@ class NGBoxFragmentPainter : public BoxPainterBase {
bool NodeAtPoint(HitTestResult&, bool NodeAtPoint(HitTestResult&,
const HitTestLocation& location_in_container, const HitTestLocation& location_in_container,
const LayoutPoint& accumulated_offset, const LayoutPoint& accumulated_offset,
const LayoutPoint& accumulated_offset_for_legacy,
HitTestAction); HitTestAction);
protected: protected:
...@@ -118,7 +117,6 @@ class NGBoxFragmentPainter : public BoxPainterBase { ...@@ -118,7 +117,6 @@ class NGBoxFragmentPainter : public BoxPainterBase {
const Vector<std::unique_ptr<NGPaintFragment>>&, const Vector<std::unique_ptr<NGPaintFragment>>&,
const HitTestLocation& location_in_container, const HitTestLocation& location_in_container,
const LayoutPoint& accumulated_offset, const LayoutPoint& accumulated_offset,
const LayoutPoint& accumulated_offset_for_legacy,
HitTestAction); HitTestAction);
bool HitTestTextFragment(HitTestResult&, bool HitTestTextFragment(HitTestResult&,
const NGPaintFragment&, const NGPaintFragment&,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment