Skip to content

Commit

Permalink
Avoid copying FontPalette and FontVariantAlternates
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=273568
<rdar://problem/127380343>

Reviewed by Chris Dumez.

Reduce unnecessary copies in our font handling code:
(1) FontPalette is a struct containing an enum and an AtomString, and should be returned
    by const reference to avoid copying that string.
(2) FontVariantAlternates is lightweight, containing only a `Markable`, but the
    `Markable` is holding a very heavy-weight object that contains Vectors of Strings,
    and Strings.

* Source/WebCore/css/CSSFontFace.h:
* Source/WebCore/css/CSSFontFaceSet.cpp:
(WebCore::CSSFontFaceSet::fontFace):
* Source/WebCore/css/CSSFontFaceSet.h:
* Source/WebCore/platform/graphics/FontDescription.h:
(WebCore::FontDescription::variantAlternates const):
(WebCore::FontDescription::fontPalette const):
(WebCore::FontDescription::setVariantAlternates):
(WebCore::FontDescription::setFontPalette):
* Source/WebCore/platform/text/TextFlags.cpp:
(WebCore::operator<<):
* Source/WebCore/platform/text/TextFlags.h:
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::setFontPalette):
* Source/WebCore/rendering/style/RenderStyle.h:
* Source/WebCore/rendering/style/RenderStyleInlines.h:
(WebCore::RenderStyle::fontPalette const):
* Source/WebCore/style/StyleBuilderCustom.h:
(WebCore::Style::BuilderCustom::applyValueFontVariantAlternates):

Canonical link: https://commits.webkit.org/278351@main
  • Loading branch information
brentfulgham authored and Brent Fulgham committed May 4, 2024
1 parent 0470d0f commit 371cab8
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 16 deletions.
4 changes: 2 additions & 2 deletions Source/WebCore/animation/CSSPropertyAnimation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3930,7 +3930,7 @@ CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
new DiscretePropertyWrapper<StyleColorScheme>(CSSPropertyColorScheme, &RenderStyle::colorScheme, &RenderStyle::setColorScheme),
#endif
new PropertyWrapperAspectRatio,
new DiscretePropertyWrapper<FontPalette>(CSSPropertyFontPalette, &RenderStyle::fontPalette, &RenderStyle::setFontPalette),
new DiscretePropertyWrapper<const FontPalette&>(CSSPropertyFontPalette, &RenderStyle::fontPalette, &RenderStyle::setFontPalette),

