Implement HasRawWindowHandle trait for win/mac #1586
Conversation
Implement the HasRawWindowHandle trait for Windows and macOS platforms, to allow access to native window handles for interop.
| RawWindowHandle::Windows(handle) | ||
| } else { | ||
| panic!("Cannot retrieved HWMD for window."); | ||
| } |
There was a problem hiding this comment.
I’m not convinced panicking is right for any of these. I think they should return an empty window handle and maybe log a warning. Given that the raw window handle can represent empty handles.
There was a problem hiding this comment.
If we're okay with the idea that a RawWindowHandle can potentially be empty then I agree, otherwise we could consider making this an Option<RawWindowHandle>.
There was a problem hiding this comment.
This is an external crate, that is depended on by various things like wgpu and video libraries (and some other gui/rendering libraries). The RawWindowHandles all explicitly have an empty state and consumers have to handle that.
There was a problem hiding this comment.
Right, typo. Fixed.
There was a problem hiding this comment.
I think a panic! is justified because if we have a WindowHandle without a native HWND I'm assuming we're in serious trouble. Or is there a valid state where that happens?
There was a problem hiding this comment.
It might be possible when the window is in the process of being set up or torn down? Elsewhere in the windows backend we log when we can't get the HWND, so I might just keep doing that. 🤷
There was a problem hiding this comment.
I think it is better to log. These handles are meant to represent empty states and users of them are expected to handle those states (its better to have an error on screen than a program crash!)
|
@rjwittams are you satisfied? feel free to approve & merge if so. :) |
cmyr
added
S-waiting-on-author
|
I am happy too modulo the panics. |
|
I could imagine this just not being implemented on those platforms, since it's defined in a trait, and if you're using a raw window handle I would expect you to be compiling or |
This was discussed. I have no access to non-Windows platforms and little knowledge about them, so rather than doing a blind implementation likely to be buggy, implementations on those missing platforms is left for a future change by whoever can contribute it. I did Windows and @rjwittams did macOS. |
|
Looks good. Could you get it up to date with master (should be a quick merge or rebase, whichever you prefer as we'll squash it anyway), as I'm not sure if the log vs tracing changes may confound Githubs 'this is ok to merge' detection... |
* Implement HasRawWindowHandle trait for win/mac Implement the HasRawWindowHandle trait for Windows and macOS platforms, to allow access to native window handles for interop. * Remove libc dependency * Fix typo in HWND * Return empty handle and log error on fail * Fix typo
* Implement HasRawWindowHandle trait for win/mac Implement the HasRawWindowHandle trait for Windows and macOS platforms, to allow access to native window handles for interop. * Remove libc dependency * Fix typo in HWND * Return empty handle and log error on fail * Fix typo
Implement the HasRawWindowHandle trait for Windows and macOS platforms,
to allow access to native window handles for interop.