wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

WGPU + SDL2 on MacOS cannot find valid containerView -- with a hack solution

Open sanbox-irl opened this issue 4 years ago • 7 comments

Description Using sdl2 and wgpu together like so:

        let instance = wgpu::Instance::new(wgpu::BackendBit::PRIMARY);
        let surface = unsafe { instance.create_surface(&window.raw_handle()) };

where .raw_handle returns &sdl2::video::Window, which implements RawWindowHandle via the feature raw-window-handle in the sdl2 crate.

This will crash, because raw_window_handle on macOS has a field, ns_view, which must be non-null but is set to null by sdl2. I have no idea what this field means or what it does, but sdl2 has this comment listed in its code:

                    ns_view: 0 as *mut libc::c_void, // consumer of RawWindowHandle should determine this

This might not be correct and SDL2-rs should be setting this field to a well defined value, but I have no idea. I don't know what I am doing at all, in this, or in a variety of tasks in my life.

It appears that at ONE point, WGPU was doing that here -- https://github.com/gfx-rs/wgpu/pull/462

However, that codepath has gone stale, and I am not familiar enough with WGPU to reattach this code to fix that regression within WGPU. I am submitting this issue primarily such that someone who is familiar with WGPU should be able to take this code and hook it into WGPU fairly easily.

Help, I am lil bean who has searched for a solution to this problem and need it now

Okay, so here's the hack I did that I think some less hacky version of should get ported back into WGPU, as the above PR did, but I'm not sure where to put it within WGPU:

struct WindowHack<'a>(
    raw_window_handle::macos::MacOSHandle,
    PhantomData<&'a sdl2::video::Window>,
);

unsafe impl<'a> HasRawWindowHandle for WindowHack<'a> {
    fn raw_window_handle(&self) -> raw_window_handle::RawWindowHandle {
        raw_window_handle::RawWindowHandle::MacOS(self.0)
    }
}

impl<'a> WindowHack<'a> {
    /// Creates a new window hack
    pub fn new(video_handle: &'a sdl2::video::Window) -> Self {
        let mut h = match video_handle.raw_window_handle() {
            raw_window_handle::RawWindowHandle::MacOS(h) => h,
            _ => unimplemented!("this is only gonna run on macos"),
        };

        // THIS IS THE PART WGPU SHOULD HAVE IN IT:
        h.ns_view = if h.ns_view.is_null() {
            let ns_window = h.ns_window as *mut Object;
            unsafe { msg_send![ns_window, contentView] }
        } else {
            h.ns_view
        };
        // --

        Self(h, PhantomData)
    }
}

And then the calling code into WGPU is edited to look like this:

        let instance = wgpu::Instance::new(wgpu::BackendBit::PRIMARY);
        let hack = WindowHack::new(window.raw_handle());
        let surface = unsafe { instance.create_surface(&hack) };

Platform MacOS 11.4 (Big Sur) on an M1 Mac mini. I have no expectations that this is specific to the ARM nature of this computer.

sanbox-irl avatar Jun 16 '21 14:06 sanbox-irl

Thank you for filing this in such a great detail! How come SDL2 doesn't fill this field? Is there an issue filed to their implementation about this? Seems weird that we'd have this workaround, when we don't have any new information versus what they got already.

kvark avatar Jun 16 '21 17:06 kvark

Okay so it appears going through sdl2's commit history, at one point they did.

It appears that, effectively, there can be multiple ns_view per ns_window, so to a degree, if sdl2 were to fill in this value, they would be giving an arbitrary value, if there were multiple views.

However, multiple views per window, especially for games, which sdl2 is oriented around, is not common at all, so I will log an issue with them to remove that complexity and instead fill in the default ns_view which I have done above, and which they once did here: https://github.com/Rust-SDL2/rust-sdl2/pull/962/commits/eedfc2a1f215864319b745945239c9dffed99ed2#diff-19a4ef09c7391f4efb59c49b7cadcf29c55bb3a9d86afb7cd81054766108423cL81

Somewhat complexly, because life is cruel that way, the person who made this edit to the PR before it was accepted has deleted their account. Additionally, it appeared that WGPU had some issue which was assumed (I suspect incorrectly) to be WGPU's fault at the time, and this change was accepted even with that error. Perhaps two problems at once, but this history is a bit of a comedy of errors.

I'll comment shortly with the sdl2 issue and this can be closed if you'd like -- I think that sdl2 should be provided valid handles here now that I know more about this

sanbox-irl avatar Jun 16 '21 20:06 sanbox-irl

Set up issue in SDL2 and in the process discovered that they have an example which eerily mirrors my above code -- so I'm re-orienting back to fact finding, since some original implementer might know more than I have learned in about an hour of reading apple docs https://github.com/Rust-SDL2/rust-sdl2/issues/1116

sanbox-irl avatar Jun 16 '21 20:06 sanbox-irl

@sanbox-irl thank you for driving this!

kvark avatar Jun 17 '21 20:06 kvark

I ran into the same issue today with the latest code.

I hacked the code to pass the NSWindow->contentView, and it appears to work, as no wgpu exceptions were seen. However, the NSWindow has no rendered images displayed after calling present().

I'm not familiar with Appkit. Is there a requirement for the NSView? For example, do I need to use CAMetalLayer?

shi-yan avatar Jun 01 '22 16:06 shi-yan

Answer to my own question:

  1. the top of tree rust-sdl2 still doesn't provide a pointer to NSWindow's currentView. instead, it simply provides nil.

  2. no, I don't need to explicitly create a CAMetalLayer myself, because WGPU has a logic to create one when there is none.

  3. present() failure is related to https://github.com/gfx-rs/wgpu/issues/2727

shi-yan avatar Jun 07 '22 21:06 shi-yan

  1. no, I don't need to explicitly create a CAMetalLayer myself, because WGPU has a logic to create one when there is none.

Yes, don't need to create CAMetalLayer yourself, but must provide a view. https://github.com/gfx-rs/wgpu/blob/8e5ac75d45369b1e8693739d25b581db23043e0e/wgpu-hal/src/metal/mod.rs#L86-L98 https://github.com/gfx-rs/wgpu/blob/31c6b39c20bc672692fe61dd345e18fac4134077/wgpu-hal/src/metal/surface.rs#L97-L129

jinleili avatar Jun 07 '22 23:06 jinleili