[widget audit] toga.Font

[studioph]

Changes

• Adds dedicated tests in testbed for font code not exercised by existing tests:
  • Fallback to system font
  • Loading custom font files
  • Error handling of corrupted/missing files and libraries
  • Tests all exposed font properties (weight, style, variant, etc)

Notes

• Refs Move GTK font measuring to Canvas widget #1902 and Add font loading for GTK backend #1846
• Based on the changes from Add font loading for GTK backend #1846 and thus may need rebasing or merge of 1846 first
• Checking stdout is used for system font fallback as some backends (GTK) do not provide a reliable way to check the fallback from the widget's font
• gtk/fonts has been increased to 100% coverage

Refs #1837.

Audit checklist

• Core tests ported to pytest
• Core tests at 100% coverage
• Core docstrings complete, and in Sphinx format
• Documentation complete and consistently formatted
• cocoa backend at 100% coverage
• winforms backend at 100% coverage
• gtk backend at 100% coverage
• iOS backend at 100% coverage
• android backend at 100% coverage
• Widget support table updated
• Widget Issue backlog reviewed

[studioph]

Right now gtk/paths is using CWD which results in the wrong path when running testbed testbed/tests instead of testbed/app, so it won't find the font files in the resources folder. I will make a separate Issue/PR for that that can be merged first

[mhsmith]

Since this PR has turned into a complete audit of Font, I've renamed it and added the audit checklist to the top comment.

[mhsmith]

This had no effect because the gravity we actually need to change is that of the TextView within the R__layout.simple_spinner_item. This can be fixed later, along with color and font (#1758).

[mhsmith]

This PR doesn't fix #1038, so I've removed it from the top comment, and I don't think it's worth looking into just now.

[mhsmith]

A while back, we modified Box to add children after creating the box impl, because this removed the need for a branch in add() checking that the parent impl exists. However, for reasons I cannot work out, this breaks the GTK implementation - the font example doesn't correctly apply custom fonts at all if children are added after the impl is created.

Reverting the child creation order fixes the problem, at the cost of a single if check, so 🤷 ...

Could this problem still happen if additional children were added to the Box after it was created?

[freakboy3742]

Any reason to call it out here, rather than in the Notes section of the docs?

[mhsmith]

Where a platform limitation affects only a specific method, I think it's more useful to put the note in that method's documentation so the user will see it at the right time. There's some precedent for this in on the WebView page.

This is especially the case now that Font.register is linked from the font_family style property. Anyone following that link is unlikely to see the Notes section before they try to use the method.

In fact, for the same reason I've now moved the notes about obique and small caps under their respective style properties, which are linked from the arguments of the Font APIs.

[freakboy3742]

Fair enough about the navigation/discovery order; however, my inclination would be to include it in both places. What you've described is definitely one mode of API discovery (and an important one), but another is someone evaluating whether this widget will do what they want to do, and needing to know up front what limitations exist.

[freakboy3742]

It occurred to me that the underlying problem here is that the bulk of the documentation for Font isn't in the usage guide for Font - in fact, there is no usage guide for font at present. I've pushed an update that reorganises this documentation.

[freakboy3742]

A while back, we modified Box to add children after creating the box impl, because this removed the need for a branch in add() checking that the parent impl exists. However, for reasons I cannot work out, this breaks the GTK implementation - the font example doesn't correctly apply custom fonts at all if children are added after the impl is created.
Reverting the child creation order fixes the problem, at the cost of a single if check, so 🤷 ...

Could this problem still happen if additional children were added to the Box after it was created?

To the best of my knowledge, no; I've just added an explicit demo of this to the fonts example.

The poking around GTK that I've done since I worked on this PR has provided a little more light on what is going on here. GTK font rendering is based on creating a Pango context; the Pango context is tied to a specific display. If you add the widget after creating the box, it gets an "empty" Pango context because it isn't associated with a Window at the time it is created. If you add it before, the assignment of a Pango context to the box transfers to the child label.

This is evidently only an issue with initial window construction; once the window is visible, a new widget added to the layout will get a Pango context that knows about the custom font. I presume this has something to do with the fact that there's now an actual display that is in some way bound to the application, so the default Pango context is the right one.

[mhsmith]

If you add the widget after creating the box, it gets an "empty" Pango context because it isn't associated with a Window at the time it is created. If you add it before, the assignment of a Pango context to the box transfers to the child label.

I don't follow how avoiding the call to add_child makes any difference to this. The only thing add_child does is call child.container = self.container, but if the box isn't associated with a Window, then self.container is None, so the call should have no effect.

From GTK's point of view, everything is a direct descendant of the container anyway.

This is evidently only an issue with initial window construction; once the window is visible, a new widget added to the layout will get a Pango context that knows about the custom font.

What about widgets added after the Box is created, but before it's added to a window:

box = toga.Box()
box.add(...)
self.main_window.content = box

[mhsmith]

I'll leave macOS and iOS marked as beta in the widget support table, because they don't yet support loading custom fonts.

[freakboy3742]

If you add the widget after creating the box, it gets an "empty" Pango context because it isn't associated with a Window at the time it is created. If you add it before, the assignment of a Pango context to the box transfers to the child label.

I don't follow how avoiding the call to add_child makes any difference to this. The only thing add_child does is call child.container = self.container, but if the box isn't associated with a Window, then self.container is None, so the call should have no effect.

From GTK's point of view, everything is a direct descendant of the container anyway.

Ok - I swear I'm being gaslit here. At the point where I originally commented about needing to alter the widget creation order, I was definitely seeing issues with font rendering that would only be resolved by creating children before the parent. However, I just undid the creation order change to prove that the change was needed... and... it's not.

My best guess is that something in #2020 has subtly changed exactly which redraw calls are being made, and as a result, we can now create the children after the parent. Since that we can remove the branch check of the impl when adding children, I've reverted the change permanently.

What about widgets added after the Box is created, but before it's added to a window:

box = toga.Box()
box.add(...)
self.main_window.content = box

It's a moot point now; but I verified this as well - I tried adding a widget before the box is assigned as window content; and I tried adding a widget after the box is assigned as window content, but before the window is created. Both work fine.

[freakboy3742]

Ok - I'm happy with where this is now. I've pushed some updates addressing my review comments; if you're happy with those changes, feel free to merge.

[freakboy3742]

Based on the StackOverflow discussion, I'm utterly petrified about what will happen to this as Android releases new versions... but until then, it's a nice hack. (well, it's an utterly ugly hack, but it's a nice utterly ugly hack :-) )

[freakboy3742]

Fair enough about the navigation/discovery order; however, my inclination would be to include it in both places. What you've described is definitely one mode of API discovery (and an important one), but another is someone evaluating whether this widget will do what they want to do, and needing to know up front what limitations exist.

[freakboy3742]

It occurred to me that the underlying problem here is that the bulk of the documentation for Font isn't in the usage guide for Font - in fact, there is no usage guide for font at present. I've pushed an update that reorganises this documentation.