Dynamic font loading #1363

t-arn · 2021-11-12T08:04:35Z

This PR allows us to include a font file to a Toga project and use the font in the Toga project without needing to install the font on the system.

The PR includes 3 free fonts from here:
https://www.1001freefonts.com/d/4555/endor.zip (ENDOR___.ttf)
https://use.fontawesome.com/releases/v5.15.4/fontawesome-free-5.15.4-desktop.zip (Font Awesome 5 Free-Solid-900.otf)
https://fonts.google.com/specimen/Roboto (Roboto*.ttf)

Dynamic font loading should also work on iOS and macOS:
https://marco.org/2012/12/21/ios-dynamic-font-loading
https://stackoverflow.com/questions/2703085/how-can-you-load-a-font-ttf-from-a-file-using-core-text
But the implementation for these platforms are not in this PR.
Maybe, someone more savvy with iOS and macOS can add this.

PR Checklist:

All new features have been tested
All new features have been documented
I have read the CONTRIBUTING.md file
I will abide by the code of conduct

* added Font.register()

* added documentation

* but try...except in toga_android/fonts.py is not catching Java Exception

* changed MANIFEST.in to include TTF and OTF files in examples

t-arn · 2021-11-12T10:39:52Z

@freakboy3742 Please review this PR, thank you!

freakboy3742

This looks really good - thanks!

The implementation looks reasonable for what it's doing (once I've done some code reformatting on the example); however, I have two questions about the technical approach.

The first is that, as far as I can make out, "registration" is really not much more than associating a font name with a filename. If a font is used multiple times, it is re-created every time, rather than being created once, and then re-used. Is there any particular reason for this?

Second - the two example fonts you've provided evidently include multiple font weights. However, it's reasonably common for fonts to be distributed with different weights in different files. Using the approach you've described here, you're associating a name with a single file - which would seem to preclude using a font that comes in multiple files. The winforms API in particular would seem to allow for this, but the implementation only ever puts one font in the collection.

I'm wondering if we need to modify font registration process to accomodate these two - firstly, creating the font at the time of registration (and then returning that instance whenever it is needed); and allowing for multiple font files under a single registered name.

t-arn · 2021-11-14T12:14:20Z

@freakboy3742

once I've done some code reformatting on the example

You seem to prefer double quotes for strings , instead of single quotes. I thought, they are interchangeable.
Why do you prefer double quotes?

If a font is used multiple times, it is re-created every time, rather than being created once, and then re-used.

This is probably true for Android. But on Winforms, the loaded fonts are cached and re-used. I did not cache the fonts on Android, because I wasn't sure if there was a particular reason why this has been implemented differently to Winforms and Cocoa. Do you think, we should cache the fonts on Android as well?

firstly, creating the font at the time of registration

From what I understand, you need to define the font size when creating a font. At registration time, you do not know the size yet. That's why, I create the font only when it is actually used and when we know the family, the weight, the style and the size.

and allowing for multiple font files under a single registered name

Yes, that could be done. So, when we have a separate font file for a certain weight and style, we would use this font with "regular" weight and style, instead of trying to render the weight and style on the base font?

freakboy3742 · 2021-11-15T12:43:24Z

@freakboy3742

once I've done some code reformatting on the example

You seem to prefer double quotes for strings , instead of single quotes. I thought, they are interchangeable. Why do you prefer double quotes?

The formatting changes I applied are the result of running black over the codebase. It prefers double quotes over singles because of the escaping problem - single quotes are much more likely to be actually printed (as either apostrophes, or as user-visible quotes); using double quotes for strings minimises the amount of escaping that is required.

If a font is used multiple times, it is re-created every time, rather than being created once, and then re-used.

This is probably true for Android. But on Winforms, the loaded fonts are cached and re-used. I did not cache the fonts on Android, because I wasn't sure if there was a particular reason why this has been implemented differently to Winforms and Cocoa. Do you think, we should cache the fonts on Android as well?

Ah - I see the caching on Winforms now; it was part of the original implementation.

