-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 wgpu 0.19 and raw-window-handle 0.6 #11280
Conversation
@@ -831,6 +831,8 @@ impl RenderAsset for Image { | |||
let texture = render_device.create_texture_with_data( | |||
render_queue, | |||
&self.texture_descriptor, | |||
// TODO: Is this correct? Do we need to use `MipMajor` if it's a ktx2 file? | |||
wgpu::util::TextureDataOrder::LayerMajor, |
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.
This is a new argument, I provided it with the default value so it should work the same way it did before. :)
We can explore forwarding this configuration to user API in further PRs.
crates/bevy_window/src/raw_handle.rs
Outdated
self.0.window_handle | ||
impl HasWindowHandle for ThreadLockedRawWindowHandleWrapper { | ||
fn window_handle(&self) -> Result<WindowHandle, HandleError> { | ||
// TODO: Unsure if this is the same safety as before |
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.
I'll get this thread started since it's the biggest change here.
I think we still need to use raw handles. I believe you cannot store objects with lifetimes in the world, at least not ones with a shorter lifetime. We may be able to transmute lifetimes to outlive the World, but that would give false safety.
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.
If you wrap winit::window::Window
in an Arc<Window>
, you can share it with the surface while getting a 'static
lifetime without any unsafe
involved.
gfx-rs/wgpu#5044 may also need some work on the wasm side |
Yup, I was just thinking about that earlier. I guess I need to audit every cfg block in bevy_pbr/render/core_pipeline/sprite... edit: Current status of web: webgpu seems to work, webgl2 doesn't (probably need to remove the webgpu feature from wgpu for webgl2 builds for now?) |
Blocked on gfx-rs/wgpu#5088 and getting webgl2 working. |
RustAudio/cpal#820 and katyo/oboe-rs#57 and Elabajaba#2 to build this PR on Android |
You added a new feature but didn't add a description for it. Please update the root Cargo.toml file. |
I've added a (temporary) Currently running into an issue where there's a ton of "error[E0412]: cannot find type |
You added a new feature but didn't add a description for it. Please update the root Cargo.toml file. |
I think it's OK for now, we can work once merged to get them working together
webgpu still needs |
You added a new feature but didn't add a description for it. Please update the root Cargo.toml file. |
1 similar comment
You added a new feature but didn't add a description for it. Please update the root Cargo.toml file. |
Reworked this so that instead of making Currently some examples are broken on webgl2 ( |
…y exclusive feature with webgl2
The generated |
You added a new feature but didn't update the readme. Please run |
Tested this on my phone and android appears to work fine (audio played correctly, paused when minimized, plays when re-focused). |
@@ -831,6 +831,8 @@ impl RenderAsset for Image { | |||
let texture = render_device.create_texture_with_data( | |||
render_queue, | |||
&self.texture_descriptor, | |||
// TODO: Is this correct? Do we need to use `MipMajor` if it's a ktx2 file? |
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.
Yes we do, according to the docs.
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.
::default()
is equivalent to what we were doing before, and I can't visually see a difference with my ktx2 bistro if I change it to mipmajor, but I'm bad at spotting texture bugs.
@@ -193,7 +195,8 @@ fn extract_windows( | |||
} | |||
|
|||
struct SurfaceData { | |||
surface: wgpu::Surface, | |||
// TODO: what lifetime should this be? |
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.
Leaving a note that we have to figure this out.
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.
I think 'static
is what we want for now until we rewrite bevy_winit
to use safe winit APIs and surface creation.
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.
LGTM
I tried a bunch of examples and I didn't find any issues (well, none that weren't already on main) but I only tested on my windows 10 desktop with my 4080. I also only tested vulkan and DX12.
Transparent window is broken on Windows 11, but I don't think it's this PRs fault, I'll update this in a sec. |
# Objective Keep core dependencies up to date. ## Solution Update the dependencies. wgpu 0.19 only supports raw-window-handle (rwh) 0.6, so bumping that was included in this. The rwh 0.6 version bump is just the simplest way of doing it. There might be a way we can take advantage of wgpu's new safe surface creation api, but I'm not familiar enough with bevy's window management to untangle it and my attempt ended up being a mess of lifetimes and rustc complaining about missing trait impls (that were implemented). Thanks to @MiniaczQ for the (much simpler) rwh 0.6 version bump code. Unblocks bevyengine#9172 and bevyengine#10812 ~~This might be blocked on cpal and oboe updating their ndk versions to 0.8, as they both currently target ndk 0.7 which uses rwh 0.5.2~~ Tested on android, and everything seems to work correctly (audio properly stops when minimized, and plays when re-focusing the app). --- ## Changelog - `wgpu` has been updated to 0.19! The long awaited arcanization has been merged (for more info, see https://gfx-rs.github.io/2023/11/24/arcanization.html), and Vulkan should now be working again on Intel GPUs. - Targeting WebGPU now requires that you add the new `webgpu` feature (setting the `RUSTFLAGS` environment variable to `--cfg=web_sys_unstable_apis` is still required). This feature currently overrides the `webgl2` feature if you have both enabled (the `webgl2` feature is enabled by default), so it is not recommended to add it as a default feature to libraries without putting it behind a flag that allows library users to opt out of it! In the future we plan on supporting wasm binaries that can target both webgl2 and webgpu now that wgpu added support for doing so (see bevyengine#11505). - `raw-window-handle` has been updated to version 0.6. ## Migration Guide - `bevy_render::instance_index::get_instance_index()` has been removed as the webgl2 workaround is no longer required as it was fixed upstream in wgpu. The `BASE_INSTANCE_WORKAROUND` shaderdef has also been removed. - WebGPU now requires the new `webgpu` feature to be enabled. The `webgpu` feature currently overrides the `webgl2` feature so you no longer need to disable all default features and re-add them all when targeting `webgpu`, but binaries built with both the `webgpu` and `webgl2` features will only target the webgpu backend, and will only work on browsers that support WebGPU. - Places where you conditionally compiled things for webgl2 need to be updated because of this change, eg: - `#[cfg(any(not(feature = "webgl"), not(target_arch = "wasm32")))]` becomes `#[cfg(any(not(feature = "webgl") ,not(target_arch = "wasm32"), feature = "webgpu"))]` - `#[cfg(all(feature = "webgl", target_arch = "wasm32"))]` becomes `#[cfg(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))]` - `if cfg!(all(feature = "webgl", target_arch = "wasm32"))` becomes `if cfg!(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))` - `create_texture_with_data` now also takes a `TextureDataOrder`. You can probably just set this to `TextureDataOrder::default()` - `TextureFormat`'s `block_size` has been renamed to `block_copy_size` - See the `wgpu` changelog for anything I might've missed: https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md --------- Co-authored-by: François <[email protected]>
The shader code was removed in #11280, but we never cleaned up the rust code.
Objective
Keep core dependencies up to date.
Solution
Update the dependencies.
wgpu 0.19 only supports raw-window-handle (rwh) 0.6, so bumping that was included in this.
The rwh 0.6 version bump is just the simplest way of doing it. There might be a way we can take advantage of wgpu's new safe surface creation api, but I'm not familiar enough with bevy's window management to untangle it and my attempt ended up being a mess of lifetimes and rustc complaining about missing trait impls (that were implemented). Thanks to @MiniaczQ for the (much simpler) rwh 0.6 version bump code.
Unblocks #9172 and #10812
This might be blocked on cpal and oboe updating their ndk versions to 0.8, as they both currently target ndk 0.7 which uses rwh 0.5.2Tested on android, and everything seems to work correctly (audio properly stops when minimized, and plays when re-focusing the app).Changelog
wgpu
has been updated to 0.19! The long awaited arcanization has been merged (for more info, see https://gfx-rs.github.io/2023/11/24/arcanization.html), and Vulkan should now be working again on Intel GPUs.webgpu
feature (setting theRUSTFLAGS
environment variable to--cfg=web_sys_unstable_apis
is still required). This feature currently overrides thewebgl2
feature if you have both enabled (thewebgl2
feature is enabled by default), so it is not recommended to add it as a default feature to libraries without putting it behind a flag that allows library users to opt out of it! In the future we plan on supporting wasm binaries that can target both webgl2 and webgpu now that wgpu added support for doing so (see Support both WebGPU and WebGL2 in the same wasm binary #11505).raw-window-handle
has been updated to version 0.6.Migration Guide
bevy_render::instance_index::get_instance_index()
has been removed as the webgl2 workaround is no longer required as it was fixed upstream in wgpu. TheBASE_INSTANCE_WORKAROUND
shaderdef has also been removed.webgpu
feature to be enabled. Thewebgpu
feature currently overrides thewebgl2
feature so you no longer need to disable all default features and re-add them all when targetingwebgpu
, but binaries built with both thewebgpu
andwebgl2
features will only target the webgpu backend, and will only work on browsers that support WebGPU.#[cfg(any(not(feature = "webgl"), not(target_arch = "wasm32")))]
becomes#[cfg(any(not(feature = "webgl") ,not(target_arch = "wasm32"), feature = "webgpu"))]
#[cfg(all(feature = "webgl", target_arch = "wasm32"))]
becomes#[cfg(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))]
if cfg!(all(feature = "webgl", target_arch = "wasm32"))
becomesif cfg!(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))
create_texture_with_data
now also takes aTextureDataOrder
. You can probably just set this toTextureDataOrder::default()
TextureFormat
'sblock_size
has been renamed toblock_copy_size
wgpu
changelog for anything I might've missed: https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.mdwgpu
now surfaces errors at instance creation time, which may have you run into this error (we've also seen it with nsight instead of EOSOverlay):It just means that the program didn't properly cleanup their registry keys on an update/uninstall, and vulkan uses those keys to load validation layers. The fix is to backup your registry, then remove the offending keys in
"Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Khronos\Vulkan\ImplicitLayers"
.