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

Cosmic text 0.11 #124

Merged
merged 26 commits into from
Mar 20, 2024
Merged

Cosmic text 0.11 #124

merged 26 commits into from
Mar 20, 2024

Conversation

bytemunch
Copy link
Collaborator

@bytemunch bytemunch commented Mar 6, 2024

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

Todo before merging:

  • Clicking in a text box that has been edited without being unfocused places the cursor incorrectly
  • Fix setting a FocusedWidget before widget instantiation (i.e in Startup)
  • Configurable cursor, selection colors
  • Run layout and shaping systems only when needed (And whenever needed, dynamically setting text doesn't update)
  • Update examples with new API
  • Pass tests

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

i should really put them somewhere else on my machine
src/lib.rs Outdated
#[derive(Component)]
pub struct CosmicEditor(pub Editor);
#[derive(Component, Deref, DerefMut)]
pub struct CosmicEditor(pub Editor<'static>);
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@bytemunch
Copy link
Collaborator Author

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
Copy link
Owner

Let me try to test basic_ui today ;)

@Dimchikkk
Copy link
Owner

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

lots of cloning but seems to be the only way if buffers are in the ECS

editor widgets don't relayout when edited, but when unfocused the buffer does
@Dimchikkk
Copy link
Owner

editable banana, nice :D

Dima and others added 3 commits March 14, 2024 11:37
uhoh it's looking like im using the not a starting point pr as a starting point lol
@Dimchikkk
Copy link
Owner

@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 bytemunch changed the title Cosmic text 0.11 testing Cosmic text 0.11 Mar 17, 2024
@bytemunch
Copy link
Collaborator Author

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
Copy link
Owner

Dimchikkk commented Mar 17, 2024

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
Copy link
Collaborator Author

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 bevyengine/bevy#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
Copy link
Owner

Yep, internal plugins sounds good.

adds a util crate for common example systems
also removes history tracking structs
MV `bevy_api_testing.rs` > `sprite_and_ui_clickable.rs`
simplifys API and mirrors Bevy's Text functions more closely
@Dimchikkk
Copy link
Owner

so close no matter how far :)

@bytemunch
Copy link
Collaborator Author

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

@Dimchikkk
Copy link
Owner

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

@bytemunch
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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

@Dimchikkk
Copy link
Owner

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
Copy link
Collaborator Author

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.

@bytemunch bytemunch marked this pull request as ready for review March 20, 2024 08:01
@Dimchikkk
Copy link
Owner

Yeah, abstracting renderer sounds cool 👍

Then, bevy_cosmit_edit can have multiple renderers...

@Dimchikkk
Copy link
Owner

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

@Dimchikkk Dimchikkk merged commit 696fe6f into main Mar 20, 2024
4 checks passed
@Dimchikkk Dimchikkk deleted the testing_cosmic-text_0.11 branch March 20, 2024 10:16
@bytemunch
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants