From a47dacd5cd235ea056da4f9e5c62594dca992059 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Thu, 8 Aug 2024 19:35:25 -0700 Subject: [PATCH] Implement `Send` for `Image` 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. https://github.com/Smithay/smithay/issues/1500 --- pixman/src/image/bits.rs | 10 ++++++++-- pixman/src/image/mod.rs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pixman/src/image/bits.rs b/pixman/src/image/bits.rs index 2140c82..4e5cde3 100644 --- a/pixman/src/image/bits.rs +++ b/pixman/src/image/bits.rs @@ -13,6 +13,10 @@ pub struct Image<'bits, 'alpha> { _alpha: PhantomData<&'alpha ()>, } +// SAFETY: An image is safe to `Send` if the image and its alpha map have a +// reference count of one. So references will not exist on multiple threads. +unsafe impl<'bits, 'alpha> Send for Image<'bits, 'alpha> {} + impl<'bits, 'alpha> std::ops::Deref for Image<'bits, 'alpha> { type Target = ImageRef; @@ -130,7 +134,7 @@ impl<'bits, 'a> Image<'bits, 'a> { /// used as a src in a blit operation pub fn set_alpha_map<'alpha: 'a>( self, - alpha_map: &'alpha Image<'_, 'static>, + alpha_map: Image<'alpha, 'static>, x: i16, y: i16, ) -> Image<'bits, 'alpha> { @@ -467,7 +471,9 @@ impl<'bits, 'alpha> Image<'bits, 'alpha> { /// /// # Safety /// - /// The pointer is expected to be valid and have a ref-count of at least one. + /// The pointer is expected to be valid and have a ref-count of at *exactly* one, + /// with an alpha map (if any) that has a ref count of one. + /// /// Ownership of the pointer is transferred and unref will be called on drop. pub unsafe fn from_ptr(ptr: *mut ffi::pixman_image_t) -> Self { Self { diff --git a/pixman/src/image/mod.rs b/pixman/src/image/mod.rs index 3b64b80..e7795e1 100644 --- a/pixman/src/image/mod.rs +++ b/pixman/src/image/mod.rs @@ -190,7 +190,7 @@ macro_rules! image_type { /// used as a src in a blit operation pub fn set_alpha_map<'alpha: 'a>( self, - alpha_map: &'alpha crate::Image<'_, 'static>, + alpha_map: crate::Image<'alpha, 'static>, x: i16, y: i16, ) -> $name<'alpha> {