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

Migrate to cosmic-text #8808

Closed
wants to merge 32 commits into from

Conversation

tigregalis
Copy link
Contributor

@tigregalis tigregalis commented Jun 10, 2023

Objective

Fixes #7616

Solution

  • Migrate to cosmic_text, simplifying and enhancing Bevy's text rendering implementation
  • Provide APIs that take advantage of the newly available features

Changelog

Changed

TextStyle::font is no longer a Handle<Font>, but a FontRef. From<Handle<Font>> is implemented for FontRef, so in your existing code, you can call .into() on the Handle<Font> to convert it into a FontRef.

The definition of the font size has changed with the migration to cosmic text. The behavior is now consistent with other platforms (e.g. the web), where the font size in pixels measures the height of the font (the distance between the top of the highest ascender and the bottom of the lowest descender). Font sizes in your app need to be rescaled to approximately 1.2x smaller; for example, if you were using a font size of 60.0, you should now use a font size of 50.0.

Added

You can now query loaded fonts by family and style using FontQuery. From<FontQuery> is implemented for FontRef, so you can call .into() on the FontQuery to convert it into a FontRef.

You can now load system fonts using TextPipeline::load_system_fonts. These fonts can then be queried by a FontQuery.

Emojis and bidirectional text are now supported, provided fonts have been loaded that support them (for example, via TextPipeline::load_system_fonts).

Check out the new system_fonts example in the Bevy repo.

Migration Guide

The definition of the font size has changed with the migration to cosmic text. Font sizes in your app need to be rescaled to approximately 1.2x smaller; for example, if you were using a font size of 60.0, you should now use a font size of 50.0..

TextStyle::font is no longer a Handle<Font>, but a FontRef. In each TextStyle, call .into() on the Handle<Font> to convert it into a FontRef.


Notes

To do

This is a draft, and there's still quite a bit of work to do. I've (liberally) peppered the code with TODOs but here is a collection:

At a high level this is what was done and still (perhaps) needs doing:

Future Work

Preferably done now:

For future PRs, or should any of these be done in this PR?

Guidance needed

I've tried to "slip under the radar" as much as possible when it comes to the API, but in some cases user-facing API changes were unavoidable. Please consider and advise how best to design and expose these APIs.

Of particular note is the new FontQuery, and TextPipeline::load_system_fonts (which probably needs to run in a parallel async task).

We also need to consider re-exports.

As far as implementation goes:

  • I'm not so confident on the Arc/Mutex I've introduced and how best to handle errors there.
  • Loading fonts are currently "infallible": ab_glyph used to parse the font when it loaded it into memory as a FontVec, but fontdb (via cosmic_text) doesn't (parsing is a separate step, and we'd have to introduce ttf-parser), so currently font data is simply a Vec<u8>.
  • I've no idea how to handle Reflect.

I think that about covers it. Have at it.

@alice-i-cecile alice-i-cecile added C-Enhancement A new feature C-Dependencies A change to the crates that Bevy depends on A-UI Graphical user interfaces, styles, layouts, and widgets C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jun 10, 2023
@alice-i-cecile alice-i-cecile added this to the 0.12 milestone Jun 10, 2023
}

#[derive(Clone, Debug)]
pub struct FontQuery {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely needs doc strings. I also think we should really try and avoid the Query name if possible, since this doesn't seem to use bevy_ecs::Query, which is going to be a source of a lot of confusion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this more, I think that "system font" is probably a better way to describe this functionality?

Copy link
Contributor Author

@tigregalis tigregalis Jun 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SystemFont doesn't quite capture the idea as you can actually use a FontQuery to refer to a font you loaded as an asset (since it's always loaded into the Database), which I've confirmed works (but I probably need to test for edge cases). *Query better captures the idea, and it's the terminology used by fontdb, but FontSearch could maybe work.

@alice-i-cecile alice-i-cecile added D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Controversial There is active debate or serious implications around merging this PR labels Jun 10, 2023
@nicopap nicopap self-requested a review July 27, 2023 08:59
@nicoburns nicoburns mentioned this pull request Aug 13, 2023
@JMS55 JMS55 removed this from the 0.12 milestone Sep 28, 2023
@TotalKrill
Copy link
Contributor

was this abandoned, and in that case, how would anyone pick this up?

@alice-i-cecile alice-i-cecile added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Oct 19, 2023
@alice-i-cecile
Copy link
Member

Not fully abandoned, but like I said in this comment, the author has been busy and would appreciate some assistance.

If I was tackling this, I would personally start by creating a branch, and attempting to recreate these changes with the latest versions of both Bevy and cosmic-text. Given the level of merge conflicts, I think that doing so manually, rather than attempting to rebase these commits is likely going to be happier.