new OffsetPathWrapper,
new OffsetDistanceWrapper,
Expand All @@ -3954,7 +3954,7 @@ CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
new DiscreteFontDescriptionTypedWrapper<FontSynthesisLonghandValue>(CSSPropertyFontSynthesisWeight, &FontCascadeDescription::fontSynthesisWeight, &FontCascadeDescription::setFontSynthesisWeight),
new DiscreteFontDescriptionTypedWrapper<FontSynthesisLonghandValue>(CSSPropertyFontSynthesisStyle, &FontCascadeDescription::fontSynthesisStyle, &FontCascadeDescription::setFontSynthesisStyle),
new DiscreteFontDescriptionTypedWrapper<FontSynthesisLonghandValue>(CSSPropertyFontSynthesisSmallCaps, &FontCascadeDescription::fontSynthesisSmallCaps, &FontCascadeDescription::setFontSynthesisSmallCaps),
new DiscreteFontDescriptionTypedWrapper<FontVariantAlternates>(CSSPropertyFontVariantAlternates, &FontCascadeDescription::variantAlternates, &FontCascadeDescription::setVariantAlternates),
new DiscreteFontDescriptionTypedWrapper<const FontVariantAlternates&>(CSSPropertyFontVariantAlternates, &FontCascadeDescription::variantAlternates, &FontCascadeDescription::setVariantAlternates),
new FontVariantEastAsianWrapper,
new FontVariantLigaturesWrapper,
new FontVariantNumericWrapper,
Expand Down
11 changes: 6 additions & 5 deletions Source/WebCore/platform/graphics/FontDescription.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class FontDescription {
std::optional<FontSelectionValue> italic() const { return m_fontSelectionRequest.slope; }
FontSelectionValue stretch() const { return m_fontSelectionRequest.width; }
FontSelectionValue weight() const { return m_fontSelectionRequest.weight; }
FontSelectionRequest fontSelectionRequest() const { return m_fontSelectionRequest; }
const FontSelectionRequest& fontSelectionRequest() const { return m_fontSelectionRequest; }
TextRenderingMode textRenderingMode() const { return static_cast<TextRenderingMode>(m_textRendering); }
TextSpacingTrim textSpacingTrim() const { return m_textSpacingTrim; }
TextAutospace textAutospace() const { return m_textAutospace; }
Expand Down Expand Up @@ -80,7 +80,7 @@ class FontDescription {
FontVariantNumericFraction variantNumericFraction() const { return static_cast<FontVariantNumericFraction>(m_variantNumericFraction); }
FontVariantNumericOrdinal variantNumericOrdinal() const { return static_cast<FontVariantNumericOrdinal>(m_variantNumericOrdinal); }
FontVariantNumericSlashedZero variantNumericSlashedZero() const { return static_cast<FontVariantNumericSlashedZero>(m_variantNumericSlashedZero); }
FontVariantAlternates variantAlternates() const { return m_variantAlternates; }
const FontVariantAlternates& variantAlternates() const { return m_variantAlternates; }
FontVariantEastAsianVariant variantEastAsianVariant() const { return static_cast<FontVariantEastAsianVariant>(m_variantEastAsianVariant); }
FontVariantEastAsianWidth variantEastAsianWidth() const { return static_cast<FontVariantEastAsianWidth>(m_variantEastAsianWidth); }
FontVariantEastAsianRuby variantEastAsianRuby() const { return static_cast<FontVariantEastAsianRuby>(m_variantEastAsianRuby); }
Expand Down Expand Up @@ -108,7 +108,7 @@ class FontDescription {
FontStyleAxis fontStyleAxis() const { return m_fontStyleAxis ? FontStyleAxis::ital : FontStyleAxis::slnt; }
AllowUserInstalledFonts shouldAllowUserInstalledFonts() const { return static_cast<AllowUserInstalledFonts>(m_shouldAllowUserInstalledFonts); }
bool shouldDisableLigaturesForSpacing() const { return m_shouldDisableLigaturesForSpacing; }
FontPalette fontPalette() const { return m_fontPalette; }
const FontPalette& fontPalette() const { return m_fontPalette; }
FontSizeAdjust fontSizeAdjust() const { return m_sizeAdjust; }

void setComputedSize(float s) { m_computedSize = clampToFloat(s); }
Expand Down Expand Up @@ -139,7 +139,8 @@ class FontDescription {
void setVariantNumericFraction(FontVariantNumericFraction variant) { m_variantNumericFraction = enumToUnderlyingType(variant); }
void setVariantNumericOrdinal(FontVariantNumericOrdinal variant) { m_variantNumericOrdinal = enumToUnderlyingType(variant); }
void setVariantNumericSlashedZero(FontVariantNumericSlashedZero variant) { m_variantNumericSlashedZero = enumToUnderlyingType(variant); }
void setVariantAlternates(FontVariantAlternates variant) { m_variantAlternates = variant; }
void setVariantAlternates(const FontVariantAlternates& variant) { m_variantAlternates = variant; }
void setVariantAlternates(FontVariantAlternates&& variant) { m_variantAlternates = WTFMove(variant); }
void setVariantEastAsianVariant(FontVariantEastAsianVariant variant) { m_variantEastAsianVariant = enumToUnderlyingType(variant); }
void setVariantEastAsianWidth(FontVariantEastAsianWidth variant) { m_variantEastAsianWidth = enumToUnderlyingType(variant); }
void setVariantEastAsianRuby(FontVariantEastAsianRuby variant) { m_variantEastAsianRuby = enumToUnderlyingType(variant); }
Expand All @@ -148,7 +149,7 @@ class FontDescription {
void setFontStyleAxis(FontStyleAxis axis) { m_fontStyleAxis = axis == FontStyleAxis::ital; }
void setShouldAllowUserInstalledFonts(AllowUserInstalledFonts shouldAllowUserInstalledFonts) { m_shouldAllowUserInstalledFonts = enumToUnderlyingType(shouldAllowUserInstalledFonts); }
void setShouldDisableLigaturesForSpacing(bool shouldDisableLigaturesForSpacing) { m_shouldDisableLigaturesForSpacing = shouldDisableLigaturesForSpacing; }
void setFontPalette(FontPalette fontPalette) { m_fontPalette = fontPalette; }
void setFontPalette(const FontPalette& fontPalette) { m_fontPalette = fontPalette; }
void setFontSizeAdjust(FontSizeAdjust fontSizeAdjust) { m_sizeAdjust = fontSizeAdjust; }

static AtomString platformResolveGenericFamily(UScriptCode, const AtomString& locale, const AtomString& familyName);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/text/TextFlags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ WTF::TextStream& operator<<(TextStream& ts, Kerning kerning)
return ts;
}

WTF::TextStream& operator<<(TextStream& ts, FontVariantAlternates alternates)
WTF::TextStream& operator<<(TextStream& ts, const FontVariantAlternates& alternates)
{
if (alternates.isNormal())
ts << "normal";
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/text/TextFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ class FontVariantAlternates {
FontVariantAlternates() = default;
};

WTF::TextStream& operator<<(WTF::TextStream&, FontVariantAlternates);
WTF::TextStream& operator<<(WTF::TextStream&, const FontVariantAlternates&);

enum class FontVariantEastAsianVariant : uint8_t {
Normal,
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/style/RenderStyle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2923,7 +2923,7 @@ void RenderStyle::setFontItalic(std::optional<FontSelectionValue> value)
fontCascade().update(selector);
}

void RenderStyle::setFontPalette(FontPalette value)
void RenderStyle::setFontPalette(const FontPalette& value)
{
auto selector = fontCascade().fontSelector();
auto description = fontDescription();
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/style/RenderStyle.h
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ class RenderStyle final : public CanMakeCheckedPtr<RenderStyle> {
inline FontSelectionValue fontWeight() const;
inline FontSelectionValue fontStretch() const;
inline std::optional<FontSelectionValue> fontItalic() const;
inline FontPalette fontPalette() const;
inline const FontPalette& fontPalette() const;
inline FontSizeAdjust fontSizeAdjust() const;

inline const Length& textIndent() const;
Expand Down Expand Up @@ -1247,7 +1247,7 @@ class RenderStyle final : public CanMakeCheckedPtr<RenderStyle> {
void setFontWeight(FontSelectionValue);
void setFontStretch(FontSelectionValue);
void setFontItalic(std::optional<FontSelectionValue>);
void setFontPalette(FontPalette);
void setFontPalette(const FontPalette&);

void setColor(const Color&);
inline void setTextIndent(Length&&);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/style/RenderStyleInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ inline float RenderStyle::flexGrow() const { return m_nonInheritedData->miscData
inline float RenderStyle::flexShrink() const { return m_nonInheritedData->miscData->flexibleBox->flexShrink; }
inline FlexWrap RenderStyle::flexWrap() const { return static_cast<FlexWrap>(m_nonInheritedData->miscData->flexibleBox->flexWrap); }
inline std::optional<FontSelectionValue> RenderStyle::fontItalic() const { return fontDescription().italic(); }
inline FontPalette RenderStyle::fontPalette() const { return fontDescription().fontPalette(); }
inline const FontPalette& RenderStyle::fontPalette() const { return fontDescription().fontPalette(); }
inline FontSizeAdjust RenderStyle::fontSizeAdjust() const { return fontDescription().fontSizeAdjust(); }
inline FontSelectionValue RenderStyle::fontStretch() const { return fontDescription().stretch(); }
inline FontVariationSettings RenderStyle::fontVariationSettings() const { return fontDescription().variationSettings(); }
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/style/StyleBuilderCustom.h
Original file line number Diff line number Diff line change
Expand Up @@ -1635,9 +1635,9 @@ inline void BuilderCustom::applyInitialFontVariantAlternates(BuilderState& build

inline void BuilderCustom::applyValueFontVariantAlternates(BuilderState& builderState, CSSValue& value)
{
auto setAlternates = [&builderState](FontVariantAlternates alternates) {
auto setAlternates = [&builderState](const FontVariantAlternates& alternates) {
auto fontDescription = builderState.fontDescription();
fontDescription.setVariantAlternates(WTFMove(alternates));
fontDescription.setVariantAlternates(alternates);
builderState.setFontDescription(WTFMove(fontDescription));
};

Expand All @@ -1649,7 +1649,7 @@ inline void BuilderCustom::applyValueFontVariantAlternates(BuilderState& builder
if (primitiveValue->valueID() == CSSValueHistoricalForms) {
auto alternates = FontVariantAlternates::Normal();
alternates.valuesRef().historicalForms = true;
setAlternates(WTFMove(alternates));
setAlternates(alternates);
return;
}
return;
Expand Down

0 comments on commit 371cab8

Please sign in to comment.