Skip to content

Commit 16f00e8

Browse files
OssianEPPlusOssianEPPlus
authored
Fix/BoldItalics in OpenTypeFontCache caches incorrectly (#2248)
* replaced unnecsary generics * Fixed font boldItalic buFixed font boldItalic bug * Added better tests for bug * Moved tests to regression * Added benchmark test for breaking out later
1 parent a980be3 commit 16f00e8

10 files changed

Lines changed: 123 additions & 57 deletions

File tree

‎src/EPPlus.Export.ImageRenderer/RenderItems/Shared/ParagraphContainer.cs‎

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -107,39 +107,26 @@ private double GetParagraphLineSpacingInPixels(double spacingValue, ITextMeasure
107107
}
108108
}
109109

110-
111-
/// <summary>
112-
/// Each specific container must specify textrun type within this
113-
/// </summary>
114-
/// <param name="origTxtRun"></param>
115-
/// <param name="runDisplayString"></param>
116-
internal protected abstract void AddTextRun(ExcelParagraphTextRunBase origTxtRun, string runDisplayString);
117-
118110
/// <summary>
119111
/// DisplayString is the text altered for display with respect to bounds etc.
120112
/// Containing line breaks appropriate for the given container
121113
/// </summary>
122114
/// <param name="origTxtRun"></param>
123-
/// <param name="runDisplayString"></param>
124-
internal protected void AddTextRun<T>(ExcelParagraphTextRunBase origTxtRun, string runDisplayString) where T : TextRunItem
115+
/// <param name="displayText"></param>
116+
internal protected void AddTextRun(ExcelParagraphTextRunBase origTxtRun, string displayText)
125117
{
126118
var maxWidth = Bounds.Width;
127119

128-
//Specify type
129-
var textRunType = typeof(T);
130-
131120
//Create object of type
132-
var targetTxtRun = (T)Activator.CreateInstance(textRunType, origTxtRun, Bounds, runDisplayString);
121+
var targetTxtRun = CreateTextRun(origTxtRun, Bounds, displayText);
122+
targetTxtRun.LineSpacingPerNewLine = ParagraphLineSpacing;
133123

134124
if (_textRunItems.Count == 0 && _isFirstParagraph == true)
135125
{
136126
targetTxtRun.BaseLineSpacing = _lineSpacingAscendantOnly;
137-
targetTxtRun.LineSpacingPerNewLine = ParagraphLineSpacing;
138127
}
139128
else
140129
{
141-
targetTxtRun.LineSpacingPerNewLine = ParagraphLineSpacing;
142-
143130
//If there are multiple sizes/multiple fonts with multiple sizes
144131
if (_lsMultiplier.HasValue)
145132
{
@@ -193,7 +180,7 @@ List<string> GetWrappedText(ExcelDrawingTextRunCollection runs)
193180
for (int i = 0; i < runs.Count(); i++)
194181
{
195182
var txtRun = runs[i];
196-
var runFont = txtRun.GetMeasureFont();
183+
var runFont = txtRun.GetMeasurementFont();
197184

198185
runContents.Add(txtRun.Text);
199186
fonts.Add(runFont);
@@ -251,17 +238,21 @@ internal double GetAlignmentHorizontal(eTextAlignment txAlignment)
251238
return TextUtils.RoundToWhole(x);
252239
}
253240

254-
//public override void Render(StringBuilder sb)
255-
//{
256-
// throw new NotImplementedException();
257-
//}
258-
259241
internal override void GetBounds(out double il, out double it, out double ir, out double ib)
260242
{
261243
il = Bounds.Left + _leftMargin;
262244
it = Bounds.Top;
263245
ir = Bounds.Right - _rightMargin;
264246
ib = Bounds.Bottom;
265247
}
248+
249+
/// <summary>
250+
/// Type of textrun defined by child type
251+
/// </summary>
252+
/// <param name="run"></param>
253+
/// <param name="parent"></param>
254+
/// <param name="DisplayString"></param>
255+
/// <returns></returns>
256+
internal abstract TextRunItem CreateTextRun(ExcelParagraphTextRunBase run, BoundingBox parent, string displayText);
266257
}
267258
}

‎src/EPPlus.Export.ImageRenderer/RenderItems/Shared/ShapeItem.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace EPPlus.Export.ImageRenderer.RenderItems.Shared
2121
/// </summary>
2222
internal abstract class ShapeItem : DrawingShape
2323
{
24-
TextBody<SvgParagraphItem> _textBox;
24+
TextBody _textBox;
2525

2626
internal ShapeItem(ExcelShape shape) : base(shape)
2727
{

‎src/EPPlus.Export.ImageRenderer/RenderItems/Shared/TextBody.cs‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace EPPlus.Export.ImageRenderer.RenderItems.Shared
1818
/// Margin left = X
1919
/// Margin right = Y
2020
/// </summary>
21-
internal abstract class TextBody<T>: SvgRenderItem where T : ParagraphContainer
21+
internal abstract class TextBody: SvgRenderItem
2222
{
2323
/// <summary>
2424
/// Shorthand for Bounds.Width
@@ -34,7 +34,7 @@ internal abstract class TextBody<T>: SvgRenderItem where T : ParagraphContainer
3434

3535
internal eTextAnchoringType VerticalAlignment = eTextAnchoringType.Top;
3636

37-
internal abstract List<T> Paragraphs { get; set; }
37+
internal abstract List<ParagraphContainer> Paragraphs { get; set; }
3838

3939
public bool AllowOverflow;
4040

@@ -63,12 +63,7 @@ public void AddParagraph(string text, FontMeasurerTrueType measurer)
6363

6464
if (_measurer != null)
6565
{
66-
//Specify type
67-
var paragraphType = typeof(T);
68-
69-
//Create object of type
70-
var paragraph = (T)Activator.CreateInstance(paragraphType, Bounds);
71-
66+
var paragraph = CreateParagraph(Bounds);
7267
Paragraphs.Add(paragraph);
7368

7469
paragraph.AddText(text, measurer);
@@ -84,11 +79,7 @@ public void ImportParagraph(ExcelDrawingParagraph item, double startingY)
8479
var measureFont = item.DefaultRunProperties.GetMeasureFont();
8580
bool isFirst = Paragraphs.Count == 0;
8681

87-
//Specify type
88-
var paragraphType = typeof(T);
89-
90-
//Create object of type
91-
var paragraph = (T)Activator.CreateInstance(paragraphType, item, Bounds);
82+
var paragraph = CreateParagraph(item, Bounds);
9283

9384
paragraph.Bounds.Top = startingY;
9485

@@ -204,5 +195,17 @@ internal override void GetBounds(out double il, out double it, out double ir, ou
204195
{
205196
il = Bounds.Left; it = Bounds.Top; ir = Bounds.Right; ib = Bounds.Bottom;
206197
}
198+
199+
/// <summary>
200+
/// Each file format defines its own paragraph
201+
/// </summary>
202+
/// <returns></returns>
203+
internal abstract ParagraphContainer CreateParagraph(ExcelDrawingParagraph paragraph, BoundingBox parent);
204+
205+
/// <summary>
206+
/// Each file format defines its own paragraph
207+
/// </summary>
208+
/// <returns></returns>
209+
internal abstract ParagraphContainer CreateParagraph(BoundingBox parent);
207210
}
208211
}

‎src/EPPlus.Export.ImageRenderer/RenderItems/SvgItem/SvgParagraphItem.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,6 @@ public SvgParagraphItem(ExcelDrawingParagraph p, BoundingBox parent) : base(p, p
3030
}
3131
}
3232

33-
internal protected override void AddTextRun(ExcelParagraphTextRunBase origTxtRun, string runDisplayString)
34-
{
35-
AddTextRun<SvgTextRunItem>(origTxtRun, runDisplayString);
36-
}
37-
3833
private string GetHorizontalAlignmentAttribute(double indentX)
3934
{
4035
string ret = "";
@@ -114,5 +109,10 @@ internal override SvgRenderItem Clone(SvgShape svgDocument)
114109
{
115110
throw new System.NotImplementedException();
116111
}
112+
113+
internal override TextRunItem CreateTextRun(ExcelParagraphTextRunBase run, BoundingBox parent, string displayText)
114+
{
115+
return new SvgTextRunItem(run, parent, displayText);
116+
}
117117
}
118118
}

‎src/EPPlus.Export.ImageRenderer/RenderItems/SvgItem/SvgTextBodyItem.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,20 @@
22
using EPPlus.Graphics;
33
using EPPlusImageRenderer.RenderItems;
44
using EPPlusImageRenderer.Svg;
5+
using OfficeOpenXml.Drawing;
56
using System;
67
using System.Collections.Generic;
78
using System.Text;
89

910
namespace EPPlus.Export.ImageRenderer.RenderItems.SvgItem
1011
{
11-
internal class SvgTextBodyItem : TextBody<SvgParagraphItem>
12+
internal class SvgTextBodyItem : TextBody
1213
{
1314
public SvgTextBodyItem(BoundingBox parent) : base(parent)
1415
{
1516
}
1617

17-
internal override List<SvgParagraphItem> Paragraphs { get; set; } = new List<SvgParagraphItem>();
18+
internal override List<ParagraphContainer> Paragraphs { get; set; } = new List<ParagraphContainer>();
1819

1920
public override void Render(StringBuilder sb)
2021
{
@@ -34,5 +35,15 @@ internal override SvgRenderItem Clone(SvgShape svgDocument)
3435
{
3536
throw new NotImplementedException();
3637
}
38+
39+
internal override ParagraphContainer CreateParagraph(BoundingBox parent)
40+
{
41+
return new SvgParagraphItem(parent);
42+
}
43+
44+
internal override ParagraphContainer CreateParagraph(ExcelDrawingParagraph paragraph, BoundingBox parent)
45+
{
46+
return new SvgParagraphItem(paragraph, parent);
47+
}
3748
}
3849
}

‎src/EPPlus.Fonts.OpenType.Tests/FontMeasurerPerformanceTest.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
using OfficeOpenXml.Interfaces.Drawing.Text;
1+
using EPPlus.Fonts.OpenType.FontCache;
2+
using OfficeOpenXml.Interfaces.Drawing.Text;
23
using System.Diagnostics;
4+
using static System.Net.Mime.MediaTypeNames;
35

46
namespace EPPlus.Fonts.OpenType.Tests
57
{
@@ -77,7 +79,8 @@ public void Wrap20Paragraphs100Times()
7779
//Assert.AreEqual(UnOptimizedOriginalWrappingString, currStr);
7880
}
7981

80-
[TestMethod]
82+
[TestMethod, Ignore("This test should not run in a multithreaded test run. If we want to keep it, it should be moved to a separate benchmark project.")]
83+
[TestCategory("Benchmark")]
8184
public void Wrap20Paragraphs100TimesMultipleTextFragments()
8285
{
8386
List<string> longTexts = new List<string>();

‎src/EPPlus.Fonts.OpenType.Tests/Regression/RegressionTests.cs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@ Date Author Change
1010
*************************************************************************************************
1111
12/22/2025 EPPlus Software AB Regression tests
1212
*************************************************************************************************/
13+
using EPPlus.Fonts.OpenType.FontCache;
1314
using EPPlus.Fonts.OpenType.Tests.Helpers;
15+
using OfficeOpenXml.Interfaces.Drawing.Text;
1416
using System.ComponentModel.DataAnnotations;
17+
using System.Diagnostics;
1518
using System.IO;
1619
using System.Runtime.InteropServices;
1720

@@ -270,5 +273,50 @@ public void Bug_20251222_CompoundLigatureComponents()
270273
// Verify in FontDrop or similar tool that ligatures render correctly
271274
FontTestHelper.AssertFontValid(subset);
272275
}
276+
277+
[TestMethod]
278+
public void SettingBoldItalicAgainShouldNotTimeout()
279+
{
280+
MeasurementFont boldItalic = new MeasurementFont()
281+
{
282+
FontFamily = "Aptos Narrow",
283+
Size = 11f,
284+
Style = MeasurementFontStyles.Bold | MeasurementFontStyles.Italic
285+
};
286+
287+
var ttTextMeasurer = new FontMeasurerTrueType();
288+
289+
Stopwatch timer = new Stopwatch();
290+
timer.Start();
291+
ttTextMeasurer.SetFont(boldItalic);
292+
timer.Stop();
293+
var firstTime = timer.ElapsedMilliseconds;
294+
295+
timer.Restart();
296+
ttTextMeasurer.SetFont(boldItalic);
297+
timer.Stop();
298+
299+
//Doing the same operation again should not be a whole second longer
300+
Assert.IsTrue((firstTime + 1000) > timer.ElapsedMilliseconds);
301+
//At time of writing OpenTypeFontCache.GetFromCache is 2s therefore it should take less
302+
Assert.IsTrue(timer.ElapsedMilliseconds < 2000);
303+
}
304+
305+
[TestMethod]
306+
public void GetFromCacheBoldItalicShouldWork()
307+
{
308+
MeasurementFont boldItalic = new MeasurementFont()
309+
{
310+
FontFamily = "Aptos Narrow",
311+
Size = 11f,
312+
Style = MeasurementFontStyles.Bold | MeasurementFontStyles.Italic
313+
};
314+
315+
var ttTextMeasurer = new FontMeasurerTrueType();
316+
ttTextMeasurer.SetFont(boldItalic);
317+
318+
var cachedFont = OpenTypeFontCache.GetFromCache("Aptos Narrow", FontSubFamily.BoldItalic);
319+
Assert.IsNotNull(cachedFont);
320+
}
273321
}
274322
}

‎src/EPPlus.Fonts.OpenType/FontCache/OpenTypeFontCache.cs‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,23 @@ internal static void Clear()
2121
}
2222
}
2323

