Upgrade winit to 0.30.2 #4849

ArthurBrussee · 2024-07-18T23:57:23Z

Closes eframe: create multiple windows in sequence #1918
Closes Update to winit 0.30 #4437
Closes eframe: Switch to winit ApplicationManager api #4709
I have followed the instructions in the PR template

Hiya,

I need new winit for a specific fix for a android_native_actvity. There are already two PRs, but both don't seem to have a lot of movement, or are entirely complete:

#4466
Seems to have gone stale & is missing some bits.

#4702
Also seems stale (if less so), and is missing a refactor to run_on_demand. I also think the accesskit integration has a mistake and can't be enabled. I've marked them as a co-author on this as I started from this branch. (I think! Haven't done that on git before...).

Sorry for the wall of text but just dumping some details / thoughts here:

There's an issue with creating child windows in winit 0.30.1 and up on macOS. The multiple_viewports, "create immediate viewport" example crashes on anything later 0.30.1, with a stack overflow in unsafe code. I've create a winit issue, it might already be fixed in 0.31.0 but I can't test as 0.31 will likely require another refactoring. For now I have just pinned things to 0.30.0 exatly.
Winit has deprecated run_on_demand, instead requiring the ApplicationHandler interface. In 0.31.0 run_on_demand is removed. I've refactored both the integration and the WinitApp trait to follow this pattern. I've left user_events a bit more opaque, as it seems 0.31.0 is doing a rework of UserEvents too.
I've used the new lazy init approach for access kit from this branch https://github.com/mwcampbell/egui/tree/accesskit-new-lazy-init and marked Matt as co-author, thanks Matt!
There was very similair but not quite the same code for run_and_return and run_and_exit. I've merged them, but looking at the github issues graveyard it seems vey finnicky. I hope this is more robust than before but it's a bit scary.
when receiving new_events this also used to check the redraw timing dictionary. That doesn't seem necesarry so left this out, but that is a slight behaviour change?
I have reeneabled serial_windows on macOS. I wondered whether it was fixed after this PR and does seem to be! However, even before this PR it seems to work, so maybe winit has sorted things out before that... Windows also works fine now without the extra hack.
I've done a very basic test of AccessKit on Windows and screen reader seems ok but I'm really not knowleadgable enough to say whether it's all good or not.
I've tested cargo tests & all examples on Windows & macOS, and ran a basic Android app. Still, testing native platforms is wel... hard so if anyone can test linux / iOs / older mac versions / windows 10 would probably be a good idea!
For consistencys sake I've made all event like functions in WinitApp return a Result<EventResult>. There's quite a bit of Ok-wrapping now, maybe too annoying? Not sure.

Thank you for having a look!

Tested on

macOS
Windows
Wayland (thanks SiebenCorgie)
X11 (thanks crumblingstatue!, SiebenCorgie)

TODO

Fix "follow system theme" not working on initial startup (winit issue, pinning to 0.30.2 for now).
Fix request_repaint_after

As per the example branch from Matt! Co-authored-by: mwcampbell <[email protected]>

Also forgot to mark j-axa co-author ealier Co-authored-by: j-axa <[email protected]>

Update to latest `accesskit` and `accesskit_winit`

Tested on WSL wayland. Required an annoying workaround for nix/fs :/

Cargo.toml

examples/serial_windows/src/main.rs

crates/egui-winit/Cargo.toml

emilk · 2024-07-30T09:31:40Z

I found a couple of issues during testing; added them under the TODO heading in the PR description. I'll do some more testing and code review soon™️. I'd like to get this in as soon as possible!

In the previous code, new_events did check repaint times, this was lost in the translation. Seperate out this check from handle_event_result as we don't need the complexity of that whole shebang.

torokati44 · 2024-07-30T21:20:42Z

Oh that's just the result of GNOME folks insisting that window decorations have to be done by the clients (applications), and not by the server (the compositor / window manager), lest they may be too consistent. 🙄 Window decorations look normal on KDE.

See: rust-windowing/winit#1967

emilk · 2024-07-31T07:42:32Z

Awesome - thanks for the help with the testing! Let's go 🚀

mwcampbell · 2024-07-31T07:58:24Z

I just did a quick test of accessibility with a screen reader on Windows, and it looks good. Thanks @ArthurBrussee for getting this across the finish line.

Since #4849, running `./scripts/check.sh` fails with: ``` ... error[E0277]: the trait bound `ActiveEventLoop: raw_window_handle::borrowed::HasDisplayHandle` is not satisfied --> crates/egui_glow/src/winit.rs:43:13 | 43 | event_loop, | ^^^^^^^^^^ the trait `raw_window_handle::borrowed::HasDisplayHandle` is not implemented for `ActiveEventLoop` | = help: the following other types implement trait `raw_window_handle::borrowed::HasDisplayHandle`: raw_window_handle::borrowed::DisplayHandle<'a> &H &mut H = note: required for the cast from `&ActiveEventLoop` to `&dyn raw_window_handle::borrowed::HasDisplayHandle` ``` This PR adds the missing `rwh_06` to the winit dependency (in egui-glow). * [x] I have followed the instructions in the PR template

torokati44 · 2024-08-07T12:28:55Z

Sounds good! I've pinned to 0.30.2 with a comment, will keep an eye on the 0.30.5.

Soon! ™️ @ArthurBrussee
rust-windowing/winit#3849

ArthurBrussee · 2024-08-07T12:46:51Z

Oh nice! Though seems actually so far 0.30.5 doesn't have the fix this needs alas. Have asked there whether it still might be included.

torokati44 · 2024-08-08T18:31:35Z

'Tis out!

This updates winit to 0.30.5. #4849 Had to pin the version to 0.30.2, as a Winit patch changed the behavior of selecting a theme. Winit 0.30.5 reverts this, so we could stick with `window.theme()`, but the newly added `ActiveEventLoop::system_theme` is more like what egui wants anyway, as individual windows can have theme overrides. Also bump `smithay-clipboard` to prevent some now duplicate dependencies.

* Closes emilk#1918 * Closes emilk#4437 * Closes emilk#4709 * [x] I have followed the instructions in the PR template Hiya, I need new winit for a specific fix for a android_native_actvity. There are already two PRs, but both don't seem to have a lot of movement, or are entirely complete: emilk#4466 Seems to have gone stale & is missing some bits. emilk#4702 Also seems stale (if less so), and is missing a refactor to run_on_demand. I also *think* the accesskit integration has a mistake and can't be enabled. I've marked them as a co-author on this as I started from this branch. (I think! Haven't done that on git before...). Sorry for the wall of text but just dumping some details / thoughts here: - There's an issue with creating child windows in winit 0.30.1 and up on macOS. The multiple_viewports, "create immediate viewport" example crashes on anything later 0.30.1, with a stack overflow in unsafe code. I've create [a winit issue](rust-windowing/winit#3800), it *might* already be fixed in 0.31.0 but I can't test as 0.31 will likely require another refactoring. For now I have just pinned things to 0.30.0 exatly. - Winit has deprecated run_on_demand, instead requiring the ApplicationHandler interface. In 0.31.0 run_on_demand is removed. I've refactored both the integration and the WinitApp trait to follow this pattern. I've left user_events a bit more opaque, as it seems 0.31.0 is doing a rework of UserEvents too. - I've used the new lazy init approach for access kit from this branch https://github.com/mwcampbell/egui/tree/accesskit-new-lazy-init and marked Matt as co-author, thanks Matt! - There was very similair but not quite the same code for run_and_return and run_and_exit. I've merged them, but looking at the github issues graveyard it seems vey finnicky. I *hope* this is more robust than before but it's a bit scary. - when receiving new_events this also used to check the redraw timing dictionary. That doesn't seem necesarry so left this out, but that is a slight behaviour change? - I have reeneabled serial_windows on macOS. I wondered whether it was fixed after this PR and does seem to be! However, even before this PR it seems to work, so maybe winit has sorted things out before that... Windows also works fine now without the extra hack. - I've done a very basic test of AccessKit on Windows and screen reader seems ok but I'm really not knowleadgable enough to say whether it's all good or not. - I've tested cargo tests & all examples on Windows & macOS, and ran a basic Android app. Still, testing native platforms is wel... hard so if anyone can test linux / iOs / older mac versions / windows 10 would probably be a good idea! - For consistencys sake I've made all event like functions in WinitApp return a `Result<EventResult>`. There's quite a bit of Ok-wrapping now, maybe too annoying? Not sure. Thank you for having a look! * [x] macOS * [x] Windows * [x] Wayland (thanks [SiebenCorgie](https://github.com/SiebenCorgie)) * [x] X11 (thanks [crumblingstatue](https://github.com/crumblingstatue)!, [SiebenCorgie](https://github.com/SiebenCorgie)) * [x] Fix "follow system theme" not working on initial startup (winit issue, pinning to 0.30.2 for now). * [x] Fix `request_repaint_after` --------- Co-authored-by: mwcampbell <[email protected]> Co-authored-by: j-axa <[email protected]> Co-authored-by: DataTriny <[email protected]> Co-authored-by: Emil Ernerfeldt <[email protected]>

Since emilk#4849, running `./scripts/check.sh` fails with: ``` ... error[E0277]: the trait bound `ActiveEventLoop: raw_window_handle::borrowed::HasDisplayHandle` is not satisfied --> crates/egui_glow/src/winit.rs:43:13 | 43 | event_loop, | ^^^^^^^^^^ the trait `raw_window_handle::borrowed::HasDisplayHandle` is not implemented for `ActiveEventLoop` | = help: the following other types implement trait `raw_window_handle::borrowed::HasDisplayHandle`: raw_window_handle::borrowed::DisplayHandle<'a> &H &mut H = note: required for the cast from `&ActiveEventLoop` to `&dyn raw_window_handle::borrowed::HasDisplayHandle` ``` This PR adds the missing `rwh_06` to the winit dependency (in egui-glow). * [x] I have followed the instructions in the PR template

This updates winit to 0.30.5. emilk#4849 Had to pin the version to 0.30.2, as a Winit patch changed the behavior of selecting a theme. Winit 0.30.5 reverts this, so we could stick with `window.theme()`, but the newly added `ActiveEventLoop::system_theme` is more like what egui wants anyway, as individual windows can have theme overrides. Also bump `smithay-clipboard` to prevent some now duplicate dependencies.

* Closes emilk#1918 * Closes emilk#4437 * Closes emilk#4709 * [x] I have followed the instructions in the PR template Hiya, I need new winit for a specific fix for a android_native_actvity. There are already two PRs, but both don't seem to have a lot of movement, or are entirely complete: emilk#4466 Seems to have gone stale & is missing some bits. emilk#4702 Also seems stale (if less so), and is missing a refactor to run_on_demand. I also *think* the accesskit integration has a mistake and can't be enabled. I've marked them as a co-author on this as I started from this branch. (I think! Haven't done that on git before...). Sorry for the wall of text but just dumping some details / thoughts here: - There's an issue with creating child windows in winit 0.30.1 and up on macOS. The multiple_viewports, "create immediate viewport" example crashes on anything later 0.30.1, with a stack overflow in unsafe code. I've create [a winit issue](rust-windowing/winit#3800), it *might* already be fixed in 0.31.0 but I can't test as 0.31 will likely require another refactoring. For now I have just pinned things to 0.30.0 exatly. - Winit has deprecated run_on_demand, instead requiring the ApplicationHandler interface. In 0.31.0 run_on_demand is removed. I've refactored both the integration and the WinitApp trait to follow this pattern. I've left user_events a bit more opaque, as it seems 0.31.0 is doing a rework of UserEvents too. - I've used the new lazy init approach for access kit from this branch https://github.com/mwcampbell/egui/tree/accesskit-new-lazy-init and marked Matt as co-author, thanks Matt! - There was very similair but not quite the same code for run_and_return and run_and_exit. I've merged them, but looking at the github issues graveyard it seems vey finnicky. I *hope* this is more robust than before but it's a bit scary. - when receiving new_events this also used to check the redraw timing dictionary. That doesn't seem necesarry so left this out, but that is a slight behaviour change? - I have reeneabled serial_windows on macOS. I wondered whether it was fixed after this PR and does seem to be! However, even before this PR it seems to work, so maybe winit has sorted things out before that... Windows also works fine now without the extra hack. - I've done a very basic test of AccessKit on Windows and screen reader seems ok but I'm really not knowleadgable enough to say whether it's all good or not. - I've tested cargo tests & all examples on Windows & macOS, and ran a basic Android app. Still, testing native platforms is wel... hard so if anyone can test linux / iOs / older mac versions / windows 10 would probably be a good idea! - For consistencys sake I've made all event like functions in WinitApp return a `Result<EventResult>`. There's quite a bit of Ok-wrapping now, maybe too annoying? Not sure. Thank you for having a look! # Tested on * [x] macOS * [x] Windows * [x] Wayland (thanks [SiebenCorgie](https://github.com/SiebenCorgie)) * [x] X11 (thanks [crumblingstatue](https://github.com/crumblingstatue)!, [SiebenCorgie](https://github.com/SiebenCorgie)) # TODO * [x] Fix "follow system theme" not working on initial startup (winit issue, pinning to 0.30.2 for now). * [x] Fix `request_repaint_after` --------- Co-authored-by: mwcampbell <[email protected]> Co-authored-by: j-axa <[email protected]> Co-authored-by: DataTriny <[email protected]> Co-authored-by: Emil Ernerfeldt <[email protected]>

Since emilk#4849, running `./scripts/check.sh` fails with: ``` ... error[E0277]: the trait bound `ActiveEventLoop: raw_window_handle::borrowed::HasDisplayHandle` is not satisfied --> crates/egui_glow/src/winit.rs:43:13 | 43 | event_loop, | ^^^^^^^^^^ the trait `raw_window_handle::borrowed::HasDisplayHandle` is not implemented for `ActiveEventLoop` | = help: the following other types implement trait `raw_window_handle::borrowed::HasDisplayHandle`: raw_window_handle::borrowed::DisplayHandle<'a> &H &mut H = note: required for the cast from `&ActiveEventLoop` to `&dyn raw_window_handle::borrowed::HasDisplayHandle` ``` This PR adds the missing `rwh_06` to the winit dependency (in egui-glow). * [x] I have followed the instructions in the PR template

This updates winit to 0.30.5. emilk#4849 Had to pin the version to 0.30.2, as a Winit patch changed the behavior of selecting a theme. Winit 0.30.5 reverts this, so we could stick with `window.theme()`, but the newly added `ActiveEventLoop::system_theme` is more like what egui wants anyway, as individual windows can have theme overrides. Also bump `smithay-clipboard` to prevent some now duplicate dependencies.

ArthurBrussee and others added 8 commits July 18, 2024 19:58

Initial conversion to winit 0.30

00c282d

Pin to 0.30.0 as 0.30.1+ has a bug that crashes on mac :/

5294cae

Builder -> attributes

3f42b72

Add link to problematic winit PR

2e6c0d3

Update WinitApp integration to be more like ApplicationHandler

4f0e7d5

Fix for windows & linux

4623863

Use more modern accesskit lazy init

eafed08

As per the example branch from Matt! Co-authored-by: mwcampbell <[email protected]>

Fixup comments

e6741d2

Also forgot to mark j-axa co-author ealier Co-authored-by: j-axa <[email protected]>

ArthurBrussee mentioned this pull request Jul 19, 2024

Crash on 0.30.1+ when creating immediate child viewports rust-windowing/winit#3800

Closed

Update to latest accesskit and accesskit_winit

f37e52b

DataTriny mentioned this pull request Jul 19, 2024

Update to latest accesskit and accesskit_winit ArthurBrussee/egui#1

Merged

ArthurBrussee added 3 commits July 19, 2024 11:36

Merge pull request #1 from DataTriny/fix-accesskit

fd3e552

Update to latest `accesskit` and `accesskit_winit`

Fixes for linux

cd894eb

Tested on WSL wayland. Required an annoying workaround for nix/fs :/

Merge remote-tracking branch 'origin/master' into winit-update

d90b565

emilk reviewed Jul 19, 2024

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

Cargo.toml Outdated Show resolved Hide resolved

examples/serial_windows/src/main.rs Show resolved Hide resolved

crates/egui-winit/Cargo.toml Outdated Show resolved Hide resolved

ArthurBrussee added 4 commits July 19, 2024 17:56

CI fixes

5e0f479

Add a comment about wayland-cursor.

67c456b

Fix crash when using winit 0.30.4

6d582f5

Enable nix/fs feature workaround in x11 too

2f4dbaf

ArthurBrussee requested a review from Wumpf as a code owner July 26, 2024 00:49

ArthurBrussee changed the title ~~Upgrade winit to 0.30.0~~ Upgrade winit to 0.30.4 Jul 26, 2024

ArthurBrussee added 2 commits July 26, 2024 10:50

Remove leftover comment

af1c892

Managed to leave in some typos...

c10e3fb

adrien-ben mentioned this pull request Jul 29, 2024

winit 0.30 adrien-ben/gltf-viewer-rs#35

Merged

Merge branch 'master' into winit-update

d9905d8

emilk added eframe Relates to epi and eframe egui-winit porblems related to winit dependencies Pull requests that update a dependency file labels Jul 30, 2024

Fix request_repaint_after

f10cf80

In the previous code, new_events did check repaint times, this was lost in the translation. Seperate out this check from handle_event_result as we don't need the complexity of that whole shebang.

emilk merged commit 6f2f006 into emilk:master Jul 31, 2024
19 checks passed

This was referenced Jul 31, 2024

update winit to 0.30.3 #4702

Closed

Bump winit to 0.30.0 and accesskit to 0.14.0 #4466

Closed

Fix crash when changing viewport settings #4862

Merged

Follow the system theme in egui #4860

Merged

emilk mentioned this pull request Jul 31, 2024

Upgrade Glow to 0.14 #4833

Closed

1 task

This was referenced Aug 2, 2024

Update accesskit_winit #4219

Closed

build(deps): bump winit from 0.29.15 to 0.30.3 in the winit-wgpu-egui group across 1 directory ruffle-rs/ruffle#16835

Closed

DataTriny mentioned this pull request Aug 4, 2024

High CPU usage when accesskit enabled #4527

Closed

bash mentioned this pull request Aug 4, 2024

Fix missing winit feature in egui_glow #4916

Merged

1 task

trevyn mentioned this pull request Aug 6, 2024

Init lib update winit 0.30 hakolao/glass#4

Merged

torokati44 mentioned this pull request Aug 7, 2024

core,desktop: Update to winit v0.30.5 and latest egui from git ruffle-rs/ruffle#17401

Merged

ArthurBrussee mentioned this pull request Aug 8, 2024

Unpin & upgrade winit to 0.30.5 #4939

Merged

gfolkmanis-nhgh mentioned this pull request Aug 8, 2024

touchscreen click events are not detected properly (on Linux) #4895

Closed

CrazyboyQCD mentioned this pull request Sep 26, 2024

Memory, size and exit errors with dynamic library rendering #5170

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade winit to 0.30.2 #4849

Upgrade winit to 0.30.2 #4849

ArthurBrussee commented Jul 18, 2024 •

edited by emilk

Loading

emilk commented Jul 30, 2024

torokati44 commented Jul 30, 2024

emilk commented Jul 31, 2024

mwcampbell commented Jul 31, 2024

torokati44 commented Aug 7, 2024

ArthurBrussee commented Aug 7, 2024

torokati44 commented Aug 8, 2024

Upgrade winit to 0.30.2 #4849

Upgrade winit to 0.30.2 #4849

Conversation

ArthurBrussee commented Jul 18, 2024 • edited by emilk Loading

Tested on

TODO

emilk commented Jul 30, 2024

torokati44 commented Jul 30, 2024

emilk commented Jul 31, 2024

mwcampbell commented Jul 31, 2024

torokati44 commented Aug 7, 2024

ArthurBrussee commented Aug 7, 2024

torokati44 commented Aug 8, 2024

ArthurBrussee commented Jul 18, 2024 •

edited by emilk

Loading