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

Upgrade winit 0.29.2 → 0.30.0 #1678

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

ErichDonGubler
Copy link
Contributor

@ErichDonGubler ErichDonGubler commented May 14, 2024

  • 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: I've updated examples, which I think might be sufficient?

Resolves #1674.

@ErichDonGubler

This comment was marked as outdated.

@ErichDonGubler ErichDonGubler force-pushed the winit-0.30 branch 3 times, most recently from a01c46a to bf431b9 Compare May 14, 2024 20:33
@MarijnS95 MarijnS95 mentioned this pull request May 26, 2024
4 tasks
@MarijnS95
Copy link
Member

The changes in glutin-winit's lib.rs look identical to #1675 minus some GlutinEventLoop trait to support the older EventLoop as argument to create_display().

Both PRs are refactoring the examples in a completely different way though, and I lean towards what you're doing here by loosing less code and maintaining more of the original architecture. But it remains hard to review both PRs as they (inevitably) move a lot of code around in the diff.

@ErichDonGubler
Copy link
Contributor Author

@MarijnS95: I strongly recommend going through these changes commit-by-commit, rather than looking through the overall diff. I've striven to make each as small as possible.

@marc2332
Copy link
Contributor

This PR examples look better indeed, do you want to maybe merge your PR against mine? So mine goes with your examples

Sounds good? :-)

@MarijnS95
Copy link
Member

@ErichDonGubler since you already expressed that you didn't expect this PR to get merged in favour of the other one, I didn't give this PR (nor the other one) more than a quick scroll-through.

Since your PR history is nice and readable, I propose to integrate that in one of the two branches.

@marc2332
Copy link
Contributor

marc2332 commented May 29, 2024

@ErichDonGubler Do you want me to just copy your examples straight into my PR or do you want to maybe rebase this PR on mine with my core changes and your examples so we can merge it into mine?

@ErichDonGubler

This comment was marked as resolved.

@ErichDonGubler ErichDonGubler force-pushed the winit-0.30 branch 2 times, most recently from 6501830 to 6fa9330 Compare May 31, 2024 21:04
@ErichDonGubler

This comment was marked as resolved.

@ErichDonGubler ErichDonGubler marked this pull request as ready for review May 31, 2024 21:30
@ErichDonGubler
Copy link
Contributor Author

@marc2332: I'll leave the choice up to you whether you want to force-push this branch's history onto yours at #1675, or just abandon it and get a fresh round of @MarijnS95's feedback here. I'm willing to handle feedback if it's left open here, in case that helps. 🙂

Co-Authored-By: marc2332 <[email protected]>
Co-Authored-By: Kirill Chibisov <[email protected]>
Co-Authored-By: Marijn Suijten <[email protected]>
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

I've Incorporated all the changes from #1675 and applied some minor style/doc fixes.

@marc2332
Copy link
Contributor

marc2332 commented Jun 6, 2024

I've Incorporated all the changes from #1675 and applied some minor style/doc fixes.

Not sure why you did that, I already had his changes in my PR 🤔

@kchibisov kchibisov merged commit e671fbe into rust-windowing:master Jun 6, 2024
43 checks passed
MarijnS95 added a commit to MarijnS95/glutin that referenced this pull request Jul 2, 2024
When testing the `glutin` example on Android, after suspending and
resuming the app no triangle appears.  As it turns out the `winit 0.30`
bump in rust-windowing#1678 never reuses the `not_current_context` that was set apart
in `suspend()`, but creates a new one, without re-running initialization
code such as creating the `Renderer`.

Partially restore original code before the `winit 0.30` bump, which
never created a context in `resume()` because it was already available
before the loop started (and moved back in place in `suspend()`). Note
also that more initialization code that now (unnecessarily) lives in
`resume()` used to be outside of the loop.
MarijnS95 added a commit to MarijnS95/glutin that referenced this pull request Jul 2, 2024
When testing the `glutin` example on Android, after suspending and
resuming the app no triangle appears.  As it turns out the `winit 0.30`
bump in rust-windowing#1678 never reuses the `not_current_context` that was set apart
in `suspend()`, but creates a new one, without re-running initialization
code such as creating the `Renderer`.

Partially restore original code before the `winit 0.30` bump, which
never created a context in `resume()` because it was already available
before the loop started (and moved back in place in `suspend()`). Note
also that more initialization code that now (unnecessarily) lives in
`resume()` used to be outside of the loop.
MarijnS95 added a commit to MarijnS95/glutin that referenced this pull request Jul 3, 2024
When testing the `glutin` example on Android, after suspending and
resuming the app no triangle appears.  As it turns out the `winit 0.30`
bump in rust-windowing#1678 never reuses the `not_current_context` that was set apart
in `suspend()`, but creates a new one, without re-running initialization
code such as creating the `Renderer`.

Partially restore original code before the `winit 0.30` bump, which
never created a context in `resume()` because it was already available
before the loop started (and moved back in place in `suspend()`). Note
also that more initialization code that now (unnecessarily) lives in
`resume()` used to be outside of the loop.
kchibisov pushed a commit that referenced this pull request Jul 3, 2024
When testing the `glutin` example on Android, after suspending and
resuming the app no triangle appears.  As it turns out the `winit 0.30`
bump in #1678 never reuses the `not_current_context` that was set apart
in `suspend()`, but creates a new one, without re-running initialization
code such as creating the `Renderer`.

Partially restore original code before the `winit 0.30` bump, which
never created a context in `resume()` because it was already available
before the loop started (and moved back in place in `suspend()`). Note
also that more initialization code that now (unnecessarily) lives in
`resume()` used to be outside of the loop.
kchibisov pushed a commit that referenced this pull request Sep 10, 2024
When testing the `glutin` example on Android, after suspending and
resuming the app no triangle appears.  As it turns out the `winit 0.30`
bump in #1678 never reuses the `not_current_context` that was set apart
in `suspend()`, but creates a new one, without re-running initialization
code such as creating the `Renderer`.

Partially restore original code before the `winit 0.30` bump, which
never created a context in `resume()` because it was already available
before the loop started (and moved back in place in `suspend()`). Note
also that more initialization code that now (unnecessarily) lives in
`resume()` used to be outside of the loop.
kchibisov pushed a commit that referenced this pull request Sep 11, 2024
When testing the `glutin` example on Android, after suspending and
resuming the app no triangle appears.  As it turns out the `winit 0.30`
bump in #1678 never reuses the `not_current_context` that was set apart
in `suspend()`, but creates a new one, without re-running initialization
code such as creating the `Renderer`.

Partially restore original code before the `winit 0.30` bump, which
never created a context in `resume()` because it was already available
before the loop started (and moved back in place in `suspend()`). Note
also that more initialization code that now (unnecessarily) lives in
`resume()` used to be outside of the loop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Support winit v0.30
4 participants