Skip to content

Commit e831f53

Browse files
JimBobSquarePantsJimBobSquarePants
authored
Merge pull request #485 from SixLabors/js/fix-483
Use correct font metrics for fallback glyph substitutions
2 parents 9f4b642 + 525cb77 commit e831f53

File tree

9 files changed

+116
-86
lines changed

9 files changed

+116
-86
lines changed

‎src/SixLabors.Fonts/GlyphSubstitutionCollection.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ internal sealed class GlyphSubstitutionCollection : IGlyphShapingCollection
1717
/// <summary>
1818
/// Contains a map the index of a map within the collection, non-sequential codepoint offsets, and their glyph ids.
1919
/// </summary>
20-
private readonly List<OffsetGlyphDataPair> glyphs = new();
20+
private readonly List<OffsetGlyphDataPair> glyphs = [];
2121

2222
/// <summary>
2323
/// Initializes a new instance of the <see cref="GlyphSubstitutionCollection"/> class.

‎src/SixLabors.Fonts/Tables/AdvancedTypographic/GPosTable.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ current is not ScriptClass.Common and not ScriptClass.Unknown and not ScriptClas
155155
}
156156

157157
Tag unicodeScriptTag = this.GetUnicodeScriptTag(current);
158-
BaseShaper shaper = ShaperFactory.Create(current, unicodeScriptTag, collection.TextOptions);
158+
BaseShaper shaper = ShaperFactory.Create(current, unicodeScriptTag, fontMetrics, collection.TextOptions);
159159

160160
if (shaper.MarkZeroingMode == MarkZeroingMode.PreGPos)
161161
{

‎src/SixLabors.Fonts/Tables/AdvancedTypographic/GSubTable.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ current is not ScriptClass.Common and not ScriptClass.Unknown and not ScriptClas
138138
}
139139

140140
Tag unicodeScriptTag = this.GetUnicodeScriptTag(current);
141-
BaseShaper shaper = ShaperFactory.Create(current, unicodeScriptTag, collection.TextOptions);
141+
BaseShaper shaper = ShaperFactory.Create(current, unicodeScriptTag, fontMetrics, collection.TextOptions);
142142

143143
// Plan substitution features for each glyph.
144144
// Shapers can adjust the count during initialization and feature processing so we must capture

‎src/SixLabors.Fonts/Tables/AdvancedTypographic/Shapers/HangulShaper.cs‎

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ internal sealed class HangulShaper : DefaultShaper
6464
{ new byte[] { None, 0 }, new byte[] { None, 1 }, new byte[] { None, 0 }, new byte[] { None, 0 }, new byte[] { Decompose, 2 }, new byte[] { Decompose, 3 }, new byte[] { ToneMark, 0 } },
6565
};
6666

67-
public HangulShaper(ScriptClass script, TextOptions textOptions)
67+
private readonly FontMetrics fontMetrics;
68+
69+
public HangulShaper(ScriptClass script, TextOptions textOptions, FontMetrics fontMetrics)
6870
: base(script, MarkZeroingMode.None, textOptions)
69-
{
70-
}
71+
=> this.fontMetrics = fontMetrics;
7172

