-
Notifications
You must be signed in to change notification settings - Fork 171
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
PixmanTexture
is not Send
#1500
Comments
ids1024
added a commit
to ids1024/pixman-rs
that referenced
this issue
Aug 9, 2024
This changes the assumptions of the crate to assert that a valid `pixmap::Image` has a ref-count of 1, with an alpha map that has a ref-count of 1. Consequently, `set_alpha_map` must take ownership of its argument. My understanding, looking at the pixmap code, is that this should ensure it is safe to `Send` the `Image` to another thread. Of course this does limit the API of this crate if a consumer wants to use multiple references (though `pixman_image_ref` wasn't wrapper already), or wants to use `set_alpha_map` while still keeping a reference to the alpha map. The first issue can just be solved by wrapping in `Rc`, though. Smithay/smithay#1500
It seems much cleaner to implement this in the |
ids1024
added a commit
to ids1024/pixman-rs
that referenced
this issue
Aug 9, 2024
This changes the assumptions of the crate to assert that a valid `pixmap::Image` has a ref-count of 1, with an alpha map that has a ref-count of 1. Consequently, `set_alpha_map` must take ownership of its argument. My understanding, looking at the pixmap code, is that this should ensure it is safe to `Send` the `Image` to another thread. Of course this does limit the API of this crate if a consumer wants to use multiple references (though `pixman_image_ref` wasn't wrapper already), or wants to use `set_alpha_map` while still keeping a reference to the alpha map. The first issue can just be solved by wrapping in `Rc`, though. Smithay/smithay#1500
Since |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Might as well have an issue for this.
PixmanTexture
not beingSend
makes it impossible to use withMemoryRenderBuffer
(as I noticed in #1497), and is otherwise awkward.Looking at pixman's implementation,
pixman_image_t
is not thread safe since it is ref-counted. In a way that isn't thread-safe.pixman_image_ref
isn't currently bound by thepixman
crate. But it is used once internally inpixman
: forpixman_image_set_alpha_map
.I think it should be safe to
Send
a pixman image that has a reference count of 1, and if it has an alpha map, that also has a reference count of 1.I see a couple ways to achieve this:
PixmanTexture
couldunsafe impl Send
. It can no longer implementFrom<pixman::Image<'static, 'static>>
, but could offer anunsafe
function that mentions the reference count requirement.pixman
doesn't provide accessors for the reference count or alpha map, so we can't assert those, to make it safe.pixman
crate could do the same thing internally, instead.pixman::Image::set_alpha_map
will have to be changed to take ownership of the alpha map (so the ref count goes up to two, then it drops the first reference).pixman::Image::from_ptr
will have to mention the same restriction.pixman
would want to make use of the reference counting. Though other thanset_alpha_map
, it could just wrap in anRc
, so it's not that bad.Send
type and a cloneable non-Send
type, but that gets a bit complicated.The text was updated successfully, but these errors were encountered: