Skip to content
Permalink
Browse files
Simplify ExpansionBehavior to avoid using getters/setters
https://bugs.webkit.org/show_bug.cgi?id=240823

Patch by Kiet Ho <tho22@apple.com> 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
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294758 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
tuankiet65 authored and webkit-commit-queue committed May 24, 2022
1 parent f86ff68 commit 956f033898db4b247105dd4628a8bd7bb2253a8e
Showing 7 changed files with 45 additions and 55 deletions.
@@ -123,8 +123,8 @@ void Line::applyRunExpansion(InlineLayoutUnit horizontalAvailableSpace)
if (run.hasTextCombine())
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.
auto hangingTrailingContentInCurrentRun = std::min(textContent.length, hangingTrailingContentLength);
@@ -144,7 +144,7 @@ void Line::applyRunExpansion(InlineLayoutUnit horizontalAvailableSpace)
}
// 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.
// Note that this is not about trailing collapsible whitespace as at this point we trimmed them all.
@@ -673,13 +673,13 @@ static inline std::pair<bool, bool> expansionLocation(bool ideograph, bool treat

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()
for (size_t runIndex = 0; runIndex < runCount; ++runIndex) {
@@ -962,8 +962,8 @@ bool FontCascade::isCJKIdeographOrSymbol(UChar32 c)
std::pair<unsigned, bool> FontCascade::expansionOpportunityCountInternal(const LChar* characters, unsigned length, TextDirection direction, ExpansionBehavior expansionBehavior)
{
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;
}
@@ -984,10 +984,10 @@ std::pair<unsigned, bool> FontCascade::expansionOpportunityCountInternal(const L
isAfterExpansion = false;
}
}
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;
isAfterExpansion = false;
@@ -998,8 +998,8 @@ std::pair<unsigned, bool> FontCascade::expansionOpportunityCountInternal(const L
std::pair<unsigned, bool> FontCascade::expansionOpportunityCountInternal(const UChar* characters, unsigned length, TextDirection direction, ExpansionBehavior expansionBehavior)
{
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;
}
@@ -1046,10 +1046,10 @@ std::pair<unsigned, bool> FontCascade::expansionOpportunityCountInternal(const U
isAfterExpansion = false;
}
}
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;
isAfterExpansion = false;
@@ -41,7 +41,7 @@ WidthIterator::WidthIterator(const FontCascade& font, const TextRun& run, HashSe
, m_run(run)
, 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())
, m_requiresShaping(font.requiresShaping())
@@ -381,10 +381,10 @@ auto WidthIterator::calculateAdditionalWidth(GlyphBuffer& glyphBuffer, GlyphBuff
if (!m_run.ltr())
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);

@@ -58,8 +58,7 @@ enum class NonCJKGlyphOrientation : uint8_t {
Upright
};

class ExpansionBehavior {
public:
struct ExpansionBehavior {
enum class Behavior : uint8_t {
Forbid,
Allow,
@@ -69,47 +68,38 @@ class ExpansionBehavior {
ExpansionBehavior() = default;

ExpansionBehavior(Behavior left, Behavior right)
: m_left(static_cast<uint8_t>(left))
, m_right(static_cast<uint8_t>(right))
: left(left)
, right(right)
{
}

static const ExpansionBehavior defaultBehavior()
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 };
};

enum FontSynthesisValues {
@@ -491,18 +491,18 @@ ExpansionBehavior LegacyInlineTextBox::expansionBehavior() const
ExpansionBehavior behavior;

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;
}
@@ -743,7 +743,7 @@ static inline ExpansionBehavior expansionBehaviorForInlineTextBox(RenderBlockFlo
// FIXME: This leftExpansionOpportunity doesn't actually work because it doesn't perform the UBA
if (FontCascade::leftExpansionOpportunity(downcast<RenderText>(leafChild->renderer()).stringView(), leafChild->direction())) {
setRightExpansion = true;
result.setRight(ExpansionBehavior::Behavior::Force);
result.right = ExpansionBehavior::Behavior::Force;
}
}
}
@@ -758,7 +758,7 @@ static inline ExpansionBehavior expansionBehaviorForInlineTextBox(RenderBlockFlo
// FIXME: This leftExpansionOpportunity doesn't actually work because it doesn't perform the UBA
if (FontCascade::rightExpansionOpportunity(downcast<RenderText>(leafChild->renderer()).stringView(), leafChild->direction())) {
setLeftExpansion = true;
result.setLeft(ExpansionBehavior::Behavior::Force);
result.left = ExpansionBehavior::Behavior::Force;
}
}
}
@@ -769,23 +769,23 @@ static inline ExpansionBehavior expansionBehaviorForInlineTextBox(RenderBlockFlo
RenderRubyBase& rubyBase = downcast<RenderRubyBase>(block);
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;
}

static inline void applyExpansionBehavior(LegacyInlineTextBox& textBox, ExpansionBehavior expansionBehavior)
{
switch (expansionBehavior.left()) {
switch (expansionBehavior.left) {
case ExpansionBehavior::Behavior::Force:
textBox.setForceLeftExpansion();
break;
@@ -800,7 +800,7 @@ static inline void applyExpansionBehavior(LegacyInlineTextBox& textBox, Expansio
break;
};

switch (expansionBehavior.right()) {
switch (expansionBehavior.right) {
case ExpansionBehavior::Behavior::Force:
textBox.setForceRightExpansion();
break;

0 comments on commit 956f033

Please sign in to comment.