Skip to content

Commit 92cc1eb

Browse files
committed
Improve CG thread-safety documentation a bit
CGColorSpace is marked Send+Sync in objc2-core-graphics, and CALayer won't be, for the reasons described in the comments.
1 parent 2cb7b86 commit 92cc1eb

1 file changed

Lines changed: 16 additions & 20 deletions

File tree

src/backends/cg.rs

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//! Softbuffer implementation using CoreGraphics.
12
use crate::backend_interface::*;
23
use crate::error::InitError;
34
use crate::{Rect, SoftBufferError};
@@ -26,7 +27,7 @@ use std::ptr::{self, slice_from_raw_parts_mut, NonNull};
2627
define_class!(
2728
#[unsafe(super(NSObject))]
2829
#[name = "SoftbufferObserver"]
29-
#[ivars = Retained<CALayer>]
30+
#[ivars = SendCALayer]
3031
struct Observer;
3132

3233
/// NSKeyValueObserving
@@ -44,13 +45,9 @@ define_class!(
4445
}
4546
);
4647

47-
// SAFETY: The `CALayer` that the observer contains is thread safe.
48-
unsafe impl Send for Observer {}
49-
unsafe impl Sync for Observer {}
50-
5148
impl Observer {
5249
fn new(layer: &CALayer) -> Retained<Self> {
53-
let this = Self::alloc().set_ivars(layer.retain());
50+
let this = Self::alloc().set_ivars(SendCALayer(layer.retain()));
5451
unsafe { msg_send![super(this), init] }
5552
}
5653

@@ -63,11 +60,9 @@ impl Observer {
6360

6461
let change =
6562
change.expect("requested a change dictionary in `addObserver`, but none was provided");
66-
let new = unsafe {
67-
change
68-
.objectForKey(NSKeyValueChangeNewKey)
69-
.expect("requested change dictionary did not contain `NSKeyValueChangeNewKey`")
70-
};
63+
let new = change
64+
.objectForKey(unsafe { NSKeyValueChangeNewKey })
65+
.expect("requested change dictionary did not contain `NSKeyValueChangeNewKey`");
7166

7267
// NOTE: Setting these values usually causes a quarter second animation to occur, which is
7368
// undesirable.
@@ -105,7 +100,7 @@ pub struct CGImpl<D, W> {
105100
/// Can also be retrieved from `layer.superlayer()`.
106101
root_layer: SendCALayer,
107102
observer: Retained<Observer>,
108-
color_space: SendCGColorSpace,
103+
color_space: CFRetained<CGColorSpace>,
109104
/// The width of the underlying buffer.
110105
width: usize,
111106
/// The height of the underlying buffer.
@@ -241,7 +236,7 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> for CGImpl<
241236
layer: SendCALayer(layer),
242237
root_layer: SendCALayer(root_layer),
243238
observer,
244-
color_space: SendCGColorSpace(color_space),
239+
color_space,
245240
width,
246241
height,
247242
_display: PhantomData,
@@ -320,7 +315,7 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> BufferInterface for BufferImpl<'_,
320315
8,
321316
32,
322317
self.imp.width * 4,
323-
Some(&self.imp.color_space.0),
318+
Some(&self.imp.color_space),
324319
// TODO: This looks incorrect!
325320
CGBitmapInfo::ByteOrder32Little | CGBitmapInfo(CGImageAlphaInfo::NoneSkipFirst.0),
326321
Some(&data_provider),
@@ -349,16 +344,17 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> BufferInterface for BufferImpl<'_,
349344
}
350345
}
351346

352-
struct SendCGColorSpace(CFRetained<CGColorSpace>);
353-
// SAFETY: `CGColorSpace` is immutable, and can freely be shared between threads.
354-
unsafe impl Send for SendCGColorSpace {}
355-
unsafe impl Sync for SendCGColorSpace {}
356-
357347
struct SendCALayer(Retained<CALayer>);
358-
// CALayer is thread safe, like most things in Core Animation, see:
348+
349+
// SAFETY: CALayer is dubiously thread safe, like most things in Core Animation.
350+
// But since we make sure to do our changes within a CATransaction, it is
351+
// _probably_ fine for us to use CALayer from different threads.
352+
//
353+
// See also:
359354
// https://developer.apple.com/documentation/quartzcore/catransaction/1448267-lock?language=objc
360355
// https://stackoverflow.com/questions/76250226/how-to-render-content-of-calayer-on-a-background-thread
361356
unsafe impl Send for SendCALayer {}
357+
// SAFETY: Same as above.
362358
unsafe impl Sync for SendCALayer {}
363359

364360
impl Deref for SendCALayer {

0 commit comments

Comments
 (0)