Use inline iterator for SVG reverse BiDI reordering
https://bugs.webkit.org/show_bug.cgi?id=231858

Reviewed by Alan Bujtas.

Share code.

* layout/integration/InlineIteratorLogicalOrderTraversal.cpp:
(WebCore::InlineIterator::makeLineLogicalOrderCache):
(WebCore::InlineIterator::nextLeafOnLineInLogicalOrder):
(WebCore::InlineIterator::previousLeafOnLineInLogicalOrder):

No need to test for the cache, it is always there.

* layout/integration/InlineIteratorLogicalOrderTraversal.h:
(WebCore::InlineIterator::leafBoxesInLogicalOrder):

Make template and move to header.

* rendering/LegacyInlineFlowBox.cpp:
(WebCore::LegacyInlineFlowBox::collectLeafBoxesInLogicalOrder const): Deleted.

Not needed anymore.

* rendering/LegacyInlineFlowBox.h:
* rendering/svg/SVGRootInlineBox.cpp:
(WebCore::SVGRootInlineBox::computePerCharacterLayoutInformation):
(WebCore::reverseInlineBoxRangeAndValueListsIfNeeded):
(WebCore::SVGRootInlineBox::reorderValueListsToLogicalOrder):
(WebCore::SVGRootInlineBox::reorderValueLists): Deleted.

Call the generic version.

* rendering/svg/SVGRootInlineBox.h:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@284315 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 7ce13c6..b296f86 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,40 @@
+2021-10-16  Antti Koivisto  <antti@apple.com>
+
+        Use inline iterator for SVG reverse BiDI reordering
+        https://bugs.webkit.org/show_bug.cgi?id=231858
+
+        Reviewed by Alan Bujtas.
+
+        Share code.
+
+        * layout/integration/InlineIteratorLogicalOrderTraversal.cpp:
+        (WebCore::InlineIterator::makeLineLogicalOrderCache):
+        (WebCore::InlineIterator::nextLeafOnLineInLogicalOrder):
+        (WebCore::InlineIterator::previousLeafOnLineInLogicalOrder):
+
+        No need to test for the cache, it is always there.
+
+        * layout/integration/InlineIteratorLogicalOrderTraversal.h:
+        (WebCore::InlineIterator::leafBoxesInLogicalOrder):
+
+        Make template and move to header.
+
+        * rendering/LegacyInlineFlowBox.cpp:
+        (WebCore::LegacyInlineFlowBox::collectLeafBoxesInLogicalOrder const): Deleted.
+
+        Not needed anymore.
+
+        * rendering/LegacyInlineFlowBox.h:
+        * rendering/svg/SVGRootInlineBox.cpp:
+        (WebCore::SVGRootInlineBox::computePerCharacterLayoutInformation):
+        (WebCore::reverseInlineBoxRangeAndValueListsIfNeeded):
+        (WebCore::SVGRootInlineBox::reorderValueListsToLogicalOrder):
+        (WebCore::SVGRootInlineBox::reorderValueLists): Deleted.
+
+        Call the generic version.
+
+        * rendering/svg/SVGRootInlineBox.h:
+
 2021-10-16  Simon Fraser  <simon.fraser@apple.com>
 
         Make sure child layers of top layer elements are rendered and correctly z-ordered (top-layer-stacking.html fails)
diff --git a/Source/WebCore/layout/integration/InlineIteratorLogicalOrderTraversal.cpp b/Source/WebCore/layout/integration/InlineIteratorLogicalOrderTraversal.cpp
index 85eb5179..69f5746 100644
--- a/Source/WebCore/layout/integration/InlineIteratorLogicalOrderTraversal.cpp
+++ b/Source/WebCore/layout/integration/InlineIteratorLogicalOrderTraversal.cpp
@@ -92,48 +92,11 @@
 static LineLogicalOrderCache makeLineLogicalOrderCache(const LineIterator& line)
 {
     auto cache = WTF::makeUnique<LineLogicalOrderCacheData>();
+
     cache->line = line;
-
-    auto& boxes = cache->boxes;
-
-    unsigned char minLevel = 128;
-    unsigned char maxLevel = 0;
-
-    for (auto box = line->firstRun(); box; box = box.traverseNextOnLine()) {
-        minLevel = std::min(minLevel, box->bidiLevel());
-        maxLevel = std::max(maxLevel, box->bidiLevel());
-        boxes.append(box);
-    }
-
-    if (!maxLevel)
-        return cache;
-
-    if (line->containingBlock().style().rtlOrdering() == Order::Visual)
-        return cache;
-
-    // Reverse of reordering of the line (L2 according to Bidi spec):
-    // L2. From the highest level found in the text to the lowest odd level on each line,
-    // reverse any contiguous sequence of characters that are at that level or higher.
-
-    // Reversing the reordering of the line is only done up to the lowest odd level.
-    if (!(minLevel % 2))
-        ++minLevel;
-
-    auto end = boxes.end();
-    for (; minLevel <= maxLevel; ++minLevel) {
-        auto box = boxes.begin();
-        while (box < end) {
-            while (box < end && (*box)->bidiLevel() < minLevel)
-                ++box;
-
-            auto first = box;
-            while (box < end && (*box)->bidiLevel() >= minLevel)
-                ++box;
-
-            auto last = box;
-            std::reverse(first, last);
-        }
-    }
+    cache->boxes = leafBoxesInLogicalOrder(line, [](auto first, auto last) {
+        std::reverse(first, last);
+    });
 
     return cache;
 }
@@ -178,9 +141,6 @@
 {
     updateLineLogicalOrderCacheIfNeeded(box, cache);
 
-    if (!cache)
-        return box->nextOnLine();
-
     cache->index++;
 
     if (cache->index < cache->boxes.size())
@@ -193,9 +153,6 @@
 {
     updateLineLogicalOrderCacheIfNeeded(box, cache);
 
-    if (!cache)
-        return box->previousOnLine();
-
     if (!cache->index)
         return { };
 
diff --git a/Source/WebCore/layout/integration/InlineIteratorLogicalOrderTraversal.h b/Source/WebCore/layout/integration/InlineIteratorLogicalOrderTraversal.h
index 1cf5894..ac9a2f2 100644
--- a/Source/WebCore/layout/integration/InlineIteratorLogicalOrderTraversal.h
+++ b/Source/WebCore/layout/integration/InlineIteratorLogicalOrderTraversal.h
@@ -59,5 +59,49 @@
 LeafBoxIterator firstLeafOnLineInLogicalOrderWithNode(const LineIterator&, LineLogicalOrderCache&);
 LeafBoxIterator lastLeafOnLineInLogicalOrderWithNode(const LineIterator&, LineLogicalOrderCache&);
 
+template<typename ReverseFunction>
+Vector<LeafBoxIterator> leafBoxesInLogicalOrder(const LineIterator& line, ReverseFunction&& reverseFunction)
+{
+    Vector<LeafBoxIterator> boxes;
+
+    unsigned char minLevel = 128;
+    unsigned char maxLevel = 0;
+
+    for (auto box = line->firstRun(); box; box = box.traverseNextOnLine()) {
+        minLevel = std::min(minLevel, box->bidiLevel());
+        maxLevel = std::max(maxLevel, box->bidiLevel());
+        boxes.append(box);
+    }
+
+    if (line->containingBlock().style().rtlOrdering() == Order::Visual)
+        return boxes;
+
+    // Reverse of reordering of the line (L2 according to Bidi spec):
+    // L2. From the highest level found in the text to the lowest odd level on each line,
+    // reverse any contiguous sequence of characters that are at that level or higher.
+
+    // Reversing the reordering of the line is only done up to the lowest odd level.
+    if (!(minLevel % 2))
+        ++minLevel;
+
+    auto end = boxes.end();
+    for (; minLevel <= maxLevel; ++minLevel) {
+        auto box = boxes.begin();
+        while (box < end) {
+            while (box < end && (*box)->bidiLevel() < minLevel)
+                ++box;
+
+            auto first = box;
+            while (box < end && (*box)->bidiLevel() >= minLevel)
+                ++box;
+
+            auto last = box;
+            reverseFunction(first, last);
+        }
+    }
+
+    return boxes;
+}
+
 }
 }
diff --git a/Source/WebCore/rendering/LegacyInlineFlowBox.cpp b/Source/WebCore/rendering/LegacyInlineFlowBox.cpp
index 594867e..2cddedb 100644
--- a/Source/WebCore/rendering/LegacyInlineFlowBox.cpp
+++ b/Source/WebCore/rendering/LegacyInlineFlowBox.cpp
@@ -1295,59 +1295,6 @@
     return result;
 }
 
