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

WIP: Update to winit 0.29 #3496

Closed
wants to merge 0 commits into from
Closed

WIP: Update to winit 0.29 #3496

wants to merge 0 commits into from

Conversation

fornwall
Copy link
Contributor

@fornwall fornwall commented Oct 23, 2023

This is an early experiment for reference when looking at updating to winit 0.29. A lot of things patched out, broken and not tested, but at least the egui_demo_app starts with both glow and wgpu, as well as pure_glow.

Related PR:s:

@SkillerRaptor
Copy link

Next step is to update egui to raw-window-handle 6.0?

@fornwall
Copy link
Contributor Author

Next step is to update egui to raw-window-handle 6.0?

Yes! We can probably await wgpu getting raw-window-handle 0.6: gfx-rs/wgpu#4202

@SkillerRaptor
Copy link

Yes! We can probably await wgpu getting raw-window-handle 0.6: gfx-rs/wgpu#4202

Wouldn't it make sense to already change it for the main library and the winit crate. It would probably make more sense to make a seperate PR for wgpu and the other egui crates that are dependant on external crates, so that the main crate is not blocked by the wgu implementation, but I would totally understand it if you wanna implement egui-wgpu and the wgpu in eframe at the same time.

Comment on lines 123 to 124
raw-window-handle = { version = "0.5.0" }
winit = { version = "0.28.1", default-features = false }
winit = { version = "0.29.2", default-features = false, features = ["rwh_05"] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In raw-window-metal and ash-window we're also debating how to best support the new feature-based (== additive!) compatibility with multiple raw-window-handle versions in consumer crates. This seems easiest with a trait (as demonstrated in handle producer crates like winit and ndk), but especially in ash-window would also leads to lots of match duplication (unless we convert from one handle to another).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I feel like we should push to just get everyone on board with the latest raw-window-handle ASAP instead of maintaining compatibility with different versions using features across a lot of crates. But don't know how realistic that is :)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a fair point for consumer crates, though things in-between like ash-window and raw-window-metal will have a hard time catering to both sides. eframe likely feels like an in-between crate.

In the case of ash-window I even have to consider when to release the crate if I don't embed backwards compatibility, as there's a direct link to its parent ash crate which is "on the brink of" a semver-breaking release. I either give people a new version of ash-window with compatibility for raw-window-handle 0.6 - and then force the next release of ash 0.38 to only be compatible with raw-window-handle 0.6 - or release ash 0.38 first together with a matching breaking release of ash-window that is still on raw-window-handle 0.5, not giving folks on ash 0.37 a way to upgrade (and they'll have quite some breaking changes to take care of in their code, even if trivially find-replaceable).

@mwcampbell
Copy link
Contributor

accesskit_winit 0.16.0, which has been updated for winit 0.29, is out. Not sure how best to coordinate between this PR and #3475 though.

@ghost
Copy link

ghost commented Nov 7, 2023

Currently this PR causes a black screen on iOS + wgpu on my iPhone 12 when running the egui examples through Xcode and Diawi, although I'm not sure why (they had previously been working). Some iOS testing will probably be needed to fix this.

@torokati44
Copy link
Contributor

#3475 just got merged, so this is now unblocked. Except for the iOS issue above...?

@emilk
Copy link
Owner

emilk commented Nov 13, 2023

Thanks for working on this! I'd like to merge #3172 first though, as it is a huge PR that will cause a bunch of merge conflicts. I'm hoping to merge it some time this week

@emilk emilk added this to the 0.24.0 milestone Nov 16, 2023
@emilk
Copy link
Owner

emilk commented Nov 16, 2023

#3172 has been merged!

@AngelicosPhosphoros
Copy link

AngelicosPhosphoros commented Nov 19, 2023

Thank you for this, I tried to do that myself but quickly found myself overwhelmed by complexity of eframe dependencies.

I think, it would be a good thing to just bring winit 0.29 without attempting to up raw-window-handle to avoid doing to much work at once (as I understand, winit still supports rwh05), and then bump raw-window-handle before release of egui if you would be able to do that. This would allow you still publish usage of winit 0.29 even if you fail to update whole windowing/rendering ecosystem to that.

However, this is just a suggestion, and I am just a user who recently acquainted with the library; it is up to you and maintainers to decide.

@emilk emilk modified the milestones: 0.24.0, 0.25.0 Nov 21, 2023
@emilk
Copy link
Owner

emilk commented Nov 21, 2023

Yes, eframe is a complex beast, and just became more complex still with #3172 (causing all these merge conflicts) 😞

@fornwall fornwall closed this Nov 21, 2023
@torokati44
Copy link
Contributor

Um, was there a git mishap? The commits are gone...

@fornwall fornwall mentioned this pull request Nov 21, 2023
@fornwall
Copy link
Contributor Author

Sorry, got this closed by accident after starting fresh when resolving merge conflicts. Replaced by #3606

@torokati44
Copy link
Contributor

Ah, I see, okay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to winit 0.29
7 participants