Simple line layout: Merge TextFragmentIterator::findNextBreakablePosition() and TextF...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Mar 2015 17:09:52 +0000 (17:09 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Mar 2015 17:09:52 +0000 (17:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142344

Reviewed by Antti Koivisto.

This patch merges findNextBreakablePosition() and findNextNonWhitespacePosition() so that
the segment looping and position handling logic are no longer duplicated. It also unifies
the static next*() functions' signature.

No change in functionality.

* rendering/SimpleLineLayoutTextFragmentIterator.cpp:
(WebCore::SimpleLineLayout::TextFragmentIterator::nextTextFragment):
(WebCore::SimpleLineLayout::nextBreakablePosition):
(WebCore::SimpleLineLayout::nextNonWhitespacePosition):
(WebCore::SimpleLineLayout::TextFragmentIterator::skipToNextPosition):
(WebCore::SimpleLineLayout::TextFragmentIterator::findNextBreakablePosition): Deleted.
(WebCore::SimpleLineLayout::findNextNonWhitespace): Deleted.
(WebCore::SimpleLineLayout::TextFragmentIterator::findNextNonWhitespacePosition): Deleted.
* rendering/SimpleLineLayoutTextFragmentIterator.h:

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp
Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.h

index 275304a..85444b7 100644 (file)
@@ -1,3 +1,26 @@
+2015-03-09  Zalan Bujtas  <zalan@apple.com>
+
+        Simple line layout: Merge TextFragmentIterator::findNextBreakablePosition() and TextFragmentIterator::findNextNonWhitespacePosition().
+        https://bugs.webkit.org/show_bug.cgi?id=142344
+
+        Reviewed by Antti Koivisto.
+
+        This patch merges findNextBreakablePosition() and findNextNonWhitespacePosition() so that
+        the segment looping and position handling logic are no longer duplicated. It also unifies
+        the static next*() functions' signature.
+
+        No change in functionality.
+
+        * rendering/SimpleLineLayoutTextFragmentIterator.cpp:
+        (WebCore::SimpleLineLayout::TextFragmentIterator::nextTextFragment):
+        (WebCore::SimpleLineLayout::nextBreakablePosition):
+        (WebCore::SimpleLineLayout::nextNonWhitespacePosition):
+        (WebCore::SimpleLineLayout::TextFragmentIterator::skipToNextPosition):
+        (WebCore::SimpleLineLayout::TextFragmentIterator::findNextBreakablePosition): Deleted.
+        (WebCore::SimpleLineLayout::findNextNonWhitespace): Deleted.
+        (WebCore::SimpleLineLayout::TextFragmentIterator::findNextNonWhitespacePosition): Deleted.
+        * rendering/SimpleLineLayoutTextFragmentIterator.h:
+
 2015-03-09  Xabier Rodriguez Calvar <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
 
         ReadableStreamJSSource should use JSC::Strong UnspecifiedBoolType operator
index 9e5f664..6bd66d1 100644 (file)
@@ -68,25 +68,18 @@ TextFragmentIterator::TextFragment TextFragmentIterator::nextTextFragment(float
         ++m_position;
         return fragment;
     }
-    unsigned spaceCount = 0;
     unsigned startPosition = m_position;
-    unsigned endPosition = findNextNonWhitespacePosition(startPosition, spaceCount);
+    unsigned endPosition = skipToNextPosition(PositionType::NonWhitespace, startPosition);
     ASSERT(startPosition <= endPosition);
     if (endPosition > startPosition) {
         bool multipleWhitespace = startPosition + 1 < endPosition;
         bool isCollapsed = multipleWhitespace && m_style.collapseWhitespace;
         bool isBreakable = !isCollapsed && multipleWhitespace;
-        float width = 0;
-        if (isCollapsed)
-            width = m_style.spaceWidth;
-        else {
-            unsigned length = endPosition - startPosition;
-            width = length == spaceCount ? length * m_style.spaceWidth : textWidth(startPosition, endPosition, xPosition);
-        }
+        float width = isCollapsed ? m_style.spaceWidth : textWidth(startPosition, endPosition, xPosition);
         m_position = endPosition;
         return TextFragment(startPosition, endPosition, width, TextFragment::Whitespace, isCollapsed, isBreakable);
     }
-    endPosition = findNextBreakablePosition(startPosition + 1);
+    endPosition = skipToNextPosition(PositionType::Breakable, startPosition + 1);
     m_position = endPosition;
     return TextFragment(startPosition, endPosition, textWidth(startPosition, endPosition, xPosition), TextFragment::NonWhitespace, false, m_style.breakWordOnOverflow);
 }
@@ -114,58 +107,55 @@ float TextFragmentIterator::textWidth(unsigned from, unsigned to, float xPositio
 }
 
 template <typename CharacterType>
-static unsigned nextBreakablePosition(LazyLineBreakIterator& lineBreakIterator, const FlowContents::Segment& segment, unsigned position)
+static unsigned nextBreakablePosition(LazyLineBreakIterator& lineBreakIterator, const FlowContents::Segment& segment, unsigned startPosition)
 {
-    const auto* characters = segment.text.characters<CharacterType>();
-    unsigned segmentLength = segment.end - segment.start;
-    unsigned segmentPosition = position - segment.start;
-    return nextBreakablePositionNonLoosely<CharacterType, NBSPBehavior::IgnoreNBSP>(lineBreakIterator, characters, segmentLength, segmentPosition);
-}
-
-unsigned TextFragmentIterator::findNextBreakablePosition(unsigned position) const
-{
-    while (!isEnd(position)) {
-        auto& segment = m_flowContents.segmentForPosition(position);
-        if (segment.text.impl() != m_lineBreakIterator.string().impl()) {
-            UChar lastCharacter = segment.start > 0 ? characterAt(segment.start - 1) : 0;
-            UChar secondToLastCharacter = segment.start > 1 ? characterAt(segment.start - 2) : 0;
-            m_lineBreakIterator.setPriorContext(lastCharacter, secondToLastCharacter);
-            m_lineBreakIterator.resetStringAndReleaseIterator(segment.text, m_style.locale, LineBreakIteratorModeUAX14);
-        }
-
-        unsigned breakable = segment.text.is8Bit() ? nextBreakablePosition<LChar>(m_lineBreakIterator, segment, position) : nextBreakablePosition<UChar>(m_lineBreakIterator, segment, position);
-        position = segment.start + breakable;
-        if (position < segment.end)
-            break;
-    }
-    return position;
+    return nextBreakablePositionNonLoosely<CharacterType, NBSPBehavior::IgnoreNBSP>(lineBreakIterator, segment.text.characters<CharacterType>(), segment.end - segment.start, startPosition);
 }
 
 template <typename CharacterType>
-static bool findNextNonWhitespace(const FlowContents::Segment& segment, const TextFragmentIterator::Style& style, unsigned& position, unsigned& spaceCount)
+static unsigned nextNonWhitespacePosition(const FlowContents::Segment& segment, unsigned startPosition, const TextFragmentIterator::Style& style)
 {
     const auto* text = segment.text.characters<CharacterType>();
+    unsigned position = startPosition;
     for (; position < segment.end; ++position) {
-        auto character = text[position - segment.start];
-        bool isSpace = character == ' ';
-        bool isWhitespace = isSpace || character == '\t' || (!style.preserveNewline && character == '\n');
+        auto character = text[position];
+        bool isWhitespace = character == ' ' || character == '\t' || (!style.preserveNewline && character == '\n');
         if (!isWhitespace)
-            return true;
-        if (isSpace)
-            ++spaceCount;
+            return position;
     }
-    return false;
+    return position;
 }
 
-unsigned TextFragmentIterator::findNextNonWhitespacePosition(unsigned position, unsigned& spaceCount) const
+unsigned TextFragmentIterator::skipToNextPosition(PositionType positionType, unsigned startPosition) const
 {
-    FlowContents::Iterator it(m_flowContents, m_flowContents.segmentIndexForPosition(position));
+    if (isEnd(startPosition))
+        return startPosition;
+
+    unsigned currentPosition = startPosition;
+    FlowContents::Iterator it(m_flowContents, m_flowContents.segmentIndexForPosition(currentPosition));
     for (auto end = m_flowContents.end(); it != end; ++it) {
-        bool foundNonWhitespace = (*it).text.is8Bit() ? findNextNonWhitespace<LChar>(*it, m_style, position, spaceCount) : findNextNonWhitespace<UChar>(*it, m_style, position, spaceCount);
-        if (foundNonWhitespace)
+        auto& segment = *it;
+        unsigned currentPositonRelativeToSegment = currentPosition - segment.start;
+        unsigned nextPositionRelativeToSegment = 0;
+        if (positionType == NonWhitespace) {
+            nextPositionRelativeToSegment = segment.text.is8Bit() ? nextNonWhitespacePosition<LChar>(segment, currentPositonRelativeToSegment, m_style) :
+                nextNonWhitespacePosition<UChar>(segment, currentPositonRelativeToSegment, m_style);
+        } else if (positionType == Breakable) {
+            if (segment.text.impl() != m_lineBreakIterator.string().impl()) {
+                UChar lastCharacter = segment.start > 0 ? characterAt(segment.start - 1) : 0;
+                UChar secondToLastCharacter = segment.start > 1 ? characterAt(segment.start - 2) : 0;
+                m_lineBreakIterator.setPriorContext(lastCharacter, secondToLastCharacter);
+                m_lineBreakIterator.resetStringAndReleaseIterator(segment.text, m_style.locale, LineBreakIteratorModeUAX14);
+            }
+            nextPositionRelativeToSegment = segment.text.is8Bit() ? nextBreakablePosition<LChar>(m_lineBreakIterator, segment, currentPositonRelativeToSegment) :
+                nextBreakablePosition<UChar>(m_lineBreakIterator, segment, currentPositonRelativeToSegment);
+        } else
+            ASSERT_NOT_REACHED();
+        currentPosition = segment.start + nextPositionRelativeToSegment;
+        if (currentPosition < segment.end)
             break;
     }
-    return position;
+    return currentPosition;
 }
 
 template <typename CharacterType>
index 683721b..c8c1eb4 100644 (file)
@@ -92,8 +92,8 @@ public:
     const FlowContents::Segment& segmentForPosition(unsigned position) const { return m_flowContents.segmentForPosition(position); };
 
 private:
-    unsigned findNextNonWhitespacePosition(unsigned position, unsigned& spaceCount) const;
-    unsigned findNextBreakablePosition(unsigned position) const;
+    enum PositionType { Breakable, NonWhitespace };
+    unsigned skipToNextPosition(PositionType, unsigned startPosition) const;
     UChar characterAt(unsigned position) const;
     bool isLineBreak(unsigned position) const;
     bool isEnd(unsigned position) const;