Changeset 294758 in webkit

Timestamp:

May 24, 2022, 12:43:40 PM (4 years ago)

Author:

commit-queue@webkit.org

Message:

Simplify ExpansionBehavior to avoid using getters/setters
​https://bugs.webkit.org/show_bug.cgi?id=240823

Patch by Kiet Ho <Kiet Ho> on 2022-05-24
Reviewed by Cameron McCormack.

• Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp:

(WebCore::Layout::Line::applyRunExpansion):

• Source/WebCore/platform/graphics/ComplexTextController.cpp:

(WebCore::ComplexTextController::adjustGlyphsAndAdvances):

• Source/WebCore/platform/graphics/FontCascade.cpp:

(WebCore::FontCascade::expansionOpportunityCountInternal):

• Source/WebCore/platform/graphics/TextRun.cpp:
• Source/WebCore/platform/graphics/WidthIterator.cpp:

(WebCore::WidthIterator::WidthIterator):
(WebCore::WidthIterator::calculateAdditionalWidth const):

• Source/WebCore/platform/text/TextFlags.h:

(WebCore::ExpansionBehavior::ExpansionBehavior):
(WebCore::ExpansionBehavior::defaultBehavior):
(WebCore::ExpansionBehavior::allowRightOnly):
(WebCore::ExpansionBehavior::allowLeftOnly):
(WebCore::ExpansionBehavior::forceLeftOnly):
(WebCore::ExpansionBehavior::forbidAll):
(WebCore::ExpansionBehavior::left const): Deleted.
(WebCore::ExpansionBehavior::setLeft): Deleted.
(WebCore::ExpansionBehavior::right const): Deleted.
(WebCore::ExpansionBehavior::setRight): Deleted.

• Source/WebCore/rendering/LegacyInlineTextBox.cpp:

(WebCore::LegacyInlineTextBox::expansionBehavior const):

• Source/WebCore/rendering/LegacyLineLayout.cpp:

(WebCore::expansionBehaviorForInlineTextBox):
(WebCore::applyExpansionBehavior):

Canonical link: ​https://commits.webkit.org/250926@main

Location:

trunk/Source/WebCore

Files:

• layout/formattingContexts/inline/InlineLine.cpp (modified) (2 diffs)
• platform/graphics/ComplexTextController.cpp (modified) (1 diff)
• platform/graphics/FontCascade.cpp (modified) (4 diffs)
• platform/graphics/WidthIterator.cpp (modified) (2 diffs)
• platform/text/TextFlags.h (modified) (2 diffs)
• rendering/LegacyInlineTextBox.cpp (modified) (1 diff)
• rendering/LegacyLineLayout.cpp (modified) (5 diffs)

trunk/Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp

