feat: [SystemFontProvider] - Implement GetFontPath for Linux #2295
Conversation
Jklawreszuk
force-pushed
the
system-font-linux
branch
from
4eda6d8 to
3fa6b0d
Compare
| } | ||
| private string GetFontPathLinux(AssetCompilerResult result) | ||
| { | ||
| var bold = Style.IsBold() ? "Bold" : ""; | ||
| var italic = Style.IsItalic() ? "Italic" : ""; | ||
| string systemFontDirectory = "/usr/share/fonts"; | ||
| string[] files = System.IO.Directory.GetFiles(systemFontDirectory, "*.ttf", System.IO.SearchOption.AllDirectories); | ||
|
|
||
| using (var fontCollection = factory.GetSystemFontCollection(false)) | ||
| foreach (string file in files) | ||
| { | ||
| if (file.Contains(FontName) && file.Contains(bold) && file.Contains(italic)) | ||
| { | ||
| int index; | ||
| if (!fontCollection.FindFamilyName(FontName, out index)) | ||
| { | ||
| result?.Error($"Cannot find system font '{FontName}'. Make sure it is installed on this machine."); | ||
| return null; | ||
| } | ||
|
|
||
| using (var fontFamily = fontCollection.GetFontFamily(index)) | ||
| { | ||
| var weight = Style.IsBold() ? FontWeight.Bold : FontWeight.Regular; | ||
| var style = Style.IsItalic() ? SharpDX.DirectWrite.FontStyle.Italic : SharpDX.DirectWrite.FontStyle.Normal; | ||
| font = fontFamily.GetFirstMatchingFont(weight, FontStretch.Normal, style); | ||
| if (font == null) | ||
| { | ||
| result?.Error($"Cannot find style '{Style}' for font family {FontName}. Make sure it is installed on this machine."); | ||
| return null; | ||
| } | ||
| } | ||
| return file; |
There was a problem hiding this comment.
I'm guessing there are less hacky ways to get the style from a font, probably have to read the .ttf though ? If that's the case, can you add a todo here for this ?
Insert a line break between this function and the previous one.
GetFiles is fairly inefficient for this specific use case, best replace it with a Directory.EnumerateFiles(systemFontDirectory, "*.ttf", System.IO.SearchOption.AllDirectories) instead.
There was a problem hiding this comment.
I'm guessing there are less hacky ways to get the style from a font, probably have to read the .ttf though
Yeah, I'll try to see if it is possible to verify the style with our current FreeType binding, but common convetion I noticed is also separating styles into multiple files. I'm not though sure is always the case - see C:\Windows\Fonts where all fonts have single file possibly containing multiple styles. Looks like separating font styles into multiple files is standard but assuming font has 'Bold' or 'Italics' in their name is bad idea
Perhaps I should also consider adding a specific font to the Search regex. Something like this:
Directory.EnumerateFiles(systemFontDirectory, $"{FontName}*.ttf", System.IO.SearchOption.AllDirectories)
| using System.Linq; | ||
| using SharpDX.DirectWrite; | ||
| using Stride.Core.Assets.Compiler; | ||
| using Stride.Core; | ||
| using Stride.Core.Diagnostics; | ||
| using Stride.Assets.SpriteFont.Compiler; | ||
| using Stride.Graphics.Font; | ||
| using System; |
There was a problem hiding this comment.
Usually. System usings are first and then the rest in dictionary order. But that's ok here.
We might even use implicit usings later which would hide those.
| } | ||
|
|
||
| return new FontFace(font); | ||
| } | ||
|
|
||
| public override string GetFontPath(AssetCompilerResult result = null) | ||
| { | ||
| using (var factory = new Factory()) | ||
| if(OperatingSystem.IsWindows()) |
There was a problem hiding this comment.
Nit: code style, space after opening bracket
There was a problem hiding this comment.
Right 🤦 Done, I also updated other if statements
| } | ||
| return null; | ||
| } | ||
| private string GetFontPathLinux(AssetCompilerResult result) |
There was a problem hiding this comment.
NIt: code style, empty line between methods
There was a problem hiding this comment.
Thanks, I added new line
|
Thanks ! |
PR Details
This PR refactors GetFontPath to find system fonts for Linux for Asset Compiler.
Types of changes
Checklist