<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[285104] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/285104">285104</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2021-11-01 08:40:32 -0700 (Mon, 01 Nov 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>[LFC][IFC] Introduce paragraph content building to InlineItemsBuilder::handleTextContent
https://bugs.webkit.org/show_bug.cgi?id=232546

Reviewed by Antti Koivisto.

Let's check for directional control characters in text content and build
the paragraph content accordingly. Now that we've got both the inline box and
the text bidi content (atomic inline level boxes are still missing) breakInlineItemsAtBidiBoundaries
can start calling into ubidi to find content boundaries.

* layout/formattingContexts/inline/InlineItemsBuilder.cpp:
(WebCore::Layout::InlineItemsBuilder::handleTextContent):
(WebCore::Layout::InlineItemsBuilder::enterBidiContext):
(WebCore::Layout::InlineItemsBuilder::buildPreviousTextContent):
* layout/formattingContexts/inline/InlineItemsBuilder.h:
* layout/integration/LayoutIntegrationCoverage.cpp:
(WebCore::LayoutIntegration::canUseForFontAndText):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorelayoutformattingContextsinlineInlineItemsBuildercpp">trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp</a></li>
<li><a href="#trunkSourceWebCorelayoutformattingContextsinlineInlineItemsBuilderh">trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (285103 => 285104)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2021-11-01 14:11:59 UTC (rev 285103)
+++ trunk/Source/WebCore/ChangeLog      2021-11-01 15:40:32 UTC (rev 285104)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2021-11-01  Alan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Introduce paragraph content building to InlineItemsBuilder::handleTextContent
+        https://bugs.webkit.org/show_bug.cgi?id=232546
+
+        Reviewed by Antti Koivisto.
+
+        Let's check for directional control characters in text content and build
+        the paragraph content accordingly. Now that we've got both the inline box and
+        the text bidi content (atomic inline level boxes are still missing) breakInlineItemsAtBidiBoundaries
+        can start calling into ubidi to find content boundaries.
+
+        * layout/formattingContexts/inline/InlineItemsBuilder.cpp:
+        (WebCore::Layout::InlineItemsBuilder::handleTextContent):
+        (WebCore::Layout::InlineItemsBuilder::enterBidiContext):
+        (WebCore::Layout::InlineItemsBuilder::buildPreviousTextContent):
+        * layout/formattingContexts/inline/InlineItemsBuilder.h:
+        * layout/integration/LayoutIntegrationCoverage.cpp:
+        (WebCore::LayoutIntegration::canUseForFontAndText):
+
</ins><span class="cx"> 2021-11-01  Daniel Kolesa  <dkolesa@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         Fix build with GCC 8.4 on Ubuntu 18.04
</span></span></pre></div>
<a id="trunkSourceWebCorelayoutformattingContextsinlineInlineItemsBuildercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp (285103 => 285104)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp     2021-11-01 14:11:59 UTC (rev 285103)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp        2021-11-01 15:40:32 UTC (rev 285104)
</span><span class="lines">@@ -33,6 +33,8 @@
</span><span class="cx"> namespace WebCore {
</span><span class="cx"> namespace Layout {
</span><span class="cx"> 
</span><ins>+#define ALLOW_BIDI_CONTENT 0
+
</ins><span class="cx"> struct WhitespaceContent {
</span><span class="cx">     size_t length { 0 };
</span><span class="cx">     bool isWordSeparator { true };
</span><span class="lines">@@ -139,7 +141,8 @@
</span><span class="cx"> void InlineItemsBuilder::handleTextContent(const InlineTextBox& inlineTextBox, InlineItems& inlineItems)
</span><span class="cx"> {
</span><span class="cx">     auto text = inlineTextBox.content();
</span><del>-    if (!text.length())
</del><ins>+    auto contentLength = text.length();
+    if (!contentLength)
</ins><span class="cx">         return inlineItems.append(InlineTextItem::createEmptyItem(inlineTextBox));
</span><span class="cx"> 
</span><span class="cx">     auto& style = inlineTextBox.style();
</span><span class="lines">@@ -158,20 +161,29 @@
</span><span class="cx">         return TextUtil::width(inlineTextBox, fontCascade, startPosition, startPosition + length, { });
</span><span class="cx">     };
</span><span class="cx"> 
</span><del>-    while (currentPosition < text.length()) {
-        auto isSegmentBreakCandidate = [](auto character) {
-            return character == '\n';
</del><ins>+    if (!m_paragraphContentBuilder.isEmpty()) {
+        m_contentOffsetMap.set(&inlineTextBox, m_paragraphContentBuilder.length());
+        m_paragraphContentBuilder.append(text);
+    }
+
+    while (currentPosition < contentLength) {
+        auto handleSegmentBreak = [&] {
+            // Segment breaks with preserve new line style (white-space: pre, pre-wrap, break-spaces and pre-line) compute to forced line break.
+            auto isSegmentBreakCandidate = text[currentPosition] == newlineCharacter;
+            if (!isSegmentBreakCandidate || !shouldPreserveNewline)
+                return false;
+            inlineItems.append(InlineSoftLineBreakItem::createSoftLineBreakItem(inlineTextBox, currentPosition++));
+            return true;
</ins><span class="cx">         };
</span><del>-
-        // Segment breaks with preserve new line style (white-space: pre, pre-wrap, break-spaces and pre-line) compute to forced line break.
-        if (isSegmentBreakCandidate(text[currentPosition]) && shouldPreserveNewline) {
-            inlineItems.append(InlineSoftLineBreakItem::createSoftLineBreakItem(inlineTextBox, currentPosition));
-            ++currentPosition;
</del><ins>+        if (handleSegmentBreak())
</ins><span class="cx">             continue;
</span><del>-        }
</del><span class="cx"> 
</span><del>-        auto stopAtWordSeparatorBoundary = shouldPreserveSpacesAndTabs && fontCascade.wordSpacing();
-        if (auto whitespaceContent = moveToNextNonWhitespacePosition(text, currentPosition, shouldPreserveNewline, shouldPreserveSpacesAndTabs, shouldTreatNonBreakingSpaceAsRegularSpace, stopAtWordSeparatorBoundary)) {
</del><ins>+        auto handleWhitespace = [&] {
+            auto stopAtWordSeparatorBoundary = shouldPreserveSpacesAndTabs && fontCascade.wordSpacing();
+            auto whitespaceContent = moveToNextNonWhitespacePosition(text, currentPosition, shouldPreserveNewline, shouldPreserveSpacesAndTabs, shouldTreatNonBreakingSpaceAsRegularSpace, stopAtWordSeparatorBoundary);
+            if (!whitespaceContent)
+                return false;
+
</ins><span class="cx">             ASSERT(whitespaceContent->length);
</span><span class="cx">             auto appendWhitespaceItem = [&] (auto startPosition, auto itemLength) {
</span><span class="cx">                 auto simpleSingleWhitespaceContent = inlineTextBox.canUseSimplifiedContentMeasuring() && (itemLength == 1 || whitespaceContentIsTreatedAsSingleSpace);
</span><span class="lines">@@ -187,33 +199,60 @@
</span><span class="cx">             } else
</span><span class="cx">                 appendWhitespaceItem(currentPosition, whitespaceContent->length);
</span><span class="cx">             currentPosition += whitespaceContent->length;
</span><ins>+            return true;
+        };
+        if (handleWhitespace())
</ins><span class="cx">             continue;
</span><del>-        }
</del><span class="cx"> 
</span><del>-        auto hasTrailingSoftHyphen = false;
-        auto initialNonWhitespacePosition = currentPosition;
-        auto isAtSoftHyphen = [&](auto position) {
-            return text[position] == softHyphen;
-        };
-        if (style.hyphens() == Hyphens::None) {
-            // Let's merge candidate InlineTextItems separated by soft hyphen when the style says so.
-            while (currentPosition < text.length()) {
-                auto nonWhiteSpaceLength = moveToNextBreakablePosition(currentPosition, lineBreakIterator, style);
-                ASSERT(nonWhiteSpaceLength);
-                currentPosition += nonWhiteSpaceLength;
-                if (!isAtSoftHyphen(currentPosition - 1))
</del><ins>+        auto handleNonWhitespace = [&] {
+            auto startPosition = currentPosition;
+            auto endPosition = startPosition;
+            auto hasTrailingSoftHyphen = false;
+            if (style.hyphens() == Hyphens::None) {
+                // Let's merge candidate InlineTextItems separated by soft hyphen when the style says so.
+                do {
+                    endPosition += moveToNextBreakablePosition(endPosition, lineBreakIterator, style);
+                    ASSERT(startPosition < endPosition);
+                } while (endPosition < contentLength && text[endPosition - 1] == softHyphen);
+            } else {
+                endPosition += moveToNextBreakablePosition(startPosition, lineBreakIterator, style);
+                ASSERT(startPosition < endPosition);
+                hasTrailingSoftHyphen = text[endPosition - 1] == softHyphen;
+            }
+            ASSERT_IMPLIES(style.hyphens() == Hyphens::None, !hasTrailingSoftHyphen);
+            auto inlineItemLength = endPosition - startPosition;
+            inlineItems.append(InlineTextItem::createNonWhitespaceItem(inlineTextBox, startPosition, inlineItemLength, UBIDI_DEFAULT_LTR, hasTrailingSoftHyphen, inlineItemWidth(startPosition, inlineItemLength)));
+            currentPosition = endPosition;
+#if ALLOW_BIDI_CONTENT
+            // Check if the content has bidi dependency so that we have to start building the paragraph content for ubidi.
+            if (text.is8Bit() || !m_paragraphContentBuilder.isEmpty())
+                return true;
+
+            for (auto position = startPosition; position < endPosition;) {
+                UChar32 character;
+                U16_NEXT(text.characters16(), position, contentLength, character);
+
+                auto bidiCategory = u_charDirection(character);
+                auto needsBidi = bidiCategory == U_RIGHT_TO_LEFT
+                    || bidiCategory == U_RIGHT_TO_LEFT_ARABIC
+                    || bidiCategory == U_RIGHT_TO_LEFT_EMBEDDING
+                    || bidiCategory == U_RIGHT_TO_LEFT_OVERRIDE
+                    || bidiCategory == U_LEFT_TO_RIGHT_EMBEDDING
+                    || bidiCategory == U_LEFT_TO_RIGHT_OVERRIDE
+                    || bidiCategory == U_POP_DIRECTIONAL_FORMAT;
+                if (needsBidi) {
+                    buildPreviousTextContent(inlineItems);
+                    // buildPreviousTextContent takes care of this content too as some inline items have already been constructed for this text.
</ins><span class="cx">                     break;
</span><ins>+                }
</ins><span class="cx">             }
</span><del>-        } else {
-            auto nonWhiteSpaceLength = moveToNextBreakablePosition(initialNonWhitespacePosition, lineBreakIterator, style);
-            ASSERT(nonWhiteSpaceLength);
-            currentPosition += nonWhiteSpaceLength;
-            hasTrailingSoftHyphen = isAtSoftHyphen(currentPosition - 1);
-        }
-        ASSERT(initialNonWhitespacePosition < currentPosition);
-        ASSERT_IMPLIES(style.hyphens() == Hyphens::None, !hasTrailingSoftHyphen);
-        auto length = currentPosition - initialNonWhitespacePosition;
-        inlineItems.append(InlineTextItem::createNonWhitespaceItem(inlineTextBox, initialNonWhitespacePosition, length, UBIDI_DEFAULT_LTR, hasTrailingSoftHyphen, inlineItemWidth(initialNonWhitespacePosition, length)));
</del><ins>+#endif
+            return true;
+        };
+        if (handleNonWhitespace())
+            continue;
+        // Unsupported content?
+        ASSERT_NOT_REACHED();
</ins><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -315,25 +354,8 @@
</span><span class="cx"> 
</span><span class="cx"> void InlineItemsBuilder::enterBidiContext(const Box& box, UChar controlCharacter, const InlineItems& inlineItems)
</span><span class="cx"> {
</span><del>-    if (m_paragraphContentBuilder.isEmpty()) {
-        // FIXME: Move this to a dedicated function to support control characters embedded in text content.
-        auto buildPreviousTextContent = [&] {
-            const Box* lastLayoutBox = nullptr;
-            for (auto& inlineItem : inlineItems) {
-                if (!inlineItem.isText())
-                    continue;
-                auto& layoutBox = inlineItem.layoutBox();
-                if (lastLayoutBox == &layoutBox) {
-                    // We've already appended this content.
-                    continue;
-                }
-                m_contentOffsetMap.set(&layoutBox, m_paragraphContentBuilder.length());
-                m_paragraphContentBuilder.append(downcast<InlineTextBox>(layoutBox).content());
-                lastLayoutBox = &layoutBox;
-            }
-        };
-        buildPreviousTextContent();
-    }
</del><ins>+    if (m_paragraphContentBuilder.isEmpty())
+        buildPreviousTextContent(inlineItems);
</ins><span class="cx">     // Let the first control character represent the  box.
</span><span class="cx">     m_contentOffsetMap.add(&box, m_paragraphContentBuilder.length());
</span><span class="cx">     m_paragraphContentBuilder.append(controlCharacter);
</span><span class="lines">@@ -345,7 +367,27 @@
</span><span class="cx">     m_paragraphContentBuilder.append(controlCharacter);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+void InlineItemsBuilder::buildPreviousTextContent(const InlineItems& inlineItems)
+{
+    ASSERT(m_paragraphContentBuilder.isEmpty());
+    ASSERT(m_contentOffsetMap.isEmpty());
+
+    const Box* lastLayoutBox = nullptr;
+    for (auto& inlineItem : inlineItems) {
+        if (!inlineItem.isText())
+            continue;
+        auto& layoutBox = inlineItem.layoutBox();
+        if (lastLayoutBox == &layoutBox) {
+            // We've already appended this content.
+            continue;
+        }
+        m_contentOffsetMap.set(&layoutBox, m_paragraphContentBuilder.length());
+        m_paragraphContentBuilder.append(downcast<InlineTextBox>(layoutBox).content());
+        lastLayoutBox = &layoutBox;
+    }
</ins><span class="cx"> }
</span><ins>+
</ins><span class="cx"> }
</span><ins>+}
</ins><span class="cx"> 
</span><span class="cx"> #endif
</span></span></pre></div>
<a id="trunkSourceWebCorelayoutformattingContextsinlineInlineItemsBuilderh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.h (285103 => 285104)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.h       2021-11-01 14:11:59 UTC (rev 285103)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.h  2021-11-01 15:40:32 UTC (rev 285104)
</span><span class="lines">@@ -52,6 +52,8 @@
</span><span class="cx">     void enterBidiContext(const Box&, UChar, const InlineItems&);
</span><span class="cx">     void exitBidiContext(const Box&, UChar);
</span><span class="cx"> 
</span><ins>+    void buildPreviousTextContent(const InlineItems&);
+
</ins><span class="cx">     const ContainerBox& root() const { return m_root; }
</span><span class="cx"> 
</span><span class="cx">     const ContainerBox& m_root;
</span></span></pre>
</div>
</div>

</body>
</html>