commit f4ed96250592feb6e435f09305a3f067c6442d61
parent 12103fa10c380cf6a345dc57de8dc334bc7a9b6c
Author: Jonathan Kew <jkew@mozilla.com>
Date: Wed, 15 Oct 2025 21:11:23 +0000
Bug 1994193 - Try to skip applying spacing if all values are zero. r=layout-reviewers,TYLin
No behavior change is expected; this is just skipping some work
when we know it would be a no-op.
Differential Revision: https://phabricator.services.mozilla.com/D268558
Diffstat:
7 files changed, 61 insertions(+), 39 deletions(-)
diff --git a/dom/canvas/CanvasRenderingContext2D.cpp b/dom/canvas/CanvasRenderingContext2D.cpp
@@ -4635,7 +4635,7 @@ struct MOZ_STACK_CLASS CanvasBidiProcessor final
explicit PropertyProvider(const CanvasBidiProcessor& aProcessor)
: mProcessor(aProcessor) {}
- void GetSpacing(gfxTextRun::Range aRange,
+ bool GetSpacing(gfxTextRun::Range aRange,
gfxFont::Spacing* aSpacing) const {
for (auto i = aRange.start; i < aRange.end; ++i) {
auto* charGlyphs = mProcessor.mTextRun->GetCharacterGlyphs();
@@ -4665,6 +4665,7 @@ struct MOZ_STACK_CLASS CanvasBidiProcessor final
}
aSpacing++;
}
+ return mProcessor.mLetterSpacing != 0.0 || mProcessor.mWordSpacing != 0.0;
}
mozilla::StyleHyphens GetHyphensOption() const {
diff --git a/gfx/src/nsFontMetrics.cpp b/gfx/src/nsFontMetrics.cpp
@@ -101,8 +101,9 @@ class StubPropertyProvider final : public gfxTextRun::PropertyProvider {
NS_ERROR("This shouldn't be called because we never enable hyphens");
return 60;
}
- void GetSpacing(gfxTextRun::Range aRange, Spacing* aSpacing) const override {
+ bool GetSpacing(gfxTextRun::Range aRange, Spacing* aSpacing) const override {
NS_ERROR("This shouldn't be called because we never enable spacing");
+ return false;
}
gfx::ShapedTextFlags GetShapedTextFlags() const override {
NS_ERROR("This shouldn't be called because we never enable hyphens");
diff --git a/gfx/thebes/gfxFont.cpp b/gfx/thebes/gfxFont.cpp
@@ -2914,16 +2914,16 @@ bool gfxFont::MeasureGlyphs(const gfxTextRun* aTextRun, uint32_t aStart,
gfxFloat* aAdvanceMin, gfxFloat* aAdvanceMax) {
const gfxTextRun::CompressedGlyph* charGlyphs =
aTextRun->GetCharacterGlyphs();
- double x = 0;
- if (aSpacing) {
- x += aSpacing[0].mBefore;
- }
uint32_t spaceGlyph = GetSpaceGlyph();
bool allGlyphsInvisible = true;
AutoReadLock lock(aExtents->mLock);
+ double x = 0;
for (uint32_t i = aStart; i < aEnd; ++i) {
+ if (aSpacing) {
+ x += aSpacing->mBefore;
+ }
const gfxTextRun::CompressedGlyph* glyphData = &charGlyphs[i];
if (glyphData->IsSimpleGlyph()) {
double advance = glyphData->GetSimpleAdvance();
@@ -3011,11 +3011,8 @@ bool gfxFont::MeasureGlyphs(const gfxTextRun* aTextRun, uint32_t aStart,
}
}
if (aSpacing) {
- double space = aSpacing[i - aStart].mAfter;
- if (i + 1 < aEnd) {
- space += aSpacing[i + 1 - aStart].mBefore;
- }
- x += space;
+ x += aSpacing->mAfter;
+ ++aSpacing;
}
}
diff --git a/gfx/thebes/gfxTextRun.cpp b/gfx/thebes/gfxTextRun.cpp
@@ -329,14 +329,16 @@ gfxTextRun::LigatureData gfxTextRun::ComputeLigatureData(
if (aProvider && (mFlags & gfx::ShapedTextFlags::TEXT_ENABLE_SPACING)) {
gfxFont::Spacing spacing;
if (aPartRange.start == result.mRange.start) {
- aProvider->GetSpacing(Range(aPartRange.start, aPartRange.start + 1),
- &spacing);
- result.mPartWidth += spacing.mBefore;
+ if (aProvider->GetSpacing(Range(aPartRange.start, aPartRange.start + 1),
+ &spacing)) {
+ result.mPartWidth += spacing.mBefore;
+ }
}
if (aPartRange.end == result.mRange.end) {
- aProvider->GetSpacing(Range(aPartRange.end - 1, aPartRange.end),
- &spacing);
- result.mPartWidth += spacing.mAfter;
+ if (aProvider->GetSpacing(Range(aPartRange.end - 1, aPartRange.end),
+ &spacing)) {
+ result.mPartWidth += spacing.mAfter;
+ }
}
}
@@ -358,15 +360,16 @@ int32_t gfxTextRun::GetAdvanceForGlyphs(Range aRange) const {
return advance;
}
-static void GetAdjustedSpacing(
+// Returns false if there is definitely no spacing to apply.
+static bool GetAdjustedSpacing(
const gfxTextRun* aTextRun, gfxTextRun::Range aRange,
const gfxTextRun::PropertyProvider& aProvider,
gfxTextRun::PropertyProvider::Spacing* aSpacing) {
if (aRange.start >= aRange.end) {
- return;
+ return false;
}
- aProvider.GetSpacing(aRange, aSpacing);
+ bool result = aProvider.GetSpacing(aRange, aSpacing);
#ifdef DEBUG
// Check to see if we have spacing inside ligatures
@@ -385,6 +388,8 @@ static void GetAdjustedSpacing(
}
}
#endif
+
+ return result;
}
bool gfxTextRun::GetAdjustedSpacingArray(
@@ -398,8 +403,11 @@ bool gfxTextRun::GetAdjustedSpacingArray(
}
auto spacingOffset = aSpacingRange.start - aRange.start;
memset(aSpacing->Elements(), 0, sizeof(gfxFont::Spacing) * spacingOffset);
- GetAdjustedSpacing(this, aSpacingRange, *aProvider,
- aSpacing->Elements() + spacingOffset);
+ if (!GetAdjustedSpacing(this, aSpacingRange, *aProvider,
+ aSpacing->Elements() + spacingOffset)) {
+ aSpacing->Clear();
+ return false;
+ }
memset(aSpacing->Elements() + spacingOffset + aSpacingRange.Length(), 0,
sizeof(gfxFont::Spacing) * (aRange.end - aSpacingRange.end));
return true;
@@ -1227,15 +1235,16 @@ gfxFloat gfxTextRun::GetAdvanceWidth(
uint32_t i;
AutoTArray<PropertyProvider::Spacing, 200> spacingBuffer;
if (spacingBuffer.AppendElements(aRange.Length(), fallible)) {
- GetAdjustedSpacing(this, ligatureRange, *aProvider,
- spacingBuffer.Elements());
- for (i = 0; i < ligatureRange.Length(); ++i) {
- PropertyProvider::Spacing* space = &spacingBuffer[i];
- result += space->mBefore + space->mAfter;
- }
- if (aSpacing) {
- aSpacing->mBefore = spacingBuffer[0].mBefore;
- aSpacing->mAfter = spacingBuffer.LastElement().mAfter;
+ if (GetAdjustedSpacing(this, ligatureRange, *aProvider,
+ spacingBuffer.Elements())) {
+ for (i = 0; i < ligatureRange.Length(); ++i) {
+ PropertyProvider::Spacing* space = &spacingBuffer[i];
+ result += space->mBefore + space->mAfter;
+ }
+ if (aSpacing) {
+ aSpacing->mBefore = spacingBuffer[0].mBefore;
+ aSpacing->mAfter = spacingBuffer.LastElement().mAfter;
+ }
}
}
}
diff --git a/gfx/thebes/gfxTextRun.h b/gfx/thebes/gfxTextRun.h
@@ -242,8 +242,10 @@ class gfxTextRun : public gfxShapedText {
* inside clusters. In other words, if character i is not
* CLUSTER_START, then character i-1 must have zero after-spacing and
* character i must have zero before-spacing.
+ * Returns true if there is (or may be) any custom spacing; false if we
+ * are sure that aSpacing contains only zero values.
*/
- virtual void GetSpacing(Range aRange, Spacing* aSpacing) const = 0;
+ virtual bool GetSpacing(Range aRange, Spacing* aSpacing) const = 0;
// Returns a gfxContext that can be used to measure the hyphen glyph.
// Only called if the hyphen width is requested.
diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp
@@ -3815,10 +3815,11 @@ JustificationInfo nsTextFrame::PropertyProvider::ComputeJustification(
return info;
}
-// aStart, aLength in transformed string offsets
-void nsTextFrame::PropertyProvider::GetSpacing(Range aRange,
+// aStart, aLength in transformed string offsets.
+// Returns false if no non-standard spacing was required.
+bool nsTextFrame::PropertyProvider::GetSpacing(Range aRange,
Spacing* aSpacing) const {
- GetSpacingInternal(
+ return GetSpacingInternal(
aRange, aSpacing,
!(mTextRun->GetFlags2() & nsTextFrameUtils::Flags::HasTab));
}
@@ -4066,7 +4067,7 @@ static bool HasCJKGlyphRun(const gfxTextRun* aTextRun) {
return false;
}
-void nsTextFrame::PropertyProvider::GetSpacingInternal(Range aRange,
+bool nsTextFrame::PropertyProvider::GetSpacingInternal(Range aRange,
Spacing* aSpacing,
bool aIgnoreTabs) const {
MOZ_ASSERT(IsInBounds(mStart, mLength, aRange), "Range out of bounds");
@@ -4074,9 +4075,14 @@ void nsTextFrame::PropertyProvider::GetSpacingInternal(Range aRange,
std::memset(aSpacing, 0, aRange.Length() * sizeof(*aSpacing));
if (mFrame->Style()->IsTextCombined()) {
- return;
+ return false;
}
+ // Track whether any non-standard spacing is actually present in this range.
+ // If letter-spacing is non-zero this will always be true, but for the
+ /// word-spacing and text-autospace cases it will depend on the actual text.
+ bool spacingPresent = mLetterSpacing;
+
// First, compute the word spacing, letter spacing, and text-autospace
// spacing.
if (mWordSpacing || mLetterSpacing || mTextAutospace) {
@@ -4185,6 +4191,7 @@ void nsTextFrame::PropertyProvider::GetSpacingInternal(Range aRange,
&iter);
uint32_t runOffset = iter.GetSkippedOffset() - aRange.start;
aSpacing[runOffset].mAfter += mWordSpacing;
+ spacingPresent = true;
}
// Add text-autospace spacing only at cluster starts. Always check the
// character classes if the textrun includes CJK; otherwise, check only
@@ -4209,6 +4216,7 @@ void nsTextFrame::PropertyProvider::GetSpacingInternal(Range aRange,
prevClass.valueOrFrom(findPrecedingClass), currClass)) {
aSpacing[runOffsetInSubstring + i].mBefore +=
mTextAutospace->InterScriptSpacing();
+ spacingPresent = true;
}
// Even if we didn't actually need to check spacing rules here, we
// record the new prevClass. (Incidentally, this ensure that we'll
@@ -4230,6 +4238,7 @@ void nsTextFrame::PropertyProvider::GetSpacingInternal(Range aRange,
mTabWidths->ApplySpacing(aSpacing,
aRange.start - mStart.GetSkippedOffset(),
aRange.Length());
+ spacingPresent = true;
}
}
}
@@ -4250,7 +4259,10 @@ void nsTextFrame::PropertyProvider::GetSpacingInternal(Range aRange,
aSpacing[offset].mBefore += spacing.mBefore;
aSpacing[offset].mAfter += spacing.mAfter;
}
+ spacingPresent = true;
}
+
+ return spacingPresent;
}
// aX and the result are in whole appunits.
diff --git a/layout/generic/nsTextFrame.h b/layout/generic/nsTextFrame.h
@@ -160,7 +160,7 @@ class nsTextFrame : public nsIFrame {
void InitializeForMeasure();
- void GetSpacing(Range aRange, Spacing* aSpacing) const final;
+ bool GetSpacing(Range aRange, Spacing* aSpacing) const final;
gfxFloat GetHyphenWidth() const final;
void GetHyphenationBreaks(Range aRange,
HyphenType* aBreakBefore) const final;
@@ -175,7 +175,7 @@ class nsTextFrame : public nsIFrame {
return mTextRun->GetAppUnitsPerDevUnit();
}
- void GetSpacingInternal(Range aRange, Spacing* aSpacing,
+ bool GetSpacingInternal(Range aRange, Spacing* aSpacing,
bool aIgnoreTabs) const;
/**
