Upgrade to wgpu 27 and associated crates #21746
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -195,6 +195,9 @@ pub async fn initialize_renderer( | |
| }, | ||
| dx12: wgpu::Dx12BackendOptions { | ||
| shader_compiler: options.dx12_shader_compiler.clone(), | ||
| presentation_system: wgpu::wgt::Dx12SwapchainKind::from_env().unwrap_or_default(), | ||
| latency_waitable_object: wgpu::wgt::Dx12UseFrameLatencyWaitableObject::from_env() | ||
| .unwrap_or_default(), | ||
| }, | ||
| noop: wgpu::NoopBackendOptions { enable: false }, | ||
| }, | ||
|
|
@@ -431,14 +434,30 @@ pub async fn initialize_renderer( | |
| max_subgroup_size: limits | ||
| .max_subgroup_size | ||
| .min(constrained_limits.max_subgroup_size), | ||
| max_acceleration_structures_per_shader_stage: limits | ||
| .max_acceleration_structures_per_shader_stage | ||
| .min(constrained_limits.max_acceleration_structures_per_shader_stage), | ||
| max_task_workgroup_total_count: limits | ||
| .max_task_workgroup_total_count | ||
| .min(constrained_limits.max_task_workgroup_total_count), | ||
| max_task_workgroups_per_dimension: limits | ||
| .max_task_workgroups_per_dimension | ||
| .min(constrained_limits.max_task_workgroups_per_dimension), | ||
| max_mesh_output_layers: limits | ||
| .max_mesh_output_layers | ||
| .min(constrained_limits.max_mesh_output_layers), | ||
| max_mesh_multiview_count: limits | ||
| .max_mesh_multiview_count | ||
| .min(constrained_limits.max_mesh_multiview_count), | ||
| }; | ||
| } | ||
|
|
||
| let device_descriptor = wgpu::DeviceDescriptor { | ||
| label: options.device_label.as_ref().map(AsRef::as_ref), | ||
| required_features: features, | ||
| required_limits: limits, | ||
| // SAFETY: We're ok using experimental wgpu features in Bevy | ||
|
Contributor
There was a problem hiding this comment. Note that experimental features are not just features that may not work properly — they are features that can cause undefined behavior, which is why this is Regardless of the answer to that question, this
Contributor
Author
There was a problem hiding this comment. In Bevy, we enable all wgpu features by default. Then, specific plugins can check if certain features are enabled or not, and either say the plugin is not supported on this platform or fallback to other options. I don't think there's much of a use case for limiting experimental features at the device level. If you don't want e.g. raytracing to be available, you just shouldn't use Bevy plugins that depend on raytracing.
Contributor
There was a problem hiding this comment.
This is a (somewhat atypical) case of the general Rust principle that it should not be possible to cause UB via calling safe functions. “The user can just not call the function” is not considered sufficient in Rust. [cc @cwfitzgerald who implemented this requirement in #8163]
Contributor
Author
There was a problem hiding this comment.
Generally this is done via feature flags in Bevy. For instance bevy_solari is not part of DefaultPlugins - you need to specifically compile with the feature for Bevy's raytracing functionality. But we don't gate it at the device level, and I feel strongly that this is a good design choice for us, as otherwise it requires specific plugin setup ordering.
Contributor
There was a problem hiding this comment. “Can cause undefined behavior from safe code” makes something not a good design choice. Avoiding that situation is the whole point of Rust. Please don't do this. I speak both as a contributor to
Contributor
There was a problem hiding this comment. Oh, yeah, I mean, it can be called "unsafe_wgpu_experimental", I was more wondering if having a feature would be enough for this. The actual name of that feature I don't really mind. Makes sense to have unsafe in the name I guess. I'm not sure what you mean in 1.? Isn't using a feature flag exposing the decision to use the unsafe api to the user already? I guess I never specified but I was assuming the feature would not be part of the default features so a user would have to specify it.
Contributor
There was a problem hiding this comment.
Features don’t have “the” user. A feature is enabled if any dependent enables it. That dependent could be a random plugin library which enables it "because we need it" without documenting that fact, leaving the application developer unaware that the risk is being taken. (I admit this gets into “well, people can write bad code no matter what” territory.)
Contributor
There was a problem hiding this comment. Ah, right, I see what you mean. I'm not sure how we can avoid that though. Even if we used some kind of runtime check instead of feature flag a plugin could still set it without a user knowing. Right now I can't think of a way to make sure a user is always aware that they are opting into it. Other than maybe logging a warning at the start if it's set to make sure users are at least aware of it. But then people will likely want a way to hide that warning and then we are back to plugins hiding it unknowingly 🙃.
Contributor
Author
There was a problem hiding this comment. My perspective is that 99% of users are not using experimental wgpu features directly. The only real people using them are developers of rendering plugins. Bevy users should be able to trust that plugin authors used GPU features correctly, and not have to deal with enabling experimental flags themselves just to use a plugin. We already disable stuff like checked shader code and indirect draw call validation in Bevy, it's trivial to misuse GPU APIs if you really want to.
Member
There was a problem hiding this comment.
Not advocating this necessarily, but it could be a flag you have to set on app, which plugins don't control. We could also have an unsafe plugin API which doesn't solve your plugins adding plugins but could help.
IMO this is dangerously close to the "git gud" argument used by C++ partisans against safety in general. If anything, the fact that GPU APIs are easy to misuse makes safety more important. I also think it's important to distinguish UB on the GPU which sucks but is mostly limited to DoS and UB within the driver which is much more classically UB scary. I take your point that the use of these features is already behind a flag and explicitly noted as experimental, but I think there's a tension here between having experimental features in tree (which inherently get a bit less review and testing, fewer developers working on them, etc) and safety. I think an additional flag is a good practical middle ground. But minimally, whatever rationale we choose should be documented here more explicitly even if we are just okay blanket enabling this. |
||
| experimental_features: unsafe { wgpu::ExperimentalFeatures::enabled() }, | ||
| memory_hints: options.memory_hints.clone(), | ||
| // See https://github.com/gfx-rs/wgpu/issues/5974 | ||
| trace: Trace::Off, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.