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

Mutability of sub-buffers #29

Open
vmx opened this issue Aug 13, 2021 · 3 comments
Open

Mutability of sub-buffers #29

vmx opened this issue Aug 13, 2021 · 3 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@vmx
Copy link
Contributor

vmx commented Aug 13, 2021

In my code I write to a sub-buffer. Due to #27 this sub-buffer needs to be mutable. The buffer the sub-buffer is created from via Buffer::create_sub_buffer() doesn't need (in the Rust type system sense) to be mutable. Though it actually mutates.

I haven't encountered such a case myself in Rust yet, so I don't know if there are any good patterns how those things are usually solved.

This issue isn't really actionable, but I wanted to bring it up, in case someone has a good idea. So feel free to close the issue as "won't fix".

@kenba kenba added help wanted Extra attention is needed question Further information is requested labels Aug 14, 2021
@kenba
Copy link
Owner

kenba commented Aug 14, 2021

You raise an interesting point Volker. I don't know a Rust pattern for handling this situation either.
I shall leave this issue open as a question in the hope that someone does know a good solution.

@vmx
Copy link
Contributor Author

vmx commented Aug 16, 2021

A colleague of mine pointed me to https://docs.rs/bytes/1.0.1/bytes/index.html. I then also started to prepare a minimal example, to make the problem easier to understand for people that are not into OpenCl: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2f92e740e64e415dc46474fc397da96f. It doesn't compile and it made me realize how things in pure Rust, without the FFI border would be.

So perhaps there needs to be a wrapper around cl_mem. Currently there is the ClMem trait, but that might not be enough. From just a cl_mem you can't really tell if the underlying data is mutable or not. I think by default a *mut c_void should alwasy be considered mutable. Though this would create an hard to use API, if you know that all you need read-only access only.

One way could be to replace the ClMem trait with two wrapper structs around cl_mem, an immutable and a mutable one (something along the lines of struct ClMem(cl_mem) and struct ClMemMut(cl_mem)). This distinction would then bubble up and you'd have a Buffer and a BufferMut. Functions that need a mutable buffer (like enqueue_write_buffer), would then require a mutable reference to the BufferMut type. Both buffer types would implement create_sub_buffer(). If you need a mutable sub-buffer, it needs to come from a BufferMut. This way you make sure that also the original buffer also was considered mutable.

@kenba
Copy link
Owner

kenba commented Aug 17, 2021

Thank you Volker.
I think that you are correct. cl_mem needs both immutable and mutable traits like bytes in order to define immutable and mutable Buffers.
However, in addition to Buffer and BufferMut, this will also "bubble up" to Image and maybe Sampler too.
Note: I don't think that this should affect Pipe, since a Pipe must be mutable, however, it may have to in order to keep the API consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants