wpf
wpf copied to clipboard
dotnet
Optimize GlyphTypeface: Avoid create the cmap Dictionary when get Count only
Description
As the code shows:
https://github.com/dotnet/wpf/blob/f7ebd69146f3ec9b771e92c13250efa99a65a26c/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/FontCache/FontFaceLayoutInfo.cs#L585-L609
We can know that the cost of creating CMap is expensive. When we get the Count only, we do not need to create the CMap Dictionary.
We can get the count from FontFace directly:
https://github.com/dotnet/wpf/blob/f7ebd69146f3ec9b771e92c13250efa99a65a26c/src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/DWriteWrapper/FontFace.h#L126
Customer Impact
Performance
Regression
Testing
CI only.
Risk
Low. I do not change the behavior. The old behavior is get the count after the CMap Dictionary created, and the new behavior is get the count from FontFace directly.
Microsoft Reviewers: Open in CodeFlow
To be fair if you just want a count you can use GlyphTypeface.GlyphCount. How did you come with this, is it in some hot path?
I might be misremembering, but doesn't CMap get created the first time you do a single text run / line anyways? Not mentioning that the generation method is slow af and should be rewritten but as miloush says, you have the prop on GlyphTypeface directly for count on public interface and FontFaceLayoutInfo does have it as well so now you're code duping baiscally.
So I'm indeed curious about the scenario where you don't end up creating it and can get away with this.
@miloush
To be fair if you just want a count you can use GlyphTypeface.GlyphCount. How did you come with this, is it in some hot path?
We just use the CharacterToGlyphMap as the Dictionary and pass it to other business code. The hot path will count the CharacterToGlyphMap Dictionary for all fonts.
@h3xds1nz As you can see, ont of the IDictionary hot path is TryGetValue method, and which do not create the CMap:
https://github.com/dotnet/wpf/blob/f7ebd69146f3ec9b771e92c13250efa99a65a26c/src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/FontCache/FontFaceLayoutInfo.cs#L634-L657
So that, it is meaningful to do this.
@lindexi I see, yeah if you're passing it around to other logic, that makes sense.
Eitherway, I'd probably extract it into a separate function instead of duplicating the code. Otherwise LGTM