-
Notifications
You must be signed in to change notification settings - Fork 962
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
[d3d12 wgl] Upgrade to windows 0.59
crates
#6876
base: trunk
Are you sure you want to change the base?
Conversation
unsafe { OpenGL::wglMakeCurrent(None, None) } | ||
unsafe { OpenGL::wglMakeCurrent(Default::default(), Default::default()) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kennykerr can you help us here? I did not do my due diligence this time to search through windows-rs
and win32metadata
for relevant changes, but while most handles have been made optional (see other Some()
wrapping changes in this PR), quite strangely these parameters are no longer optional in wglMakeCurrent()
where they have a specific use-case to uncurrent the context.
Is this change intended or accidental? We can clearly get around it by passing a defaulted HDC
/HGLRC
, but it seems strange when compared to other functions where None
can be passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code generator now faithfully honors the metadata's [Optional]
attribute - in the case of wglMakeCurrent
the handle parameters are not optional whereas in the case of CreateSwapChainForCompositionSurfaceHandle
the handle parameter is optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kennykerr. So this'll take another trip through win32metadata
to annotate correctly.
https://github.com/microsoft/windows-rs/releases/tag/0.61.0 The latest `windows 0.59` and `windows-core 0.59` crates were just released (strangely tagged `0.61`), including some minor code improvements for us. The MSRV has been bumped to `1.74`, but `wgpu` is already on `1.76` anyway.
7841fa7
to
d9cd506
Compare
Just to set expectations here, we likely can't merge this for a bit, as firefox is currently on 0.58 and migration will be a bit of a thing. @ErichDonGubler will take charge of this. I don't expect there to be a ton of problems with this PR just hanging out for a while. I'm going to mark this as draft as we can't merge it, but @Wumpf will review it still. |
@cwfitzgerald that's all good, I don't expect this to be merged until the upgrade has fully trickled through the ecosystem. Just making sure we have a PR open with all the necessary changes that I couldn't make in the original |
There are a few things that I'm already aware need to happen in
|
Regarding |
Under the assumption that windows-sys = { version = ">=0.58, <=0.59" See quinn-rs/quinn#2021 as an example from one of our upstream dependencies. @ErichDonGubler let me know what works best for you. |
Connections
Depends on Traverse-Research/gpu-allocator#258 and Xudong-Huang/generator-rs#72.
https://github.com/microsoft/windows-rs/releases/tag/0.61.0
Description
The latest
windows 0.59
andwindows-core 0.59
crates were just released (strangely tagged0.61
), including some minor code improvements for us. The MSRV has been bumped to1.74
, butwgpu
is already on1.76
anyway.Testing
Not yet, only cross-compiled.
Checklist
cargo fmt
.taplo format
.cargo clippy
. If applicable, add:--target wasm32-unknown-unknown
--target wasm32-unknown-emscripten
cargo xtask test
to run tests.CHANGELOG.md
. See simple instructions inside file.