Dimchikkk / bevy_cosmic_edit

Apache License 2.0
76 stars 9 forks source link

Cosmic text 0.11 #124

Closed bytemunch closed 6 months ago

bytemunch commented 6 months ago

Updates cosmic-text to 0.11, rips out inbuilt functionality in favour of a more modular design.

Todo before merging:

Embrace regression, create modularity - [ ] Reimplement placeholders - [ ] Reimplement password fields - [ ] Reimplement undo/redo - [ ] Check for other regressions

bytemunch commented 6 months ago

Example basic_ui is where I'm testing these changes, the widget works for editing when defocused (ESC) and then focused again. Still doesn't modify the underlying buffer, borrow checker skill issues lol

Dimchikkk commented 6 months ago

Let me try to test basic_ui today ;)

Dimchikkk commented 6 months ago

It's white screen for me :) but I see some widget text in console

Dimchikkk commented 6 months ago

editable banana, nice :D

Dimchikkk commented 6 months ago

@bytemunch we need to have some plan how bring this branch into main... what the scope of tasks we have to deliver to make it work, any ideas? :)

bytemunch commented 6 months ago

Updated the PR message with a checklist (from the top of my head, I'm sure there'll be more to get done but so far unknown unknowns)

Hopefully most of the work can be easily reimported from the main branch, but with so many code changes already compatibility is uncertain.

Feel free to edit the checklist with anything you can think of that I have missed, lmk if this scope sounds appropriate

Dimchikkk commented 6 months ago

What comes to mind...

would be cool to have core module that supports only essential things: like input handling and drawing text and then move everything else (like password, placeholder, auto-height, undo-redo, etc) into separate modules (bevy plugins maybe?). Undo-redo wasn't implemented perfectly for example... there is https://github.com/pop-os/cosmic_undo_2 that we could take inspiration from... if we split undo-redo functionality to another plugin it would be easier to refactor/improve it in this case.

another thing is ab_glyph that used for text rendering in bevy can give position of "text curves" to draw text in 3d... would be cool to know whether cosmic-text can give the same thing....

another thing is trying to marry vello + cosmic-text + bevy to use GPU for text rendering.. if it's not feasible we could think about how we can speed text drawing up... maybe to use rayon to utilise all CPUs for pixel calculation.

bytemunch commented 6 months ago

Yeah that sounds like a more maintainable way forward, though I do find the bevy plugin dependency issue pretty difficult to work with. Maybe for now we could use internal plugins in the same crate (lose for binary size but makes plugin interop a non issue), and once https://github.com/bevyengine/bevy/issues/69 is fixed split modules out to their own crates?

Binary size issue can be addressed with feature flags for toggling internal plugins on and off.

Dropping scope to a core module is a great move though, unix philosophy and all that.

Dimchikkk commented 6 months ago

Yep, internal plugins sounds good.

Dimchikkk commented 6 months ago

so close no matter how far :)

bytemunch commented 6 months ago

Aah i have duplicate commits from working offline :laughing: Just pushed cos I couldn't figure out how to marry them >.<

Dimchikkk commented 6 months ago

@bytemunch could you elaborate on "Fix setting a FocusedWidget"?

bytemunch commented 6 months ago

@bytemunch could you elaborate on "Fix setting a FocusedWidget"?

Yeah I'm trying to sort it now, it's a systems ordering issue that means that sometimes a widget that should have text in it that is focused at startup appears blank. Currently fixed in all examples by never starting with a focused widget. I don't think it's too important to start up with focus, but I don't know if the issue would persist in a scene change or something similar where auto-focus is desirable.

Now I've written that I realise it's because I'm setting the focus in Update while layout runs in PostUpdate, but explicitly ordering the relevant systems is still giving me buggy behaviour :(

bytemunch commented 6 months ago

Got it :) was letting set_widget_size and set_buffer_size run unordered, when they need to be in order

Dimchikkk commented 6 months ago

Makes sense.... good job 👍

Just throwing some info at you :D

I made a small check yesterday: it's indeed possible to get outline curves using cosmic-text for the given font... and I know that it's possible to create meshes for each glyph using lyon... I am brainstorming whether it makes sense to adopt this approach in the future... instead of generating pixels using cosmic-text draw calls and replacing bevy asset... do something like this: https://github.com/tigregalis/bevy_text3d but using cosmic-text for obtaining outline commands to draw text.

bytemunch commented 6 months ago

Definitely something to look into, could be better for both perf and rendering.

I'll throw some ideas back lol.

If we can somehow make the rendering itself an (internal) plugin then creating a mesh renderer would be simpler. Figuring out how the upcoming internal plugin architecture works should show if and how such a system can work. I'll try implementing placeholders and password fields as plugins after this PR meets goals, that should set the groundwork for a pluggable renderer. Rambling a bit but I guess I'm saying decoupling rendering and data allows for everything above and more. Plus, a data-only core module could just slot straight into bevy upstream, far as I understand the inner bevy components are essentially plugins themselves.

Dimchikkk commented 6 months ago

Yeah, abstracting renderer sounds cool 👍

Then, bevy_cosmit_edit can have multiple renderers...

Dimchikkk commented 6 months ago

There is tiny bug: when widget becomes active cursor first jumps to the beginning of buffer then on desired position but I think we can live with that

bytemunch commented 6 months ago

There is tiny bug: when widget becomes active cursor first jumps to the beginning of buffer then on desired position but I think we can live with that

Yeah I noticed, will have to time it to find out more, if it's a single frame delay then it could be a systems ordering thing