Fix for issue #10307, embedded fonts not working on UWP #11741
Conversation
(cherry picked from commit 4ed0819)
|
This would require a documentation change on embedded fonts. The |
| var file = FontFile.FromString(IOPath.GetFileName(fontPostScriptName)); | ||
| var formated = $"{fontPostScriptName}#{file.GetPostScriptNameWithSpaces()}"; | ||
| yield return formated; | ||
| yield return fontFamily; |
There was a problem hiding this comment.
Note: this line was not needed when the font was found in the FontRegistrar. In fact, it caused problems when setting FontFamily in a CanvasTextFormat object (see FontImageSourceHandler.cs)
samhouts
added
the
retarget-branch-required
|
@samhouts Is anything else needed before this can be reviewed? |
|
@activa This looks great! Since this is an API change, can we move it to 5.0.0? Thanks! |
|
Hey wait there @samhouts, before we start moving this "bug" 😄 to next releases... @activa I was thinking that I probably wouldn't want to scratch around in files to find things and add names and all that... seems like too much work 😆 So I had a look at maybe trying to just ask the font file what the correct name was. Seems this is totally doable without adding any new dependencies or special logic: I just made use of the already used Win2D: var name = "fa-solid-900.ttf";
var uriString = $"ms-appdata:///local/fonts/{name}";
var uri = new Uri(uriString);
var cfs = new CanvasFontSet(uri);
var familyName = cfs.GetPropertyValues(CanvasFontPropertyIdentifier.FamilyName).FirstOrDefault().Value;
var fullFontName = $"{uriString}#{familyName}";
Content = new TextBlock
{
Text = "hello \uf0c8",
VerticalAlignment = VerticalAlignment.Center,
FontFamily = new FontFamily(fullFontName)
};If there is no font name, then I am not sure what is supposed to happen... but, we can probably just fall back to the plain filename for now. Unless you have an example font that has no name and can test things? Maybe you could tweak your PR to do this instead? And, then this will also clean up all the other logic bits when loading fonts. Together @activa my fine friend, we shall merge this into the next release of Xamarin.Forms 4.8! We shall oppose this "API change" and "move it to 5.0.0" oppression of our beloved UWP! o7 |
|
Using Win2D will also work on the WPF platform as well. Might be able to use things there too. |
|
My gosh, why do I have the power to close issues? I can't be trusted! My mouse slipped! Sorry for this mess. |
|
@mattleibow I'll update the PR with your suggested changes. |
…nt file using Win2D
|
PR was updated. No API changes were introduced and the font family is now extracted from the font file on UWP. |
| if (fontSet.Fonts.Count == 0) | ||
| return null; | ||
|
|
||
| return fontSet.GetPropertyValues(CanvasFontPropertyIdentifier.FamilyName).FirstOrDefault().Value; |
There was a problem hiding this comment.
Will FirstOrDefault here always return something?
Does this need a null check?
There was a problem hiding this comment.
The result is a struct: https://microsoft.github.io/Win2D/html/T_Microsoft_Graphics_Canvas_Text_CanvasFontProperty.htm
so the .Value should be null.
There was a problem hiding this comment.
What @mattleibow said. Also, I was thinking about caching the return value but it would be pointless because the resulting FontFamily objects are already being cached here.
|
/azp run |
|
No pipelines are associated with this pull request. |
|
Confirmation that it just runs on UWP without any changes to my existing app: @activa you are a beautiful dev! We need more like you in the world! Thanks for getting this PR in! Now we just got to get the Team to merge this for the next SR! |
samhouts
removed
the
retarget-branch-required
|
So much yay!!! My app will now look beautiful again. Thanks! |
Embedded fonts on UWP don't work when the font name does not match the file name of the font, which is the case with many Google fonts (and FontAwesome icons)
To fix this, the font family name has to be extracted from the font file and appended to the font name when the font is created from the font file.
Issues Resolved
API Changes
None
Platforms Affected
Behavioral/Visual Changes
None (fixes bug #10307)
Before/After Screenshots
Before:
After:
Not applicable
PR Checklist