Use std::sync::Arc<cosmic-text::Buffer> in bevy component

[bytemunch]

I'm not too sure about using make_mut whenever modification to the buffer is needed, as I don't have the best understanding of copy-on-write and how that will work, though I'd like to think bevy's scheduler will prevent race conditions.

Needs a fair bit more testing, but this change should remove the need for double checking the editor buffer everywhere there could be two, as there is now one buffer per widget.

Fixes #145
Fixes #138 (untested so far)

[Dimchikkk]

copy on write shouldn't happen with current code changes, right?
since we don't have more than one arc pointers pointing to the same allocation.

[bytemunch]

That's what I was thinking but I'm a bit new to the concept. I think that anything that touches the buffer while it's in an editor would cause a copy? Unsure, but I did break text centering when using the single buffer so there must still be two allocations somewhere

[Dimchikkk]

I see, what do you think about Arc<Mutex<...>> ? :)

[bytemunch]

Cosmic text doesn't accept a mutex, that's what tripped me up when I tried this before

…

On Wed, 5 Jun 2024, 16:06 Dimchikkk, ***@***.***> wrote: I see, what do you think about Arc<Mutex<...>> ? :) — Reply to this email directly, view it on GitHub <#146 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKMCYIY632YFOXLDIA4DE4DZF4SOPAVCNFSM6AAAAABI2VV3EWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJQGMYDQMJSHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[bytemunch]

I think the solution is that we can never use make_mut to access a buffer if it's already owned by an editor. Maybe matching on get_mut and then if that fails there needs to be access through editor.with_buffer_mut().

Having all of this be hidden from the user would be ideal. Perhaps the CosmicBuffer holds an optional reference to an Editor, and the functions on the struct allow for buffer access?

[Dimchikkk]

I want to undestand issue better xD a bit lost

[bytemunch]

Internally cosmic-text calls Arc::make_mut() on the buffer it is provided with, when the buffer is in an Arc. In order to prevent clone-on-write (allocating a clone of a buffer for operations that should happen to one buffer), we need to make sure to only have one strong reference to the Arc when the Editor is created, and ensure that while the Editor holds a strong reference we don't try to create any more. (I need to read up on the difference between strong and weak refs so take this with a pinch of salt!)

I think that if our code never calls make_mut when an Editor holds a Buffer, we should be able to keep the reference count at 1.

The problem is that we'd like user code and plugin code to be able to modify a buffer, both when it is not owned by an Editor, and when it is.

So the solution I'm thinking of is tracking when a given buffer is owned by an editor, and redirecting any modifications through the editor reference when it is, while still being able to directly modify the buffer when it's not in an editor, without the user having to use different functions. I think that this could be done with pattern matching on the result of get_mut, but I've not tried yet.

All the problems that are solved with a Mutex, which feels very backwards, and usually when something feels this backwards in rust it means I have a lack of understanding somewhere, which is why I stopped exploring this path a couple months ago. But I've not come up with anything better since, so think I'll just push forward with this approach and see if the way reveals itself in time lol.

[Dimchikkk]

Pattern-matching seems to be possible:

I see

pub enum BufferRef<'buffer> {
    Owned(Buffer),
    Borrowed(&'buffer mut Buffer),
    Arc(Arc<Buffer>),
}

so we use Arc<Buffer>, can we use Borrowed version?

[bytemunch]

I think with the borrowed version we run into lifetime issues, holding a mutable borrow to the buffer past the function's return. Maybe a static lifetime saves us here, maybe not

After a bit more reading I feel even the Arc approach will always lead to a forced clone, as the bevy component holds an Arc at the same time the editor calls make_mut.

I'll make another branch and try out my optional call redirection idea, perhaps using a shared trait on the CosmicBuffer and CosmicEditor wrappers for simplicity, and the public impl on the buffer can decide which call should be used based on some sort of editor relationship stored on the buffer wrapper.

Another approach could be to store the editor and buffer in a struct, and then that decides where to send the calls to buffer functions?

Lots of thinking out loud, I'll have a play around and see what I come up with

[Dimchikkk]

Nice, have a nice play :D

[Dimchikkk]

closing for now