Skip to content

Commit d683d9b

Browse files
Improve docs and naming for RawWindowHandle functionality (#4335)
# Objective - As noticed in #4333 by @x-52, the exact purpose and logic of `HasRawWIndowHandleWrapper` is unclear - Unfortunately, there are rather good reasons why this design is needed (and why we can't just `impl HasRawWindowHandle for RawWindowHandleWrapper` ## Solution - Rename `HasRawWindowHandleWrapper` to `ThreadLockedRawWindowHandleWrapper`, reflecting the primary distinction - Document how this design is intended to be used - Leave comments explaining why this design must exist ## Migration Guide - renamed `HasRawWindowHandleWrapper` to `ThreadLockedRawWindowHandleWrapper`
1 parent f271d73 commit d683d9b

File tree

1 file changed

+25
-8
lines changed

1 file changed

+25
-8
lines changed

crates/bevy_window/src/raw_window_handle.rs

+25-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
use raw_window_handle::{HasRawWindowHandle, RawWindowHandle};
22

3-
/// This wrapper exist to enable safely passing a [`RawWindowHandle`] across threads. Extracting the handle
4-
/// is still an unsafe operation, so the caller must still validate that using the raw handle is safe for a given context.
3+
/// A wrapper over [`RawWindowHandle`] that allows us to safely pass it across threads.
4+
///
5+
/// Depending on the platform, the underlying pointer-containing handle cannot be used on all threads,
6+
/// and so we cannot simply make it (or any type that has a safe operation to get a [`RawWindowHandle`])
7+
/// thread-safe.
58
#[derive(Debug, Clone)]
69
pub struct RawWindowHandleWrapper(RawWindowHandle);
710

@@ -10,12 +13,14 @@ impl RawWindowHandleWrapper {
1013
Self(handle)
1114
}
1215

16+
/// Returns a [`HasRawWindowHandle`] impl, which exposes [`RawWindowHandle`].
17+
///
1318
/// # Safety
14-
/// This returns a [`HasRawWindowHandle`] impl, which exposes [`RawWindowHandle`]. Some platforms
15-
/// have constraints on where/how this handle can be used. For example, some platforms don't support doing window
19+
///
20+
/// Some platforms have constraints on where/how this handle can be used. For example, some platforms don't support doing window
1621
/// operations off of the main thread. The caller must ensure the [`RawWindowHandle`] is only used in valid contexts.
17-
pub unsafe fn get_handle(&self) -> HasRawWindowHandleWrapper {
18-
HasRawWindowHandleWrapper(self.0)
22+
pub unsafe fn get_handle(&self) -> ThreadLockedRawWindowHandleWrapper {
23+
ThreadLockedRawWindowHandleWrapper(self.0)
1924
}
2025
}
2126

@@ -27,10 +32,22 @@ impl RawWindowHandleWrapper {
2732
unsafe impl Send for RawWindowHandleWrapper {}
2833
unsafe impl Sync for RawWindowHandleWrapper {}
2934

30-
pub struct HasRawWindowHandleWrapper(RawWindowHandle);
35+
/// A [`RawWindowHandleWrapper`] that cannot be sent across threads.
36+
///
37+
/// This safely exposes a [`RawWindowHandle`], but care must be taken to ensure that the construction itself is correct.
38+
///
39+
/// This can only be constructed via the [`RawWindowHandleWrapper::get_handle()`] method;
40+
/// be sure to read the safety docs there about platform-specific limitations.
41+
/// In many cases, this should only be constructed on the main thread.
42+
pub struct ThreadLockedRawWindowHandleWrapper(RawWindowHandle);
3143

3244
// SAFE: the caller has validated that this is a valid context to get RawWindowHandle
33-
unsafe impl HasRawWindowHandle for HasRawWindowHandleWrapper {
45+
// as otherwise an instance of this type could not have been constructed
46+
// NOTE: we cannot simply impl HasRawWindowHandle for RawWindowHandleWrapper,
47+
// as the `raw_window_handle` method is safe. We cannot guarantee that all calls
48+
// of this method are correct (as it may be off the main thread on an incompatible platform),
49+
// and so exposing a safe method to get a `RawWindowHandle` directly would be UB.
50+
unsafe impl HasRawWindowHandle for ThreadLockedRawWindowHandleWrapper {
3451
fn raw_window_handle(&self) -> RawWindowHandle {
3552
self.0
3653
}

0 commit comments

Comments
 (0)