Title: [143812] trunk/Source/WebCore
Revision
143812
Author
[email protected]
Date
2013-02-22 16:00:27 -0800 (Fri, 22 Feb 2013)

Log Message

Add descriptive names for different addMidpoint use cases
https://bugs.webkit.org/show_bug.cgi?id=110644

Reviewed by Ryosuke Niwa.

Midpoints are used to delineate ranges where we don't add line boxes for contents (collapsed spaces),
and to explicitly split a RenderText into multiple text runs so that text paragraph seperators get
their own line boxes. This patch encapsulates the different cases where midpoints are added to
lineMidpointState into 4 helper functions to make it clearer what's going on in each case.

No new tests. No change in functionality.

* rendering/RenderBlockLineLayout.cpp:
(WebCore::deprecatedAddMidpoint): Original function simply adds a midpoint to the lineMidpointState.
Renaming to deprecated to discourage callers.
(WebCore::startIgnoringSpaces): Adds a midpoint to start collapsing subsequent spaces. Asserts that
we have an even number of midpoints.
(WebCore::stopIgnoringSpaces): Adds the corresponding midpoint from startIgnoringSpaces and asserts
that it's an odd number.
(WebCore::ensureLineBoxInsideIgnoredSpaces): When ignoring spaces and we come across a RenderInline
that needs a line box, this function adds a pair of midpoints which ensures we'll later add a line
box for it.
(WebCore::ensureCharacterGetsLineBox): Adds a pair of midpoints in a text renderer to mark that
the current character needs its own line box. This is used by svg for absolutely positioned
characters, or for text paragraph seperators.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (143811 => 143812)


--- trunk/Source/WebCore/ChangeLog	2013-02-22 23:37:20 UTC (rev 143811)
+++ trunk/Source/WebCore/ChangeLog	2013-02-23 00:00:27 UTC (rev 143812)
@@ -1,3 +1,31 @@
+2013-02-22  Levi Weintraub  <[email protected]>
+
+        Add descriptive names for different addMidpoint use cases
+        https://bugs.webkit.org/show_bug.cgi?id=110644
+
+        Reviewed by Ryosuke Niwa.
+
+        Midpoints are used to delineate ranges where we don't add line boxes for contents (collapsed spaces),
+        and to explicitly split a RenderText into multiple text runs so that text paragraph seperators get
+        their own line boxes. This patch encapsulates the different cases where midpoints are added to
+        lineMidpointState into 4 helper functions to make it clearer what's going on in each case.
+
+        No new tests. No change in functionality.
+
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::deprecatedAddMidpoint): Original function simply adds a midpoint to the lineMidpointState.
+        Renaming to deprecated to discourage callers.
+        (WebCore::startIgnoringSpaces): Adds a midpoint to start collapsing subsequent spaces. Asserts that
+        we have an even number of midpoints.
+        (WebCore::stopIgnoringSpaces): Adds the corresponding midpoint from startIgnoringSpaces and asserts
+        that it's an odd number.
+        (WebCore::ensureLineBoxInsideIgnoredSpaces): When ignoring spaces and we come across a RenderInline
+        that needs a line box, this function adds a pair of midpoints which ensures we'll later add a line
+        box for it.
+        (WebCore::ensureCharacterGetsLineBox): Adds a pair of midpoints in a text renderer to mark that
+        the current character needs its own line box. This is used by svg for absolutely positioned
+        characters, or for text paragraph seperators.
+
 2013-02-22  Justin Schuh  <[email protected]>
 
         RenderArena masking has low entropy

Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (143811 => 143812)


--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp	2013-02-22 23:37:20 UTC (rev 143811)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp	2013-02-23 00:00:27 UTC (rev 143812)
@@ -359,7 +359,8 @@
     }
 }
 
-static void addMidpoint(LineMidpointState& lineMidpointState, const InlineIterator& midpoint)
+// Don't call this directly. Use one of the descriptive helper functions below.
+static void deprecatedAddMidpoint(LineMidpointState& lineMidpointState, const InlineIterator& midpoint)
 {
     if (lineMidpointState.midpoints.size() <= lineMidpointState.numMidpoints)
         lineMidpointState.midpoints.grow(lineMidpointState.numMidpoints + 10);
@@ -368,6 +369,35 @@
     midpoints[lineMidpointState.numMidpoints++] = midpoint;
 }
 