7273
/// <inheritdoc/>
7374
protected override void PlanFeatures(IGlyphShapingCollection collection, int index, int count)
@@ -121,13 +122,13 @@ protected override void AssignFeatures(IGlyphShapingCollection collection, int i
121122
case Compose:
122123

123124
// Found a decomposed syllable. Try to compose if supported by the font.
124-
i = this.ComposeGlyph(substitutionCollection, data, i, type);
125+
i = this.ComposeGlyph(substitutionCollection, i, type);
125126
break;
126127

127128
case ToneMark:
128129

129130
// Got a valid syllable, followed by a tone mark. Move the tone mark to the beginning of the syllable.
130-
ReOrderToneMark(substitutionCollection, data, i);
131+
this.ReOrderToneMark(substitutionCollection, data, i);
131132
break;
132133

133134
case Invalid:
@@ -172,8 +173,6 @@ protected override void AssignFeatures(IGlyphShapingCollection collection, int i
172173
collection.EnableShapingFeature(i, VjmoTag);
173174
collection.EnableShapingFeature(i, TjmoTag);
174175
break;
175-
default:
176-
break;
177176
}
178177
}
179178
}
@@ -216,7 +215,7 @@ private int DecomposeGlyph(GlyphSubstitutionCollection collection, GlyphShapingD
216215
int l = (LBase + (s / VCount)) | 0;
217216
int v = VBase + (s % VCount);
218217

219-
FontMetrics metrics = data.TextRun.Font!.FontMetrics;
218+
FontMetrics metrics = this.fontMetrics;
220219

221220
// Don't decompose if all of the components are not available
222221
if (!metrics.TryGetGlyphId(new(l), out ushort ljmo) ||
@@ -252,7 +251,7 @@ private int DecomposeGlyph(GlyphSubstitutionCollection collection, GlyphShapingD
252251
return index + 2;
253252
}
254253

255-
private int ComposeGlyph(GlyphSubstitutionCollection collection, GlyphShapingData data, int index, int type)
254+
private int ComposeGlyph(GlyphSubstitutionCollection collection, int index, int type)
256255
{
257256
if (index == 0)
258257
{
@@ -306,8 +305,7 @@ private int ComposeGlyph(GlyphSubstitutionCollection collection, GlyphShapingDat
306305

307306
// Replace with a composed glyph if supported by the font,
308307
// otherwise apply the proper OpenType features to each component.
309-
FontMetrics metrics = data.TextRun.Font!.FontMetrics;
310-
if (metrics.TryGetGlyphId(s, out ushort id))
308+
if (this.fontMetrics.TryGetGlyphId(s, out ushort id))
311309
{
312310
int del = prevType == V ? 3 : 2;
313311
int idx = index - del + 1;
@@ -338,15 +336,14 @@ private int ComposeGlyph(GlyphSubstitutionCollection collection, GlyphShapingDat
338336
// Sequence was originally <L,V>, which got combined earlier.
339337
// Either the T was non-combining, or the LVT glyph wasn't supported.
340338
// Decompose the glyph again and apply OT features.
341-
data = collection[index - 1];
342-
this.DecomposeGlyph(collection, data, index - 1);
339+
this.DecomposeGlyph(collection, collection[index - 1], index - 1);
343340
return index + 1;
344341
}
345342

346343
return index;
347344
}
348345

349-
private static void ReOrderToneMark(GlyphSubstitutionCollection collection, GlyphShapingData data, int index)
346+
private void ReOrderToneMark(GlyphSubstitutionCollection collection, GlyphShapingData data, int index)
350347
{
351348
if (index == 0)
352349
{
@@ -355,17 +352,15 @@ private static void ReOrderToneMark(GlyphSubstitutionCollection collection, Glyp
355352

356353
// Move tone mark to the beginning of the previous syllable, unless it is zero width
357354
// We don't have access to the glyphs metrics as an array when substituting so we have to loop.
358-
FontMetrics fontMetrics = data.TextRun.Font!.FontMetrics;
355+
FontMetrics fontMetrics = this.fontMetrics;
359356
TextAttributes textAttributes = data.TextRun.TextAttributes;
360357
TextDecorations textDecorations = data.TextRun.TextDecorations;
361358
LayoutMode layoutMode = collection.TextOptions.LayoutMode;
362359
ColorFontSupport colorFontSupport = collection.TextOptions.ColorFontSupport;
363-
if (fontMetrics.TryGetGlyphMetrics(data.CodePoint, textAttributes, textDecorations, layoutMode, colorFontSupport, out GlyphMetrics? metrics))
360+
if (fontMetrics.TryGetGlyphMetrics(data.CodePoint, textAttributes, textDecorations, layoutMode, colorFontSupport, out GlyphMetrics? metrics)
361+
&& metrics.AdvanceWidth == 0)
364362
{
365-
if (metrics.AdvanceWidth == 0)
366-
{
367-
return;
368-
}
363+
return;
369364
}
370365

371366
GlyphShapingData prev = collection[index - 1];
@@ -376,20 +371,18 @@ private static void ReOrderToneMark(GlyphSubstitutionCollection collection, Glyp
376371
private int InsertDottedCircle(GlyphSubstitutionCollection collection, GlyphShapingData data, int index)
377372
{
378373
bool after = false;
379-
FontMetrics fontMetrics = data.TextRun.Font!.FontMetrics;
374+
FontMetrics fontMetrics = this.fontMetrics;
380375

381376
if (fontMetrics.TryGetGlyphId(new(DottedCircle), out ushort id))
382377
{
383378
TextAttributes textAttributes = data.TextRun.TextAttributes;
384379
TextDecorations textDecorations = data.TextRun.TextDecorations;
385380
LayoutMode layoutMode = collection.TextOptions.LayoutMode;
386381
ColorFontSupport colorFontSupport = collection.TextOptions.ColorFontSupport;
387-
if (fontMetrics.TryGetGlyphMetrics(data.CodePoint, textAttributes, textDecorations, layoutMode, colorFontSupport, out GlyphMetrics? metrics))
382+
if (fontMetrics.TryGetGlyphMetrics(data.CodePoint, textAttributes, textDecorations, layoutMode, colorFontSupport, out GlyphMetrics? metrics)
383+
&& metrics.AdvanceWidth != 0)
388384
{
389-
if (metrics.AdvanceWidth != 0)
390-
{
391-
after = true;
392-
}
385+
after = true;
393386
}
394387

395388
// If the tone mark is zero width, insert the dotted circle before, otherwise after

‎src/SixLabors.Fonts/Tables/AdvancedTypographic/Shapers/IndicShaper.cs‎

Lines changed: 29 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,15 @@ internal sealed class IndicShaper : DefaultShaper
4444
private const int DottedCircle = 0x25cc;
4545

4646
private readonly TextOptions textOptions;
47+
private readonly FontMetrics fontMetrics;
4748
private ShapingConfiguration indicConfiguration;
4849
private readonly bool isOldSpec;
4950

50-
public IndicShaper(ScriptClass script, Tag unicodeScriptTag, TextOptions textOptions)
51+
public IndicShaper(ScriptClass script, Tag unicodeScriptTag, TextOptions textOptions, FontMetrics fontMetrics)
5152
: base(script, MarkZeroingMode.None, textOptions)
5253
{
5354
this.textOptions = textOptions;
55+
this.fontMetrics = fontMetrics;
5456

5557
if (IndicConfigurations.TryGetValue(script, out ShapingConfiguration value))
5658
{
@@ -101,23 +103,24 @@ protected override void AssignFeatures(IGlyphShapingCollection collection, int i
101103
return;
102104
}
103105

106+
FontMetrics fontMetrics = this.fontMetrics;
107+
104108
// Decompose split matras
105109
Span<ushort> buffer = stackalloc ushort[16];
106110
int end = index + count;
107111
for (int i = end - 1; i >= 0; i--)
108112
{
109113
GlyphShapingData data = substitutionCollection[i];
110-
FontMetrics fontMetrics = data.TextRun.Font!.FontMetrics;
111-
112114
if ((Decompositions.TryGetValue(data.CodePoint.Value, out int[]? decompositions) ||
113115
UniversalShapingData.Decompositions.TryGetValue(data.CodePoint.Value, out decompositions)) &&
114116
decompositions != null)
115117
{
116118
Span<ushort> ids = buffer[..decompositions.Length];
117119
for (int j = 0; j < decompositions.Length; j++)
118120
{
119-
// Font should always contain the decomposed glyph.
120-
fontMetrics.TryGetGlyphId(new CodePoint(decompositions[j]), out ushort id);
121+
// Font should always contain the decomposed glyph since the shaper
122+
// is assigned based on features supported by the font.
123+
_ = fontMetrics.TryGetGlyphId(new CodePoint(decompositions[j]), out ushort id);
121124
ids[j] = id;
122125
}
123126

@@ -207,23 +210,22 @@ private void InitialReorder(IGlyphShapingCollection collection, int index, int c
207210
Span<GlyphShapingData> tempBuffer = new GlyphShapingData[3];
208211

209212
ShapingConfiguration indicConfiguration = this.indicConfiguration;
210-
for (int i = 0; i < count; i++)
211-
{
212-
GlyphShapingData data = substitutionCollection[i + index];
213-
FontMetrics fontMetrics = data.TextRun.Font!.FontMetrics;
213+
FontMetrics fontMetrics = this.fontMetrics;
214+
CodePoint viramaPoint = new(indicConfiguration.Virama);
214215

215-
fontMetrics.TryGetGlyphId(new(0x0020), out ushort spc);
216-
217-
IndicShapingEngineInfo? info = data.IndicShapingEngineInfo;
218-
if (info?.Position == Positions.Base_C)
216+
if (fontMetrics.TryGetGlyphId(viramaPoint, out ushort viramaId))
217+
{
218+
for (int i = 0; i < count; i++)
219219
{
220-
CodePoint cp = new(indicConfiguration.Virama);
221-
if (fontMetrics.TryGetGlyphId(cp, out ushort id))
220+
GlyphShapingData data = substitutionCollection[i + index];
221+
IndicShapingEngineInfo? info = data.IndicShapingEngineInfo;
222+
223+
if (info?.Position == Positions.Base_C)
222224
{
223225
GlyphShapingData virama = new(data, false)
224226
{
225-
GlyphId = id,
226-
CodePoint = cp
227+
GlyphId = viramaId,
228+
CodePoint = viramaPoint
227229
};
228230

229231
tempBuffer[2] = virama;
@@ -235,6 +237,8 @@ private void InitialReorder(IGlyphShapingCollection collection, int index, int c
235237
}
236238
}
237239

240+
bool hasDottedCircle = fontMetrics.TryGetGlyphId(new(DottedCircle), out ushort circleId);
241+
_ = fontMetrics.TryGetGSubTable(out GSubTable? gSubTable);
238242
int max = index + count;
239243
int start = index;
240244
int end = NextSyllable(substitutionCollection, index, max);
@@ -250,13 +254,7 @@ private void InitialReorder(IGlyphShapingCollection collection, int index, int c
250254
goto Increment;
251255
}
252256

253-
FontMetrics fontMetrics = data.TextRun.Font!.FontMetrics;
254-
if (!fontMetrics.TryGetGSubTable(out GSubTable? gSubTable))
255-
{
256-
break;
257-
}
258-
259-
if (dataInfo != null && type == "broken_cluster" && fontMetrics.TryGetGlyphId(new(DottedCircle), out ushort id))
257+
if (dataInfo != null && hasDottedCircle && type == "broken_cluster")
260258
{
261259
// Insert after possible Repha.
262260
int i = start;
@@ -272,7 +270,7 @@ private void InitialReorder(IGlyphShapingCollection collection, int index, int c
272270
}
273271

274272
glyphs[0] = current.GlyphId;
275-
glyphs[1] = id;
273+
glyphs[1] = circleId;
276274

277275
substitutionCollection.Replace(i, glyphs, FeatureTags.GlyphCompositionDecomposition);
278276

@@ -303,7 +301,7 @@ private void InitialReorder(IGlyphShapingCollection collection, int index, int c
303301
// base consonants.
304302
if (start + 3 <= end &&
305303
indicConfiguration.RephPosition != Positions.Ra_To_Become_Reph &&
306-
gSubTable.TryGetFeatureLookups(in RphfTag, this.ScriptClass, out _) &&
304+
gSubTable?.TryGetFeatureLookups(in RphfTag, this.ScriptClass, out _) == true &&
307305
((indicConfiguration.RephMode == RephMode.Implicit && !IsJoiner(substitutionCollection[start + 2])) ||
308306
(indicConfiguration.RephMode == RephMode.Explicit && substitutionCollection[start + 2].IndicShapingEngineInfo?.Category == Categories.ZWJ)))
309307
{
@@ -692,7 +690,7 @@ private void InitialReorder(IGlyphShapingCollection collection, int index, int c
692690

693691
const int prefLen = 2;
694692
if (basePosition + prefLen < end &&
695-
gSubTable.TryGetFeatureLookups(in PrefTag, this.ScriptClass, out _))
693+
gSubTable?.TryGetFeatureLookups(in PrefTag, this.ScriptClass, out _) == true)
696694
{
697695
// Find a Halant,Ra sequence and mark it for pre-base reordering processing.
698696
for (int i = basePosition + 1; i + prefLen - 1 < end; i++)
@@ -790,9 +788,7 @@ private bool WouldSubstitute(GlyphSubstitutionCollection collection, in Tag feat
790788
collection.EnableShapingFeature(i, featureTag);
791789
}
792790

793-
GlyphShapingData data = buffer[0];
794-
FontMetrics fontMetrics = data.TextRun.Font!.FontMetrics;
795-
791+
FontMetrics fontMetrics = this.fontMetrics;
796792
if (fontMetrics.TryGetGSubTable(out GSubTable? gSubTable))
797793
{
798794
const int index = 0;
@@ -865,6 +861,8 @@ private void FinalReorder(IGlyphShapingCollection collection, int index, int cou
865861
int max = index + count;
866862
int start = index;
867863
int end = NextSyllable(substitutionCollection, index, max);
864+
FontMetrics fontMetrics = this.fontMetrics;
865+
_ = fontMetrics.TryGetGSubTable(out GSubTable? gSubTable);
868866
while (start < max)
869867
{
870868
// 4. Final reordering:
@@ -873,14 +871,7 @@ private void FinalReorder(IGlyphShapingCollection collection, int index, int cou
873871
// applied (see below), the shaping engine performs some final glyph
874872
// reordering before applying all the remaining font features to the entire
875873
// cluster.
876-
GlyphShapingData data = substitutionCollection[start];
877-
FontMetrics fontMetrics = data.TextRun.Font!.FontMetrics;
878-
if (!fontMetrics.TryGetGSubTable(out GSubTable? gSubTable))
879-
{
880-
break;
881-
}
882-
883-
bool tryPref = gSubTable.TryGetFeatureLookups(in PrefTag, this.ScriptClass, out _);
874+
bool tryPref = gSubTable?.TryGetFeatureLookups(in PrefTag, this.ScriptClass, out _) == true;
884875

885876
// Find base consonant again.
886877
int basePosition = start;

‎src/SixLabors.Fonts/Tables/AdvancedTypographic/Shapers/ShaperFactory.cs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,18 @@ namespace SixLabors.Fonts.Tables.AdvancedTypographic.Shapers;
88
internal static class ShaperFactory
99
{
1010
/// <summary>
11-
/// Creates a Shaper based on the given script language.
11+
/// Creates a shaper based on the given script language.
1212
/// </summary>
1313
/// <param name="script">The script language.</param>
1414
/// <param name="unicodeScriptTag">The unicode script tag found in the font matching the script.</param>
15-
/// <param name="textOptions">The text options.</param>
15+
/// <param name="fontMetrics">The current font metrics.</param>
16+
/// <param name="textOptions">The global text options.</param>
1617
/// <returns>A shaper for the given script.</returns>
17-
public static BaseShaper Create(ScriptClass script, Tag unicodeScriptTag, TextOptions textOptions)
18+
public static BaseShaper Create(
19+
ScriptClass script,
20+
Tag unicodeScriptTag,
21+
FontMetrics fontMetrics,
22+
TextOptions textOptions)
1823
=> script switch
1924
{
2025
// Arabic
@@ -28,7 +33,7 @@ or ScriptClass.Manichaean
2833
or ScriptClass.PsalterPahlavi => new ArabicShaper(script, textOptions),
2934

3035
// Hangul
31-
ScriptClass.Hangul => new HangulShaper(script, textOptions),
36+
ScriptClass.Hangul => new HangulShaper(script, textOptions, fontMetrics),
3237

3338
// Indic
3439
ScriptClass.Bengali
@@ -40,7 +45,7 @@ or ScriptClass.Malayalam
4045
or ScriptClass.Oriya
4146
or ScriptClass.Tamil
4247
or ScriptClass.Telugu
43-
or ScriptClass.Khmer => new IndicShaper(script, unicodeScriptTag, textOptions),
48+
or ScriptClass.Khmer => new IndicShaper(script, unicodeScriptTag, textOptions, fontMetrics),
4449

4550
// Universal
4651
ScriptClass.Balinese
@@ -82,7 +87,7 @@ or ScriptClass.Takri
8287
or ScriptClass.Tibetan
8388
or ScriptClass.Tifinagh
8489
or ScriptClass.Tirhuta
85-
=> new UniversalShaper(script, textOptions),
90+
=> new UniversalShaper(script, textOptions, fontMetrics),
8691
_ => new DefaultShaper(script, textOptions),
8792
};
8893
}

0 commit comments

Comments
 (0)