-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Utilizing a Cow
like type for RichText / LayoutJob
#1098
Comments
I did a similar experiment in #698. The main difference is that I opted for It did improve performance by quite a bit, but ergonomics became worse. Adding lifetime parameters to everything is not very appealing to me. Especially it is nice to be able to store a There are some other alternatives to consider, such as using string-interning with ref-counting (e.g. using https://docs.rs/refcount-interner/latest/refcount_interner/). That would avoid lifetimes, but at the cost of an extra hashing. |
String interning would be nice, I was also thinking utilizing types like I also agree that ergonomics would be hurt with lifetimes, my main problem with it is new users of Rust may find it complex. But glad to see that it was already considered. |
It'd be nice if there was a convenient way to opt in to something like The only alternative right now is to pause any playing audio while the UI is visible or let the audio crackle from time to time. |
I think there's a strong argument for at least using Side note: arbitrary string typesMy use case may, in the future, include those strings dynamically changing as well (additions/deletions at arbitrary locations). In that case I would ideally use something like https://crates.io/crates/ropey, however that wouldn't work here without using something like Another approach is to potentially have a "string manager" that is used across all layout calls, and that manager can yield identifiers which are stored in |
I've just quickly implemented a string manager scheme in
I don't have the repo forked to push my changes up, but if this API is of interest I will definitely do so. |
This is a great idea, similar to managing textures, given the sheer number of small allocations large UIs can require for strings. Hopefully the design could be generic enough to allow using a per frame bump arena allocator - as strings can change a lot more frequently than textures and a bulk allocation per frame is preferable to thousands of individual ones strewn throughout. I'd love to see that POC! |
It's a fairly straightforward change! master...afranchuk:egui:string-manager |
Indeed it is! Looks great and seems like a design could be compatible with a typed arena for strings. Why the char iterator design instead of a method to return &str? It precludes using byte range which I assume is important for some features like horizontal scrolling. Edit: oh because of the ropey use case? How would that work with scrolling byte offsets? |
Yeah the main reason was the ropey (or other string type) use case, though it's also the minimum interface necessary to abstract the call site (i.e., the layout code loops over |
Layout code should iterate over a higher layer of abstraction than The trait it provides is probably not the right interface for being generic over strings, though. |
I'm open to suggestions, I was just making the change that abstracts the existing code. But it seems like that concern might be (at this time) unrelated to this change, as the layout code doesn't seem to care about the |
That is correct at present. I don't want to derail this thread, but that has been a concern for quite some time. #2532 has details. Providing an interface for |
Currently, a
RichText
(and alsoLayoutJob
) holds aString
to store the data. I propose we change these to take a new enum type, withSingle(&str)
,Multiple(&[&str])
and anOwned(String)
variants. The lifetimes should essentially be generic; so users can pass any string they want, without creating a new string. This would get rid of creating a new string 60 times per second for all labels / buttons etc. which could add up a lot if there is a lot of different widgets that have text.RichText
would essentially use theSingle
variant. TheMultiple
variant is for the macro mentioned inLayoutJob
docs, it should allow the user to style text without needing to create a new string.The downside is that this requires putting lifetimes everywhere, which can be a PITA to look at / work with sometimes. The lifetimes wouldn't be visible to a user most of the time, since widgets are created and used immediately and not stored. To make sure this is worth the effort, this should be implemented first and then benchmarked in different scenarios. I may try to implement this later, but I'm putting this out there so that I don't forget it.
The text was updated successfully, but these errors were encountered: