Update to new raw-window-handle strategy #3126

notgull · 2023-10-01T23:42:45Z

Tested on all platforms changed
Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
Created or updated an example program if it would help users understand this functionality
Updated feature matrix, if new features were added or implemented

Essentially a combination of #3075 and #2943. This PR adds explicit support for raw-window-handle v0.6 through the rwh_06 feature, which is enabled by default. It also adds optional support for raw-window-handle v0.5 and raw-window-handle v0.4, which is exposed through features that are not exposed by default.

This also adds a lifetime to WindowAttributes.

daxpedda

Web is LGTM.

madsmtm

Thanks for following up on this @notgull!

macOS and iOS impl approved, have opened some discussion on the API though that I would like to see resolved.

src/window.rs

madsmtm · 2023-10-02T14:04:26Z

src/window.rs

    /// Returns a [`raw_window_handle::RawDisplayHandle`] used by the [`EventLoop`] that
    /// created a window.
    ///
    /// [`EventLoop`]: crate::event_loop::EventLoop


Nit: This documentation is also relevant for v0.6

you mean, that this should also be added to the implementation of HasDisplayHandle for v0.6?

src/lib.rs

examples/child_window.rs

src/platform/windows.rs

src/platform_impl/ios/window.rs

src/platform_impl/macos/window.rs

kchibisov

Could you expect instead of doing unchecked? I'd rather not have more unsafe and have an actual assert that it got a Null for whatever reason.

It'll basically never happen, but the cost of checking null in a function you rarely call is non-existent.

Cargo.toml

examples/util/fill.rs

TornaxO7 · 2023-10-04T20:12:39Z

@notgull maybe not needed, but just in case, I fixed the CI build in this PR

TornaxO7 · 2023-10-06T10:17:41Z

Could you expect instead of doing unchecked? I'd rather not have more unsafe and have an actual assert that it got a Null for whatever reason.

It'll basically never happen, but the cost of checking null in a function you rarely call is non-existent.

may I ask which unchecked places you mean?
rg -trust "unchecked" returns no entries, if I'm seeing it correctly, related with the HasDisplayHandle and HasWindowHandle.

kchibisov · 2023-10-06T10:23:27Z

@TornaxO7 comments are getting resolved over time. This particular one was removed. See 9e15334

MarijnS95 · 2023-10-07T15:59:07Z

src/platform_impl/android/mod.rs

+        if let Some(native_window) = self.app.native_window().as_ref() {
+            let handle = rwh_06::AndroidNdkWindowHandle::new(native_window.ptr().cast());
+            Ok(rwh_06::RawWindowHandle::AndroidNdk(handle))
+        } else {


I'm implementing this in the NDK, as part of version bumps.

Done in rust-mobile/ndk#434.

May I ask what that means? Does anything has to do be done here or can this be marked as resolved?

This means that once I publish ndk 0.8.0, winit should upgrade to it (it's on -beta.0 now) and reuse the traits provided there.

Does this block this PR? I think that this should be merged in its current state and then we can upgrade to the latest ndk later.

It does not and I can patch this when submitting the final ndk bump inside winit.

MarijnS95 · 2023-10-07T16:10:03Z

src/platform_impl/android/mod.rs

+    }
+
+    #[cfg(feature = "rwh_06")]
+    pub fn raw_window_handle_rwh_06(&self) -> Result<rwh_06::RawWindowHandle, rwh_06::HandleError> {


Question while implementing the ndk portion for this: implementing the Has**RAW**WindowHandle trait is deprecated and there are blanket implementations when HasWindowHandle is implemented. Should this function also return a non-raw handle (in terms of that implementation when it is released)?

I considered this, but it would add an additional unsafe { WIndowHandle::borrow_raw } call for each backend. I could go either way.

Right. At least for Android I'm willing to have that unsafe { borrow_raw() } inside the ndk with appropriate // SAFETY docs, so you could call through to that. But it'd have to be consistent across all the backends...

Did you consider whether or not the safe borrow handles should be implemented by winit?

For the time being it leads to a lot more unsafe throughout the codebase. So I think that we shouldn't for now.

…0.5) Following [the same strategy in winit], move `raw-window-handle 0.5` behind a `rwh_05` feature and add `raw-window-handle 0.6` behind a corresponding `rwh_06` feature. Implement the new non-raw trait and **lifetimed** type for `0.6` (where `raw-window-handle` provides blanket implementations to coerce/convert back to `Raw*` types). [the same strategy in winit]: rust-windowing/winit#3126

…aintain 0.5) Following [the same strategy in winit], move `raw-window-handle 0.5` behind a `rwh_05` feature and add `raw-window-handle 0.4` and the newly released `raw-window-handle 0.6` behind a corresponding `rwh04`/`rwh_06` feature. The new **non-raw** trait and **lifetimed** type since `0.6` have been implemented instead, as `raw-window-handle` provides blanket implementations to coerce/convert back to `Raw*` types. [the same strategy in winit]: rust-windowing/winit#3126