As for Android - yes, I think caching should be added. It was less important when most typefaces were being loaded as system defaults; but if we're encouraging people to create new typefaces, we should ensure we're not needlessly re-creating them.

firstly, creating the font at the time of registration

From what I understand, you need to define the font size when creating a font. At registration time, you do not know the size yet. That's why, I create the font only when it is actually used and when we know the family, the weight, the style and the size.

In both Android and Windows, there's a differentiation between the set of objects that define a "font" (the face, weight, and style) - and the specific instance of a font that is passed to labels etc. The "generic" font combination of face, weight and style is definitely something that should be cached (otherwise we'll be re-loading the font file every time we render a label). This can be done at time of registration, because you're only caching the existence of a font. The specific font at a given size should also be cached if possible, as it's one less object that needs to be re-created. The latter is what Windows and Cocoa are currently caching.

and allowing for multiple font files under a single registered name

Yes, that could be done. So, when we have a separate font file for a certain weight and style, we would use this font with "regular" weight and style, instead of trying to render the weight and style on the base font?

Not quite.Many fonts - especially the common ones - will have multiple variants: Regular, Bold, Italic, Oblique, Bold Italic (and potentially many others). We need to be able to register:

a single file that contains multiple variants; or
a separate file for each of the variants.

Regardless of how the font is split into files, we then need to be able to ask "give me a 12 pt bold italic version". If we can't satisfy that because the variant doesn't exist (e.g., we don't have a bold italic version) then we fall back to a variant that we can satisfy (e.g., 12pt bold, then 12pt italic, then 12pt regular).

So - part of the registration process will involve working out what we actually have.

t-arn · 2021-11-15T19:45:08Z

@freakboy3742

I don't quite know what you mean with "generic font". Do you mean a toga.Font? Should then the REGISTERED_FONTS be the cache for these "generic fonts" and contain toga.Fonts instead of just the family name?

freakboy3742 · 2021-11-15T22:43:57Z

@t-arn It's not exactly a toga.Font, because that encompasses a size as well. By "generic", I mean capturing the "face, style, weight" attributes, independent of the size; the "generic" font is then used to instantiate a specific font instance.

REGISTERED_FONTS is a cache of these "generic" pieces; the existing _FONT_CACHE in Winforms/Cocoa is a cache of specific font instances.

That said - I'm aware that there isn't (on any platform) a clear single object that represent this "generic" font. However, there are objects being created/loaded as part of the font creation process that are being re-created if you ask for a second font with the same face/weight/style, but a different style. What I'm suggesting is we should be caching any intermediate object to avoid re-reading the font file, and accommodating the case where a single font file may contain multiple weights & styles.

* added support for loading several font file for the same family

* updated tests

* updated Android implementation

* fixed Winforms exception when adding many custom fonts * code clean up * applied black formatting

t-arn · 2021-11-19T07:14:50Z

@freakboy3742 I made following changes:

added the RegisteredFont class
added support for registering several font files for the same font family
font chaching for Android

Please re-review this PR.

freakboy3742

This definitely works... but I get the impression that it's simultaneously much more complex than it needs to be in some places, and also not as complex as it needs to be in others.

The process of registration creates a whole new object that doesn't actually seem to serve any purpose beyond being an expensive cache key - except that it also contains the one detail that actually needs to be cached - the path to the file. However, the RegisteredFont object is both the key for lookup, and the object being looked up. Is there any reason this couldn't just be a dictionary of a tuple (family, weight, style, variant) mapping to path?

However, even with the complexity that is there - I'm not sure I see how this works for "multiple font file" situations. If I have a font file that contains multiple weights/variants, it will be registered as "Normal/Normal/Normal"... so how do I get the "Bold" variant?

The _pfc object also confuses me; see the commentary inline.

freakboy3742 · 2021-11-21T02:33:46Z

src/winforms/toga_winforms/fonts.py

@@ -17,20 +18,35 @@ def points_to_pixels(points, dpi):

 class Font:
    def __init__(self, interface):
+        self._pfc = None  # this needs to be a class variable, otherwise we might get Winforms exceptions later


I'm confused... on what condition can we get a Winforms exception? AFAICT, self._pfc is only referenced between lines 32 and 36... and it's recreated every time. How does this error manifest?

And, if it does needs to be a class variable then it should be declared on the class. This is currently declared as an instance variable.

See the discussion between me and SyntheticBee on Discord and here: https://stackoverflow.com/questions/11829344/parameter-is-not-valid-when-draw-text-in-label-with-custom-font

The exception seems to happen, when there are too many custom fonts being used.
I'm not sure myself, where exactly the exception occurs and why the use of this instance variable (you're right: it is not a class variable) helps.

