Changeset 84436 in webkit

Timestamp:

Apr 20, 2011 4:48:23 PM (13 years ago)

Author:

eric@webkit.org

Message:

2011-04-20 Eric Seidel <​eric@webkit.org>

Reviewed by Ryosuke Niwa.

Split bidiFirst into two functions
​https://bugs.webkit.org/show_bug.cgi?id=59025

I don't have a full understanding of the skipInlines case yet
(hence it's not as pretty as the non-skipping case), but
this is clearly a win, as both functions are much simpler
than their combined version.

Both of these functions still have bad names. However the
bidiNext/bidiFirst names date all the way back to revision 1
of the WebKit repository. :) So I don't feel too bad keeping
them for the moment.

• rendering/InlineIterator.h: (WebCore::bidiNext): (WebCore::bidiFirstSkippingInlines): (WebCore::bidiFirstNotSkippingInlines):
• rendering/RenderBlock.cpp: (WebCore::RenderBlock::simplifiedNormalFlowLayout):
• rendering/RenderBlockLineLayout.cpp: (WebCore::RenderBlock::layoutInlineChildren): (WebCore::RenderBlock::determineStartPosition):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/InlineIterator.h (modified) (4 diffs)
• rendering/RenderBlock.cpp (modified) (1 diff)
• rendering/RenderBlockLineLayout.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-04-20  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        Split bidiFirst into two functions
+        https://bugs.webkit.org/show_bug.cgi?id=59025
+
+        I don't have a full understanding of the skipInlines case yet
+        (hence it's not as pretty as the non-skipping case), but
+        this is clearly a win, as both functions are much simpler
+        than their combined version.
+
+        Both of these functions still have bad names.  However the
+        bidiNext/bidiFirst names date all the way back to revision 1
+        of the WebKit repository. :)  So I don't feel too bad keeping
+        them for the moment.
+
+        * rendering/InlineIterator.h:
+        (WebCore::bidiNext):
+        (WebCore::bidiFirstSkippingInlines):
+        (WebCore::bidiFirstNotSkippingInlines):
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::simplifiedNormalFlowLayout):
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::RenderBlock::layoutInlineChildren):
+        (WebCore::RenderBlock::determineStartPosition):
+
 2011-04-20  Jia Pu  <jpu@apple.com>
 

trunk/Source/WebCore/rendering/InlineIterator.h

@@ -138 +138 @@
 {
     RenderObject* next = 0;
+    // oldEndOfInline denotes if when we last stopped iterating if we were at the end of an inline.
     bool oldEndOfInline = endOfInlinePtr ? *endOfInlinePtr : false;
     bool endOfInline = false;
@@ -148 +149 @@
         }
 
+        // We hit this when either current has no children, or when current is not a renderer we care about.
         if (!next) {
+            // If it is a renderer we care about, and we're doing our inline-walk, return it.
             if (!skipInlines && !oldEndOfInline && current->isRenderInline()) {
                 next = current;
@@ -189 +192 @@
 }
 
-static inline RenderObject* bidiFirst(RenderObject* root, InlineBidiResolver* resolver, bool skipInlines = true)
-{
-    if (!root->firstChild())
+static inline RenderObject* bidiFirstSkippingInlines(RenderObject* root, InlineBidiResolver* resolver)
+{
+    ASSERT(resolver);
+    RenderObject* o = root->firstChild();
+    if (!o)
         return 0;
 
-    RenderObject* o = root->firstChild();
     if (o->isRenderInline()) {
         notifyResolverEnteredObject(resolver, o);
-        if (skipInlines && o->firstChild())
-            o = bidiNext(root, o, resolver, skipInlines);
+        if (o->firstChild())
+            o = bidiNext(root, o, resolver, true);
         else {
             // Never skip empty inlines.
@@ -207 +211 @@
     }
 
+    // FIXME: Unify this with the bidiNext call above.
     if (o && !o->isText() && !o->isReplaced() && !o->isFloating() && !o->isPositioned())
-        o = bidiNext(root, o, resolver, skipInlines);
-
-    if (resolver)
-        resolver->commitExplicitEmbedding();
+        o = bidiNext(root, o, resolver, true);
+
+    resolver->commitExplicitEmbedding();
     return o;
+}
+
+// FIXME: This method needs to be renamed when bidiNext finds a good name.
+static inline RenderObject* bidiFirstNotSkippingInlines(RenderObject* root)
+{
+    RenderObject* o = root->firstChild();
+    // If either there are no children to walk, or the first one is correct
+    // then just return it.
+    if (!o || o->isRenderInline() || o->isText() || o->isReplaced() || o->isFloating() || o->isPositioned())
+        return o;
+
+    return bidiNext(root, o, 0, false);
 }
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -2082 +2082 @@
         ListHashSet<RootInlineBox*> lineBoxes;
         bool endOfInline = false;
-        RenderObject* o = bidiFirst(this, 0, false);
+        RenderObject* o = bidiFirstNotSkippingInlines(this);
         while (o) {
             if (!o->isPositioned() && (o->isReplaced() || o->isFloating())) {

trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

@@ -754 +754 @@
         // layout replaced elements
         bool endOfInline = false;
-        RenderObject* o = bidiFirst(this, 0, false);
+        RenderObject* o = bidiFirstNotSkippingInlines(this);
         Vector<FloatWithRect> floats;
         bool hasInlineChild = false;
@@ -1275 +1275 @@
         resolver.setContext(BidiContext::create(ltr ? 0 : 1, direction, style()->unicodeBidi() == Override, FromStyleOrDOM));
 
-        startObj = bidiFirst(this, &resolver);
+        startObj = bidiFirstSkippingInlines(this, &resolver);
     }