Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds basic support for working with system fonts #16365

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

bushrat011899
Copy link
Contributor

@bushrat011899 bushrat011899 commented Nov 13, 2024

Objective

Solution

  • Created a new feature, system_font, which will instruct cosmic_text to begin loading system fonts on startup.
  • Added a new system parameter, FontLibrary, which allows for convenient searching of all loaded fonts, returning a Handle<Font> if you have found a suitable candidate.
  • Added a new example, system_fonts, which demonstrates searching for and using system fonts.

Testing

  • CI
  • system_fonts example

Showcase

Users now have access to the FontLibrary system parameter, which allows searching for available fonts. This helps with using the new system_fonts feature, which will load system fonts at runtime.

In the below snippets, fonts is of type FontLibrary

// Find a monospaced font
let font: Option<Handle<Font>> = fonts.find(|font| font.monospaced);

// Find a font with a particular name
let font: Option<Handle<Font>> = fonts.find(|font| {
    let family_name = &font.families[0].0;
    family_name.contains("Courier")
});

system_font_demo

Notes

  • Loading system fonts increases startup time by a few seconds. I suspect this is platform dependent, and is largely out of our control. The startup cost is now negligible (I cannot measure the difference, much less than a second), since moving system font loading onto a background task. I do believe it's worth keeping this behind a feature gate as it's already organised this way, and there may be a performance cost on some platforms. More options for the user is better IMO.
  • If the system_fonts feature is enabled on a platform cosmic_text does not support, it is simply a no-op, making the feature safe to enable in all cases.
  • The FontLibrary system param could use some more methods, such as an iterator. I have deliberately only exposed a find method since we need to do some work with the Assets<Font> to ensure it's available for the TextPipeline.
  • The Font type assumes every font will have binary data associated with it (the data field). For system fonts this is not the case. I'm simply inserting no data (empty slice) for these fonts. It might be prudent to update Font to be an enum instead to make this more clear.
  • Font families are still handled the same way Bevy currently does, only selecting index 0. Changing this is out of scope for this PR.

@bushrat011899 bushrat011899 added this to the 0.16 milestone Nov 13, 2024
@bushrat011899 bushrat011899 added C-Feature A new feature, making something new possible A-Text Rendering and layout for characters X-Contentious There are nontrivial implications that should be thought through D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Needs-SME Decision or review from an SME is required labels Nov 13, 2024
@TurtIeSocks
Copy link
Contributor

Awesome! I swapped to this branch locally with my playground example and every locale now displays without the need to load any external font files 🎊

I suppose this does increase the need for users to have the ability to control cosmic_text fallbacks. I should have time later in the week to dig into that and see what's viable.

@Bcompartment
Copy link

If it's enabled by default, add an example that shows how to disable this feature. Reasoning is simple: if an app is shipped with a font, why waste time loading all system fonts? IMO add something like: 
fn setup(....) {    ....    let system_fonts: SystemFonts = asset_loader.load_sys_fonts(); .... }

So user will have simple way, and control when to load.

@bushrat011899
Copy link
Contributor Author

If it's enabled by default...

I've left it is a feature that's disabled by default. To enable system fonts, you enable the system_font feature in Bevy. This feature does have a startup time cost and I completely agree that the functionality should be opt-in, at least while it has a noticeable impact.

@TurtIeSocks
Copy link
Contributor

I've just noticed #14150, was that effort abandoned?

@bushrat011899
Copy link
Contributor Author

I never saw that! Looks like it wasn't linked to the original issue. I'm happy to incorporate changes from that PR, or close this out in favour of that one, I'll leave that up to the reviewers.

@rparrett
Copy link
Contributor

rparrett commented Feb 26, 2025

Most likely not a Bevy issue, but the example hangs on my macbook when it eventually tries to load Noto Nastaliq Urdu after spending a while mashing the space bar.

edit: I think this reproduces in bare cosmic, but was fixed in an unreleased commit. I'll make an issue to beg for a release over there.

Also added a mapping between `AssetId<Font>` and `fontdb::ID` to avoid creating new assets and handles when it's not required.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Text Rendering and layout for characters C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Needs-SME Decision or review from an SME is required X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support using system fonts
6 participants