Remove TextRun::setCharactersLength() and TextRun::charactersLength()
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Sep 2017 23:08:27 +0000 (23:08 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Sep 2017 23:08:27 +0000 (23:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177620

Reviewed by Zalan Bujtas.

The purpose of TextRun::setCharactersLength() and TextRun::charactersLength() predate the
use of WidthIterator to safely iterate over characters in a TextRun that may contain
surrogate halves due to how it was created (without thought of surrogate pairs). Historically
TextRun::charactersLength() complemented TextRun::length() and represented the length of the
text to render ignoring and respecting truncation, respectively. We not longer need either
of these member functions with the advent of WidthIterator.

No functionality changed. So, no new tests.

* platform/graphics/ComplexTextController.cpp:
(WebCore::TextLayout::constructTextRun):
* platform/graphics/TextRun.cpp: Remove one unsigned field from ExpectedTextRunSize as we
reduced the size of TextRun with the removal of TextRun::m_charactersLength.
* platform/graphics/TextRun.h:
(WebCore::TextRun::TextRun):
(WebCore::TextRun::charactersLength const): Deleted.
(WebCore::TextRun::setCharactersLength): Deleted.
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::constructTextRun const):
* rendering/InlineTextBox.h:
(WebCore::InlineTextBox::constructTextRun): Deleted overload that took const RenderStyle& style
and StringView.
* rendering/RenderText.cpp:
(WebCore::RenderText::widthFromCache const):
(WebCore::maxWordFragmentWidth):
(WebCore::RenderText::computePreferredLogicalWidths):
(WebCore::RenderText::width const):
* rendering/line/BreakingContext.h:
(WebCore::textWidth):
(WebCore::tryHyphenating):
* rendering/svg/SVGInlineTextBox.cpp:
(WebCore::SVGInlineTextBox::constructTextRun const):
* rendering/svg/SVGTextMetrics.cpp:
(WebCore::SVGTextMetrics::constructTextRun):
* rendering/svg/SVGTextMetricsBuilder.cpp:
(WebCore::SVGTextMetricsBuilder::currentCharacterStartsSurrogatePair const): Use TextRun::length().
(WebCore::SVGTextMetricsBuilder::advance): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222637 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/ComplexTextController.cpp
Source/WebCore/platform/graphics/TextRun.cpp
Source/WebCore/platform/graphics/TextRun.h
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/rendering/InlineTextBox.h
Source/WebCore/rendering/RenderText.cpp
Source/WebCore/rendering/line/BreakingContext.h
Source/WebCore/rendering/svg/SVGInlineTextBox.cpp
Source/WebCore/rendering/svg/SVGTextMetrics.cpp
Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp

index e527640..d15b1dd 100644 (file)
@@ -1,3 +1,48 @@
+2017-09-28  Daniel Bates  <dabates@apple.com>
+
+        Remove TextRun::setCharactersLength() and TextRun::charactersLength()
+        https://bugs.webkit.org/show_bug.cgi?id=177620
+
+        Reviewed by Zalan Bujtas.
+
+        The purpose of TextRun::setCharactersLength() and TextRun::charactersLength() predate the
+        use of WidthIterator to safely iterate over characters in a TextRun that may contain
+        surrogate halves due to how it was created (without thought of surrogate pairs). Historically
+        TextRun::charactersLength() complemented TextRun::length() and represented the length of the
+        text to render ignoring and respecting truncation, respectively. We not longer need either
+        of these member functions with the advent of WidthIterator.
+
+        No functionality changed. So, no new tests.
+
+        * platform/graphics/ComplexTextController.cpp:
+        (WebCore::TextLayout::constructTextRun):
+        * platform/graphics/TextRun.cpp: Remove one unsigned field from ExpectedTextRunSize as we
+        reduced the size of TextRun with the removal of TextRun::m_charactersLength.
+        * platform/graphics/TextRun.h:
+        (WebCore::TextRun::TextRun):
+        (WebCore::TextRun::charactersLength const): Deleted.
+        (WebCore::TextRun::setCharactersLength): Deleted.
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::constructTextRun const):
+        * rendering/InlineTextBox.h:
+        (WebCore::InlineTextBox::constructTextRun): Deleted overload that took const RenderStyle& style
+        and StringView.
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::widthFromCache const):
+        (WebCore::maxWordFragmentWidth):
+        (WebCore::RenderText::computePreferredLogicalWidths):
+        (WebCore::RenderText::width const):
+        * rendering/line/BreakingContext.h:
+        (WebCore::textWidth):
+        (WebCore::tryHyphenating):
+        * rendering/svg/SVGInlineTextBox.cpp:
+        (WebCore::SVGInlineTextBox::constructTextRun const):
+        * rendering/svg/SVGTextMetrics.cpp:
+        (WebCore::SVGTextMetrics::constructTextRun):
+        * rendering/svg/SVGTextMetricsBuilder.cpp:
+        (WebCore::SVGTextMetricsBuilder::currentCharacterStartsSurrogatePair const): Use TextRun::length().
+        (WebCore::SVGTextMetricsBuilder::advance): Ditto.
+
 2017-09-28  Tim Horton  <timothy_horton@apple.com>
 
         Remove constant() in favor of env()