freakboy3742 · 2021-11-21T02:36:30Z

src/core/toga/fonts.py

+        _REGISTERED_FONT_CACHE[registered_font.key] = registered_font
+
+    @staticmethod
+    def find_registered_font(family, weight=NORMAL, style=NORMAL, variant=NORMAL):


Having a simple register method, and a complex lookup method effectively puts the load on the wrong end of the lookup. Registration is something that is done once; font lookup will be a regular event. If we pre-insert font at all the cache locations where it might be useful, we can make cache retrieval a single operation, rather than needing to have complex logic every time a font is requested. This also removes the need for a "find_registered_font" function - it's just a lookup on a dictionary.

OK, I addressed this issue

freakboy3742 · 2021-11-21T02:37:12Z

src/core/toga/fonts.py

+        registered_font = RegisteredFont(
+            family, path, weight=weight, style=style, variant=variant
+        )
+        _REGISTERED_FONT_CACHE[registered_font.key] = registered_font


If you define __hash__() on a class, you don't need to explicitly reference a key like this; you can just use the object as the key.

I'm also a little unclear about what is going on here. You've got a dictionary mapping from a font... to a font. Why is the path on the RegisteredFont, rather than being the thing that is being registered?

OK, I addressed this issue

freakboy3742 · 2021-11-21T02:37:54Z

src/core/toga/fonts.py

+    def key(self):
+        return "<RegisteredFont family={} weight={} style={} variant={}>".format(
+            self.family, self.weight, self.style, self.variant
+        )


Strings are a great object to use as a key - it's better to use tuples; or better still, the actual object.

OK, issue addressed

freakboy3742 · 2021-11-21T02:39:31Z

src/core/toga/fonts.py

