Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to winit 0.30 #4437

Closed
emilk opened this issue Apr 30, 2024 · 6 comments · Fixed by #4849
Closed

Update to winit 0.30 #4437

emilk opened this issue Apr 30, 2024 · 6 comments · Fixed by #4849
Labels
eframe Relates to epi and eframe egui-winit porblems related to winit help wanted Extra attention is needed

Comments

@emilk
Copy link
Owner

emilk commented Apr 30, 2024

Also needed:

@emilk emilk added help wanted Extra attention is needed eframe Relates to epi and eframe egui-winit porblems related to winit labels Apr 30, 2024
@emilk emilk added this to the Next Major Release milestone Apr 30, 2024
@mwcampbell
Copy link
Contributor

The accesskit_winit update for winit 0.30 is released now.

@lylythechosenone
Copy link

Updating to wgpu 0.20.0 should be an important goal here as well. Not sure if that should be a separate issue.

@torokati44
Copy link
Contributor

That's already done: #4433

@AndriBaal
Copy link

I updated winit to 0.30.0 and was able to successfully run the hello-world example successfully with wgpu.

Here is a summary of the changes I made:

  • Bump accesskit to version 0.14.0
  • Bump winit to 0.30.0
  • Partially done the glow integration
  • Roughly implemented accesskit
  • Implemented winit / wgpu

Here is a summary of things that need to be done:

  • Integration with glow. We have to wait until glutin-winit updates to winit 0.30.0
  • Not all accesskit_winit::Event are being processed, temporarily I have marked them with todo! (found in winit_integration.rs)
  • Move from deprecated winit::EventLoop::run_on_demand to new Application trait
  • Documentation and testing
  • Code review

I have madew a pull request with my changes here: #4466

@mwcampbell
Copy link
Contributor

You can see how to properly handle all the accesskit_winit events in this branch that I never got around to completing and submitting as a PR because, at the time, updating accesskit_winit led to cargo deny problems in CI. That problem is solved now, so feel free to take code from my branch, including my changes to the egui crate.

@emilk
Copy link
Owner Author

emilk commented Jul 15, 2024

We have two competing PRs, neither of which are ready to be merged:

@emilk emilk closed this as completed in 6f2f006 Jul 31, 2024
rib pushed a commit to EmbarkStudios/egui that referenced this issue Sep 30, 2024
* 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]>
486c pushed a commit to 486c/egui that referenced this issue Oct 9, 2024
* 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]>
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe egui-winit porblems related to winit help wanted Extra attention is needed
Projects
None yet
5 participants