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 #7616

Closed
alice-i-cecile opened this issue Feb 11, 2023 · 17 comments · Fixed by #10193
Closed

Migrate to cosmic-text #7616

alice-i-cecile opened this issue Feb 11, 2023 · 17 comments · Fixed by #10193
Labels
A-Rendering Drawing game state to the screen A-Text Rendering and layout for characters A-UI Graphical user interfaces, styles, layouts, and widgets C-Dependencies A change to the crates that Bevy depends on S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

Our font rendering is subpar. See #2404 for an example of the rendering problems and #1325 / #1017 for some missing features.

What solution would you like?

Migrate to the well-made, well-supported [cosmic-text] crate that is quickly becoming the ecosystem standard.

What alternative(s) have you considered?

We could try and fix all of these issues in the assorted font crates we depend on.

Additional context

Iced's migration should be informative: iced-rs/iced#1697

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on A-UI Graphical user interfaces, styles, layouts, and widgets labels Feb 11, 2023
@alice-i-cecile alice-i-cecile added this to the 0.11 milestone Feb 11, 2023
@tigregalis
Copy link
Contributor

tigregalis commented Feb 17, 2023

Some things to consider / possible limitations, when using cosmic_text's existing high level APIs:

It is not clear how to mix spans of different font sizes, see pop-os/cosmic-text#64

You can't currently layout text like this

image

<div>
  <span>Some text is quite </span>
  <span class="big">big</span>
  <span> but some text is quite </span>
  <span class="small">small</span>
  <span>, and sometimes text has </span>
  <span class="wide-lines">wide lines, </span>
  <span>but other times text has </span>
  <span class="thin-lines">thin lines.</span>
</div>
<style>
div {
  width: 140px;
  font-size: 16pt;
  line-height: 1.5;
}

.big {
  font-size: 32pt;
}

.small {
  font-size: 8pt;
}

.wide-lines {
  line-height: 4;
}

.thin-lines {
  line-height: 1;
}
</style>

It is not clear how you supply your own font (i.e. as bytes, needed to load a font as an asset), see comment https://github.com/iced-rs/iced/pull/1610/files#r1054666431 - Iced seems to tackle this elsewhere as part of its implementation

Perhaps we could reimplement cosmic-text's layout and font-loading functionality using some of its lower level APIs but that doesn't feel great.

@grovesNL
Copy link

It is not clear how you supply your own font (i.e. as bytes, needed to load a font as an asset)

You can use load_font_data to load a font from bytes

@tigregalis
Copy link
Contributor

tigregalis commented Feb 23, 2023

It is not clear how you supply your own font (i.e. as bytes, needed to load a font as an asset)

You can use load_font_data to load a font from bytes

The challenge I had with it was that...

cosmic_text starts with a FontSystem and borrows the FontSystem for all of its high level APIs:

// A FontSystem provides access to detected system fonts, create one per application
let font_system = FontSystem::new();

// A SwashCache stores rasterized glyphs, create one per application
let mut swash_cache = SwashCache::new(&font_system);

// Text metrics indicate the font size and line height of a buffer
let metrics = Metrics::new(14, 20);

// A Buffer provides shaping and layout for a UTF-8 string, create one per text widget
let mut buffer = Buffer::new(&font_system, metrics);

FontSystem holds the fontdb::Database privately.

To add fonts as the application is running (such as when the asset server loads font files) we would need to access the inner Database somehow.

FontSystem only exposes a method to access the Database immutably via db; there is no mutable method. Actually this makes sense given there are multiple borrowers, starting with SwashCache.

Note I am assuming here that FontSystem and SwashCache are long-lived and owned by the ECS.

As a really hacky workaround:

  • We could perhaps clone the Database.
  • We could then replace the existing FontSystem resource (presuming it is a resource) with a new FontSystem constructed using new_with_locale_and_db.
    • Using that method though we would also have to duplicate the locale checking logic which you don't get out of the box, as you would with new.
  • We would also need to replace the SwashCache resource (presuming it is a resource) as well, as it holds a reference to the original FontSystem, but it doesn't seem ideal.
    • Actually can replacing the SwashCache even be done?
    • If it can, can it be done cheaply?
    • Does SwashCache holding a reference to FontSystem play nicely with Bevy's ECS that wants to own and control all of its data?
    • If that's not workable, would we have to effectively reimplement SwashCache?

The easiest solution would probably be to fork cosmic_text and expose the APIs we need exposed, and contribute back upstream if and when they are open to the changes. Otherwise I don't know how you support Bevy's existing text functionality.