+        if variant not in constants.FONT_VARIANTS:
+            variant = NORMAL
+        return "<RegisteredFont family={} weight={} style={} variant={}>".format(
+            family, weight, style, variant


This effectively duplicates the key construction logic. This should be a single utility function, rather than a duplication.

OK, I removed the RegisteredFont class

freakboy3742 · 2021-11-21T02:40:18Z

src/core/toga/fonts.py

+                return None
+
+
+class RegisteredFont:


I'm not sure I see what having a second class gains us here. It's an internal cache lookup; a tuple is more than enough to operate as a key.

OK, I addressed this issue

freakboy3742 · 2021-11-21T02:56:43Z

src/android/toga_android/fonts.py

+                    family = Typeface.createFromFile(
+                        str(self.interface.factory.paths.app / registered_font.path)
+                    )
+                except Exception as ex:


This is a red flag; blanket exception handling can accidentally hide a wide range of problems. Can we narrow this down to a specific error?

I can think of these cases:

The font file does not exist

The file is a folder, not a file

The file is not a supported font file

What Python exceptions correspond to these cases?

How to find out if there are other cases?

In PrivateFontCollection, it says If you try to use a font that is not supported, such as an unsupported OpenType font or a Bitmap font, an exception will occur. But what exception exactly?

OK, I addressed this issue cleanly on Winforms.
On Android I can only handle the (crashing) file-not-found exception. The exception when using an invalid font file cannot be catched, but luckily, it does not crash the app

I now catch the file-not-found exception when loading the font. Would it be better to catch this exception already in Font.register()? But then, register could not be a static method anymore because we need access to factory.paths.app

freakboy3742 · 2021-11-21T02:57:18Z

src/core/tests/test_font.py

@@ -38,3 +44,9 @@ def test_variant(self):

    def test_weight(self):
        self.assertEqual(self.font.weight, self.weight)
+
+    def test_register(self):


There's a few other use cases here that should be tested - especially on the fallback lookup path.

OK, there is no fallback lookup path anymore

t-arn · 2021-11-21T08:13:03Z

@freakboy3742 Regarding the case where you have multiple styles/weights in 1 font file: You will register this font as normal/normal/normal and when requesting "bold", the find method will not find a specific font file for bold and therefore default to normal/normal/normal and apply bold there which should create the intended font.
Or, the user explicitly registers all style/weight combinations available in the font file with the register method.

t-arn · 2021-11-21T10:24:38Z

I googled a bit about fonts and found following:

ttc and otc are font collection files that contain several font style / weight combinations
There are OpenType Font Variations where a font file can contain several font style / weight combinations (e.g. Bahnschrift.ttf on Windows 10)

https://en.wikipedia.org/wiki/TrueType
https://en.wikipedia.org/wiki/OpenType
https://en.wikipedia.org/wiki/Variable_font

I'm not sure yet how to handle ttc / otc files. Maybe, it works with calling the register() method for every combination with the same collection file. Or, maybe, ttc / otc files are not supported for dynamic font loading at all...

* catch exception when font file cannot be loaded (only on Winforms - not possible on Android)

t-arn · 2021-11-21T18:02:03Z

@freakboy3742 I addressed all issues you raised on your second review.
Please re-review this PR.

* catch exception when font file cannot be loaded (only on Winforms - not possible on Android)

t-arn · 2021-12-09T14:10:32Z

@freakboy3742
I fear that the font caching on Android is not working correctly.
I have an Android app where I show a new GUI after tapping a button. And when I use a Toga branch that includes this font PR, the app crashes when the first label or button of the new GUI is created:

java_vm_ext.cc:545] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x2005
java_vm_ext.cc:545]     from java.lang.Object org.beeware.rubicon.PythonInstance.invoke(java.lang.Object, java.lang.reflect.Method, java.lang.Object[])
java_vm_ext.cc:545] "main" prio=5 tid=1 Runnable
java_vm_ext.cc:545]   | group="main" sCount=0 dsCount=0 flags=0 obj=0x741c7f50 self=0xea374000
java_vm_ext.cc:545]   | sysTid=17029 nice=-10 cgrp=default sched=0/0 handle=0xeef49494
java_vm_ext.cc:545]   | state=R schedstat=( 2667825654 1577720597 2136 ) utm=186 stm=80 core=0 HZ=100
java_vm_ext.cc:545]   | stack=0xff1b9000-0xff1bb000 stackSize=8MB
java_vm_ext.cc:545]   | held mutexes= "mutator lock"(shared held)
java_vm_ext.cc:545]   native: #00 pc 004151b6  /system/lib/libart.so (art::DumpNativeStack(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, int, BacktraceMap*, char const*, art::ArtMethod*, void*, bool)+198)
java_vm_ext.cc:545]   native: #01 pc 0051034e  /system/lib/libart.so (art::Thread::DumpStack(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, bool, BacktraceMap*, bool) const+382)
java_vm_ext.cc:545]   native: #02 pc 0050b603  /system/lib/libart.so (art::Thread::Dump(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, bool, BacktraceMap*, bool) const+83)
java_vm_ext.cc:545]   native: #03 pc 0031a720  /system/lib/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+1088)
java_vm_ext.cc:545]   native: #04 pc 0031ac35  /system/lib/libart.so (art::JavaVMExt::JniAbortF(char const*, char const*, ...)+117)
java_vm_ext.cc:545]   native: #05 pc 00516b37  /system/lib/libart.so (art::Thread::DecodeJObject(_jobject*) const+871)
java_vm_ext.cc:545]   native: #06 pc 000d53d9  /system/lib/libart.so (art::(anonymous namespace)::ScopedCheck::CheckInstance(art::ScopedObjectAccess&, art::(anonymous namespace)::ScopedCheck::InstanceKind, _jobject*, bool)+153)
java_vm_ext.cc:545]   native: #07 pc 000d4637  /system/lib/libart.so (art::(anonymous namespace)::ScopedCheck::CheckPossibleHeapValue(art::ScopedObjectAccess&, char, art::(anonymous namespace)::JniValueType)+551)
java_vm_ext.cc:545]   native: #08 pc 000d4718  /system/lib/libart.so (art::(anonymous namespace)::ScopedCheck::CheckPossibleHeapValue(art::ScopedObjectAccess&, char, art::(anonymous namespace)::JniValueType)+776)
java_vm_ext.cc:545]   native: #09 pc 000d3a5b  /system/lib/libart.so (art::(anonymous namespace)::ScopedCheck::Check(art::ScopedObjectAccess&, bool, char const*, art::(anonymous namespace)::JniValueType*)+811)
java_vm_ext.cc:545]   native: #10 pc 000d9758  /system/lib/libart.so (art::(anonymous namespace)::CheckJNI::CheckCallArgs(art::ScopedObjectAccess&, art::(anonymous namespace)::ScopedCheck&, _JNIEnv*, _jobject*, _jclass*, _jmethodID*, art::InvokeType, art::(anonymous namespace)::VarArgs const*)+200)
java_vm_ext.cc:545]   native: #11 pc 000d8974  /system/lib/libart.so (art::(anonymous namespace)::CheckJNI::CallMethodV(char const*, _JNIEnv*, _jobject*, _jclass*, _jmethodID*, char*, art::Primitive::Type, art::InvokeType)+948)
java_vm_ext.cc:545]   native: #12 pc 000c4a99  /system/lib/libart.so (art::(anonymous namespace)::CheckJNI::CallVoidMethodV(_JNIEnv*, _jobject*, _jmethodID*, char*)+73)
java_vm_ext.cc:545]   native: #13 pc 00003c08  /data/app/ch.tanapro.taapplister-KnOG6mrHMlrGtQmW37syBg==/lib/x86/librubicon.so (CallVoidMethod+56)
java_vm_ext.cc:545]   at org.beeware.rubicon.PythonInstance.invoke(Native method)