…aintain 0.5) (#434) Following [the same strategy in winit], move `raw-window-handle 0.5` behind a `rwh_05` feature and add `raw-window-handle 0.4` and the newly released `raw-window-handle 0.6` behind a corresponding `rwh04`/`rwh_06` feature. The new **non-raw** trait and **lifetimed** type since `0.6` have been implemented instead, as `raw-window-handle` provides blanket implementations to coerce/convert back to `Raw*` types. [the same strategy in winit]: rust-windowing/winit#3126

Signed-off-by: John Nunley <dev@notgull.net>

Co-Authored-By: TornaxO7 <tornax@proton.me> Signed-off-by: John Nunley <dev@notgull.net>

MarijnS95

Approving the android changes, but still have a question about borrowed handles.

Depends on #3126, rust-mobile/android-activity#128

This allows using accesskit with current versions of wgpu. Winit has a similar system to widden its support. See rust-windowing/winit#3126

notgull requested review from jackpot51, msiglreith, daxpedda, madsmtm and kchibisov as code owners October 1, 2023 23:42

daxpedda approved these changes Oct 2, 2023

View reviewed changes

madsmtm requested changes Oct 2, 2023

View reviewed changes

kchibisov requested changes Oct 2, 2023

View reviewed changes

Cargo.toml Show resolved Hide resolved

examples/util/fill.rs Outdated Show resolved Hide resolved

TornaxO7 mentioned this pull request Oct 3, 2023

Bump raw window handle to 0.6.0 gfx-rs/wgpu#4202

Merged

3 tasks

madsmtm approved these changes Oct 3, 2023

View reviewed changes

notgull force-pushed the notgull/rwh06 branch from 79c5961 to 101905c Compare October 4, 2023 23:54

MarijnS95 requested changes Oct 7, 2023

View reviewed changes

MarijnS95 reviewed Oct 7, 2023

View reviewed changes

MarijnS95 mentioned this pull request Oct 7, 2023

ndk/native_window: Implement HasWindowHandle for 0.4 and 0.6 (and maintain 0.5) rust-mobile/ndk#434

Merged

pengiie mentioned this pull request Oct 10, 2023

Bump raw-window-handle to 0.6.0 ash-rs/ash#795

Closed

madsmtm mentioned this pull request Oct 13, 2023

Make with_parent_window safe #3142

Draft

3 tasks

ids1024 mentioned this pull request Oct 13, 2023

Use raw-window-handle version 0.6 rust-windowing/softbuffer#132

Merged

notgull force-pushed the notgull/rwh06 branch from 101905c to bd80244 Compare October 14, 2023 03:35

notgull added 2 commits October 14, 2023 09:12

Make the compile errors go away on X11

5692a26

Signed-off-by: John Nunley <dev@notgull.net>

Make the errors go away on macOS

ebab7cb

Signed-off-by: John Nunley <dev@notgull.net>

notgull and others added 10 commits October 14, 2023 09:12

Make the compile errors go away on WIndows

4a89e32

Signed-off-by: John Nunley <dev@notgull.net>

Make the compile errors go away on web

3013c3c

Signed-off-by: John Nunley <dev@notgull.net>

Make the Android build errors go away

8509157

Signed-off-by: John Nunley <dev@notgull.net>

Finish the iOS and Orbital backends

36439bd

Signed-off-by: John Nunley <dev@notgull.net>

Fix CI errors

dee38df

Signed-off-by: John Nunley <dev@notgull.net>

More fixes for CI errors

2cf5efa

Signed-off-by: John Nunley <dev@notgull.net>

Fix X11 CI error

e2d1301

Signed-off-by: John Nunley <dev@notgull.net>

Partially fix review comments

b20c0ce

Signed-off-by: John Nunley <dev@notgull.net>

Remaining review comments

9a861b6

Signed-off-by: John Nunley <dev@notgull.net>

Fix windows lifetimes

2933b0a

Co-Authored-By: TornaxO7 <tornax@proton.me> Signed-off-by: John Nunley <dev@notgull.net>

notgull force-pushed the notgull/rwh06 branch from bd80244 to 2933b0a Compare October 14, 2023 16:12

notgull mentioned this pull request Oct 14, 2023

Port to use raw-window-handle v0.6 #2943

Closed

5 tasks

notgull requested review from kchibisov and MarijnS95 October 14, 2023 17:39

MarijnS95 approved these changes Oct 14, 2023

View reviewed changes

MarijnS95 added a commit that referenced this pull request Oct 14, 2023

Upgrade ndk(-sys) to stable 0.8 / 0.5 releases

f5b4a5d

Depends on #3126, rust-mobile/android-activity#128

MarijnS95 mentioned this pull request Oct 14, 2023

Upgrade to ndk 0.8, ndk-sys 0.5 and android-activity 0.5 releases #3145

Merged

kchibisov approved these changes Oct 15, 2023

View reviewed changes

kchibisov merged commit e41fac8 into master Oct 15, 2023
50 checks passed

kchibisov deleted the notgull/rwh06 branch October 15, 2023 02:07

Vrixyz mentioned this pull request Oct 22, 2023

Update winit dependency to 0.29 bevyengine/bevy#8745

Closed

11 tasks

Vrixyz mentioned this pull request Dec 13, 2023

fix: Use raw-window-handle 0.6 AccessKit/accesskit#310

Merged

nicopap added a commit to nicopap/accesskit that referenced this pull request Dec 14, 2023

Add a rwh_05 feature flag

6443c22

This allows using accesskit with current versions of wgpu. Winit has a similar system to widden its support. See rust-windowing/winit#3126

nicopap mentioned this pull request Dec 14, 2023

Add a rwh_05 feature flag AccessKit/accesskit#319

Merged

3 tasks

nicopap added a commit to nicopap/accesskit that referenced this pull request Dec 14, 2023

Add a rwh_05 feature flag

daa6c6c

This allows using accesskit with current versions of wgpu. Winit has a similar system to widden its support. See rust-windowing/winit#3126

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to new raw-window-handle strategy #3126

Update to new raw-window-handle strategy #3126

notgull commented Oct 1, 2023

daxpedda left a comment

madsmtm left a comment

madsmtm Oct 2, 2023

TornaxO7 Oct 6, 2023

madsmtm Oct 6, 2023

kchibisov left a comment

TornaxO7 commented Oct 4, 2023

TornaxO7 commented Oct 6, 2023 •

edited

Loading

kchibisov commented Oct 6, 2023

MarijnS95 Oct 7, 2023

MarijnS95 Oct 7, 2023

TornaxO7 Oct 10, 2023

MarijnS95 Oct 10, 2023

notgull Oct 14, 2023

MarijnS95 Oct 14, 2023

MarijnS95 Oct 7, 2023

notgull Oct 7, 2023

MarijnS95 Oct 7, 2023

MarijnS95 Oct 14, 2023

notgull Oct 14, 2023

MarijnS95 left a comment

Update to new raw-window-handle strategy #3126

Update to new raw-window-handle strategy #3126

Conversation

notgull commented Oct 1, 2023

daxpedda left a comment

Choose a reason for hiding this comment

madsmtm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kchibisov left a comment

Choose a reason for hiding this comment

TornaxO7 commented Oct 4, 2023

TornaxO7 commented Oct 6, 2023 • edited Loading

kchibisov commented Oct 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarijnS95 left a comment

Choose a reason for hiding this comment

TornaxO7 commented Oct 6, 2023 •

edited

Loading