commit 685b45182095a24670a2e8a7bba4f228d0641e7e
parent fb4b653594efa6646a5f62030a4129e387b77347
Author: Jonathan Kew <jkew@mozilla.com>
Date: Sat, 18 Oct 2025 07:26:09 +0000
Bug 1994197 - patch 3 - Convert gfxTextRun text advance computation to nscoord values. r=layout-reviewers,emilio
There's no point in doing these calculations with gfxFloat values,
as they're destined to become nscoord in layout, which restricts the
possible range. With huge font sizes/spacing/etc, we can end up with
values that are out of range for nscoord, which will lead to broken
layout (at best) or possibly assertions (e.g. about advances that
appear hugely negative after nscoord overflow).
Doing all the advance computations in nscoord values, with saturating
addition to handle potentially huge values, should result in more stable
and predictable results (although layout will still not really be
meaningful when dimensions exceed what nscoord can handle).
(This resolves the assertions that were seen when I originally tried
to land the patch stack here.)
Differential Revision: https://phabricator.services.mozilla.com/D269085
Diffstat:
4 files changed, 50 insertions(+), 49 deletions(-)
diff --git a/gfx/src/nsFontMetrics.cpp b/gfx/src/nsFontMetrics.cpp
@@ -312,8 +312,7 @@ nscoord nsFontMetrics::GetWidth(const char* aString, uint32_t aLength,
StubPropertyProvider provider;
AutoTextRun textRun(this, aDrawTarget, aString, aLength);
if (textRun.get()) {
- return NSToCoordRound(
- textRun->GetAdvanceWidth(gfxTextRun::Range(0, aLength), &provider));
+ return textRun->GetAdvanceWidth(gfxTextRun::Range(0, aLength), &provider);
}
return 0;
}
@@ -329,8 +328,7 @@ nscoord nsFontMetrics::GetWidth(const char16_t* aString, uint32_t aLength,
StubPropertyProvider provider;
AutoTextRun textRun(this, aDrawTarget, aString, aLength);
if (textRun.get()) {
- return NSToCoordRound(
- textRun->GetAdvanceWidth(gfxTextRun::Range(0, aLength), &provider));
+ return textRun->GetAdvanceWidth(gfxTextRun::Range(0, aLength), &provider);
}
return 0;
}
diff --git a/gfx/thebes/gfxFont.cpp b/gfx/thebes/gfxFont.cpp
@@ -968,7 +968,7 @@ void gfxFont::RunMetrics::CombineWith(const RunMetrics& aOther,
mBoundingBox =
mBoundingBox.Union(aOther.mBoundingBox + gfxPoint(mAdvanceWidth, 0));
}
- mAdvanceWidth += aOther.mAdvanceWidth;
+ mAdvanceWidth = NSCoordSaturatingAdd(mAdvanceWidth, aOther.mAdvanceWidth);
}
gfxFont::gfxFont(const RefPtr<UnscaledFont>& aUnscaledFont,
diff --git a/gfx/thebes/gfxTextRun.cpp b/gfx/thebes/gfxTextRun.cpp
@@ -281,7 +281,7 @@ gfxTextRun::LigatureData gfxTextRun::ComputeLigatureData(
}
result.mRange.end = i;
- int32_t ligatureWidth = GetAdvanceForGlyphs(result.mRange);
+ nscoord ligatureWidth = GetAdvanceForGlyphs(result.mRange);
// Count the number of started clusters we have seen
uint32_t totalClusterCount = 0;
uint32_t partClusterIndex = 0;
@@ -307,7 +307,7 @@ gfxTextRun::LigatureData gfxTextRun::ComputeLigatureData(
// so that measuring all parts of a ligature and summing them is equal to
// the ligature width.
if (aPartRange.end == result.mRange.end) {
- gfxFloat allParts = totalClusterCount * (ligatureWidth / totalClusterCount);
+ nscoord allParts = totalClusterCount * (ligatureWidth / totalClusterCount);
result.mPartWidth += ligatureWidth - allParts;
}
@@ -331,13 +331,15 @@ gfxTextRun::LigatureData gfxTextRun::ComputeLigatureData(
if (aPartRange.start == result.mRange.start) {
if (aProvider->GetSpacing(Range(aPartRange.start, aPartRange.start + 1),
&spacing)) {
- result.mPartWidth += spacing.mBefore;
+ result.mPartWidth =
+ NSCoordSaturatingAdd(result.mPartWidth, spacing.mBefore);
}
}
if (aPartRange.end == result.mRange.end) {
if (aProvider->GetSpacing(Range(aPartRange.end - 1, aPartRange.end),
&spacing)) {
- result.mPartWidth += spacing.mAfter;
+ result.mPartWidth =
+ NSCoordSaturatingAdd(result.mPartWidth, spacing.mAfter);
}
}
}
@@ -345,17 +347,19 @@ gfxTextRun::LigatureData gfxTextRun::ComputeLigatureData(
return result;
}
-gfxFloat gfxTextRun::ComputePartialLigatureWidth(
+nscoord gfxTextRun::ComputePartialLigatureWidth(
Range aPartRange, const PropertyProvider* aProvider) const {
- if (aPartRange.start >= aPartRange.end) return 0;
+ if (aPartRange.start >= aPartRange.end) {
+ return 0;
+ }
LigatureData data = ComputeLigatureData(aPartRange, aProvider);
return data.mPartWidth;
}
-int32_t gfxTextRun::GetAdvanceForGlyphs(Range aRange) const {
- int32_t advance = 0;
+nscoord gfxTextRun::GetAdvanceForGlyphs(Range aRange) const {
+ nscoord advance = 0;
for (auto i = aRange.start; i < aRange.end; ++i) {
- advance += GetAdvanceForGlyph(i);
+ advance = NSCoordSaturatingAdd(advance, GetAdvanceForGlyph(i));
}
return advance;
}
@@ -975,19 +979,19 @@ uint32_t gfxTextRun::BreakAndMeasureText(
}
}
- gfxFloat width = 0;
- gfxFloat advance = 0;
+ nscoord width = 0;
+ nscoord advance = 0;
// The number of space characters that can be trimmed or hang at a soft-wrap
uint32_t trimmableChars = 0;
// The amount of space removed by ignoring trimmableChars
- gfxFloat trimmableAdvance = 0;
+ nscoord trimmableAdvance = 0;
int32_t lastBreak = -1;
int32_t lastBreakTrimmableChars = -1;
- gfxFloat lastBreakTrimmableAdvance = -1;
+ nscoord lastBreakTrimmableAdvance = -1;
// Cache the last candidate break
int32_t lastCandidateBreak = -1;
int32_t lastCandidateBreakTrimmableChars = -1;
- gfxFloat lastCandidateBreakTrimmableAdvance = -1;
+ nscoord lastCandidateBreakTrimmableAdvance = -1;
bool lastCandidateBreakUsedHyphenation = false;
gfxBreakPriority lastCandidateBreakPriority = gfxBreakPriority::eNoBreak;
bool aborted = false;
@@ -1099,7 +1103,7 @@ uint32_t gfxTextRun::BreakAndMeasureText(
: gfxBreakPriority::eWordWrapBreak;
}
- width += advance;
+ width = NSCoordSaturatingAdd(width, advance);
advance = 0;
if (width - trimmableAdvance > aWidth) {
// No more text fits. Abort
@@ -1135,23 +1139,24 @@ uint32_t gfxTextRun::BreakAndMeasureText(
continue;
}
- gfxFloat charAdvance;
+ nscoord charAdvance;
if (i >= ligatureRange.start && i < ligatureRange.end) {
charAdvance = GetAdvanceForGlyphs(Range(i, i + 1));
if (haveSpacing) {
PropertyProvider::Spacing* space =
&spacingBuffer[i - bufferRange.start];
- charAdvance += space->mBefore + space->mAfter;
+ charAdvance =
+ NSCoordSaturatingAdd(charAdvance, space->mBefore + space->mAfter);
}
} else {
charAdvance = ComputePartialLigatureWidth(Range(i, i + 1), &aProvider);
}
- advance += charAdvance;
+ advance = NSCoordSaturatingAdd(advance, charAdvance);
if (aOutTrimmableWhitespace) {
if (mCharacterGlyphs[i].CharIsSpace()) {
++trimmableChars;
- trimmableAdvance += charAdvance;
+ trimmableAdvance = NSCoordSaturatingAdd(trimmableAdvance, charAdvance);
} else {
trimmableAdvance = 0;
trimmableChars = 0;
@@ -1160,7 +1165,7 @@ uint32_t gfxTextRun::BreakAndMeasureText(
}
if (!aborted) {
- width += advance;
+ width = NSCoordSaturatingAdd(width, advance);
}
// There are three possibilities:
@@ -1210,20 +1215,20 @@ uint32_t gfxTextRun::BreakAndMeasureText(
return charsFit;
}
-gfxFloat gfxTextRun::GetAdvanceWidth(
- Range aRange, const PropertyProvider* aProvider,
- PropertyProvider::Spacing* aSpacing) const {
+nscoord gfxTextRun::GetAdvanceWidth(Range aRange,
+ const PropertyProvider* aProvider,
+ PropertyProvider::Spacing* aSpacing) const {
NS_ASSERTION(aRange.end <= GetLength(), "Substring out of range");
Range ligatureRange = aRange;
bool adjusted = ShrinkToLigatureBoundaries(&ligatureRange);
- gfxFloat result =
+ nscoord result =
adjusted ? ComputePartialLigatureWidth(
Range(aRange.start, ligatureRange.start), aProvider) +
ComputePartialLigatureWidth(
Range(ligatureRange.end, aRange.end), aProvider)
- : 0.0;
+ : 0;
if (aSpacing) {
aSpacing->mBefore = aSpacing->mAfter = 0;
@@ -1239,7 +1244,7 @@ gfxFloat gfxTextRun::GetAdvanceWidth(
spacingBuffer.Elements())) {
for (i = 0; i < ligatureRange.Length(); ++i) {
PropertyProvider::Spacing* space = &spacingBuffer[i];
- result += space->mBefore + space->mAfter;
+ result = NSCoordSaturatingAdd(result, space->mBefore + space->mAfter);
}
if (aSpacing) {
aSpacing->mBefore = spacingBuffer[0].mBefore;
@@ -1249,33 +1254,34 @@ gfxFloat gfxTextRun::GetAdvanceWidth(
}
}
- return result + GetAdvanceForGlyphs(ligatureRange);
+ return NSCoordSaturatingAdd(result, GetAdvanceForGlyphs(ligatureRange));
}
-gfxFloat gfxTextRun::GetMinAdvanceWidth(Range aRange) {
+nscoord gfxTextRun::GetMinAdvanceWidth(Range aRange) {
MOZ_ASSERT(aRange.end <= GetLength(), "Substring out of range");
Range ligatureRange = aRange;
bool adjusted = ShrinkToLigatureBoundaries(&ligatureRange);
- gfxFloat result =
+ nscoord result =
adjusted
? std::max(ComputePartialLigatureWidth(
Range(aRange.start, ligatureRange.start), nullptr),
ComputePartialLigatureWidth(
Range(ligatureRange.end, aRange.end), nullptr))
- : 0.0;
+ : 0;
// Compute min advance width by assuming each grapheme cluster takes its own
// line.
- gfxFloat clusterAdvance = 0;
+ nscoord clusterAdvance = 0;
for (uint32_t i = ligatureRange.start; i < ligatureRange.end; ++i) {
if (mCharacterGlyphs[i].CharIsSpace()) {
// Skip space char to prevent its advance width contributing to the
// result. That is, don't consider a space can be in its own line.
continue;
}
- clusterAdvance += GetAdvanceForGlyph(i);
+ clusterAdvance =
+ NSCoordSaturatingAdd(clusterAdvance, GetAdvanceForGlyph(i));
if (i + 1 == ligatureRange.end || IsClusterStart(i + 1)) {
result = std::max(result, clusterAdvance);
clusterAdvance = 0;
diff --git a/gfx/thebes/gfxTextRun.h b/gfx/thebes/gfxTextRun.h
@@ -338,10 +338,10 @@ class gfxTextRun : public gfxShapedText {
* the substring would be returned in it. NOTE: the spacing is
* included in the advance width.
*/
- gfxFloat GetAdvanceWidth(Range aRange, const PropertyProvider* aProvider,
- PropertyProvider::Spacing* aSpacing = nullptr) const;
+ nscoord GetAdvanceWidth(Range aRange, const PropertyProvider* aProvider,
+ PropertyProvider::Spacing* aSpacing = nullptr) const;
- gfxFloat GetAdvanceWidth() const {
+ nscoord GetAdvanceWidth() const {
return GetAdvanceWidth(Range(this), nullptr);
}
@@ -349,7 +349,7 @@ class gfxTextRun : public gfxShapedText {
* Computes the minimum advance width for a substring assuming line
* breaking is allowed everywhere.
*/
- gfxFloat GetMinAdvanceWidth(Range aRange);
+ nscoord GetMinAdvanceWidth(Range aRange);
/**
* Clear all stored line breaks for the given range (both before and after),
@@ -724,11 +724,11 @@ class gfxTextRun : public gfxShapedText {
Range mRange;
// appunits advance to the start of the ligature part within the ligature;
// never includes any spacing
- gfxFloat mPartAdvance;
+ nscoord mPartAdvance;
// appunits width of the ligature part; includes before-spacing
// when the part is at the start of the ligature, and after-spacing
// when the part is as the end of the ligature
- gfxFloat mPartWidth;
+ nscoord mPartWidth;
bool mClipBeforePart;
bool mClipAfterPart;
@@ -822,7 +822,7 @@ class gfxTextRun : public gfxShapedText {
// **** general helpers ****
// Get the total advance for a range of glyphs.
- int32_t GetAdvanceForGlyphs(Range aRange) const;
+ nscoord GetAdvanceForGlyphs(Range aRange) const;
// Spacing for characters outside the range aSpacingStart/aSpacingEnd
// is assumed to be zero; such characters are not passed to aProvider.
@@ -844,8 +844,8 @@ class gfxTextRun : public gfxShapedText {
// if aProvider is null then mBeforeSpacing and mAfterSpacing are set to zero
LigatureData ComputeLigatureData(Range aPartRange,
const PropertyProvider* aProvider) const;
- gfxFloat ComputePartialLigatureWidth(Range aPartRange,
- const PropertyProvider* aProvider) const;
+ nscoord ComputePartialLigatureWidth(Range aPartRange,
+ const PropertyProvider* aProvider) const;
void DrawPartialLigature(gfxFont* aFont, Range aRange,
mozilla::gfx::Point* aPt,
const PropertyProvider* aProvider,
@@ -856,9 +856,6 @@ class gfxTextRun : public gfxShapedText {
// aRange->start == aRange->end.
// Returns whether any adjustment was made.
bool ShrinkToLigatureBoundaries(Range* aRange) const;
- // result in appunits
- gfxFloat GetPartialLigatureWidth(Range aRange,
- const PropertyProvider* aProvider) const;
void AccumulatePartialLigatureMetrics(
gfxFont* aFont, Range aRange, gfxFont::BoundingBoxType aBoundingBoxType,
DrawTarget* aRefDrawTarget, const PropertyProvider* aProvider,
