WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
[patch]
patch
lfc-absoluteQuadsForRange-2.patch (text/plain), 19.65 KB, created by
Antti Koivisto
on 2020-11-02 11:38:58 PST
(
hide
)
Description:
patch
Filename:
MIME Type:
Creator:
Antti Koivisto
Created:
2020-11-02 11:38:58 PST
Size:
19.65 KB
patch
obsolete
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index f732e13bded7..4b4d40830022 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,16 @@ >+2020-11-02 Antti Koivisto <antti@apple.com> >+ >+ [LFC][Integration] RenderText::absoluteQuadsForRange should use iterator >+ https://bugs.webkit.org/show_bug.cgi?id=218444 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * 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-01 Mason Xiao <me@masonx.ca> > > REGRESSION (r252689): box-shadow with inset and negative spread renders incorrectly >diff --git a/LayoutTests/fast/dom/Range/getBoundingClientRect-expected.txt b/LayoutTests/fast/dom/Range/getBoundingClientRect-expected.txt >index 5e4f966e5b7a..38a257cba5b7 100644 >--- a/LayoutTests/fast/dom/Range/getBoundingClientRect-expected.txt >+++ b/LayoutTests/fast/dom/Range/getBoundingClientRect-expected.txt >@@ -48,9 +48,9 @@ PASS rect.bottom is rect.top + rect.height > > 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 > >diff --git a/LayoutTests/fast/dom/Range/getBoundingClientRect.html b/LayoutTests/fast/dom/Range/getBoundingClientRect.html >index f432bc95a779..caeab3abf484 100644 >--- a/LayoutTests/fast/dom/Range/getBoundingClientRect.html >+++ b/LayoutTests/fast/dom/Range/getBoundingClientRect.html >@@ -105,7 +105,7 @@ body { > /*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. >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index ef9962a6e40c..fb6c23602c1b 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,35 @@ >+2020-11-02 Antti Koivisto <antti@apple.com> >+ >+ [LFC][Integration] RenderText::absoluteQuadsForRange should use iterator >+ https://bugs.webkit.org/show_bug.cgi?id=218444 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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 Youenn Fablet <youenn@apple.com> > > Stop sending origins from WebProcess to UIProcess for getUserMedia requests >diff --git a/Source/WebCore/layout/integration/LayoutIntegrationLineIteratorModernPath.h b/Source/WebCore/layout/integration/LayoutIntegrationLineIteratorModernPath.h >index 77c01f577432..aa18dc972a7d 100644 >--- a/Source/WebCore/layout/integration/LayoutIntegrationLineIteratorModernPath.h >+++ b/Source/WebCore/layout/integration/LayoutIntegrationLineIteratorModernPath.h >@@ -52,9 +52,9 @@ public: > 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()); } > >diff --git a/Source/WebCore/layout/integration/LayoutIntegrationRunIterator.h b/Source/WebCore/layout/integration/LayoutIntegrationRunIterator.h >index 03da8ced02e3..d36a1117d0d5 100644 >--- a/Source/WebCore/layout/integration/LayoutIntegrationRunIterator.h >+++ b/Source/WebCore/layout/integration/LayoutIntegrationRunIterator.h >@@ -109,6 +109,9 @@ public: > > 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 unsigned PathTextRun::offsetForPosition(float x) const > }); > } > >+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) { >diff --git a/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorLegacyPath.h b/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorLegacyPath.h >index 466abad09107..5e183a0223f2 100644 >--- a/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorLegacyPath.h >+++ b/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorLegacyPath.h >@@ -61,6 +61,9 @@ public: > > 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(); >diff --git a/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorModernPath.h b/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorModernPath.h >index de425e6e704d..af09b338244c 100644 >--- a/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorModernPath.h >+++ b/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorModernPath.h >@@ -77,18 +77,38 @@ public: > 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 @@ public: > 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()); } >diff --git a/Source/WebCore/rendering/RenderText.cpp b/Source/WebCore/rendering/RenderText.cpp >index 436cc2d13fdc..3cbbce3f6da4 100644 >--- a/Source/WebCore/rendering/RenderText.cpp >+++ b/Source/WebCore/rendering/RenderText.cpp >@@ -313,19 +313,9 @@ void RenderText::absoluteRects(Vector<IntRect>& rects, const LayoutPoint& accumu > > 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 @@ void RenderText::absoluteQuads(Vector<FloatQuad>& quads, bool* wasFixed) const > 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 @@ Vector<FloatQuad> RenderText::absoluteQuadsForRange(unsigned start, unsigned end > 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) >diff --git a/Source/WebCore/rendering/RenderTextLineBoxes.cpp b/Source/WebCore/rendering/RenderTextLineBoxes.cpp >index b75639aff8b7..addbf58fdd43 100644 >--- a/Source/WebCore/rendering/RenderTextLineBoxes.cpp >+++ b/Source/WebCore/rendering/RenderTextLineBoxes.cpp >@@ -237,59 +237,6 @@ void RenderTextLineBoxes::collectSelectionRectsForRange(unsigned start, unsigned > } > } > >-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; >diff --git a/Source/WebCore/rendering/RenderTextLineBoxes.h b/Source/WebCore/rendering/RenderTextLineBoxes.h >index 02d30ab3c0e0..e2f9f55806ae 100644 >--- a/Source/WebCore/rendering/RenderTextLineBoxes.h >+++ b/Source/WebCore/rendering/RenderTextLineBoxes.h >@@ -64,8 +64,6 @@ public: > > 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();
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=412950">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 218444
:
412916
| 412950