@@ -124 +124 @@
                 expansionBehavior = ExpansionBehavior::forbidAll();
             else {
-                expansionBehavior.setLeft(runIsAfterExpansion ? ExpansionBehavior::Behavior::Forbid : ExpansionBehavior::Behavior::Allow);
-                expansionBehavior.setRight(ExpansionBehavior::Behavior::Allow);
+                expansionBehavior.left = runIsAfterExpansion ? ExpansionBehavior::Behavior::Forbid : ExpansionBehavior::Behavior::Allow;
+                expansionBehavior.right = ExpansionBehavior::Behavior::Allow;
                 auto& textContent = *run.textContent();
                 // Trailing hanging whitespace sequence is ignored when computing the expansion opportunities.
@@ -145 +145 @@
     // Forbid right expansion in the last run to prevent trailing expansion at the end of the line.
     if (lastRunIndexWithContent && runsExpansionOpportunities[*lastRunIndexWithContent]) {
-        runsExpansionBehaviors[*lastRunIndexWithContent].setRight(ExpansionBehavior::Behavior::Forbid);
+        runsExpansionBehaviors[*lastRunIndexWithContent].right = ExpansionBehavior::Behavior::Forbid;
         if (runIsAfterExpansion) {
             // When the last run has an after expansion (e.g. CJK ideograph) we need to remove this trailing expansion opportunity.

trunk/Source/WebCore/platform/graphics/ComplexTextController.cpp

@@ -674 +674 @@
 void ComplexTextController::adjustGlyphsAndAdvances()
 {
-    bool afterExpansion = m_run.expansionBehavior().left() == ExpansionBehavior::Behavior::Forbid;
+    bool afterExpansion = m_run.expansionBehavior().left == ExpansionBehavior::Behavior::Forbid;
     size_t runCount = m_complexTextRuns.size();
     bool hasExtraSpacing = (m_font.letterSpacing() || m_font.wordSpacing() || m_expansion) && !m_run.spacingDisabled();
-    bool runForcesLeftExpansion = m_run.expansionBehavior().left() == ExpansionBehavior::Behavior::Force;
-    bool runForcesRightExpansion = m_run.expansionBehavior().right() == ExpansionBehavior::Behavior::Force;
-    bool runForbidsLeftExpansion = m_run.expansionBehavior().left() == ExpansionBehavior::Behavior::Forbid;
-    bool runForbidsRightExpansion = m_run.expansionBehavior().right() == ExpansionBehavior::Behavior::Forbid;
+    bool runForcesLeftExpansion = m_run.expansionBehavior().left == ExpansionBehavior::Behavior::Force;
+    bool runForcesRightExpansion = m_run.expansionBehavior().right == ExpansionBehavior::Behavior::Force;
+    bool runForbidsLeftExpansion = m_run.expansionBehavior().left == ExpansionBehavior::Behavior::Forbid;
+    bool runForbidsRightExpansion = m_run.expansionBehavior().right == ExpansionBehavior::Behavior::Forbid;
 
     // We are iterating in glyph order, not string order. Compare this to WidthIterator::advanceInternal()

trunk/Source/WebCore/platform/graphics/FontCascade.cpp

@@ -963 +963 @@
 {
     unsigned count = 0;
-    bool isAfterExpansion = expansionBehavior.left() == ExpansionBehavior::Behavior::Forbid;
-    if (expansionBehavior.left() == ExpansionBehavior::Behavior::Force) {
+    bool isAfterExpansion = expansionBehavior.left == ExpansionBehavior::Behavior::Forbid;
+    if (expansionBehavior.left == ExpansionBehavior::Behavior::Force) {
         ++count;
         isAfterExpansion = true;
@@ -985 +985 @@
         }
     }
-    if (!isAfterExpansion && expansionBehavior.right() == ExpansionBehavior::Behavior::Force) {
+    if (!isAfterExpansion && expansionBehavior.right == ExpansionBehavior::Behavior::Force) {
         ++count;
         isAfterExpansion = true;
-    } else if (isAfterExpansion && expansionBehavior.right() == ExpansionBehavior::Behavior::Forbid) {
+    } else if (isAfterExpansion && expansionBehavior.right == ExpansionBehavior::Behavior::Forbid) {
         ASSERT(count);
         --count;
@@ -999 +999 @@
 {
     unsigned count = 0;
-    bool isAfterExpansion = expansionBehavior.left() == ExpansionBehavior::Behavior::Forbid;
-    if (expansionBehavior.left() == ExpansionBehavior::Behavior::Force) {
+    bool isAfterExpansion = expansionBehavior.left == ExpansionBehavior::Behavior::Forbid;
+    if (expansionBehavior.left == ExpansionBehavior::Behavior::Force) {
         ++count;
         isAfterExpansion = true;
@@ -1047 +1047 @@
         }
     }
-    if (!isAfterExpansion && expansionBehavior.right() == ExpansionBehavior::Behavior::Force) {
+    if (!isAfterExpansion && expansionBehavior.right == ExpansionBehavior::Behavior::Force) {
         ++count;
         isAfterExpansion = true;
-    } else if (isAfterExpansion && expansionBehavior.right() == ExpansionBehavior::Behavior::Forbid) {
+    } else if (isAfterExpansion && expansionBehavior.right == ExpansionBehavior::Behavior::Forbid) {
         ASSERT(count);
         --count;

trunk/Source/WebCore/platform/graphics/WidthIterator.cpp

@@ -42 +42 @@
     , m_fallbackFonts(fallbackFonts)
     , m_expansion(run.expansion())
-    , m_isAfterExpansion(run.expansionBehavior().left() == ExpansionBehavior::Behavior::Forbid)
+    , m_isAfterExpansion(run.expansionBehavior().left == ExpansionBehavior::Behavior::Forbid)
     , m_accountForGlyphBounds(accountForGlyphBounds)
     , m_enableKerning(font.enableKerning())
@@ -382 +382 @@
                 std::swap(isLeftmostCharacter, isRightmostCharacter);
 
-            bool forceLeftExpansion = isLeftmostCharacter && m_run.expansionBehavior().left() == ExpansionBehavior::Behavior::Force;
-            bool forceRightExpansion = isRightmostCharacter && m_run.expansionBehavior().right() == ExpansionBehavior::Behavior::Force;
-            bool forbidLeftExpansion = isLeftmostCharacter && m_run.expansionBehavior().left() == ExpansionBehavior::Behavior::Forbid;
-            bool forbidRightExpansion = isRightmostCharacter && m_run.expansionBehavior().right() == ExpansionBehavior::Behavior::Forbid;
+            bool forceLeftExpansion = isLeftmostCharacter && m_run.expansionBehavior().left == ExpansionBehavior::Behavior::Force;
+            bool forceRightExpansion = isRightmostCharacter && m_run.expansionBehavior().right == ExpansionBehavior::Behavior::Force;
+            bool forbidLeftExpansion = isLeftmostCharacter && m_run.expansionBehavior().left == ExpansionBehavior::Behavior::Forbid;
+            bool forbidRightExpansion = isRightmostCharacter && m_run.expansionBehavior().right == ExpansionBehavior::Behavior::Forbid;
 
             bool isIdeograph = FontCascade::canExpandAroundIdeographsInComplexText() && FontCascade::isCJKIdeographOrSymbol(character);

trunk/Source/WebCore/platform/text/TextFlags.h

@@ -59 +59 @@
 };
 
-class ExpansionBehavior {
-public:
+struct ExpansionBehavior {
     enum class Behavior : uint8_t {
         Forbid,
@@ -70 +69 @@
 
     ExpansionBehavior(Behavior left, Behavior right)
-        : m_left(static_cast<uint8_t>(left))
-        , m_right(static_cast<uint8_t>(right))
-    {
-    }
-
-    static const ExpansionBehavior defaultBehavior()
+        : left(left)
+        , right(right)
+    {
+    }
+
+    static ExpansionBehavior defaultBehavior()
     {
         return { };
     }
 
-    static const ExpansionBehavior allowRightOnly()
+    static ExpansionBehavior allowRightOnly()
     {
         return { Behavior::Forbid, Behavior::Allow };
     }
 
-    static const ExpansionBehavior allowLeftOnly()
+    static ExpansionBehavior allowLeftOnly()
     {
         return { Behavior::Allow, Behavior::Forbid };
     }
 
-    static const ExpansionBehavior forceLeftOnly()
+    static ExpansionBehavior forceLeftOnly()
     {
         return { Behavior::Force, Behavior::Forbid };
     }
 
-    static const ExpansionBehavior forbidAll()
+    static ExpansionBehavior forbidAll()
     {
         return { Behavior::Forbid, Behavior::Forbid };
     }
 
-    Behavior left() const { return static_cast<Behavior>(m_left); }
-    void setLeft(Behavior behavior) { m_left = static_cast<uint8_t>(behavior); }
-
-    Behavior right() const { return static_cast<Behavior>(m_right); }
-    void setRight(Behavior behavior) { m_right = static_cast<uint8_t>(behavior); }
-
-private:
-    // Default behavior follows the previous implementation:
-    // forbids left and allows right expansions.
-    uint8_t m_left : 2 { static_cast<uint8_t>(Behavior::Forbid) };
-    uint8_t m_right : 2 { static_cast<uint8_t>(Behavior::Allow) };
+    Behavior left : 2 { Behavior::Forbid };
+    Behavior right : 2 { Behavior::Allow };
 };
 

trunk/Source/WebCore/rendering/LegacyInlineTextBox.cpp

@@ -492 +492 @@
 
     if (forceLeftExpansion())
-        behavior.setLeft(ExpansionBehavior::Behavior::Force);
+        behavior.left = ExpansionBehavior::Behavior::Force;
     else if (canHaveLeftExpansion())
-        behavior.setLeft(ExpansionBehavior::Behavior::Allow);
+        behavior.left = ExpansionBehavior::Behavior::Allow;
     else
-        behavior.setLeft(ExpansionBehavior::Behavior::Forbid);
+        behavior.left = ExpansionBehavior::Behavior::Forbid;
 
     if (forceRightExpansion())
-        behavior.setRight(ExpansionBehavior::Behavior::Force);
+        behavior.right = ExpansionBehavior::Behavior::Force;
     else if (expansion() && nextLeafOnLine() && !nextLeafOnLine()->isLineBreak())
-        behavior.setRight(ExpansionBehavior::Behavior::Allow);
+        behavior.right = ExpansionBehavior::Behavior::Allow;
     else
-        behavior.setRight(ExpansionBehavior::Behavior::Forbid);
+        behavior.right = ExpansionBehavior::Behavior::Forbid;
 
     return behavior;

trunk/Source/WebCore/rendering/LegacyLineLayout.cpp

@@ -744 +744 @@
                         if (FontCascade::leftExpansionOpportunity(downcast<RenderText>(leafChild->renderer()).stringView(), leafChild->direction())) {
                             setRightExpansion = true;
-                            result.setRight(ExpansionBehavior::Behavior::Force);
+                            result.right = ExpansionBehavior::Behavior::Force;
                         }
                     }
@@ -759 +759 @@
                         if (FontCascade::rightExpansionOpportunity(downcast<RenderText>(leafChild->renderer()).stringView(), leafChild->direction())) {
                             setLeftExpansion = true;
-                            result.setLeft(ExpansionBehavior::Behavior::Force);
+                            result.left = ExpansionBehavior::Behavior::Force;
                         }
                     }
@@ -770 +770 @@
             if (&textBox == rubyBase.firstRootBox()->firstLeafDescendant()) {
                 setLeftExpansion = true;
-                result.setLeft(ExpansionBehavior::Behavior::Forbid);
+                result.left = ExpansionBehavior::Behavior::Forbid;
             } if (&textBox == rubyBase.firstRootBox()->lastLeafDescendant()) {
                 setRightExpansion = true;
-                result.setRight(ExpansionBehavior::Behavior::Forbid);
+                result.right = ExpansionBehavior::Behavior::Forbid;
             }
         }
     }
     if (!setLeftExpansion)
-        result.setLeft(isAfterExpansion ? ExpansionBehavior::Behavior::Forbid : ExpansionBehavior::Behavior::Allow);
+        result.left = isAfterExpansion ? ExpansionBehavior::Behavior::Forbid : ExpansionBehavior::Behavior::Allow;
     if (!setRightExpansion)
-        result.setRight(ExpansionBehavior::Behavior::Allow);
+        result.right = ExpansionBehavior::Behavior::Allow;
     return result;
 }
@@ -786 +786 @@
 static inline void applyExpansionBehavior(LegacyInlineTextBox& textBox, ExpansionBehavior expansionBehavior)
 {
-    switch (expansionBehavior.left()) {
+    switch (expansionBehavior.left) {
     case ExpansionBehavior::Behavior::Force:
         textBox.setForceLeftExpansion();
@@ -801 +801 @@
     };
 
-    switch (expansionBehavior.right()) {
+    switch (expansionBehavior.right) {
     case ExpansionBehavior::Behavior::Force:
         textBox.setForceRightExpansion();