I have a strong feeling that the cached native fonts become stale which is the reason for the above crash message.
How can we prevent the cached native fonts from becoming stale?

paulproteus · 2021-12-09T14:29:16Z

@t-arn One thing I saw in that log is:

java_vm_ext.cc:545] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x2005

It seems to me that if the references were JNI global refs, the problem would be fixed.

Can you try that and see if it solves the issue?

t-arn · 2021-12-09T14:46:48Z

@paulproteus
How (and where) do I change the references to JNI global refs?

freakboy3742 · 2021-12-11T23:48:03Z

@t-arn The __global__method that I added in the development version of Rubicon is what this is for; in the meantime, you can see how global refs are used in the DetailedList implementation.

t-arn · 2021-12-12T11:35:58Z

@freakboy3742
Yes, the global() method fixed the problem :-)
I added following line, before adding the font to the cache:

family = family.__global__()  # store a JNI global reference to prevent objects from becoming stale

Please re-review

freakboy3742

Ok - I've made a couple of minor stylistic tweaks, but I think this is good to go. The API is very simple, but it makes sense; and it should be relatively easy to adapt to Cocoa, iOS and GTK.

I might take a quick swing to see if I can make it work on macOS at least; if I can't, I'll merge as is.

codecov · 2022-01-07T04:46:03Z

Codecov Report

Merging #1363 (2912073) into master (c621cfd) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted Files	Coverage Δ
src/core/toga/fonts.py	`96.66% <100.00%> (+2.91%)`	⬆️

freakboy3742 · 2022-01-07T05:35:53Z

Ok - looks like it's not that simple. It's easy enough to register a font... but it looks like Cocoa uses the font name that is internal to the font, which won't necessarily be the registration name - and it's not trivial to extract the internal registration name from a registered font. I've put my WIP on #1399; if anyone wants to try their hand at a fix before I get there, feel free.

