Skip to content

Commit 67d6bda

Browse files
mstenshoChromium LUCI CQ
authored andcommitted
Check for intersection earlier when hit-testing.
Simply creating an NGBoxFragmentPainter has a "significant" cost. When hit-testing, we're typically going to miss most of the elements (in search of the one to hit (along with its ancestors)), so this needs to be as cheap as possible. Therefore, check for intersection earlier, and only create an NGBoxFragmentPainter for hit-testing if it's likely at all that we're going to hit something inside the fragment. This fixes most of the performance regression in the following tests: perf_tests/events/hit-test-lots-of-layers.html [1] perf_tests/layout/hittest-block-children.html [2] [1] Same, or possibly slightly faster (0.9%) [2] Still slower (9%) I suspect that perf_tests/layout/hittest-block-children.html is still slower because of the overhead of creating NGInlineCursor and NGInlineBackwardCursor objects without checking if we intersect first. See crbug.com/1240036 We are also performing duplicate intersection checks in some cases now, which has a tiny (but I *think* it's measurable) cost. With this change, we'll check for intersection before creating an NGBoxFragmentPainter, but, if we intersect, we'll check again nevertheless in e.g. NGBoxFragmentPainter::NodeAtPoint(). Ideally, we should turn the latter into a DCHECK, but there are still a few call-sites that would need to be updated first. Added a couple of TODOs for now. It may be that we could wait until the legacy layout engine is gone, before following up, as that would be slightly easier (and the gain isn't huge anyway). Bug: 1240034, 1240036 Change-Id: I11a91038de180b30d99263038f65c7c8bf6fa923 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3113968 Reviewed-by: Koji Ishii <[email protected]> Commit-Queue: Morten Stenshorne <[email protected]> Cr-Commit-Position: refs/heads/main@{#914849}
1 parent a6a88b4 commit 67d6bda

File tree

2 files changed

+26
-12
lines changed

2 files changed

+26
-12
lines changed

third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,9 @@ bool HitTestAllPhasesInFragment(const NGPhysicalBoxFragment& fragment,
306306
*result, hit_test_location, accumulated_offset);
307307
}
308308

309+
if (!fragment.MayIntersect(*result, hit_test_location, accumulated_offset))
310+
return false;
311+
309312
return NGBoxFragmentPainter(To<NGPhysicalBoxFragment>(fragment))
310313
.HitTestAllPhases(*result, hit_test_location, accumulated_offset);
311314
}
@@ -320,6 +323,9 @@ bool NodeAtPointInFragment(const NGPhysicalBoxFragment& fragment,
320323
*result, hit_test_location, accumulated_offset, action);
321324
}
322325

326+
if (!fragment.MayIntersect(*result, hit_test_location, accumulated_offset))
327+
return false;
328+
323329
return NGBoxFragmentPainter(fragment).NodeAtPoint(*result, hit_test_location,
324330
accumulated_offset, action);
325331
}
@@ -1870,6 +1876,8 @@ bool NGBoxFragmentPainter::NodeAtPoint(HitTestResult& result,
18701876
bool NGBoxFragmentPainter::NodeAtPoint(const HitTestContext& hit_test,
18711877
const PhysicalOffset& physical_offset) {
18721878
const NGPhysicalBoxFragment& fragment = PhysicalFragment();
1879+
// TODO(mstensho): Make sure that we never create an NGBoxFragmentPainter for
1880+
// a fragment that doesn't intersect, and turn this into a DCHECK.
18731881
if (!fragment.MayIntersect(*hit_test.result, hit_test.location,
18741882
physical_offset))
18751883
return false;
@@ -1985,6 +1993,9 @@ bool NGBoxFragmentPainter::HitTestAllPhases(
19851993
const HitTestLocation& hit_test_location,
19861994
const PhysicalOffset& accumulated_offset,
19871995
HitTestFilter hit_test_filter) {
1996+
// TODO(mstensho): Make sure that we never create an NGBoxFragmentPainter for
1997+
// a fragment that doesn't intersect, and DCHECK for that here.
1998+
19881999
// Logic taken from LayoutObject::HitTestAllPhases().
19892000
HitTestContext hit_test(kHitTestForeground, hit_test_location,
19902001
accumulated_offset, &result);
@@ -2139,6 +2150,10 @@ bool NGBoxFragmentPainter::HitTestChildBoxFragment(
21392150
const NGFragmentItem* item = cursor.Current().Item();
21402151
DCHECK(item);
21412152
DCHECK_EQ(item->BoxFragment(), &fragment);
2153+
if (!fragment.MayIntersect(*hit_test.result, hit_test.location,
2154+
physical_offset))
2155+
return false;
2156+
21422157
if (fragment.IsInlineBox()) {
21432158
return NGBoxFragmentPainter(cursor, *item, fragment)
21442159
.NodeAtPoint(hit_test, physical_offset);
@@ -2416,16 +2431,10 @@ bool NGBoxFragmentPainter::HitTestFloatingChildren(
24162431
// We need to properly visit this fragment for hit-testing, rather than
24172432
// jumping directly to its children (which is what we normally do when
24182433
// looking for floats), in order to set up the clip rectangle.
2419-
if (child_fragment.CanTraverse()) {
2420-
if (NGBoxFragmentPainter(To<NGPhysicalBoxFragment>(child_fragment))
2421-
.NodeAtPoint(*hit_test.result, hit_test.location, child_offset,
2422-
kHitTestFloat))
2423-
return true;
2424-
} else if (child_fragment.GetMutableLayoutObject()->NodeAtPoint(
2425-
*hit_test.result, hit_test.location, child_offset,
2426-
kHitTestFloat)) {
2434+
if (NodeAtPointInFragment(To<NGPhysicalBoxFragment>(child_fragment),
2435+
hit_test.location, child_offset, kHitTestFloat,
2436+
hit_test.result))
24272437
return true;
2428-
}
24292438
continue;
24302439
}
24312440

third_party/blink/renderer/core/paint/paint_layer.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2506,9 +2506,14 @@ bool PaintLayer::HitTestContents(HitTestResult& result,
25062506

25072507
bool did_hit;
25082508
if (physical_fragment) {
2509-
did_hit = NGBoxFragmentPainter(*physical_fragment)
2510-
.HitTestAllPhases(result, hit_test_location, fragment_offset,
2511-
hit_test_filter);
2509+
if (!physical_fragment->MayIntersect(result, hit_test_location,
2510+
fragment_offset)) {
2511+
did_hit = false;
2512+
} else {
2513+
did_hit = NGBoxFragmentPainter(*physical_fragment)
2514+
.HitTestAllPhases(result, hit_test_location,
2515+
fragment_offset, hit_test_filter);
2516+
}
25122517
} else {
25132518
did_hit = GetLayoutObject().HitTestAllPhases(
25142519
result, hit_test_location, fragment_offset, hit_test_filter);

0 commit comments

Comments
 (0)