@nicoburns
Copy link
Contributor

The easiest solution would probably be to fork cosmic_text and expose the APIs we need exposed, and contribute back upstream if and when they are open to the changes.

FWIW, cosmic-text seems very open to changes in general. They have already accepted PRs from both the Iced and Vizia developers. Discussion of cosmic-text development happens partly in it's github issues, but also partly in this thread in Iced's discord https://discord.com/channels/628993209984614400/1027584473631555604

@hecrj
Copy link

hecrj commented Feb 26, 2023

@tigregalis The lifetime dependencies will be most likely gone soon (see pop-os/cosmic-text#39), but in the meantime you can use ouroboros to safely deal with self-referential types.

For font loading, you can leverage FontSystem::into_locale_and_db to mutate the Database and then create a new FontSystem with new_with_locale_and_db. Here's how iced does it!

@tigregalis
Copy link
Contributor

@tigregalis The lifetime dependencies will be most likely gone soon (see pop-os/cosmic-text#39),

Great news. That looks exactly like the design I was envisioning to resolve this. Glad to learn someone's already working on it.

but in the meantime you can use ouroboros to safely deal with self-referential types.

In this case, I guess it means we store the SwashCache and FontSystem together under a single resource, at least until then?

For font loading, you can leverage FontSystem::into_locale_and_db to mutate the Database and then create a new FontSystem with new_with_locale_and_db. Here's how iced does it!

Ah, that's a smart workaround. And once pop-os/cosmic-text#39 is merged, with nothing long-borrowing the FontSystem, it should be possible to expose a method to mutate the database directly right?

@jackpot51
Copy link

pop-os/cosmic-text#97 has been merged, this should now be possible.

@tpotancok
Copy link

FontSystem::db_mut is now released.

@Zaplime
Copy link

Zaplime commented Jun 8, 2023

Is this migration for 0.11 or should we expect it for another update?

@VitalyAnkh
Copy link
Contributor

I'm working on this, but the progress is slow. Since 0.11 will be released one or two weeks later, it's hopefully done in 0.12.

@argxentakato
Copy link

It is very disappointing that we are unlikely to meet the 0.12 milestone. Do you have anything I can contribute?

@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Oct 18, 2023

my PR is blocked on my own work on multi-size text upstream in cosmic-text (which is blocked on IRL commitments)
More than happy for you to adopt the PR. There's been a lot of changes upstream in cosmic-text which will simplify this PR too, so that could be a good start.

From the OP on Discord. pop-os/cosmic-text#150 appears to be the linked PR.

Adopting either #8808 or that PR would be incredibly useful: I'm sure the OP will be around to give feedback and review.

@viridia
Copy link
Contributor

viridia commented Jan 6, 2024

@alice-i-cecile Apparently there is a new contender for text layout: https://github.com/dfrg/parley/blob/master/doc/concept.md

@alice-i-cecile
Copy link
Member Author

Neat! IMO for this sort of dependency we should prioritize stability and longevity though, and cosmic-text has a funded open source team with a long history behind it.

@viridia
Copy link
Contributor

viridia commented Jan 6, 2024

Agreed. The reason this popped up on my radar is that I've been chatting with Raph Levien, and apparently the Xilem team is going to be working on this.

@alice-i-cecile
Copy link
Member Author

Ah, that's also a sensible team to rely on. Probably worth evaluating on the merits then.

@alice-i-cecile alice-i-cecile modified the milestones: 0.13, 0.14 Jan 24, 2024
@JMS55 JMS55 removed this from the 0.14 milestone Apr 11, 2024
@nicoburns nicoburns added the A-Text Rendering and layout for characters label May 16, 2024
@alice-i-cecile
Copy link
Member Author

alice-i-cecile commented Jun 6, 2024

We've opened a new working group to handle this migration, and decide between cosmic-text and parley.

Draft design doc: https://hackmd.io/@alice-i-cecile/rJhG8KR40/edit

Open membership working group (join us!): https://discord.com/channels/691052431525675048/1248074018612051978

@alice-i-cecile alice-i-cecile added the S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged label Jun 6, 2024
github-merge-queue bot pushed a commit that referenced this issue 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 issue 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-Rendering Drawing game state to the screen A-Text Rendering and layout for characters A-UI Graphical user interfaces, styles, layouts, and widgets C-Dependencies A change to the crates that Bevy depends on S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.