t-arn added 10 commits November 9, 2021 13:33

created example for testing

62f6fd4

* extended example

7bc4b1c

* added Font.register()

* working for Windows, but only with full paths

080b5fa

* finished Windows implementation

5e23567

* added documentation

* implementation for Android working

76da4a2

* but try...except in toga_android/fonts.py is not catching Java Exception

* added font_style for Winforms

d2f140d

* fixed flakes

1586df7

* fixed flakes

f6a9f48

* cleanup

1814e29

* added fonts again

4f752bd

* changed MANIFEST.in to include TTF and OTF files in examples

t-arn and others added 2 commits November 12, 2021 15:02

* added test for Font.register()

4179e16

Applied black formatting to new example.

51d2c17

freakboy3742 requested changes Nov 14, 2021

View reviewed changes

t-arn added 4 commits November 18, 2021 09:26

* added RegisteredFont class

79b5fa8

* added support for loading several font file for the same family

* fixed flake

e5cfc0e

* updated tests

* applied black formatting to font example

34915d8

* updated Android implementation

* added font cache for Android

5f376cd

* fixed Winforms exception when adding many custom fonts * code clean up * applied black formatting

freakboy3742 requested changes Nov 21, 2021

View reviewed changes

t-arn added 2 commits November 21, 2021 18:25

* simplified the registered font cache

232a2f6

* catch exception when font file cannot be found

9ee6d0e

* catch exception when font file cannot be loaded (only on Winforms - not possible on Android)

cleanup

049d374

t-arn added 4 commits November 22, 2021 15:53

* catch exception when font file cannot be found

3cf2cd2

* catch exception when font file cannot be loaded (only on Winforms - not possible on Android)

Merge remote-tracking branch 'origin/dynamic_fonts' into dynamic_fonts

11f06b6

fixed flake

87d53a3

fixed flake

88551df

t-arn added 2 commits December 12, 2021 12:15

Merge branch 'master' into dynamic_fonts

f005dad

* fixed app crash caused by stale cached fonts

3a5244a

freakboy3742 added 4 commits January 7, 2022 10:06

Merge branch 'master' into dynamic_fonts

b966def

Make better use of constants in the example app.

02a43e2

Modified Pythonnet API usage.

a7c8c49

Minor code cleanups.

9f33d2b

freakboy3742 approved these changes Jan 7, 2022

View reviewed changes

Clean up import ordering.

2912073

freakboy3742 merged commit 7e5f590 into beeware:master Jan 7, 2022

t-arn deleted the dynamic_fonts branch January 7, 2022 07:09

mhsmith mentioned this pull request Sep 12, 2023

Add support for custom fonts on remaining platforms #1837

Closed

Dynamic font loading #1363

Dynamic font loading #1363

Conversation

t-arn commented Nov 12, 2021 • edited Loading

PR Checklist:

t-arn commented Nov 12, 2021

freakboy3742 left a comment

Choose a reason for hiding this comment

t-arn commented Nov 14, 2021 • edited Loading

freakboy3742 commented Nov 15, 2021

t-arn commented Nov 15, 2021

freakboy3742 commented Nov 15, 2021

t-arn commented Nov 19, 2021

freakboy3742 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t-arn commented Nov 21, 2021 • edited Loading

t-arn commented Nov 21, 2021 • edited Loading

t-arn commented Nov 21, 2021

t-arn commented Dec 9, 2021 • edited Loading

paulproteus commented Dec 9, 2021

t-arn commented Dec 9, 2021

freakboy3742 commented Dec 11, 2021

t-arn commented Dec 12, 2021

freakboy3742 left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 7, 2022

Codecov Report

freakboy3742 commented Jan 7, 2022

t-arn commented Nov 12, 2021 •

edited

Loading

t-arn commented Nov 14, 2021 •

edited

Loading

t-arn commented Nov 21, 2021 •

edited

Loading

t-arn commented Nov 21, 2021 •

edited

Loading

t-arn commented Dec 9, 2021 •

edited

Loading