Diff
Modified: trunk/LayoutTests/ChangeLog (269271 => 269272)
--- trunk/LayoutTests/ChangeLog 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/LayoutTests/ChangeLog 2020-11-02 21:43:04 UTC (rev 269272)
@@ -1,3 +1,16 @@
+2020-11-02 Antti Koivisto <[email protected]>
+
+ [LFC][Integration] RenderText::absoluteQuadsForRange should use iterator
+ https://bugs.webkit.org/show_bug.cgi?id=218444
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/dom/Range/getBoundingClientRect-expected.txt:
+ * fast/dom/Range/getBoundingClientRect.html:
+
+ On legacy path <br> y position is miscomputed affecting getBoundingClientRect().
+ Update the test to match the new behavior. Results now match Firefox and Chrome.
+
2020-11-02 Chris Dumez <[email protected]>
[ macOS ] webaudio/OfflineAudioContext/onstatechange.html is a flaky failure
Modified: trunk/LayoutTests/fast/dom/Range/getBoundingClientRect-expected.txt (269271 => 269272)
--- trunk/LayoutTests/fast/dom/Range/getBoundingClientRect-expected.txt 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/LayoutTests/fast/dom/Range/getBoundingClientRect-expected.txt 2020-11-02 21:43:04 UTC (rev 269272)
@@ -48,9 +48,9 @@
Test 7
PASS rect.left.toFixed(3) is "8.000"
-PASS rect.top.toFixed(3) is "2168.000"
+PASS rect.top.toFixed(3) is "2180.000"
PASS rect.width.toFixed(3) is "176.000"
-PASS rect.height.toFixed(3) is "108.000"
+PASS rect.height.toFixed(3) is "96.000"
PASS rect.right is rect.left + rect.width
PASS rect.bottom is rect.top + rect.height
Modified: trunk/LayoutTests/fast/dom/Range/getBoundingClientRect.html (269271 => 269272)
--- trunk/LayoutTests/fast/dom/Range/getBoundingClientRect.html 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/LayoutTests/fast/dom/Range/getBoundingClientRect.html 2020-11-02 21:43:04 UTC (rev 269272)
@@ -105,7 +105,7 @@
/*4*/ { left: 0, top: 0, width: 0, height: 0 },
/*5*/ { left: -14.574, top: 1761.947, width: 504.009, height: 535.849 },
/*6*/ { left: 0, top: 0, width: 0, height: 0 },
- /*7*/ { left: 8, top: 2168, width: 176, height: 108 },
+ /*7*/ { left: 8, top: 2180, width: 176, height: 96 },
];
// Range over entire element.
Modified: trunk/Source/WebCore/ChangeLog (269271 => 269272)
--- trunk/Source/WebCore/ChangeLog 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/Source/WebCore/ChangeLog 2020-11-02 21:43:04 UTC (rev 269272)
@@ -1,3 +1,35 @@
+2020-11-02 Antti Koivisto <[email protected]>
+
+ [LFC][Integration] RenderText::absoluteQuadsForRange should use iterator
+ https://bugs.webkit.org/show_bug.cgi?id=218444
+
+ Reviewed by Zalan Bujtas.
+
+ Add the required iterator support and convert absoluteQuadsForRange to use it.
+ This eliminates ensureLineBoxes from two places.
+
+ * layout/integration/LayoutIntegrationRunIterator.h:
+ (WebCore::LayoutIntegration::PathTextRun::isSelectable const):
+ (WebCore::LayoutIntegration::PathTextRun::selectionRect const):
+ * layout/integration/LayoutIntegrationRunIteratorLegacyPath.h:
+ (WebCore::LayoutIntegration::RunIteratorLegacyPath::isSelectable const):
+ (WebCore::LayoutIntegration::RunIteratorLegacyPath::selectionRect const):
+ * layout/integration/LayoutIntegrationRunIteratorModernPath.h:
+ (WebCore::LayoutIntegration::RunIteratorModernPath::offsetForPosition const):
+ (WebCore::LayoutIntegration::RunIteratorModernPath::isSelectable const):
+ (WebCore::LayoutIntegration::RunIteratorModernPath::selectionRect const):
+ (WebCore::LayoutIntegration::RunIteratorModernPath::clampedOffset const):
+ (WebCore::LayoutIntegration::RunIteratorModernPath::createTextRun const):
+ * rendering/RenderText.cpp:
+ (WebCore::RenderText::absoluteRectsForRange const):
+ (WebCore::localQuadForTextRun):
+ (WebCore::RenderText::absoluteQuadsForRange const):
+ * rendering/RenderTextLineBoxes.cpp:
+ (WebCore::localQuadForTextBox): Deleted.
+ (WebCore::RenderTextLineBoxes::absoluteQuadsForRange const): Deleted.
+ (WebCore::RenderTextLineBoxes::absoluteRectsForRange const): Deleted.
+ * rendering/RenderTextLineBoxes.h:
+
2020-11-02 Chris Dumez <[email protected]>
[ macOS ] webaudio/OfflineAudioContext/onstatechange.html is a flaky failure
Modified: trunk/Source/WebCore/layout/integration/LayoutIntegrationLineIteratorModernPath.h (269271 => 269272)
--- trunk/Source/WebCore/layout/integration/LayoutIntegrationLineIteratorModernPath.h 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/Source/WebCore/layout/integration/LayoutIntegrationLineIteratorModernPath.h 2020-11-02 21:43:04 UTC (rev 269272)
@@ -52,9 +52,9 @@
LayoutUnit top() const { return LayoutUnit::fromFloatRound(line().enclosingRect().y()); }
LayoutUnit bottom() const { return LayoutUnit::fromFloatRound(line().enclosingRect().maxY()); }
// FIXME: What should these really be?
- LayoutUnit selectionTop() const { return lineBoxTop(); }
- LayoutUnit selectionTopForHitTesting() const { return lineBoxTop(); }
- LayoutUnit selectionBottom() const { return lineBoxBottom(); }
+ LayoutUnit selectionTop() const { return top(); }
+ LayoutUnit selectionTopForHitTesting() const { return top(); }
+ LayoutUnit selectionBottom() const { return bottom(); }
LayoutUnit lineBoxTop() const { return LayoutUnit::fromFloatRound(line().rect().y()); }
LayoutUnit lineBoxBottom() const { return LayoutUnit::fromFloatRound(line().rect().maxY()); }
Modified: trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIterator.h (269271 => 269272)
--- trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIterator.h 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIterator.h 2020-11-02 21:43:04 UTC (rev 269272)
@@ -109,6 +109,9 @@
unsigned offsetForPosition(float x) const;
+ bool isSelectable(unsigned start, unsigned end) const;
+ LayoutRect selectionRect(unsigned start, unsigned end) const;
+
bool isLastTextRunOnLine() const;
bool isLastTextRun() const;
@@ -325,6 +328,20 @@
});
}
+inline bool PathTextRun::isSelectable(unsigned start, unsigned end) const
+{
+ return WTF::switchOn(m_pathVariant, [&](auto& path) {
+ return path.isSelectable(start, end);
+ });
+}
+
+inline LayoutRect PathTextRun::selectionRect(unsigned start, unsigned end) const
+{
+ return WTF::switchOn(m_pathVariant, [&](auto& path) {
+ return path.selectionRect(start, end);
+ });
+}
+
inline bool PathTextRun::isLastTextRunOnLine() const
{
return WTF::switchOn(m_pathVariant, [](auto& path) {
Modified: trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorLegacyPath.h (269271 => 269272)
--- trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorLegacyPath.h 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorLegacyPath.h 2020-11-02 21:43:04 UTC (rev 269272)
@@ -61,6 +61,9 @@
inline unsigned offsetForPosition(float x) const { return inlineTextBox()->offsetForPosition(x); }
+ bool isSelectable(unsigned start, unsigned end) const { return inlineTextBox()->isSelected(start, end); }
+ LayoutRect selectionRect(unsigned start, unsigned end) const { return inlineTextBox()->localSelectionRect(start, end); }
+
bool isLastTextRunOnLine() const
{
auto* next = nextInlineTextBoxInTextOrder();
Modified: trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorModernPath.h (269271 => 269272)
--- trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorModernPath.h 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorModernPath.h 2020-11-02 21:43:04 UTC (rev 269272)
@@ -77,18 +77,38 @@
if (localX < 0)
return 0;
- auto& style = run().style();
+ bool includePartialGlyphs = true;
+ return run().style().fontCascade().offsetForPosition(createTextRun(HyphenMode::Ignore), localX, includePartialGlyphs);
+ }
- auto createTextRun = [&] {
- auto expansion = run().expansion();
- auto xPos = rect.x() - (line().rect().x() + line().horizontalAlignmentOffset());
- TextRun textRun { text(), xPos, expansion.horizontalExpansion, expansion.behavior };
- textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
- return textRun;
- };
+ bool isSelectable(unsigned start, unsigned end) const
+ {
+ return clampedOffset(start) < clampedOffset(end);
+ }
- bool includePartialGlyphs = true;
- return run().style().fontCascade().offsetForPosition(createTextRun(), localX, includePartialGlyphs);
+ LayoutRect selectionRect(unsigned rangeStart, unsigned rangeEnd) const
+ {
+ unsigned clampedStart = clampedOffset(rangeStart);
+ unsigned clampedEnd = clampedOffset(rangeEnd);
+
+ if (clampedStart >= clampedEnd && !(rangeStart == rangeEnd && rangeStart >= start() && rangeStart <= end()))
+ return { };
+
+ auto logicalLeft = LayoutUnit(isHorizontal() ? rect().x() : rect().y());
+ auto logicalRight = LayoutUnit(isHorizontal() ? rect().maxX() : rect().maxY());
+ auto logicalWidth = logicalRight - logicalLeft;
+
+ // FIXME: These should share implementation with the line iterator.
+ auto selectionTop = LayoutUnit::fromFloatRound(line().enclosingRect().y());
+ auto selectionHeight = LayoutUnit::fromFloatRound(line().enclosingRect().height());
+
+ LayoutRect selectionRect { logicalLeft, selectionTop, logicalWidth, selectionHeight };
+
+ TextRun textRun = createTextRun(HyphenMode::Include);
+ if (clampedStart != start() || clampedEnd != textRun.length())
+ run().style().fontCascade().adjustSelectionRectForText(textRun, selectionRect, clampedStart - start(), clampedEnd - start());
+
+ return snappedSelectionRect(selectionRect, logicalRight, selectionTop, selectionHeight, isHorizontal());
}
bool isLastTextRunOnLine() const
@@ -176,6 +196,36 @@
private:
friend class RunIterator;
+ unsigned clampedOffset(unsigned offset) const
+ {
+ auto clampedOffset = std::max(start(), std::min(offset, end()));
+ // We treat the last codepoint in this run and the hyphen as a single unit.
+ if (hasHyphen() && clampedOffset == end())
+ clampedOffset += run().style().hyphenString().length();
+
+ return clampedOffset;
+ }
+
+ enum class HyphenMode { Include, Ignore };
+ TextRun createTextRun(HyphenMode hyphenMode) const
+ {
+ auto& style = run().style();
+ auto expansion = run().expansion();
+ auto rect = this->rect();
+ auto xPos = rect.x() - (line().rect().x() + line().horizontalAlignmentOffset());
+
+ auto textForRun = [&] {
+ if (hyphenMode == HyphenMode::Ignore || !hasHyphen())
+ return text().toStringWithoutCopying();
+
+ return makeString(text(), style.hyphenString());
+ }();
+
+ TextRun textRun { textForRun, xPos, expansion.horizontalExpansion, expansion.behavior };
+ textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
+ return textRun;
+ };
+
const InlineContent::Runs& runs() const { return m_inlineContent->runs; }
const Run& run() const { return runs()[m_runIndex]; }
const Line& line() const { return m_inlineContent->lineForRun(run()); }
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (269271 => 269272)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2020-11-02 21:43:04 UTC (rev 269272)
@@ -313,19 +313,9 @@
Vector<IntRect> RenderText::absoluteRectsForRange(unsigned start, unsigned end, bool useSelectionHeight, bool* wasFixed) const
{
- const_cast<RenderText&>(*this).ensureLineBoxes();
-
- // Work around signed/unsigned issues. This function takes unsigneds, and is often passed UINT_MAX
- // to mean "all the way to the end". InlineTextBox coordinates are unsigneds, so changing this
- // function to take ints causes various internal mismatches. But selectionRect takes ints, and
- // passing UINT_MAX to it causes trouble. Ideally we'd change selectionRect to take unsigneds, but
- // that would cause many ripple effects, so for now we'll just clamp our unsigned parameters to INT_MAX.
- ASSERT(end == UINT_MAX || end <= INT_MAX);
- ASSERT(start <= INT_MAX);
- start = std::min(start, static_cast<unsigned>(INT_MAX));
- end = std::min(end, static_cast<unsigned>(INT_MAX));
-
- return m_lineBoxes.absoluteRectsForRange(*this, start, end, useSelectionHeight, wasFixed);
+ return absoluteQuadsForRange(start, end, useSelectionHeight, false /* ignoreEmptyTextSelections */, wasFixed).map([](auto& quad) {
+ return quad.enclosingBoundingBox();
+ });
}
#if PLATFORM(IOS_FAMILY)
@@ -433,6 +423,26 @@
quads.appendVector(m_lineBoxes.absoluteQuads(*this, wasFixed, RenderTextLineBoxes::NoClipping));
}
+static FloatRect localQuadForTextRun(const LayoutIntegration::PathTextRun& run, unsigned start, unsigned end, bool useSelectionHeight)
+{
+ unsigned realEnd = std::min(run.end(), end);
+ LayoutRect boxSelectionRect = run.selectionRect(start, realEnd);
+ if (!boxSelectionRect.height())
+ return { };
+ if (useSelectionHeight)
+ return boxSelectionRect;
+
+ auto rect = run.rect();
+ if (run.isHorizontal()) {
+ boxSelectionRect.setHeight(rect.height());
+ boxSelectionRect.setY(rect.y());
+ } else {
+ boxSelectionRect.setWidth(rect.width());
+ boxSelectionRect.setX(rect.x());
+ }
+ return boxSelectionRect;
+}
+
Vector<FloatQuad> RenderText::absoluteQuadsForRange(unsigned start, unsigned end, bool useSelectionHeight, bool ignoreEmptyTextSelections, bool* wasFixed) const
{
// Work around signed/unsigned issues. This function takes unsigneds, and is often passed UINT_MAX
@@ -445,8 +455,35 @@
start = std::min(start, static_cast<unsigned>(INT_MAX));
end = std::min(end, static_cast<unsigned>(INT_MAX));
- const_cast<RenderText&>(*this).ensureLineBoxes();
- return m_lineBoxes.absoluteQuadsForRange(*this, start, end, useSelectionHeight, ignoreEmptyTextSelections, wasFixed);
+ Vector<FloatQuad> quads;
+ for (auto& run : LayoutIntegration::textRunsFor(*this)) {
+ if (ignoreEmptyTextSelections && !run.isSelectable(start, end))
+ continue;
+ if (start <= run.start() && run.end() <= end) {
+ auto boundaries = [&] {
+ if (run.legacyInlineBox() && run.legacyInlineBox()->isSVGInlineTextBox())
+ return run.legacyInlineBox()->calculateBoundaries();
+ return run.rect();
+ }();
+
+ if (useSelectionHeight) {
+ LayoutRect selectionRect = run.selectionRect(start, end);
+ if (run.isHorizontal()) {
+ boundaries.setHeight(selectionRect.height());
+ boundaries.setY(selectionRect.y());
+ } else {
+ boundaries.setWidth(selectionRect.width());
+ boundaries.setX(selectionRect.x());
+ }
+ }
+ quads.append(localToAbsoluteQuad(boundaries, UseTransforms, wasFixed));
+ continue;
+ }
+ FloatRect rect = localQuadForTextRun(run, start, end, useSelectionHeight);
+ if (!rect.isZero())
+ quads.append(localToAbsoluteQuad(rect, UseTransforms, wasFixed));
+ }
+ return quads;
}
Position RenderText::positionForPoint(const LayoutPoint& point)
Modified: trunk/Source/WebCore/rendering/RenderTextLineBoxes.cpp (269271 => 269272)
--- trunk/Source/WebCore/rendering/RenderTextLineBoxes.cpp 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/Source/WebCore/rendering/RenderTextLineBoxes.cpp 2020-11-02 21:43:04 UTC (rev 269272)
@@ -237,59 +237,6 @@
}
}
-static FloatRect localQuadForTextBox(const InlineTextBox& box, unsigned start, unsigned end, bool useSelectionHeight)
-{
- unsigned realEnd = std::min(box.end(), end);
- LayoutRect boxSelectionRect = box.localSelectionRect(start, realEnd);
- if (!boxSelectionRect.height())
- return FloatRect();
- if (useSelectionHeight)
- return boxSelectionRect;
- // Change the height and y position (or width and x for vertical text)
- // because selectionRect uses selection-specific values.
- if (box.isHorizontal()) {
- boxSelectionRect.setHeight(box.height());
- boxSelectionRect.setY(box.y());
- } else {
- boxSelectionRect.setWidth(box.width());
- boxSelectionRect.setX(box.x());
- }
- return boxSelectionRect;
-}
-
-Vector<FloatQuad> RenderTextLineBoxes::absoluteQuadsForRange(const RenderText& renderer, unsigned start, unsigned end, bool useSelectionHeight, bool ignoreEmptyTextSelections, bool* wasFixed) const
-{
- Vector<FloatQuad> quads;
- for (auto* box = m_first; box; box = box->nextTextBox()) {
- if (ignoreEmptyTextSelections && !box->isSelected(start, end))
- continue;
- if (start <= box->start() && box->end() <= end) {
- FloatRect boundaries = box->calculateBoundaries();
- if (useSelectionHeight) {
- LayoutRect selectionRect = box->localSelectionRect(start, end);
- if (box->isHorizontal()) {
- boundaries.setHeight(selectionRect.height());
- boundaries.setY(selectionRect.y());
- } else {
- boundaries.setWidth(selectionRect.width());
- boundaries.setX(selectionRect.x());
- }
- }
- quads.append(renderer.localToAbsoluteQuad(boundaries, UseTransforms, wasFixed));
- continue;
- }
- FloatRect rect = localQuadForTextBox(*box, start, end, useSelectionHeight);
- if (!rect.isZero())
- quads.append(renderer.localToAbsoluteQuad(rect, UseTransforms, wasFixed));
- }
- return quads;
-}
-
-Vector<IntRect> RenderTextLineBoxes::absoluteRectsForRange(const RenderText& renderer, unsigned start, unsigned end, bool useSelectionHeight, bool* wasFixed) const
-{
- return absoluteQuadsForRange(renderer, start, end, useSelectionHeight, false /* ignoreEmptyTextSelections */, wasFixed).map([](auto& quad) { return quad.enclosingBoundingBox(); });
-}
-
Vector<FloatQuad> RenderTextLineBoxes::absoluteQuads(const RenderText& renderer, bool* wasFixed, ClippingOption option) const
{
Vector<FloatQuad> quads;
Modified: trunk/Source/WebCore/rendering/RenderTextLineBoxes.h (269271 => 269272)
--- trunk/Source/WebCore/rendering/RenderTextLineBoxes.h 2020-11-02 21:21:43 UTC (rev 269271)
+++ trunk/Source/WebCore/rendering/RenderTextLineBoxes.h 2020-11-02 21:43:04 UTC (rev 269272)
@@ -64,8 +64,6 @@
enum ClippingOption { NoClipping, ClipToEllipsis };
Vector<FloatQuad> absoluteQuads(const RenderText&, bool* wasFixed, ClippingOption) const;
- Vector<FloatQuad> absoluteQuadsForRange(const RenderText&, unsigned start, unsigned end, bool useSelectionHeight, bool ignoreEmptyTextSelections, bool* wasFixed) const;
- Vector<IntRect> absoluteRectsForRange(const RenderText&, unsigned start, unsigned end, bool useSelectionHeight, bool* wasFixed) const;
#if ASSERT_ENABLED
~RenderTextLineBoxes();
