Commit e56348f0 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

[LayoutNG] Paint / hit-test block-fragmented relpos inlines correctly.

PaintLayer does one fragmentainer at a time, but we were painting and
hit-testing everything in each iteration, leading to DCHECK failures
(duplicate paint entries) for painting, and false hit-test matches.

Bug: 829028
Change-Id: Iededd014d84b4db9c29e4df6332033b0138da42b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490111Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821117}
parent 5b173084
...@@ -86,6 +86,16 @@ HitTestLocation::HitTestLocation(const HitTestLocation& other, ...@@ -86,6 +86,16 @@ HitTestLocation::HitTestLocation(const HitTestLocation& other,
Move(offset); Move(offset);
} }
HitTestLocation::HitTestLocation(const HitTestLocation& other,
wtf_size_t fragment_index)
: point_(other.point_),
bounding_box_(other.bounding_box_),
transformed_point_(other.transformed_point_),
transformed_rect_(other.transformed_rect_),
fragment_index_(fragment_index),
is_rect_based_(other.is_rect_based_),
is_rectilinear_(other.is_rectilinear_) {}
HitTestLocation::HitTestLocation(const HitTestLocation& other) = default; HitTestLocation::HitTestLocation(const HitTestLocation& other) = default;
HitTestLocation& HitTestLocation::operator=(const HitTestLocation& other) = HitTestLocation& HitTestLocation::operator=(const HitTestLocation& other) =
......
...@@ -61,12 +61,15 @@ class CORE_EXPORT HitTestLocation { ...@@ -61,12 +61,15 @@ class CORE_EXPORT HitTestLocation {
const PhysicalRect& bounding_box); const PhysicalRect& bounding_box);
HitTestLocation(const HitTestLocation&, const PhysicalOffset& offset); HitTestLocation(const HitTestLocation&, const PhysicalOffset& offset);
HitTestLocation(const HitTestLocation&, wtf_size_t fragment_index);
HitTestLocation(const HitTestLocation&); HitTestLocation(const HitTestLocation&);
HitTestLocation& operator=(const HitTestLocation&); HitTestLocation& operator=(const HitTestLocation&);
const PhysicalOffset& Point() const { return point_; } const PhysicalOffset& Point() const { return point_; }
IntPoint RoundedPoint() const { return RoundedIntPoint(point_); } IntPoint RoundedPoint() const { return RoundedIntPoint(point_); }
int FragmentIndex() const { return fragment_index_; }
// Rect-based hit test related methods. // Rect-based hit test related methods.
bool IsRectBasedTest() const { return is_rect_based_; } bool IsRectBasedTest() const { return is_rect_based_; }
bool IsRectilinear() const { return is_rectilinear_; } bool IsRectilinear() const { return is_rectilinear_; }
...@@ -104,6 +107,13 @@ class CORE_EXPORT HitTestLocation { ...@@ -104,6 +107,13 @@ class CORE_EXPORT HitTestLocation {
FloatPoint transformed_point_; FloatPoint transformed_point_;
FloatQuad transformed_rect_; FloatQuad transformed_rect_;
// Index of fragment (FragmentData) to hit-test. If it's -1, all fragments
// will be hit-tested. This is used to hit test items inside one NG block
// fragment at a time. This is necessary for relatively positioned non-atomic
// inlines. Note that this member is intentionally NOT copied when copying the
// object.
int fragment_index_ = -1;
bool is_rect_based_; bool is_rect_based_;
bool is_rectilinear_; bool is_rectilinear_;
}; };
......
...@@ -1115,7 +1115,22 @@ bool LayoutInline::NodeAtPoint(HitTestResult& result, ...@@ -1115,7 +1115,22 @@ bool LayoutInline::NodeAtPoint(HitTestResult& result,
// which case the element must have self painting layer. // which case the element must have self painting layer.
DCHECK(HasSelfPaintingLayer()); DCHECK(HasSelfPaintingLayer());
NGInlineCursor cursor; NGInlineCursor cursor;
for (cursor.MoveTo(*this); cursor; cursor.MoveToNextForSameLayoutObject()) { cursor.MoveTo(*this);
if (!cursor)
return false;
int target_fragment_idx = hit_test_location.FragmentIndex();
DCHECK(!CanTraversePhysicalFragments() || target_fragment_idx >= 0);
// Convert from inline fragment index to container fragment index, as the
// inline may not start in the first fragment generated for the inline
// formatting context.
if (target_fragment_idx != -1)
target_fragment_idx += cursor.CurrentContainerFragmentIndex();
for (; cursor; cursor.MoveToNextForSameLayoutObject()) {
if (target_fragment_idx != -1 &&
wtf_size_t(target_fragment_idx) !=
cursor.CurrentContainerFragmentIndex())
continue;
if (const NGPaintFragment* paint_fragment = if (const NGPaintFragment* paint_fragment =
cursor.Current().PaintFragment()) { cursor.Current().PaintFragment()) {
// NGBoxFragmentPainter::NodeAtPoint() takes an offset that is // NGBoxFragmentPainter::NodeAtPoint() takes an offset that is
......
...@@ -254,6 +254,10 @@ class CORE_EXPORT NGInlineCursor { ...@@ -254,6 +254,10 @@ class CORE_EXPORT NGInlineCursor {
// Returns the |LayoutBlockFlow| containing this cursor. // Returns the |LayoutBlockFlow| containing this cursor.
const LayoutBlockFlow* GetLayoutBlockFlow() const; const LayoutBlockFlow* GetLayoutBlockFlow() const;
// Return the index of the current physical box fragment of the containing
// block. An inline formatting context may be block fragmented.
wtf_size_t CurrentContainerFragmentIndex() const { return fragment_index_; }
// //
// Functions to query the current position. // Functions to query the current position.
// //
......
...@@ -380,7 +380,17 @@ void NGInlineBoxFragmentPainter::PaintAllFragments( ...@@ -380,7 +380,17 @@ void NGInlineBoxFragmentPainter::PaintAllFragments(
NGInlineCursor cursor(*block_flow); NGInlineCursor cursor(*block_flow);
cursor.MoveTo(layout_inline); cursor.MoveTo(layout_inline);
if (!cursor)
return;
// Convert from inline fragment index to container fragment index, as the
// inline may not start in the first fragment generated for the inline
// formatting context.
wtf_size_t target_fragment_idx =
cursor.CurrentContainerFragmentIndex() +
paint_info.context.GetPaintController().CurrentFragment();
for (; cursor; cursor.MoveToNextForSameLayoutObject()) { for (; cursor; cursor.MoveToNextForSameLayoutObject()) {
if (target_fragment_idx != cursor.CurrentContainerFragmentIndex())
continue;
const NGFragmentItem* item = cursor.CurrentItem(); const NGFragmentItem* item = cursor.CurrentItem();
DCHECK(item); DCHECK(item);
const NGPhysicalBoxFragment* box_fragment = item->BoxFragment(); const NGPhysicalBoxFragment* box_fragment = item->BoxFragment();
......
...@@ -2254,8 +2254,18 @@ bool PaintLayer::HitTestContentsForFragments( ...@@ -2254,8 +2254,18 @@ bool PaintLayer::HitTestContentsForFragments(
inside_clip_rect = true; inside_clip_rect = true;
PhysicalOffset fragment_offset = offset; PhysicalOffset fragment_offset = offset;
fragment_offset += fragment.layer_bounds.offset; fragment_offset += fragment.layer_bounds.offset;
// When hit-testing a relatively positioned inline, we'll search for it in
// each fragment of the containing block. Each fragment has its own offset,
// and we need to do one fragment at a time.
int limit_to_fragment = -1;
if (GetLayoutObject().IsLayoutInline() &&
GetLayoutObject().CanTraversePhysicalFragments())
limit_to_fragment = i;
HitTestLocation location_for_fragment(hit_test_location, limit_to_fragment);
if (HitTestContents(result, fragment.physical_fragment, fragment_offset, if (HitTestContents(result, fragment.physical_fragment, fragment_offset,
hit_test_location, hit_test_filter)) location_for_fragment, hit_test_filter))
return true; return true;
} }
......
...@@ -990,6 +990,8 @@ virtual/layout_ng_block_frag/external/wpt/css/css-break/fieldset-004.html [ Pass ...@@ -990,6 +990,8 @@ virtual/layout_ng_block_frag/external/wpt/css/css-break/fieldset-004.html [ Pass
virtual/layout_ng_block_frag/external/wpt/css/css-break/forced-break-at-fragmentainer-start-000.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-break/forced-break-at-fragmentainer-start-000.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-break/overflowed-block-with-no-room-after-000.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-break/overflowed-block-with-no-room-after-000.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-break/overflowed-block-with-no-room-after-001.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-break/overflowed-block-with-no-room-after-001.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-break/relpos-inline-hit-testing.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-break/relpos-inline.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-break/tall-line-in-short-fragmentainer-000.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-break/tall-line-in-short-fragmentainer-000.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-break/tall-line-in-short-fragmentainer-001.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-break/tall-line-in-short-fragmentainer-001.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-break/tall-line-in-short-fragmentainer-002.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-break/tall-line-in-short-fragmentainer-002.html [ Pass ]
...@@ -3209,6 +3211,8 @@ crbug.com/829028 external/wpt/css/css-break/fieldset-004.html [ Failure ] ...@@ -3209,6 +3211,8 @@ crbug.com/829028 external/wpt/css/css-break/fieldset-004.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/forced-break-at-fragmentainer-start-000.html [ Failure ] crbug.com/829028 external/wpt/css/css-break/forced-break-at-fragmentainer-start-000.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/overflowed-block-with-no-room-after-001.html [ Failure ] crbug.com/829028 external/wpt/css/css-break/overflowed-block-with-no-room-after-001.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/overflowed-block-with-no-room-after-000.html [ Failure ] crbug.com/829028 external/wpt/css/css-break/overflowed-block-with-no-room-after-000.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/relpos-inline-hit-testing.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/relpos-inline.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/tall-line-in-short-fragmentainer-000.html [ Failure ] crbug.com/829028 external/wpt/css/css-break/tall-line-in-short-fragmentainer-000.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/tall-line-in-short-fragmentainer-001.html [ Failure ] crbug.com/829028 external/wpt/css/css-break/tall-line-in-short-fragmentainer-001.html [ Failure ]
crbug.com/829028 external/wpt/css/css-break/tall-line-in-short-fragmentainer-002.html [ Failure ] crbug.com/829028 external/wpt/css/css-break/tall-line-in-short-fragmentainer-002.html [ Failure ]
......
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-break-3/#transforms">
<link rel="help" href="https://www.w3.org/TR/CSS22/visuren.html#relative-positioning">
<link rel="match" href="relpos-inline-ref.html">
<style>
body {
margin: 8px;
}
#target {
position: relative;
left: -100px;
top: 100px;
border: 1px solid;
background: hotpink;
}
</style>
<div style="columns:4; column-gap:0; column-fill:auto; line-height:30px; width:600px; height:60px; orphans:1; widows:1;">
<br><br><br>
<span id="target">
line1<br>
line2<br>
line3<br>
line4<br>
</span>
</div>
<div id="log" style="margin-top:100px;"></div>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
test(()=> { assert_equals(document.elementFromPoint(58, 153), target); }, "line1");
test(()=> { assert_equals(document.elementFromPoint(208, 123), target); }, "line2");
test(()=> { assert_equals(document.elementFromPoint(208, 153), target); }, "line3");
test(()=> { assert_equals(document.elementFromPoint(358, 123), target); }, "line4");
test(()=> { assert_not_equals(document.elementFromPoint(58, 123), target); }, "Above line1");
test(()=> { assert_not_equals(document.elementFromPoint(358, 153), target); }, "Below line4");
test(()=> { assert_not_equals(document.elementFromPoint(158, 48), target); }, "line1 before offsetting");
</script>
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<style>
span {
position: relative;
left: -100px;
top: 100px;
border: 1px solid;
background:hotpink;
}
.fakecol {
float: left;
width: 150px;
height: 100%;
}
</style>
<div style="line-height:30px; height:60px;">
<div class="fakecol"></div>
<div class="fakecol">
<br>
<span style="border-right:none;">
line1<br>
</span>
</div>
<div class="fakecol">
<span style="border-left:none; border-right:none;">
line2<br>
line3<br>
</span>
</div>
<div class="fakecol">
<span style="border-left:none;">
line4<br>
</span>
</div>
</div>
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-break-3/#transforms">
<link rel="help" href="https://www.w3.org/TR/CSS22/visuren.html#relative-positioning">
<link rel="match" href="relpos-inline-ref.html">
<div style="columns:4; column-gap:0; column-fill:auto; line-height:30px; width:600px; height:60px; orphans:1; widows:1;">
<br><br><br>
<span style="position:relative; left:-100px; top:100px; border:1px solid; background:hotpink;">
line1<br>
line2<br>
line3<br>
line4<br>
</span>
</div>
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