Skip to content

feat(raw_window_handle): Improve raw-window-handle api & bump wgpu version#394

Open
What42Pizza wants to merge 10 commits into
vhspace:masterfrom
What42Pizza:master
Open

feat(raw_window_handle): Improve raw-window-handle api & bump wgpu version#394
What42Pizza wants to merge 10 commits into
vhspace:masterfrom
What42Pizza:master

Conversation

@What42Pizza

@What42Pizza What42Pizza commented May 30, 2026

Copy link
Copy Markdown
Contributor

This does three things:

  • Improves the api for using sdl3::video::Window as a raw_window_handle::HasDisplayHandle type (previously the example needed a custom type with its own unsafe code, and now it's just window.as_window_handle()?)
  • Improves the code's threading safety by caching values instead of sending a struct which might call main-thread-only functions on other threads
  • Fix the api annoyance that creating a window handle struct removes your ability to use &mut self methods on sdl3::video::Window due to lifetime restrictions
  • Bumps wgpu's version from 29.0.1 to 30.0.0

Edit: latest safety review

@What42Pizza What42Pizza changed the title Improve raw-window-handle support & bump wgpu version feat(raw-window-handle): Improve raw-window-handle api & bump wgpu version May 30, 2026
@What42Pizza What42Pizza changed the title feat(raw-window-handle): Improve raw-window-handle api & bump wgpu version feat(raw_window_handle): Improve raw-window-handle api & bump wgpu version May 30, 2026
@What42Pizza

What42Pizza commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

Also, I'm kinda questioning whether this raw-window-handle compatibility should be in the higher-level sdl3 crate considering the fact that the implementation only needs the functions and types from the sdl3-sys crate

Here's what I'm envisioning: there's a new 'sdl3-raw-window-handle' crate that provides unsafe fn get_window_handle(*mut SDL_Window) -> SdlWindowHandle<'static>. This crate can be used even if you're only using sdl3-sys, but also the sdl3 crate still has a feature flag that gives the Window type the method fn as_window_handle<'a>(&'a self) -> SdlWindowHandle<'a> (which just passes window.raw() to sdl3-raw-window-handle then replaces the handle's static lifetime with the window's lifetime)

If this needs more explaining or an example implementation then let me know

@What42Pizza

Copy link
Copy Markdown
Contributor Author

I've been looking into the safety of this caching approach some more, and safety documentation for handles like this are annoying absent, but it does seem like this should be safe.

Here's a list of every pointer/id that is queried by the raw-window-handle compat code:

windows window handle:
SDL_PROP_WINDOW_WIN32_HWND_POINTER
SDL_PROP_WINDOW_WIN32_INSTANCE_POINTER

macos window handle:
SDL_PROP_WINDOW_COCOA_WINDOW_POINTER
msg_send![ns_window as *mut NSObject, contentView]

ios window handle:
SDL_PROP_WINDOW_UIKIT_WINDOW_POINTER

android window handle:
SDL_PROP_WINDOW_ANDROID_WINDOW_POINTER

x11 window handle:
SDL_PROP_WINDOW_X11_WINDOW_NUMBER

wayland window handle:
SDL_PROP_WINDOW_WAYLAND_SURFACE_POINTER

x11 display handle:
SDL_PROP_WINDOW_X11_DISPLAY_POINTER
SDL_PROP_WINDOW_X11_SCREEN_NUMBER

wayland display handle:
SDL_PROP_WINDOW_WAYLAND_DISPLAY_POINTER

If any of these values were to change while the window is still alive, that would cause huge issues with essentially any graphics api, so I think it's safe to assume that these values can be cached.

Also, for what it's worth, SDL tends to document when values need to be re-queried, and it doesn't document that any of these values need to be re-queried.

@revmischa

Copy link
Copy Markdown
Member

The overall approach here (querying the native handles once and caching them in a wrapper) is reasonable, and splitting the shared vs mut variants makes sense. A few things before it can land:

  • This drops the HasWindowHandle/HasDisplayHandle impls on Window itself, so it's a breaking change for anyone passing &window straight to wgpu/glutin. That's fine, but it should ship in a release with a short migration note pointing people at window.as_window_handle().
  • mut_as_window_handle reads backwards; as_window_handle_mut matches Rust naming conventions. The WindowAsWindowHandle type name is also a bit of a mouthful (WindowHandles or similar might read better).
  • The unsafe impl Send/Sync for WindowAsWindowHandle needs a safety comment. Native handles (HWND, NSView, X11/Wayland pointers) aren't Send/Sync in general, so it's worth spelling out why it's sound for the cached values here, and whether that holds on every backend.
  • The safety comment on mut_as_window_handle ("does break rust's safety rule...") should explain what actually keeps it sound, since handing back both &mut Window and a handle that borrows from it is the crux.

Unrelated to your change: the red Windows CI is a build-from-source break on the GitHub runners, fixed separately in #407. Once that lands a rebase should clear it.

@What42Pizza

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback, I did change the names as requested but I'm unsure of what to do for the unsafe impls.

As far as I understand it, for a struct like WindowHandles (as it is now called), it is only considered thread-safe if both the struct itself and all possible usages of it are thread-safe. In this case, the struct itself is safe, but it can be used in unsafe ways (e.g. you move it to another thread, retrieve the window handle, then give that to a method which can only be called on the main thread). So my conclusion is that the unsafe impls for WindowHandles are not sound, but I'm not sure how much we should care about that or what should be done about it. It would be nice if it wasn't needed, but unfortunately it's a requirement for wgpu compatibility.

Also, I'm not familiar with adding migration notes, so an example of what to do for that would be nice. And about the safety comment on as_window_handles_mut, I realized it is not needed because the returned value does not reference any data in the source Window.

It seems like the unsafe impls are the biggest problem, and the core of it is that the actual window and display pointers shouldn't be sent across threads, but wgpu requires the given window struct to be send/sync anyways. And even if the unsafe impls are removed, anyone who wants to use sdl3-rs with wgpu will just add their own unsafe impls, so the problem is just moved instead of fixed. So I'm in favor of keeping the unsafe impls, which now have a safety comment explaining why it technically isn't sound but is still likely safe in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants