Skip to content

GitLab

T
tangled
Commit f5eb2e72 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot
Browse files
Options

Heurstically use fragments in |MoveToIncludingCulledInline| 

crbug.com/1111154 discovered that |getBoundingClientRect()|
for culled inline has regressed a lot in LayoutNG. r793989
crrev.com/c/2332083 fixed this by traversing |LayoutObject|
tree instead of fragments.

This change, however, regressed a hit-testing performance
case previously raised at crbug.com/1008523, because hit-
testing uses |MoveToIncludingCulledInline| for every leaf, by
limiting the search range to the containing line. This can
easily lead to thousands of |MoveToIncludingCulledInline| for
one hit-test request.

This patch changes the function to revert the change if the
search range is limited. In this case, most of |LayoutObject|
descendants may not be in the search range, and that the
searching fragments can be faster than searching
|LayoutObject| descendants.

                Case A Case B
Before r793989:     19    420
ToT:             2,556  9,064
This CL:            16    403

Numbers in milliseconds
Case A: test attached to crbug/1008523
Case B: culled-inline-hittest.html in this CL

Note, the design to call |MoveToIncludingCulledInline| so
many times is questionable, but this CL does not try to
change it to reduce the risk of regressions.

Bug: 1127976, 1008523
Change-Id: Id21dd07cba0d0746b779b913ae9f180a77e23c46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2409734Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807114}
parent 79486b9c
Hide whitespace changes
Inline Side-by-side
Showing with 119 additions and 2 deletions
third_party/blink/perf_tests/layout/culled-inline-hittest.html 0 → 100644
View file @ f5eb2e72
<!DOCTYPE html>
<title>Hit-testing culled inline</title>
<script src="../resources/runner.js"></script>
<body>
<div id="container" style="width: 800px"></div>
<script>
let span_count = 0;
function createTree(container, children_list) {
let children = children_list.shift();
for (let i = 0; i < children; ++i) {
const span = document.createElement('span');
span.appendChild(document.createTextNode('1234567'));
container.appendChild(span);
container.appendChild(document.createTextNode(' '));
++span_count;
}
if (children_list.length) {
const span = document.createElement('span');
createTree(span, children_list);
container.appendChild(span);
}
}
function setup() {
container.textContent = '';
createTree(container, [1, 1, 1, 1, 3000]);
}
function test() {
let bounds = container.getBoundingClientRect();
for (let y = 0; y < 200; y += 10) {
for (let x = bounds.x + 5; x < bounds.x + 800; x += 100)
document.elementFromPoint(x, y);
document.elementFromPoint(bounds.right - 2, y);
}
}
function run() {
PerfTestRunner.measureTime({
description: `Measures performance of hit-testing ${span_count} culled inline.`,
run: test
});
}
setup();
run();
</script>
</body>
third_party/blink/renderer/core/layout/ng/inline/ng_inline_cursor.cc
View file @ f5eb2e72
......@@ -269,6 +269,32 @@ bool NGInlineCursorPosition::IsInlineLeaf() const {
return !IsListMarker();
}
bool NGInlineCursorPosition::IsPartOfCulledInlineBox(
const LayoutInline& layout_inline) const {
DCHECK(!layout_inline.ShouldCreateBoxFragment());
DCHECK(*this);
const LayoutObject* const layout_object = GetLayoutObject();
// We use |IsInline()| to exclude floating and out-of-flow objects.
if (!layout_object || !layout_object->IsInline() ||
layout_object->IsAtomicInlineLevel())
return false;
DCHECK(!layout_object->IsFloatingOrOutOfFlowPositioned());
DCHECK(!BoxFragment() || !BoxFragment()->IsFormattingContextRoot());
for (const LayoutObject* parent = layout_object->Parent(); parent;
parent = parent->Parent()) {
// Children of culled inline should be included.
if (parent == &layout_inline)
return true;
// Grand children should be included only if children are also culled.
if (const auto* parent_layout_inline = ToLayoutInlineOrNull(parent)) {
if (!parent_layout_inline->ShouldCreateBoxFragment())
continue;
}
return false;
}
return false;
}
bool NGInlineCursor::IsLastLineInInlineBlock() const {
DCHECK(Current().IsLineBox());
if (!GetLayoutBlockFlow()->IsAtomicInlineLevel())
......@@ -1665,14 +1691,23 @@ const LayoutObject* NGInlineCursor::CulledInlineTraversal::Find(
return nullptr;
}
void NGInlineCursor::CulledInlineTraversal::SetUseFragmentTree(
const LayoutInline& layout_inline) {
layout_inline_ = &layout_inline;
use_fragment_tree_ = true;
}
const LayoutObject* NGInlineCursor::CulledInlineTraversal::MoveToFirstFor(
const LayoutInline& layout_inline) {
layout_inline_ = &layout_inline;
use_fragment_tree_ = false;
current_object_ = Find(layout_inline.FirstChild());
return current_object_;
}
const LayoutObject* NGInlineCursor::CulledInlineTraversal::MoveToNext() {
if (!current_object_)
return nullptr;
current_object_ =
Find(current_object_->NextInPreOrderAfterChildren(layout_inline_));
return current_object_;
......@@ -1680,6 +1715,19 @@ const LayoutObject* NGInlineCursor::CulledInlineTraversal::MoveToNext() {
void NGInlineCursor::MoveToFirstForCulledInline(
const LayoutInline& layout_inline) {
// When |this| is a descendant cursor, |this| may be limited to a very small
// subset of the |LayoutObject| descendants, and that traversing
// |LayoutObject| descendants is much more expensive. Prefer checking every
// fragment in that case.
if (IsDescendantsCursor()) {
culled_inline_.SetUseFragmentTree(layout_inline);
DCHECK(!CanMoveAcrossFragmentainer());
MoveToFirst();
while (Current() && !Current().IsPartOfCulledInlineBox(layout_inline))
MoveToNext();
return;
}
if (const LayoutObject* layout_object =
culled_inline_.MoveToFirstFor(layout_inline)) {
MoveTo(*layout_object);
......@@ -1691,6 +1739,16 @@ void NGInlineCursor::MoveToFirstForCulledInline(
void NGInlineCursor::MoveToNextForCulledInline() {
DCHECK(culled_inline_);
if (culled_inline_.UseFragmentTree()) {
const LayoutInline* layout_inline = culled_inline_.GetLayoutInline();
DCHECK(layout_inline);
DCHECK(!CanMoveAcrossFragmentainer());
do {
MoveToNext();
} while (Current() && !Current().IsPartOfCulledInlineBox(*layout_inline));
return;
}
MoveToNextForSameLayoutObjectExceptCulledInline();
// If we're at the end of fragments for the current |LayoutObject| that
// contributes to the current culled inline, find the next |LayoutObject|.
......
third_party/blink/renderer/core/layout/ng/inline/ng_inline_cursor.h
View file @ f5eb2e72
......@@ -195,6 +195,9 @@ class CORE_EXPORT NGInlineCursorPosition {
item_ = nullptr;
}
// True if current position is part of culled inline box |layout_inline|.
bool IsPartOfCulledInlineBox(const LayoutInline& layout_inline) const;
const NGPaintFragment* paint_fragment_ = nullptr;
const NGFragmentItem* item_ = nullptr;
ItemsSpan::iterator item_iter_;
......@@ -527,8 +530,13 @@ class CORE_EXPORT NGInlineCursor {
public:
CulledInlineTraversal() = default;
explicit operator bool() const { return current_object_; }
void Reset() { current_object_ = nullptr; }
const LayoutInline* GetLayoutInline() const { return layout_inline_; }
explicit operator bool() const { return layout_inline_; }
void Reset() { layout_inline_ = nullptr; }
bool UseFragmentTree() const { return use_fragment_tree_; }
void SetUseFragmentTree(const LayoutInline& layout_inline);
// Returns first/next |LayoutObject| that contribute to |layout_inline|.
const LayoutObject* MoveToFirstFor(const LayoutInline& layout_inline);
......@@ -539,6 +547,7 @@ class CORE_EXPORT NGInlineCursor {
const LayoutObject* current_object_ = nullptr;
const LayoutInline* layout_inline_ = nullptr;
bool use_fragment_tree_ = false;
};
void MoveToFirstForCulledInline(const LayoutInline& layout_inline);
......
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