24-
25-
26-
static string BuildCacheKey(string familyName, string subFamily)
24+
/// <summary>
25+
/// THE SUBFAMILY ENUM SHOULD ALWAYS BE INPUT PARAMETER
26+
/// Fonts can name themselves however they want. But we map other values in the font to the subfamily
27+
/// Therefore Never change it to e.g. a string input-parameter
28+
/// and Never use e.g font.GetEnglishSubfamily name as this could create miss-matches.
29+
/// </summary>
30+
/// <param name="familyName"></param>
31+
/// <param name="subFamily">MUST STAY as FontSubFamily enum</param>
32+
/// <returns></returns>
33+
static string BuildCacheKey(string familyName, FontSubFamily subFamily)
2734
{
28-
return $"{familyName}-{subFamily}";
35+
return $"{familyName}-{subFamily.ToString()}";
2936
}
3037

3138
public static bool Contains(string familyName, FontSubFamily subFamily)
3239
{
33-
var key = BuildCacheKey(familyName, subFamily.ToString());
40+
var key = BuildCacheKey(familyName, subFamily);
3441
lock (_syncRoot)
3542
{
3643

@@ -44,7 +51,7 @@ public static void BeginCache(string familyName, FontSubFamily subFamily)
4451
{
4552
lock (_syncRoot)
4653
{
47-
var key = BuildCacheKey(familyName, subFamily.ToString());
54+
var key = BuildCacheKey(familyName, subFamily);
4855
if (!_cache.ContainsKey(key))
4956
{
5057
_cache[key] = new CachedOpenTypeFont()
@@ -55,11 +62,11 @@ public static void BeginCache(string familyName, FontSubFamily subFamily)
5562
}
5663
}
5764

58-
public static void AddToCache(OpenTypeFont font)
65+
public static void AddToCache(OpenTypeFont font, string familyName, FontSubFamily subFamily)
5966
{
6067
lock (_syncRoot)
6168
{
62-
var key = BuildCacheKey(font.GetEnglishFontFamilyName(), font.GetEnglishFontSubFamilyName());
69+
var key = BuildCacheKey(familyName, subFamily);
6370
if (!_cache.ContainsKey(key))
6471
{
6572
_cache[key] = new CachedOpenTypeFont();
@@ -76,7 +83,7 @@ public static void AddToCache(OpenTypeFont font)
7683

7784
public static CachedOpenTypeFont GetFromCache(string familyName, FontSubFamily subFamily)
7885
{
79-
var key = BuildCacheKey(familyName, subFamily.ToString());
86+
var key = BuildCacheKey(familyName, subFamily);
8087
lock (_syncRoot)
8188
{
8289
if (_cache.TryGetValue(key, out var cached))

‎src/EPPlus.Fonts.OpenType/OpenTypeFonts.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public static OpenTypeFont GetFontDataOpen(
144144
var font = OpenTypeFontFactory.CreateFromFace(face);
145145

146146
if (!ignoreCache)
147-
OpenTypeFontCache.AddToCache(font);
147+
OpenTypeFontCache.AddToCache(font, fontName, subFamily);
148148

149149
return font;
150150
}

‎src/EPPlus.Fonts.OpenType/TrueTypeMeasurer/FontMeasurerTrueType.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ public class FontMeasurerTrueType : ITextMeasurerWrap
3131
private OpenTypeFont _defaultFont = TextData.GetFontData("Calibri", FontSubFamily.Regular);
3232
private double _defaultSize = 11d;
3333

34+
OpenTypeFont LastFont = null;
35+
3436
OpenTypeFont CurrentFont;
3537
private double _widthInPoints;
3638
private double _heightInPoints;
@@ -93,7 +95,8 @@ public FontMeasurerTrueType(MeasurementFont mFont)
9395
public void SetFont(double fontSize, string fontName, FontSubFamily subFamily = FontSubFamily.Regular)
9496
{
9597
CurrentFontName = fontName;
96-
CurrentFont = TextData.GetFontData(fontName, subFamily);
98+
var newFont = TextData.GetFontData(fontName, subFamily);
99+
CurrentFont = newFont;
97100
FontSize = fontSize;
98101
}
99102

0 commit comments

Comments
 (0)