Skip to content

Commit 24c9f51

Browse files
jakobhellermannItsDoot
authored andcommitted
use error scope to handle errors on shader module creation (bevyengine#3675)
related: bevyengine#3289 In addition to validating shaders early when debug assertions are enabled, use the new [error scopes](https://gpuweb.github.io/gpuweb/#error-scopes) API when creating a shader module. I chose to keep the early validation (and thereby parsing twice) when debug assertions are enabled in, because it lets as handle errors ourselves and display them with pretty colors, while the error scopes API just gives us a string we can display. This change pulls in `futures-util` as a new dependency for `future.now_or_never()`. I can inline that part of futures-lite into `bevy_render` to keep the compilation time lower if that's preferred.
1 parent 32bdec2 commit 24c9f51

File tree

4 files changed

+61
-6
lines changed

4 files changed

+61
-6
lines changed

crates/bevy_render/src/render_resource/pipeline_cache.rs

+24-3
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,24 @@ impl ShaderCache {
129129
return Err(PipelineCacheError::AsModuleDescriptorError(err, processed));
130130
}
131131
};
132-
entry.insert(Arc::new(
133-
render_device.create_shader_module(&module_descriptor),
134-
))
132+
133+
render_device
134+
.wgpu_device()
135+
.push_error_scope(wgpu::ErrorFilter::Validation);
136+
let shader_module = render_device.create_shader_module(&module_descriptor);
137+
let error = render_device.wgpu_device().pop_error_scope();
138+
139+
// `now_or_never` will return Some if the future is ready and None otherwise.
140+
// On native platforms, wgpu will yield the error immediatly while on wasm it may take longer since the browser APIs are asynchronous.
141+
// So to keep the complexity of the ShaderCache low, we will only catch this error early on native platforms,
142+
// and on wasm the error will be handled by wgpu and crash the application.
143+
if let Some(Some(wgpu::Error::Validation { description, .. })) =
144+
bevy_utils::futures::now_or_never(error)
145+
{
146+
return Err(PipelineCacheError::CreateShaderModule(description));
147+
}
148+
149+
entry.insert(Arc::new(shader_module))
135150
}
136151
};
137152

@@ -479,6 +494,10 @@ impl PipelineCache {
479494
log_shader_error(source, err);
480495
continue;
481496
}
497+
PipelineCacheError::CreateShaderModule(description) => {
498+
error!("failed to create shader module: {}", description);
499+
continue;
500+
}
482501
}
483502
}
484503
}
@@ -626,6 +645,8 @@ pub enum PipelineCacheError {
626645
AsModuleDescriptorError(AsModuleDescriptorError, ProcessedShader),
627646
#[error("Shader import not yet available.")]
628647
ShaderImportNotYetAvailable,
648+
#[error("Could not create shader module: {0}")]
649+
CreateShaderModule(String),
629650
}
630651

631652
struct ErrorSources<'a> {

crates/bevy_render/src/render_resource/shader.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,9 @@ impl ProcessedShader {
161161
source: match self {
162162
ProcessedShader::Wgsl(source) => {
163163
#[cfg(debug_assertions)]
164-
// This isn't neccessary, but catches errors early during hot reloading of invalid wgsl shaders.
165-
// Eventually, wgpu will have features that will make this unneccessary like compilation info
166-
// or error scopes, but until then parsing the shader twice during development the easiest solution.
164+
// Parse and validate the shader early, so that (e.g. while hot reloading) we can
165+
// display nicely formatted error messages instead of relying on just displaying the error string
166+
// returned by wgpu upon creating the shader module.
167167
let _ = self.reflect()?;
168168

169169
ShaderSource::Wgsl(source.clone())

crates/bevy_utils/src/futures.rs

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
use std::{
2+
future::Future,
3+
pin::Pin,
4+
task::{Context, Poll, RawWaker, RawWakerVTable, Waker},
5+
};
6+
7+
pub fn now_or_never<F: Future>(mut future: F) -> Option<F::Output> {
8+
let noop_waker = noop_waker();
9+
let mut cx = Context::from_waker(&noop_waker);
10+
11+
// Safety: `future` is not moved and the original value is shadowed
12+
let future = unsafe { Pin::new_unchecked(&mut future) };
13+
14+
match future.poll(&mut cx) {
15+
Poll::Ready(x) => Some(x),
16+
_ => None,
17+
}
18+
}
19+
20+
unsafe fn noop_clone(_data: *const ()) -> RawWaker {
21+
noop_raw_waker()
22+
}
23+
unsafe fn noop(_data: *const ()) {}
24+
25+
const NOOP_WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new(noop_clone, noop, noop, noop);
26+
27+
fn noop_raw_waker() -> RawWaker {
28+
RawWaker::new(std::ptr::null(), &NOOP_WAKER_VTABLE)
29+
}
30+
31+
fn noop_waker() -> Waker {
32+
unsafe { Waker::from_raw(noop_raw_waker()) }
33+
}

crates/bevy_utils/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ pub mod prelude {
22
pub use crate::default;
33
}
44

5+
pub mod futures;
56
pub mod label;
67

78
mod default;

0 commit comments

Comments
 (0)