+static inline void startIgnoringSpaces(LineMidpointState& lineMidpointState, const InlineIterator& midpoint)
+{
+    ASSERT(!(lineMidpointState.numMidpoints % 2));
+    deprecatedAddMidpoint(lineMidpointState, midpoint);
+}
+
+static inline void stopIgnoringSpaces(LineMidpointState& lineMidpointState, const InlineIterator& midpoint)
+{
+    ASSERT(lineMidpointState.numMidpoints % 2);
+    deprecatedAddMidpoint(lineMidpointState, midpoint);
+}
+
+// When ignoring spaces, this needs to be called for objects that need line boxes such as RenderInlines or
+// hard line breaks to ensure that they're not ignored.
+static inline void ensureLineBoxInsideIgnoredSpaces(LineMidpointState& lineMidpointState, RenderObject* renderer)
+{
+    InlineIterator midpoint(0, renderer, 0);
+    stopIgnoringSpaces(lineMidpointState, midpoint);
+    startIgnoringSpaces(lineMidpointState, midpoint);
+}
+
+// Adding a pair of midpoints before a character will split it out into a new line box.
+static inline void ensureCharacterGetsLineBox(LineMidpointState& lineMidpointState, InlineIterator& textParagraphSeparator)
+{
+    InlineIterator midpoint(0, textParagraphSeparator.m_obj, textParagraphSeparator.m_pos);
+    startIgnoringSpaces(lineMidpointState, InlineIterator(0, textParagraphSeparator.m_obj, textParagraphSeparator.m_pos - 1));
+    stopIgnoringSpaces(lineMidpointState, InlineIterator(0, textParagraphSeparator.m_obj, textParagraphSeparator.m_pos));
+}
+
 static inline BidiRun* createRun(int start, int end, RenderObject* obj, InlineBidiResolver& resolver)
 {
     return new (obj->renderArena()) BidiRun(start, end, obj, resolver.context(), resolver.dir());
@@ -2325,7 +2355,7 @@
         RenderText* nextText = toRenderText(next);
         UChar nextChar = nextText->characterAt(0);
         if (nextText->style()->isCollapsibleWhiteSpace(nextChar)) {
-            addMidpoint(lineMidpointState, InlineIterator(0, o, 0));
+            startIgnoringSpaces(lineMidpointState, InlineIterator(0, o, 0));
             return true;
         }
     }
@@ -2478,9 +2508,7 @@
         for (size_t i = 0; i < m_boxes.size(); ++i) {
             if (currentMidpoint >= lineMidpointState.numMidpoints) {
                 // We don't have a midpoint for this box yet.
-                InlineIterator ignoreStart(0, m_boxes[i], 0);
-                addMidpoint(lineMidpointState, ignoreStart); // Stop ignoring.
-                addMidpoint(lineMidpointState, ignoreStart); // Start ignoring again.
+                ensureLineBoxInsideIgnoredSpaces(lineMidpointState, m_boxes[i]);
             } else {
                 ASSERT(lineMidpointState.midpoints[currentMidpoint].m_obj == m_boxes[i]);
                 ASSERT(lineMidpointState.midpoints[currentMidpoint + 1].m_obj == m_boxes[i]);
@@ -2494,11 +2522,9 @@
         unsigned length = m_whitespace->textLength();
         unsigned pos = length >= 2 ? length - 2 : UINT_MAX;
         InlineIterator endMid(0, m_whitespace, pos);
-        addMidpoint(lineMidpointState, endMid);
+        startIgnoringSpaces(lineMidpointState, endMid);
         for (size_t i = 0; i < m_boxes.size(); ++i) {
-            InlineIterator ignoreStart(0, m_boxes[i], 0);
-            addMidpoint(lineMidpointState, ignoreStart); // Stop ignoring spaces.
-            addMidpoint(lineMidpointState, ignoreStart); // Start ignoring again.
+            ensureLineBoxInsideIgnoredSpaces(lineMidpointState, m_boxes[i]);
         }
     }
 }
@@ -2641,10 +2667,8 @@
                 // A <br> with clearance always needs a linebox in case the lines below it get dirtied later and 
                 // need to check for floats to clear - so if we're ignoring spaces, stop ignoring them and add a
                 // run for this object.
-                if (ignoringSpaces && currentStyle->clear() != CNONE) {
-                    addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, 0)); // Stop ignoring spaces.
-                    addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, 0)); // Start ignoring again.
-                }
+                if (ignoringSpaces && currentStyle->clear() != CNONE)
+                    ensureLineBoxInsideIgnoredSpaces(lineMidpointState, current.m_obj);
 
                 if (!lineInfo.isEmpty())
                     m_clear = currentStyle->clear();
@@ -2668,12 +2692,8 @@
             // If we're ignoring spaces, we have to stop and include this object and
             // then start ignoring spaces again.
             if (isInlineType || current.m_obj->container()->isRenderInline()) {
-                if (ignoringSpaces) {
-                    ignoreStart.m_obj = current.m_obj;
-                    ignoreStart.m_pos = 0;
-                    addMidpoint(lineMidpointState, ignoreStart); // Stop ignoring spaces.
-                    addMidpoint(lineMidpointState, ignoreStart); // Start ignoring again.
-                }
+                if (ignoringSpaces)
+                    ensureLineBoxInsideIgnoredSpaces(lineMidpointState, current.m_obj);
                 trailingObjects.appendBoxIfNeeded(box);
             } else
                 m_positionedObjects.append(box);
