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

Uncouple glyph storage from text processing #16901

Open
djeedai opened this issue Dec 19, 2024 · 2 comments
Open

Uncouple glyph storage from text processing #16901

djeedai opened this issue Dec 19, 2024 · 2 comments
Labels
A-Text Rendering and layout for characters C-Feature A new feature, making something new possible S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@djeedai
Copy link
Contributor

djeedai commented Dec 19, 2024

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

I have a crate doing text rendering in a different way than Bevy does. Originally, I was reusing the Bevy text pipeline, it was really great. Later some changes forced me to make a copy of the entire TextPipeline and related classes, which added a lot of maintenance overhead, just because the way Bevy stored glyphs (one texture per font per size) was in my opinion quite wasteful, and I wanted a different glyph storage. Now that Bevy switched from ab to cosmic, I find myself once again copy-pasting and very lightly modifying the TextPipeline and other classes, just because of that storage decision. This is a lot of wasted time, while there's no real good reason other than lack of previous need why the glyph storage solution is intertwined with the text processing of Cosmic.

What solution would you like?

Add an interface to decouple where/how glyphs as stored in textures, from the rest of the text processing (collecting glyphs, rasterizing them, layouting multi-line text, etc.).

What alternative(s) have you considered?

What I'm doing currently, which is copy/pasting bevy_text and doing some light modifications. This however invariably lead to subtle issues, which requires days of debugging to figure out why glyphs are not properly aligned etc.. This works entirely duplicate what bevy_text authors already did.

Additional context

I believe we should also really reconsider that pattern of creating one texture atlas per font per size. It's unlikely any app has more than a few fonts in use, and possibly some of them are used for a few words only (title, etc.) which means we're wasting large atlas textures and force (in the absence of bindless) separate draw calls for no good reason, as there's no technical limitation I know of to packing different font families and sizes into the same atlas.

@djeedai djeedai added A-Text Rendering and layout for characters C-Feature A new feature, making something new possible S-Needs-Design This issue requires design work to think about how it would best be accomplished labels Dec 19, 2024
@ickshonpe
Copy link
Contributor

ickshonpe commented Dec 19, 2024

Yeah I've wanted support for multiple font instances per texture forever, there are always critical bugs etc to fix though so I've never made a pass at it.

@rparrett
Copy link
Contributor

rparrett commented Dec 22, 2024

I know there is some interest in alternative text rendering, including not using our rasterizer at all. It probably would be nice for folks to be able to continue using our text layout stuff and their own renderer without resorting to copy/pasting everything text related in Bevy.

I am not sure if abstracting away glyph raster storage specifically is the best thing to do here. But if it can be done without introducing a bunch of complexity or compromising performance, then I'm not opposed or anything.

I believe we should also really reconsider that pattern of creating one texture atlas per font per size.
...
we're wasting large atlas textures

I've been playing around with related improvements to try to form an opinion about this.

At the very least, we need to use separate textures for TextFonts with font_smoothing = false so the underlying texture can have ImageSampler::nearest().

The only reason I can think of for doing separate textures for each font size is to help with sizing the textures to minimize memory usage, but if our atlas textures could grow, this would no longer be a factor.

(side note) Atlas textures seem to currently be smallest power of two greater than max glyph dimension of the first processed glyph or 512 if that is bigger which feels pretty janky. That should probably just be based on font size, and possibly made configurable by the user.

I tried implementing font-atlas-growing and it seems like there probably aren't any major blockers to getting that going. The atlas allocator already supports growing the atlas, but Image can't currently be grown in a way that doesn't corrupt the texture. That wasn't too difficult to add though. rparrett@d9d0022

If atlases can grow, then a minimum of 512x512 seems really excessive as a starting point -- that's a huge number of glyphs (probably > 1000?) at the default font size.

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 S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

No branches or pull requests

3 participants