Add systemFontFamily to flutter/settings channel

[robert-ancell]

Description

This allows a shell to set the system font to use by default.

Related Issues

flutter/flutter#51831

Tests

No tests added.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the contributor guide and followed the process outlined there for submitting PRs.
• I signed the CLA.
• I read and followed the C++, Objective-C, Java style guides for the engine.
• I read the tree hygiene wiki page, which explains my responsibilities.
• I updated/added relevant documentation.
• All existing and new tests are passing.
• I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

• I have submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

• No, no existing tests failed, so this is not a breaking change.
• Yes, this is a breaking change.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[robert-ancell]

With this branch and the associated Linux shell changes in https://github.com/robert-ancell/engine/tree/linux-shell-system-font (which builds on #22486) I can get the GTK configured font to be used by adding the following to my test app:

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        fontFamily: WidgetsBinding.instance.window.systemFontFamily,
        primarySwatch: Colors.blue,
      ),
      home: MyHomePage(title: 'Font Test'),
    );
  }
}

I need some feedback on the following parts:

• WidgetsBinding.instance.window.systemFontFamily seems like the correct way to get the configured font family in Dart, but it feels convoluted. Should this be exposed anywhere else?
• The original theme was defined in Flutter in packages/flutter/lib/src/material/typography.dart - all the existing themes have constant font families set there, should we be dynamically generating a theme with systemFontFamily? This would mean the above app wouldn't need to explicitly set fontFamily in ThemeData.

[robert-ancell]

CC @gspencergoog and @stuartmorgan.

[gspencergoog]

@robert-ancell I think for now we will be keeping it as using systemFontFamily and adding the Theme widget at the top of the tree: it's somewhat unpredictable how the sizing on an arbitrary font might work out, and can cause drastic changes in the layout, so it seems like it could be better to be more explicit to set the font family to the system default. I feel a little sad saying that, though, so maybe we can add something in the future to make it easier to do, or more predictable layout-wise.

[HansMuller]

I agree with #22981 (comment). I think the support added for using a platform-specific font for Windows and Linux, flutter/flutter#51519, plus the existing support for Android and iOS/MacOS, is enough to comply with the Material Design spec. Wiring the platform's "system" font down in that way produces a relatively predictable text look/geometry.

Just providing simple access to the system font, as this PR does, does seem useful. Developers can choose to use the system font, if they're OK with the unpredictability.

[stuartmorgan-g]

I think the support added for using a platform-specific font for Windows and Linux, [...] is enough to comply with the Material Design spec.

How so? The spec says "For platforms outside of Android and web products, use a system typeface that is preferred on that platform." Currently on Linux we're using one of a set of fonts that cover the default fonts on a subset of Linux distributions, which seems hard to argue is the same thing.

Two simple examples:

• If a distro uses its own font (as Ubuntu does) and it's not on our hard-coded list, well use an essentially random font, not that platform's preferred typeface.
• If someone is using a non-Ubuntu distribution, but happens to have the Ubuntu font installed, we will use it (because we had to pick an order to try them in), not that platform's preferred typeface.

Unless we are considering every form and distribution of Linux that isn't Android to be a single monolithic platform? The fact that we're already having to carve out an exception for Android in that demonstrates why it's a problematic framing.

[gspencergoog]

What's the status of this PR: has this been resolved in another way?
(@HansMuller do you have a response to @stuartmorgan's comment?)

[HansMuller]

My apologies for letting this sit for so long. I have no problem with the PR's direction: it enables developers to discover the platform's default system font.

[cbracken]

@robert-ancell is this still WIP?

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[robert-ancell]

@robert-ancell is this still WIP?

If this is the right location for this API then this should be ready for review/landing.

[gspencergoog]

Sorry this languished for so long! It just fell off my radar.

[chinmaygarde]

Can we address the conflict so this may land?

[robert-ancell]

The build is failing due to a license break on main from a skia change, will have to merge once that is fixed

[chinmaygarde]

That has been fixed now. Can you rebase please?