@@ -2710,8 +2730,7 @@
                     lineInfo.setEmpty(false, m_block, &width);
                 if (ignoringSpaces) {
                     trailingObjects.clear();
-                    addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, 0)); // Stop ignoring spaces.
-                    addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, 0)); // Start ignoring again.
+                    ensureLineBoxInsideIgnoredSpaces(lineMidpointState, current.m_obj);
                 } else if (blockStyle->collapseWhiteSpace() && resolver.position().m_obj == current.m_obj
                     && shouldSkipWhitespaceAfterStartObject(m_block, current.m_obj, lineMidpointState)) {
                     // Like with list markers, we start ignoring spaces to make sure that any
@@ -2736,7 +2755,7 @@
             }
 
             if (ignoringSpaces)
-                addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, 0));
+                stopIgnoringSpaces(lineMidpointState, InlineIterator(0, current.m_obj, 0));
 
             lineInfo.setEmpty(false, m_block, &width);
             ignoringSpaces = false;
@@ -2855,7 +2874,7 @@
                             ignoringSpaces = false;
                             wordSpacingForWordMeasurement = 0;
                             lastSpace = current.m_pos; // e.g., "Foo    goo", don't add in any of the ignored spaces.
-                            addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos));
+                            stopIgnoringSpaces(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos));
                             stoppedIgnoringSpaces = true;
                         } else {
                             // Just keep ignoring these spaces.
@@ -2913,11 +2932,8 @@
                                     goto end;
                             }
                             if (lBreak.atTextParagraphSeparator()) {
-                                if (!stoppedIgnoringSpaces && current.m_pos > 0) {
-                                    // We need to stop right before the newline and then start up again.
-                                    addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos - 1)); // Stop
-                                    addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos)); // Start
-                                }
+                                if (!stoppedIgnoringSpaces && current.m_pos > 0)
+                                    ensureCharacterGetsLineBox(lineMidpointState, current);
                                 lBreak.increment();
                                 lineInfo.setPreviousLineBrokeCleanly(true);
                                 wordMeasurement.endOffset = lBreak.m_pos;
@@ -2931,7 +2947,7 @@
                                 }
                             }
                             // Didn't fit. Jump to the end unless there's still an opportunity to collapse whitespace.
-                            if (ignoringSpaces || !currentStyle->collapseWhiteSpace() || !currentCharacterIsSpace || !previousCharacterIsSpace)
+                            if (ignoringSpaces || !collapseWhiteSpace || !currentCharacterIsSpace || !previousCharacterIsSpace)
                                 goto end;
                         } else {
                             if (!betweenWords || (midWordBreak && !autoWrap))
@@ -2945,11 +2961,8 @@
                     }
 
                     if (c == '\n' && preserveNewline) {
-                        if (!stoppedIgnoringSpaces && current.m_pos > 0) {
-                            // We need to stop right before the newline and then start up again.
-                            addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos - 1)); // Stop
-                            addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos)); // Start
-                        }
+                        if (!stoppedIgnoringSpaces && current.m_pos > 0)
+                            ensureCharacterGetsLineBox(lineMidpointState, current);
                         lBreak.moveTo(current.m_obj, current.m_pos, current.m_nextBreakablePosition);
                         lBreak.increment();
                         lineInfo.setPreviousLineBrokeCleanly(true);
@@ -2988,7 +3001,7 @@
                             // We just entered a mode where we are ignoring
                             // spaces. Create a midpoint to terminate the run
                             // before the second space.
-                            addMidpoint(lineMidpointState, ignoreStart);
+                            startIgnoringSpaces(lineMidpointState, ignoreStart);
                             trailingObjects.updateMidpointsForTrailingBoxes(lineMidpointState, InlineIterator(), TrailingObjects::DoNotCollapseFirstSpace);
                         }
                     }
@@ -2999,16 +3012,14 @@
                     lastSpaceWordSpacing = applyWordSpacing ? wordSpacing : 0;
                     wordSpacingForWordMeasurement = (applyWordSpacing && wordMeasurements.last().width) ? wordSpacing : 0;
                     lastSpace = current.m_pos; // e.g., "Foo    goo", don't add in any of the ignored spaces.
-                    addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos));
+                    stopIgnoringSpaces(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos));
                 }
 
 #if ENABLE(SVG)
                 if (isSVGText && current.m_pos > 0) {
                     // Force creation of new InlineBoxes for each absolute positioned character (those that start new text chunks).
-                    if (toRenderSVGInlineText(t)->characterStartsNewTextChunk(current.m_pos)) {
-                        addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos - 1));
-                        addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, current.m_pos));
-                    }
+                    if (toRenderSVGInlineText(t)->characterStartsNewTextChunk(current.m_pos))
+                        ensureCharacterGetsLineBox(lineMidpointState, current);
                 }
 #endif
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to