deps!: Bump winit to 0.29

[Vrixyz]

I'm working on updating bevy to next winit version: bevyengine/bevy#8745 ; a simple winit bump seems enough, but I noticed examples failing so I updated them. I'm sharing in case it's useful.

[mwcampbell]

I'm not sure I understand the point of this PR. Is this meant to prepare for winit 0.29? If so, I can't merge it until 0.29 is published. As a published crate, accesskit_winit can only depend on published versions of other crates; it can't depend on a git checkout of winit.

[Vrixyz]

You're correct, it's not merge-able currently: The point is to prepare for a future winit 0.29, I should have put it in a draft status indeed.

if you prefer a well finished PR feel free to ignore this and I can tag you again when it's merge-able (when winit releases, if you need it, if I get to it)

[Vrixyz]

This should have minimal changes, see #257 (comment)

(I did run a cargo update out of habit too, messing up with the cargo.lock)

[Vrixyz]

not sure that's the best way to handle that. I assumed it makes little sense to test other platforms in platform/windows 🤔

[DataTriny]

The accesskit_windows crate compiles just fine on Linux, and probably on macOS as well. Therefore one might try to run these tests outside of a Windows environment. So yes, this check makes sense to me.

[YouKnow-sys]

Hey, winit 0.29.2 finally released, so are you or anyone else plan to finish this PR?

[Vrixyz]

I'll need it for bevyengine/bevy#8745 ; so I'm planning to work on it if/when needed, but it's more a "side quest" for me. If anyone wants to work on it before I'm ok with it.

If waiting for me, I guess you should expect it to be ready in 1-2 months (not taking into account reviewing)

[YouKnow-sys]

I'll need it for bevyengine/bevy#8745 ; so I'm planning to work on it if/when needed, but it's more a "side quest" for me. If anyone wants to work on it before I'm ok with it.

If waiting for me, I guess you should expect it to be ready in 1-2 months (not taking into account reviewing)

Oh ok, I want to update Vizia to use new version of winit, but Vizia also have accesskit as dependency, so I need to wait for accesskit to update to new version of winit as well.
If you don't have time to finish you PR for now I'll try to take a look at it later.

[torokati44]

I want to update Vizia to use new version of winit, but Vizia also have accesskit as dependency

I'm in a similar situation with Ruffle (actually, egui-winit directly), so if you were to finish this, I would highly appreciate it!

[Vrixyz]

I updated it just so it compiles and unblocks me for further compiling my other work. I spotted a few rough edges I think I'll pass on on fixing,I'm just sharing my work if it can be useful:

• HasRawWindowHandle is deprecated but I still used it to avoid breaking everything.

Feel free to reuse/continue my PR or not, I think I won't work on it more, unless I spot blocking issues.

[Vrixyz]

I needed to rely on rwh_05 (raw_windows_handle version 0.5) because the version of wgpu I use is still on rwh_05, ideally I guess accesskit could support all raw_window_handle versions by providing different features. I'll consider that out of the scope of that PR (which I consider focused on updating to winit 0.29, with minimal changes.

I tested on mac, other platforms are most likely not compiling.

[DataTriny]

Thank you @Vrixyz for your work.

Since interest seem high on this one, I'll push it to completion. I have everything fixed, just waiting for #301 to fix CI.

[DataTriny]

OK, I think I'm done here.

[YouKnow-sys]

So any plan for merging this?

[DataTriny]

Of course! Waiting for a review from @mwcampbell. Please be patient.

[tronical]

I've ported Slint to use this PR (we're using a newer winit version) and I can report that the changes work well for me (in slint-ui/slint#3833 ).

[DataTriny]

I think this PR was tested enough by now and I don't want to block everybody for too long. Unless I'm told otherwise, I'll merge this tomorrow.

I want to fix #300 before we release the next version, and possibly update raw-window-handle to 0.6 (feedback welcome on this one).

[mwcampbell]

I'm finally reviewing this. Sorry for the delay.

[mwcampbell]

I think we need to move the updates to the Windows adapter to a separate PR. This main PR has to be marked as a breaking change in the accesskit_winit crate. But the update to the accesskit_windows crate is not a breaking change, as it only affects test code. I know it's inconvenient, but given the way our semi-automated release process works, I think it's necessary.

[DataTriny]

You're right, it would mess up accesskit_windows semver... I'll revert these changes.

[DataTriny]

I'll need to rebase...

[Vrixyz]

I'll need to rebase...

I merged main into my branch before, so rebase might be a bit trickier than a merge (sorry about that) good luck 🤞

[DataTriny]

I merged main into my branch before, so rebase might be a bit trickier than a merge (sorry about that) good luck 🤞

Yeah, I squashed everything into a single commit because I really didn't feel like going through each one to fix conflicts. Sorry for erasing the git history as well as your name from these commits.

[DataTriny]

Hmm, interesting... The tests are failing on our Windows pipeline, but they pass on my machine. Looks like we encountered an issue with the focus of a window.

test tests::simple::navigation ... FAILED

failures:

---- tests::simple::focus stdout ----
thread 'tests::simple::focus' panicked at platforms\windows\src\tests\simple.rs:189:9:
assertion failed: equal
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- tests::simple::navigation stdout ----
thread 'tests::simple::navigation' panicked at platforms\windows\src\tests\mod.rs:199:36:
called `Result::unwrap()` on an `Err` value: PoisonError { .. }

[mwcampbell]

I wonder if our recent change to use Rc<ReceivedFocusEvent> rather than Arc<ReceivedFocusEvent> was actually incorrect. I'm going to review that code and figure out if that pointer is actually sent across threads.

[DataTriny]

Re-running the job didn't fix the issue so at least it's reproduceable. I still can't make it fail on my machine, even if I run the tests from Powershell.

[mwcampbell]

Yes, it appears my guess was right. Because I deliberately decided to create the UIA client on a thread where COM is initialized in MTA mode, any UIA event handlers are called on a separate thread, and using Rc in such event handlers is unsafe. I'll have to fix this in a separate PR.

[mwcampbell]

I couldn't make it fail either, on my quad-core laptop. As with many concurrency bugs, it's probably only intermittently reproducible, and the conditions happen to be right in the GitHub Actions runner at the moment.

[DataTriny]

Well... At least now we don't panic, but we still have one failing test:

thread 'tests::simple::focus' panicked at platforms\windows\src\tests\simple.rs:189:9:
assertion failed: equal

[mwcampbell]

Why change the macOS backend, and only the macOS backend, to use crate::WindowEvent rather than importing WindowEvent from winit?

[mwcampbell]

I don't know what to do about the failing test. It passed in the CI run for my PR about half an hour ago. Maybe a VM is being reused for the CI runs for this PR, and it's in a weird state. The test also passes for me locally on this branch.

[DataTriny]

No idea, I don't remember making this change as I can't test it anyway. I see that I forgot to remove a comment on this file as well.

[DataTriny]

I was able to get the test failing once on my machine through the newer Terminal application. It occurred the first time I tried running the tests from this app and all subsequent runs passed.

[mwcampbell]

OK, this PR looks good to me now. And now the Windows tests passed. Go figure. May I merge?

[DataTriny]

I went through all the changes once again and I think we are good!

These spurious test failures probably have nothing to do with this PR, so let's merge this.

[mwcampbell]

For anyone who missed it, I published a new version of accesskit_winit yesterday. So if you've been waiting on AccessKit to update to winit 0.29 before you do the same, you're now unblocked.