-void LegacyInlineFlowBox::collectLeafBoxesInLogicalOrder(Vector<LegacyInlineBox*>& leafBoxesInLogicalOrder, CustomInlineBoxRangeReverse customReverseImplementation, void* userData) const
-{
-    LegacyInlineBox* leaf = firstLeafDescendant();
-
-    // FIXME: The reordering code is a copy of parts from BidiResolver::createBidiRunsForLine, operating directly on InlineBoxes, instead of BidiRuns.
-    // Investigate on how this code could possibly be shared.
-    unsigned char minLevel = 128;
-    unsigned char maxLevel = 0;
-
-    // First find highest and lowest levels, and initialize leafBoxesInLogicalOrder with the leaf boxes in visual order.
-    for (; leaf; leaf = leaf->nextLeafOnLine()) {
-        minLevel = std::min(minLevel, leaf->bidiLevel());
-        maxLevel = std::max(maxLevel, leaf->bidiLevel());
-        leafBoxesInLogicalOrder.append(leaf);
-    }
-
-    if (renderer().style().rtlOrdering() == Order::Visual)
-        return;
-
-    // Reverse of reordering of the line (L2 according to Bidi spec):
-    // L2. From the highest level found in the text to the lowest odd level on each line,
-    // reverse any contiguous sequence of characters that are at that level or higher.
-
-    // Reversing the reordering of the line is only done up to the lowest odd level.
-    if (!(minLevel % 2))
-        ++minLevel;
-
-    Vector<LegacyInlineBox*>::iterator end = leafBoxesInLogicalOrder.end();
-    while (minLevel <= maxLevel) {
-        Vector<LegacyInlineBox*>::iterator it = leafBoxesInLogicalOrder.begin();
-        while (it != end) {
-            while (it != end) {
-                if ((*it)->bidiLevel() >= minLevel)
-                    break;
-                ++it;
-            }
-            Vector<LegacyInlineBox*>::iterator first = it;
-            while (it != end) {
-                if ((*it)->bidiLevel() < minLevel)
-                    break;
-                ++it;
-            }
-            Vector<LegacyInlineBox*>::iterator last = it;
-            if (customReverseImplementation) {
-                ASSERT(userData);
-                (*customReverseImplementation)(userData, first, last);
-            } else
-                std::reverse(first, last);
-        }                
-        ++minLevel;
-    }
-}
-
 void LegacyInlineFlowBox::computeReplacedAndTextLineTopAndBottom(LayoutUnit& lineTop, LayoutUnit& lineBottom) const
 {
     for (const auto* box = firstChild(); box; box = box->nextOnLine()) {
diff --git a/Source/WebCore/rendering/LegacyInlineFlowBox.h b/Source/WebCore/rendering/LegacyInlineFlowBox.h
index 4e32085..38cb17a 100644
--- a/Source/WebCore/rendering/LegacyInlineFlowBox.h
+++ b/Source/WebCore/rendering/LegacyInlineFlowBox.h
@@ -93,9 +93,6 @@
     LegacyInlineBox* firstLeafDescendant() const;
     LegacyInlineBox* lastLeafDescendant() const;
 
-    typedef void (*CustomInlineBoxRangeReverse)(void* userData, Vector<LegacyInlineBox*>::iterator first, Vector<LegacyInlineBox*>::iterator last);
-    void collectLeafBoxesInLogicalOrder(Vector<LegacyInlineBox*>&, CustomInlineBoxRangeReverse customReverseImplementation = nullptr, void* userData = nullptr) const;
-
     void setConstructed() final
     {
         LegacyInlineBox::setConstructed();
diff --git a/Source/WebCore/rendering/svg/SVGRootInlineBox.cpp b/Source/WebCore/rendering/svg/SVGRootInlineBox.cpp
index 9a8f73d..3cfcb2a 100644
--- a/Source/WebCore/rendering/svg/SVGRootInlineBox.cpp
+++ b/Source/WebCore/rendering/svg/SVGRootInlineBox.cpp
@@ -25,6 +25,7 @@
 #include "SVGRootInlineBox.h"
 
 #include "GraphicsContext.h"
+#include "InlineIteratorLogicalOrderTraversal.h"
 #include "RenderSVGText.h"
 #include "RenderSVGTextPath.h"
 #include "SVGElementTypeHelpers.h"
@@ -85,7 +86,7 @@
         return;
 
     if (textRoot.needsReordering())
-        reorderValueLists(layoutAttributes);
+        reorderValueListsToLogicalOrder(layoutAttributes);
 
     // Perform SVG text layout phase two (see SVGTextLayoutEngine for details).
     SVGTextLayoutEngine characterLayout(layoutAttributes);
@@ -270,26 +271,24 @@
     ASSERT(last);
 }
 
-static inline void reverseInlineBoxRangeAndValueListsIfNeeded(void* userData, Vector<LegacyInlineBox*>::iterator first, Vector<LegacyInlineBox*>::iterator last)
+static inline void reverseInlineBoxRangeAndValueListsIfNeeded(Vector<SVGTextLayoutAttributes*>& attributes, Vector<InlineIterator::LeafBoxIterator>::iterator first, Vector<InlineIterator::LeafBoxIterator>::iterator last)
 {
-    ASSERT(userData);
-    Vector<SVGTextLayoutAttributes*>& attributes = *reinterpret_cast<Vector<SVGTextLayoutAttributes*>*>(userData);
-
     // This is a copy of std::reverse(first, last). It additionally assures that the metrics map within the renderers belonging to the InlineBoxes are reordered as well.
     while (true)  {
         if (first == last || first == --last)
             return;
-
-        if (!is<SVGInlineTextBox>(**last) || !is<SVGInlineTextBox>(**first)) {
-            LegacyInlineBox* temp = *first;
+        auto* legacyFirst = (*first)->legacyInlineBox();
+        auto* legacyLast = (*last)->legacyInlineBox();
+        if (!is<SVGInlineTextBox>(legacyFirst) || !is<SVGInlineTextBox>(legacyLast)) {
+            auto temp = *first;
             *first = *last;
             *last = temp;
             ++first;
             continue;
         }
 
-        auto& firstTextBox = downcast<SVGInlineTextBox>(**first);
-        auto& lastTextBox = downcast<SVGInlineTextBox>(**last);
+        auto& firstTextBox = downcast<SVGInlineTextBox>(*legacyFirst);
+        auto& lastTextBox = downcast<SVGInlineTextBox>(*legacyLast);
 
         // Reordering is only necessary for BiDi text that is _absolutely_ positioned.
         if (firstTextBox.len() == 1 && firstTextBox.len() == lastTextBox.len()) {
@@ -302,7 +301,7 @@
             swapItemsInLayoutAttributes(firstAttributes, lastAttributes, firstTextBox.start(), lastTextBox.start());
         }
 
-        LegacyInlineBox* temp = *first;
+        auto temp = *first;
         *first = *last;
         *last = temp;
 
@@ -310,10 +309,14 @@
     }
 }
 
-void SVGRootInlineBox::reorderValueLists(Vector<SVGTextLayoutAttributes*>& attributes)
+void SVGRootInlineBox::reorderValueListsToLogicalOrder(Vector<SVGTextLayoutAttributes*>& attributes)
 {
-    Vector<LegacyInlineBox*> leafBoxesInLogicalOrder;
-    collectLeafBoxesInLogicalOrder(leafBoxesInLogicalOrder, reverseInlineBoxRangeAndValueListsIfNeeded, &attributes);
+    auto line = InlineIterator::LineIterator(this);
+
+    InlineIterator::leafBoxesInLogicalOrder(line, [&](auto first, auto last) {
+        reverseInlineBoxRangeAndValueListsIfNeeded(attributes, first, last);
+    });
+
 }
 
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/svg/SVGRootInlineBox.h b/Source/WebCore/rendering/svg/SVGRootInlineBox.h
index 145f05b..0d16269 100644
--- a/Source/WebCore/rendering/svg/SVGRootInlineBox.h
+++ b/Source/WebCore/rendering/svg/SVGRootInlineBox.h
@@ -50,7 +50,7 @@
 
 private:
     bool isSVGRootInlineBox() const override { return true; }
-    void reorderValueLists(Vector<SVGTextLayoutAttributes*>&);
+    void reorderValueListsToLogicalOrder(Vector<SVGTextLayoutAttributes*>&);
     void layoutCharactersInTextBoxes(LegacyInlineFlowBox*, SVGTextLayoutEngine&);
     void layoutChildBoxes(LegacyInlineFlowBox*, FloatRect* = nullptr);
     void layoutRootBox(const FloatRect&);