[LayoutNG] Stop getClientRects/getBoundingClientRects from adding unnecessary collapsed rects
Currently the two functions adds rects from all text fragments intersecting
the input range, even if they only touch at boundaries. For example:
Text fragments: ABC DEF GHI
Text offsets: 012 345 678
Input range: start = 3, end = 6
The current implementation returns three rects, one from each fragment,
even though the rects from "ABC" and "GHI" are collapsed.
This patch stops the inclusion of such collapsed rects; they are included
only when we can't find non-collapsed rects.
Bug: 755750
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I1ca65d1aee02e1eb33b8c72605daca4d6dbd119b
Reviewed-on: https://chromium-review.googlesource.com/1142685
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576935}
diff --git a/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG b/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
index 1907bf3..5b63006 100644
--- a/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
+++ b/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
@@ -542,8 +542,6 @@
crbug.com/591099 fast/css3-text/css3-text-indent/negative-text-indent-leading-out-of-flow.html [ Failure ]
crbug.com/591099 fast/css3-text/css3-text-indent/text-indent-leading-out-of-flow.html [ Failure ]
crbug.com/591099 fast/dom/HTMLAreaElement/area-download.html [ Failure ]
-crbug.com/755750 fast/dom/Range/get-bounding-client-rect-empty-and-non-empty.html [ Failure Pass ]
-crbug.com/755750 fast/dom/Range/getBoundingClientRect-linebreak-character.html [ Failure ]
crbug.com/591099 fast/dom/nodesFromRect/nodesFromRect-basic.html [ Failure ]
crbug.com/591099 fast/dynamic/first-letter-after-list-marker.html [ Failure ]
crbug.com/591099 fast/dynamic/text-combine.html [ Failure ]
diff --git a/third_party/blink/renderer/core/layout/layout_text.cc b/third_party/blink/renderer/core/layout/layout_text.cc
index bb8a4d45..a2ff463 100644
--- a/third_party/blink/renderer/core/layout/layout_text.cc
+++ b/third_party/blink/renderer/core/layout/layout_text.cc
@@ -480,6 +480,18 @@
if (!MapDOMOffsetToTextContentOffset(*mapping, &start, &end))
return;
+ // We don't want to add collapsed (i.e., start == end) quads from text
+ // fragments that intersect [start, end] only at the boundary, unless they
+ // are the only quads found. For example, when we have
+ // - text fragments: ABC DEF GHI
+ // - text offsets: 012 345 678
+ // and input range [3, 6], since fragment "DEF" gives non-collapsed quad,
+ // we no longer add quads from "ABC" and "GHI" since they are collapsed.
+ // TODO(layout-dev): This heuristic doesn't cover all cases, as we return
+ // 2 collapsed quads (instead of 1) for range [3, 3] in the above example.
+ bool found_non_collapsed_quad = false;
+ Vector<FloatQuad, 1> collapsed_quads_candidates;
+
// Find fragments that have text for the specified range.
DCHECK_LE(start, end);
auto fragments = NGPaintFragment::InlineFragmentsFor(this);
@@ -489,12 +501,22 @@
if (start > text_fragment.EndOffset() ||
end < text_fragment.StartOffset())
continue;
+ const unsigned clamped_start =
+ std::max(start, text_fragment.StartOffset());
+ const unsigned clamped_end = std::min(end, text_fragment.EndOffset());
NGPhysicalOffsetRect rect =
- text_fragment.LocalRect(std::max(start, text_fragment.StartOffset()),
- std::min(end, text_fragment.EndOffset()));
+ text_fragment.LocalRect(clamped_start, clamped_end);
rect.offset += fragment->InlineOffsetToContainerBox();
- quads.push_back(LocalToAbsoluteQuad(rect.ToFloatRect()));
+ const FloatQuad quad = LocalToAbsoluteQuad(rect.ToFloatRect());
+ if (clamped_start < clamped_end) {
+ quads.push_back(quad);
+ found_non_collapsed_quad = true;
+ } else {
+ collapsed_quads_candidates.push_back(quad);
+ }
}
+ if (!found_non_collapsed_quad)
+ quads.AppendVector(collapsed_quads_candidates);
return;
}