raw_window_handle feature is broken on all non-Windows OSes.

[Elabajaba]

#53 broke all the non-windows backends for for raw_window_handle by updating the dependency and changing the return type, then only fixing the #[cfg(target_os = "windows")] blocks and not any of the other blocks.

[What42Pizza]

Yeah I'm the one that did that, I only have windows so I can't test for other OSes, but fixing it should be pretty easy. The code as mostly all the same for each os before I updated it, so I'm guessing you'd just need to copy the code tweak it just a bit. I guess I can do that myself and just hope that the resulting code works, but if it doesn't then I won't be able to help with that

[revmischa]

Want to get check out #60 @Elabajaba ?

[What42Pizza]

Okay dang it I didn't check the code thoroughly enough and there's some more commented out code that's breaking raw-window-handle for me. That's definitely my fault, I'll make another pull request to quickly fix it but I'd say that you don't have to bother making a new release just for this fix unless someone actually needs it

[What42Pizza]

Oh yeah but other that I think this issue is fixed

[codec-xyz]

That same pull #53 seems to break the raw_window_handle feature for me on Windows. Using sdl3 v0.11.14 and rustc v1.83.0 I get the error...

Compiling sdl3 v0.11.14
error[E0658]: use of unstable library feature 'strict_provenance'
  --> C:\-redacted-\.cargo\registry\src\index.crates.io-6f17d22bba15001f\sdl3-0.11.14\src/sdl3\raw_window_handle.rs:31:81
   |
31 |             let mut handle = Win32WindowHandle::new(NonZero::new_unchecked(hwnd.addr() as isize));
   |                                                                                 ^^^^
   |
   = note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information

error[E0658]: use of unstable library feature 'strict_provenance'
  --> C:\-redacted-\.cargo\registry\src\index.crates.io-6f17d22bba15001f\sdl3-0.11.14\src/sdl3\raw_window_handle.rs:32:70
   |
32 |             handle.hinstance = Some(NonZero::new_unchecked(hinstance.addr() as isize));
   |                                                                      ^^^^
   |
   = note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information

For more information about this error, try `rustc --explain E0658`.
error: could not compile `sdl3` (lib) due to 2 previous errors

[Elabajaba]

Yeah, looks like those are nightly only for the moment.

Seems like they'll be in rust 1.84 on February 20th. January 9th.
rust-lang/rust#130350

edit: Whoops, got the date wrong for rust 1.84 it's out now as of this edit.

[What42Pizza]

I'm a little bit late to this, is any action still needed? If it's working fine then maybe we can just leave it, but it would probably nice to have a lower min rust version, right? I could easily just make another pull request, but there's not even any min supported rust version listed in the readme or toml

[Goldenbough44]

I think there is still an issue on MacOS.

Current implementation uses SDL_PROP_WINDOW_COCOA_WINDOW_POINTER to get NSView for MacOS, and directly use the return value to create AppKitWindowHandle of raw-window-handle.

But from SDL3 documentation, it says that the prop returns the pointer of NSWindow, not NSView.

So it seems that some action is needed to work properly on MacOS.

[What42Pizza]

Do you think this is something that you could fix yourself?

[Goldenbough44]

I handled the issue using another crate (https://crates.io/crates/objc2-app-kit) that provide rust bindings of Apple's Appkit for my project. It seems SDL3 itself doesn't provide an api for NSview object.
If I can use same crate, yes I think, but needs some days.
Sorry if this comment seems rediculus. I’m not familiar with the open source community.

[What42Pizza]

Wait, have you tried using SDL_PROP_WINDOW_CREATE_COCOA_VIEW_POINTER instead of SDL_PROP_WINDOW_COCOA_WINDOW_POINTER? According to the documentation (link), I think that will give the nsview

[Goldenbough44]

That is about creating sdl window, as far as I know. When creating sdl window, you can specify your pre-created nsview as ptr using SDL_CreateWindowWithProperties function and SDL_PROP_WINDOW_CREATE_COCOA_WINDOW_POINTER prop. SDL_GetWindowProperties function cannot use the property.

[What42Pizza]

Oh yeah, I guess I should've seen that from the name. If that's the case then yeah I guess obj2-app-kit is necessary and I think it makes sense to add it as an optional dependency. Can you share how you were able to make it work?

[What42Pizza]

After A LOT of using chatgpt, it seems like this might work? It's using objc instead of the crate you mentioned, and I have no clue if it actually works, but I thought I'd just put it out here anyways

let ns_window = SDL_GetPointerProperty(
    window_properties,
    sys::video::SDL_PROP_WINDOW_COCOA_WINDOW_POINTER,
    std::ptr::null_mut(),
);
let ns_view = msg_send![ns_window as *mut Object, contentView];
let handle = AppKitWindowHandle::new(NonNull::new_unchecked(ns_view));

It's giving a warning about a cfg macro but that's just something the objc devs added for clippy and it's okay to ignore

[Goldenbough44]

Yeah it seems have to work, Can you try to run it? If you can't, I'll try in few days.

[What42Pizza]

No I don't have a mac and I can't test it so yeah you'll have to test it

[What42Pizza]

I just pushed these changes to my own fork so that you can test it whenever you're ready (link)

[Goldenbough44]

I fixed deps for compatibility and test to your fork and opened PR. link
This code works propely on my macbook, and it seems need some more test on not macos device.

[What42Pizza]

Okay I've opened a PR to move from my repo to this one, and I've edited the cargo.toml to make the objc dependency only activate on MacOs

[revmischa]

Great, if it works for people I can merge and release it

[What42Pizza]

@Goldenbough44 can you test my fork again? I changed the dep from objc to objc2 and I'm not sure if it still works

[Goldenbough44]

@What42Pizza I fixed objc2's deprecated apis and opend PR for your fork. This works well.

[What42Pizza]

Thanks a lot, I'll merge to mine and hopefully merge from mine to this one

[Elabajaba]

Seems to be working now with #69, and is now tested in CI with #72 (at least, mac and linux are currently).