index 907bdc5..4b29465 100644 (file)
@@ -78,8 +78,6 @@ private:
     static TextRun constructTextRun(RenderText& text, float xPos)
     {
         TextRun run = RenderBlock::constructTextRun(text, text.style());
-        run.setCharactersLength(text.textLength());
-        ASSERT(run.charactersLength() >= run.length());
         run.setXPos(xPos);
         return run;
     }
index 884cf9b..26d400e 100644 (file)
@@ -31,7 +31,6 @@ namespace WebCore {
 struct ExpectedTextRunSize {
     StringView text;
     unsigned integer1;
-    unsigned integer2;
     float float1;
     float float2;
     float float3;
index 2911cba..d2b5c1f 100644 (file)
@@ -46,7 +46,6 @@ class TextRun {
 public:
     explicit TextRun(StringView text, float xpos = 0, float expansion = 0, ExpansionBehavior expansionBehavior = DefaultExpansion, TextDirection direction = LTR, bool directionalOverride = false, bool characterScanForCodePath = true)
         : m_text(text)
-        , m_charactersLength(text.length())
         , m_tabSize(0)
         , m_xpos(xpos)
         , m_horizontalGlyphStretch(1)
@@ -83,13 +82,11 @@ public:
 
     bool is8Bit() const { return m_text.is8Bit(); }
     unsigned length() const { return m_text.length(); }
-    unsigned charactersLength() const { return m_charactersLength; }
     String string() const { return m_text.toString(); }
 
     void setText(const LChar* c, unsigned len) { m_text = StringView(c, len); }
     void setText(const UChar* c, unsigned len) { m_text = StringView(c, len); }
     void setText(StringView stringView) { m_text = stringView; }
-    void setCharactersLength(unsigned charactersLength) { m_charactersLength = charactersLength; }
 
     float horizontalGlyphStretch() const { return m_horizontalGlyphStretch; }
     void setHorizontalGlyphStretch(float scale) { m_horizontalGlyphStretch = scale; }
@@ -117,7 +114,6 @@ public:
 
 private:
     StringView m_text;
-    unsigned m_charactersLength; // Marks the end of the characters buffer. Default equals to length of m_text.
 
     unsigned m_tabSize;
 
index 5c2b63a..ef6d3a8 100644 (file)
@@ -1091,22 +1091,14 @@ String InlineTextBox::hyphenatedStringForTextRun(const RenderStyle& style, std::
 
 TextRun InlineTextBox::constructTextRun(const RenderStyle& style, StringView alternateStringToRender, std::optional<unsigned> alternateLength) const
 {
+    auto actuallyConstructTextRun = [&] (StringView string) {
+        TextRun run(string, textPos(), expansion(), expansionBehavior(), direction(), dirOverride() || style.rtlOrdering() == VisualOrder, !renderer().canUseSimpleFontCodePath());
+        run.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
+        return run;
+    };
     if (alternateStringToRender.isNull())
-        return constructTextRun(style, substringToRender(alternateLength), renderer().textLength() - start());
-    return constructTextRun(style, alternateStringToRender, alternateStringToRender.length());
-}
-
-TextRun InlineTextBox::constructTextRun(const RenderStyle& style, StringView string, unsigned maximumLength) const
-{
-    ASSERT(maximumLength >= string.length());
-
-    TextRun run(string, textPos(), expansion(), expansionBehavior(), direction(), dirOverride() || style.rtlOrdering() == VisualOrder, !renderer().canUseSimpleFontCodePath());
-    run.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
-
-    // Propagate the maximum length of the characters buffer to the TextRun, even when we're only processing a substring.
-    run.setCharactersLength(maximumLength);
-    ASSERT(run.charactersLength() >= run.length());
-    return run;
+        return actuallyConstructTextRun(substringToRender(alternateLength));
+    return actuallyConstructTextRun(alternateStringToRender);
 }
 
 inline const RenderCombineText* InlineTextBox::combinedText() const
index a6b5176..48fd385 100644 (file)
@@ -110,7 +110,6 @@ private:
     StringView substringToRender(std::optional<unsigned> overridingLength = { }) const;
     String hyphenatedStringForTextRun(const RenderStyle&, std::optional<unsigned> alternateLength = { }) const;
     TextRun constructTextRun(const RenderStyle&, StringView alternateStringToRender = { }, std::optional<unsigned> alternateLength = { }) const;
-    TextRun constructTextRun(const RenderStyle&, StringView, unsigned maximumLength) const;
 
 public:
     FloatRect calculateBoundaries() const override { return FloatRect(x(), y(), width(), height()); }
index e9d18ae..403d188 100644 (file)
@@ -514,9 +514,6 @@ ALWAYS_INLINE float RenderText::widthFromCache(const FontCascade& f, unsigned st
     }
 
     TextRun run = RenderBlock::constructTextRun(*this, start, len, style);
-    run.setCharactersLength(textLength() - start);
-    ASSERT(run.charactersLength() >= run.length());
-
     run.setCharacterScanForCodePath(!canUseSimpleFontCodePath());
     run.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
     run.setXPos(xPos);
@@ -781,7 +778,6 @@ static float maxWordFragmentWidth(RenderText& renderer, const RenderStyle& style
         fragmentWithHyphen.append(style.hyphenString());
 
         TextRun run = RenderBlock::constructTextRun(fragmentWithHyphen.toString(), style);
-        run.setCharactersLength(fragmentWithHyphen.length());
         run.setCharacterScanForCodePath(!renderer.canUseSimpleFontCodePath());
         float fragmentWidth = font.width(run, &fallbackFonts, &glyphOverflow);
 
@@ -1007,8 +1003,6 @@ void RenderText::computePreferredLogicalWidths(float leadWidth, HashSet<const Fo
                 currMaxWidth = 0;
             } else {
                 TextRun run = RenderBlock::constructTextRun(*this, i, 1, style);
-                run.setCharactersLength(len - i);
-                ASSERT(run.charactersLength() >= run.length());
                 run.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
                 run.setXPos(leadWidth + currMaxWidth);
 
@@ -1395,12 +1389,10 @@ float RenderText::width(unsigned from, unsigned len, const FontCascade& f, float
             w = widthFromCache(f, from, len, xPos, fallbackFonts, glyphOverflow, style);
     } else {
         TextRun run = RenderBlock::constructTextRun(*this, from, len, style);
-        run.setCharactersLength(textLength() - from);
-        ASSERT(run.charactersLength() >= run.length());
-
         run.setCharacterScanForCodePath(!canUseSimpleFontCodePath());
         run.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
         run.setXPos(xPos);
+
         w = f.width(run, fallbackFonts, glyphOverflow);
     }
 
index dfd3e5e..056601e 100644 (file)
@@ -628,9 +628,6 @@ ALWAYS_INLINE float textWidth(RenderText& text, unsigned from, unsigned len, con
         return FontCascade::width(*layout, from, len, &fallbackFonts);
 
     TextRun run = RenderBlock::constructTextRun(text, from, len, style);
-    run.setCharactersLength(text.textLength() - from);
-    ASSERT(run.charactersLength() >= run.length());
-
     run.setCharacterScanForCodePath(!text.canUseSimpleFontCodePath());
     run.setTabSize(!collapseWhiteSpace, style.tabSize());
     run.setXPos(xPos);
@@ -675,9 +672,6 @@ inline void tryHyphenating(RenderText& text, const FontCascade& font, const Atom
 
     const RenderStyle& style = text.style();
     TextRun run = RenderBlock::constructTextRun(text, lastSpace, pos - lastSpace, style);
-    run.setCharactersLength(text.textLength() - lastSpace);
-    ASSERT(run.charactersLength() >= run.length());
-
     run.setTabSize(!collapseWhiteSpace, style.tabSize());
     run.setXPos(xPos + lastSpaceWordSpacing);
 
index 68a929c..d1a2ad8 100644 (file)
@@ -396,10 +396,6 @@ TextRun SVGInlineTextBox::constructTextRun(const RenderStyle& style, const SVGTe
 
     // We handle letter & word spacing ourselves.
     run.disableSpacing();
-
-    // Propagate the maximum length of the characters buffer to the TextRun, even when we're only processing a substring.
-    run.setCharactersLength(renderer().textLength() - fragment.characterOffset);
-    ASSERT(run.charactersLength() >= run.length());
     return run;
 }
 
index 7627fe5..78e06b5 100644 (file)
@@ -73,10 +73,6 @@ TextRun SVGTextMetrics::constructTextRun(RenderSVGInlineText& text, unsigned pos
 
     // We handle letter & word spacing ourselves.
     run.disableSpacing();
-
-    // Propagate the maximum length of the characters buffer to the TextRun, even when we're only processing a substring.
-    run.setCharactersLength(text.textLength() - position);
-    ASSERT(run.charactersLength() >= run.length());
     return run;
 }
 
index bb2593d..2efb27e 100644 (file)
@@ -38,13 +38,13 @@ SVGTextMetricsBuilder::SVGTextMetricsBuilder()
 
 inline bool SVGTextMetricsBuilder::currentCharacterStartsSurrogatePair() const
 {
-    return U16_IS_LEAD(m_run[m_textPosition]) && (m_textPosition + 1) < m_run.charactersLength() && U16_IS_TRAIL(m_run[m_textPosition + 1]);
+    return U16_IS_LEAD(m_run[m_textPosition]) && (m_textPosition + 1) < m_run.length() && U16_IS_TRAIL(m_run[m_textPosition + 1]);
 }
 
 bool SVGTextMetricsBuilder::advance()
 {
     m_textPosition += m_currentMetrics.length();
-    if (m_textPosition >= m_run.charactersLength())
+    if (m_textPosition >= m_run.length())
         return false;
 
     if (m_isComplexText)