Skip to content

Commit

Permalink
Bug 1721612 - (followup) Rename AdjustAdvancesForSyntheticBold to App…
Browse files Browse the repository at this point in the history
…lyTrackingToClusters, and clamp the adjusted advance to avoid becoming negative. r=gfx-reviewers,lsalzman

This is not necessary to fix the observed bug, but as a precaution against excessive
negative tracking, let's clamp the adjusted advance so that it can't become negative
(which seems unlikely to end well).

Also rename the method, given that it is not only used for synthetic bold adjustments
any longer.

(No change in behavior, except in the edge-case of a font that has such small
advances and extreme tracking that it tries to go backwards...)

Depends on D193288

Differential Revision: https://phabricator.services.mozilla.com/D193289
  • Loading branch information
jfkthame committed Nov 11, 2023
1 parent 737870d commit e23f480
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 19 deletions.
28 changes: 12 additions & 16 deletions gfx/thebes/gfxFont.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -840,18 +840,18 @@ bool gfxShapedText::FilterIfIgnorable(uint32_t aIndex, uint32_t aCh) {
return false;
}

void gfxShapedText::AdjustAdvancesForSyntheticBold(float aSynBoldOffset,
uint32_t aOffset,
uint32_t aLength) {
int32_t synAppUnitOffset = aSynBoldOffset * mAppUnitsPerDevUnit;
void gfxShapedText::ApplyTrackingToClusters(float aTrackingAdjustment,
uint32_t aOffset,
uint32_t aLength) {
int32_t appUnitAdjustment = aTrackingAdjustment * mAppUnitsPerDevUnit;
CompressedGlyph* charGlyphs = GetCharacterGlyphs();
for (uint32_t i = aOffset; i < aOffset + aLength; ++i) {
CompressedGlyph* glyphData = charGlyphs + i;
if (glyphData->IsSimpleGlyph()) {
// simple glyphs ==> just add the advance
int32_t advance = glyphData->GetSimpleAdvance();
if (advance > 0) {
advance += synAppUnitOffset;
advance = std::max(0, advance + appUnitAdjustment);
if (CompressedGlyph::IsSimpleAdvance(advance)) {
glyphData->SetSimpleGlyph(advance, glyphData->GetSimpleGlyph());
} else {
Expand All @@ -872,14 +872,10 @@ void gfxShapedText::AdjustAdvancesForSyntheticBold(float aSynBoldOffset,
if (!details) {
continue;
}
if (IsRightToLeft()) {
if (details[0].mAdvance > 0) {
details[0].mAdvance += synAppUnitOffset;
}
} else {
if (details[detailedLength - 1].mAdvance > 0) {
details[detailedLength - 1].mAdvance += synAppUnitOffset;
}
auto& advance = IsRightToLeft() ? details[0].mAdvance
: details[detailedLength - 1].mAdvance;
if (advance > 0) {
advance = std::max(0, advance + appUnitAdjustment);
}
}
}
Expand Down Expand Up @@ -3341,7 +3337,7 @@ bool gfxFont::ShapeText(DrawTarget* aDrawTarget, const char16_t* aText,
// synthetic bold: we want to apply between clusters, not to
// non-spacing glyphs within a cluster. So we can reuse that
// helper here.
aShapedText->AdjustAdvancesForSyntheticBold(tracking, aOffset, aLength);
aShapedText->ApplyTrackingToClusters(tracking, aOffset, aLength);
}
return true;
}
Expand All @@ -3357,8 +3353,8 @@ void gfxFont::PostShapingFixup(DrawTarget* aDrawTarget, const char16_t* aText,
const Metrics& metrics = GetMetrics(aVertical ? nsFontMetrics::eVertical
: nsFontMetrics::eHorizontal);
if (metrics.maxAdvance > metrics.aveCharWidth) {
aShapedText->AdjustAdvancesForSyntheticBold(GetSyntheticBoldOffset(),
aOffset, aLength);
aShapedText->ApplyTrackingToClusters(GetSyntheticBoldOffset(), aOffset,
aLength);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions gfx/thebes/gfxFont.h
Original file line number Diff line number Diff line change
Expand Up @@ -1036,8 +1036,8 @@ class gfxShapedText {
return mDetailedGlyphs->Get(aCharIndex);
}
void AdjustAdvancesForSyntheticBold(float aSynBoldOffset, uint32_t aOffset,
uint32_t aLength);
void ApplyTrackingToClusters(float aTrackingAdjustment, uint32_t aOffset,
uint32_t aLength);
// Mark clusters in the CompressedGlyph records, starting at aOffset,
// based on the Unicode properties of the text in aString.
Expand Down
2 changes: 1 addition & 1 deletion gfx/thebes/gfxMacFont.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ bool gfxMacFont::ShapeText(DrawTarget* aDrawTarget, const char16_t* aText,
// synthetic bold: we want to apply between clusters, not to
// non-spacing glyphs within a cluster. So we can reuse that
// helper here.
aShapedText->AdjustAdvancesForSyntheticBold(tracking, aOffset, aLength);
aShapedText->ApplyTrackingToClusters(tracking, aOffset, aLength);
}
return true;
}
Expand Down

0 comments on commit e23f480

Please sign in to comment.