Once you have something to show (it doesn't need to be complete), open up a new PR and link this one in the description.

@TotalKrill TotalKrill mentioned this pull request Oct 19, 2023
@TotalKrill
Copy link
Contributor

Okay so basically, I took and did the quickest way I could figure out, and that was to just squash all the commits by @tigregalis and then merge that one large commit into a new branch, and then do a PR.

I probably messed something up, but at least I got the examples/ui/text.rs to run, and show similar text.

You can see that PR here: #10193

@alice-i-cecile
Copy link
Member

Adopted in #10193. Please follow (and chime in) there!

@nicoburns nicoburns mentioned this pull request Nov 23, 2023
1 task
github-merge-queue bot pushed a commit that referenced this pull request Apr 30, 2024
# Objective

- Enables support for `Display::Block`
- Enables support for `Overflow::Hidden`
- Allows for cleaner integration with text, image and other content
layout.
- Unblocks #8104
- Unlocks the possibility of Bevy creating a custom layout tree over
which Taffy operates.
- Enables #8808 / #10193 to remove a Mutex around the font system.

## Todo

- [x] ~Fix rendering of text/images to account for padding/border on
nodes (should size/position to content box rather than border box)~ In
order get this into a mergeable state this PR instead zeroes out
padding/border when syncing leaf node styles into Taffy to preserve the
existing behaviour. #6879 can
be fixed in a followup PR.

## Solution

- Update the version of Taffy
- Update code to work with the new version

Note: Taffy 0.4 has not yet been released. This PR is being created in
advance of the release to ensure that there are no blockers to upgrading
once the release occurs.

---

## Changelog

- Bevy now supports the `Display::Block` and `Overflow::Hidden` styles.
github-merge-queue bot pushed a commit that referenced this pull request Jul 4, 2024
# Replace ab_glyph with the more capable cosmic-text

Fixes #7616.

Cosmic-text is a more mature text-rendering library that handles scripts
and ligatures better than ab_glyph, it can also handle system fonts
which can be implemented in bevy in the future

Rebase of #8808

## Changelog

Replaces text renderer ab_glyph with cosmic-text

The definition of the font size has changed with the migration to cosmic
text. The behavior is now consistent with other platforms (e.g. the
web), where the font size in pixels measures the height of the font (the
distance between the top of the highest ascender and the bottom of the
lowest descender). Font sizes in your app need to be rescaled to
approximately 1.2x smaller; for example, if you were using a font size
of 60.0, you should now use a font size of 50.0.

## Migration guide

- `Text2dBounds` has been replaced with `TextBounds`, and it now accepts
`Option`s to the bounds, instead of using `f32::INFINITY` to inidicate
lack of bounds
- Textsizes should be changed, dividing the current size with 1.2 will
result in the same size as before.
- `TextSettings` struct is removed
- Feature `subpixel_alignment` has been removed since cosmic-text
already does this automatically
- TextBundles and things rendering texts requires the `CosmicBuffer`
Component on them as well

## Suggested followups:

- TextPipeline: reconstruct byte indices for keeping track of eventual
cursors in text input
- TextPipeline: (future work) split text entities into section entities
- TextPipeline: (future work) text editing
- Support line height as an option. Unitless `1.2` is the default used
in browsers (1.2x font size).
- Support System Fonts and font families
- Example showing of animated text styles. Eg. throbbing hyperlinks

---------

Co-authored-by: tigregalis <anak.harimau@gmail.com>
Co-authored-by: Nico Burns <nico@nicoburns.com>
Co-authored-by: sam edelsten <samedelsten1@gmail.com>
Co-authored-by: Dimchikkk <velo.app1@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
MrGVSV pushed a commit to MrGVSV/bevy that referenced this pull request Jul 5, 2024
# Replace ab_glyph with the more capable cosmic-text

Fixes bevyengine#7616.

Cosmic-text is a more mature text-rendering library that handles scripts
and ligatures better than ab_glyph, it can also handle system fonts
which can be implemented in bevy in the future

Rebase of bevyengine#8808

## Changelog

Replaces text renderer ab_glyph with cosmic-text

The definition of the font size has changed with the migration to cosmic
text. The behavior is now consistent with other platforms (e.g. the
web), where the font size in pixels measures the height of the font (the
distance between the top of the highest ascender and the bottom of the
lowest descender). Font sizes in your app need to be rescaled to
approximately 1.2x smaller; for example, if you were using a font size
of 60.0, you should now use a font size of 50.0.

## Migration guide

- `Text2dBounds` has been replaced with `TextBounds`, and it now accepts
`Option`s to the bounds, instead of using `f32::INFINITY` to inidicate
lack of bounds
- Textsizes should be changed, dividing the current size with 1.2 will
result in the same size as before.
- `TextSettings` struct is removed
- Feature `subpixel_alignment` has been removed since cosmic-text
already does this automatically
- TextBundles and things rendering texts requires the `CosmicBuffer`
Component on them as well

## Suggested followups:

- TextPipeline: reconstruct byte indices for keeping track of eventual
cursors in text input
- TextPipeline: (future work) split text entities into section entities
- TextPipeline: (future work) text editing
- Support line height as an option. Unitless `1.2` is the default used
in browsers (1.2x font size).
- Support System Fonts and font families
- Example showing of animated text styles. Eg. throbbing hyperlinks

---------

Co-authored-by: tigregalis <anak.harimau@gmail.com>
Co-authored-by: Nico Burns <nico@nicoburns.com>
Co-authored-by: sam edelsten <samedelsten1@gmail.com>
Co-authored-by: Dimchikkk <velo.app1@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Dependencies A change to the crates that Bevy depends on C-Enhancement A new feature D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to cosmic-text
5 participants