Move font-loading logic into core

[HalfWhitt]

All the font backends already perform essentially the same logic, but sometimes in a different order, with individual quirks, or just formatted differently enough that the commonalities are less obvious. (For instance, unlike the others, Windows checks for a custom user-registered font before checking for a system font.)

This standardizes the overall logic and moves it up into core. When a new Font instance is created, it first checks a cache to see if a suitable implementation already exists¹. If it doesn't, it creates a new one and calls a series of methods on it, to search for the following, in order:

1. Toga's built-in system fonts
2. User-registered fonts
3. Fonts available on the system (Currently only supported on Windows and GTK)

This doesn't fix any bugs (that I know of), but it gives a more even playing field from which to consider #3566 and #3567.

¹ Since Font objects (both core and backend) are effectively read-only, it should be fine (as far as I know) for core fonts to share implementations.

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

[HalfWhitt]

Okay I didn't fix any bugs, but apparently I did create one! Will see about fixing the Windows error.

[HalfWhitt]

I may have fixed a bug — or removed an undocumented feature?

The current / main version of the Windows backend is capable of using arbitrary system-installed fonts. Try setting it to something like Arial or Impact — it'll just work, as long as it's available on your system. (I haven't tested it yet, but the code for GTK looks like it does as well.)

Is this intentional? If so, it should probably be documented, and hopefully implemented on more backends for consistency. Either way, I can add a test to ensure that it either does or doesn't work, with xfails if needed.

[freakboy3742]

Is this intentional? If so, it should probably be documented, and hopefully implemented on more backends for consistency. Either way, I can add a test to ensure that it either does or doesn't work, with xfails if needed.

It definitely wasn't intentional, but I can imagine how it happened by accident. If we can make the same behavior work on other platforms, that would be great; but in the meantime, it definitely sounds like something that should be documented as a platform quirk, and validated in the testbed when it's available.

[HalfWhitt]

Is this intentional? If so, it should probably be documented, and hopefully implemented on more backends for consistency. Either way, I can add a test to ensure that it either does or doesn't work, with xfails if needed.

It definitely wasn't intentional, but I can imagine how it happened by accident. If we can make the same behavior work on other platforms, that would be great; but in the meantime, it definitely sounds like something that should be documented as a platform quirk, and validated in the testbed when it's available.

Should I restore it, then? This branch currently removes it. I'm also in the midst of setting up a Linux VM so I can test on GTK.

[freakboy3742]

Should I restore it, then? This branch currently removes it. I'm also in the midst of setting up a Linux VM so I can test on GTK.

I guess it depends how hard it is to accommodate. If it's a relatively simple re-org, then sure; but if it's going to be difficult to do while maintaining consistency between backends, then it's probably not worth it.

[HalfWhitt]

Done (I think). I'll see about making a testbed test for this, and... hopefully at some point getting Toga working on Linux so I can actually test there hands-on. I strongly suspect that backend has this feature too, but I wouldn't swear to it.

[HalfWhitt]

Turns out the GTK backend wasn't currently capable of loading an arbitrary font installed on the system, but it was so close to do doing so that enabling it was pretty trivial.

[HalfWhitt]

I'm honestly surprised, now, that Windows isn't failing testbed coverage the way GTK is. Currently GTK does:

# Check for a font installed on the system.
installed = PangoCairo.FontMap.get_default().get_family(
    self.interface.family
)
if installed is None:
    raise UnknownFontError(f"Unknown font '{self.interface}'")

And Windows does:

try:
    # Check for a font installed on the system.
    font_family = FontFamily(self.interface.family)
except ArgumentException:
    raise UnknownFontError(f"Unknown font '{self.interface}'")

Either Coverage is treating the try/except differently than the if for some reason, or something unexpected is going on in the Windows font-setting at some point.

[freakboy3742]

I know this is still a draft, but I'm intrigued on the direction you're heading here. I've noticed that the base logic of all the individual font backends is starting to converge on a common pattern... is the end goal here to factor out the common pattern into core, and then make the backend implement the specific logic? If not.. could that be an end goal?

[HalfWhitt]

I know this is still a draft, but I'm intrigued on the direction you're heading here. I've noticed that the base logic of all the individual font backends is starting to converge on a common pattern... is the end goal here to factor out the common pattern into core, and then make the backend implement the specific logic? If not.. could that be an end goal?

I've certainly considered it... There are still just enough platform-specific quirks in the logic flow that I'm not 100% sure what that will look like, but if I can wrangle it, I'd love to.

[HalfWhitt]

Okay, it turns out that Coverage ignores implicit else clauses of try blocks. If you just have try / except — without an explicit else — and you always trigger the exception, Coverage won't complain about missing a branch. I confirmed this by adding an else to the installed-font-finding bit in the Windows backend code, and sure enough, now the missed branch shows up.

I worry, now, that we could be missing all sorts of cases.

[HalfWhitt]

I think this is ready to look at... I've edited the initial post up top to reflect the current state; the main change is that the logic is now in core, and _impl is cached instead of native directly. I don't think that should cause any issues, but there may be corner cases I'm unaware of.

[freakboy3742]

A couple of really minor tweaks inline, but otherwise Thit looks really good - much easier to follow, and more consistent to boot.

[freakboy3742]

LGTM - thanks for the cleanup!