massive lines of text cause halting when touch down
Reported by
oicqc...@gmail.com,
Dec 23 2016
|
|||||||||||
Issue descriptionChrome Version : 55.0.2883.91 URLs (if applicable) : http://www.19lou.com/wap/board-48471392532152117-thread-49051392640627310-1.html Other browsers tested: Add OK or FAIL, along with the version, after other browsers where you have tested this issue: Safari: OK Firefox: OK IE: What steps will reproduce the problem? (1) open the url above, or the attached reduced test case page, with an Android mobile (2) scroll to some areas containing massive lines of text (3) touch down and up on the texts, than try to scroll What is the expected result? Page scrolls as normal. What happens instead? It halts a few seconds before scrolling. Please provide any additional information below. Attach a screenshot if possible. (1) attached file "a_large_number_of_text_nodes.html" is a reduced test case. (2) attached file "trace_massive_texts_hittest.json" shows that hittest take a long time. (3) root cause is that, when calling generateCulledLineBoxRects() inside LayoutInline::hitTestCulledInline(), it unites all the text line rects into a Region. (4) attached file "chromium_bugfix_hitTestCulledInline.patch" show a verified locally solution. we don't need to unites all the rects but only the intersaction with hittest rect.
,
Dec 27 2016
I'll look at the patch when I get a chance.
,
Jan 13 2017
Patch is up. For the record, both these tests show a 3x runtime improvement with the patch. I was worried that the call to intersects might be worse than the call to unite(enclosingIntRect(...)) but thankfully that's not the case.
TEST_F(LayoutInlineTest, RegionHitBigPerfTest) {
String innerHTMLString("<div><span id='lotsOfBoxes'>");
for (int i = 0; i < 5000; ++i) {
innerHTMLString.append(
"This is a test line<br>This is a test line<br>");
}
innerHTMLString.append("</span></div>");
setBodyInnerHTML((char*)innerHTMLString.impl()->bytes());
document().view()->updateAllLifecyclePhases();
LayoutInline* lotsOfBoxes =
toLayoutInline(getLayoutObjectByElementId("lotsOfBoxes"));
ASSERT_TRUE(lotsOfBoxes);
for (int i = 0; i < 1000; ++i) {
HitTestRequest hitRequest(HitTestRequest::TouchEvent |
HitTestRequest::ListBased);
LayoutPoint hitLocation(2, 5);
HitTestResult hitResult(hitRequest, hitLocation, 2, 1, 2, 1);
LayoutPoint hitOffset;
bool hitOutcome = lotsOfBoxes->hitTestCulledInline(
hitResult, hitResult.hitTestLocation(), hitOffset);
// Assert checks that we both hit something and that the area covered
// by "something" totally contains the hit region.
EXPECT_TRUE(hitOutcome);
}
}
TEST_F(LayoutInlineTest, RegionHitSmallPerfTest) {
String innerHTMLString("<div><span id='lotsOfBoxes'>");
for (int i = 0; i < 50; ++i) {
innerHTMLString.append(
"This is a test line<br>This is a test line<br>");
}
innerHTMLString.append("</span></div>");
setBodyInnerHTML((char*)innerHTMLString.impl()->bytes());
document().view()->updateAllLifecyclePhases();
LayoutInline* lotsOfBoxes =
toLayoutInline(getLayoutObjectByElementId("lotsOfBoxes"));
ASSERT_TRUE(lotsOfBoxes);
for (int i = 0; i < 100000; ++i) {
HitTestRequest hitRequest(HitTestRequest::TouchEvent |
HitTestRequest::ListBased);
LayoutPoint hitLocation(2, 5);
HitTestResult hitResult(hitRequest, hitLocation, 2, 1, 2, 1);
LayoutPoint hitOffset;
bool hitOutcome = lotsOfBoxes->hitTestCulledInline(
hitResult, hitResult.hitTestLocation(), hitOffset);
// Assert checks that we both hit something and that the area covered
// by "something" totally contains the hit region.
EXPECT_TRUE(hitOutcome);
}
,
Jan 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4f24093ff9c2a18dcddb8728edf40be941b8aadc commit 4f24093ff9c2a18dcddb8728edf40be941b8aadc Author: schenney <schenney@chromium.org> Date: Fri Jan 13 21:46:54 2017 Reduce rectangle merges when doing region-based hit test on text boxes Only unite the box rect with the hit test result if it intersects the hit test rect. That is sufficient because the only use for the united rect is to check that it contains the location rect for early termination of testing. Patch based on work by oicqchen@gmail.com. R=wkorman@chromium.org BUG= 676764 Review-Url: https://codereview.chromium.org/2616483002 Cr-Commit-Position: refs/heads/master@{#443684} [modify] https://crrev.com/4f24093ff9c2a18dcddb8728edf40be941b8aadc/third_party/WebKit/Source/core/layout/LayoutInline.cpp [modify] https://crrev.com/4f24093ff9c2a18dcddb8728edf40be941b8aadc/third_party/WebKit/Source/core/layout/LayoutInlineTest.cpp
,
Jan 17 2017
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by sureshkumari@chromium.org
, Dec 26 2016