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

[Merged by Bors] - use error scope to handle errors on shader module creation #3675

Closed
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 58 additions & 3 deletions crates/bevy_render/src/render_resource/pipeline_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,24 @@ impl ShaderCache {
return Err(PipelineCacheError::AsModuleDescriptorError(err, processed));
}
};
entry.insert(Arc::new(
render_device.create_shader_module(&module_descriptor),
))

render_device
.wgpu_device()
.push_error_scope(wgpu::ErrorFilter::Validation);
let shader_module = render_device.create_shader_module(&module_descriptor);
let error = render_device.wgpu_device().pop_error_scope();

// `now_or_never` will return Some if the future is ready and None otherwise.
// On native platforms, wgpu will yield the error immediatly while on wasm it may take longer since the browser APIs are asynchronous.
// So to keep the complexity of the ShaderCache low, we will only catch this error early on native platforms,
// and on wasm the error will be handled by wgpu and crash the application.
if let Some(Some(wgpu::Error::Validation { description, .. })) =
futures_helper::now_or_never(error)
{
return Err(PipelineCacheError::CreateShaderModule(description));
}

entry.insert(Arc::new(shader_module))
}
};

Expand Down Expand Up @@ -479,6 +494,10 @@ impl PipelineCache {
log_shader_error(source, err);
continue;
}
PipelineCacheError::CreateShaderModule(description) => {
error!("failed to create shader module: {}", description);
continue;
}
}
}
}
Expand Down Expand Up @@ -626,6 +645,8 @@ pub enum PipelineCacheError {
AsModuleDescriptorError(AsModuleDescriptorError, ProcessedShader),
#[error("Shader import not yet available.")]
ShaderImportNotYetAvailable,
#[error("Could not create shader module: {0}")]
CreateShaderModule(String),
}

struct ErrorSources<'a> {
Expand All @@ -649,3 +670,37 @@ impl<'a> Iterator for ErrorSources<'a> {
current
}
}

mod futures_helper {
jakobhellermann marked this conversation as resolved.
Show resolved Hide resolved
use std::{
future::Future,
task::{Context, Poll, RawWaker, RawWakerVTable, Waker},
};

pub fn now_or_never<F: Future>(future: F) -> Option<F::Output> {
let noop_waker = noop_waker();
let mut cx = Context::from_waker(&noop_waker);

futures_lite::pin!(future);
match future.poll(&mut cx) {
Poll::Ready(x) => Some(x),
_ => None,
}
}

unsafe fn noop_clone(_data: *const ()) -> RawWaker {
noop_raw_waker()
}

unsafe fn noop(_data: *const ()) {}

const NOOP_WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new(noop_clone, noop, noop, noop);

fn noop_raw_waker() -> RawWaker {
RawWaker::new(std::ptr::null(), &NOOP_WAKER_VTABLE)
}

fn noop_waker() -> Waker {
unsafe { Waker::from_raw(noop_raw_waker()) }
}
}
6 changes: 3 additions & 3 deletions crates/bevy_render/src/render_resource/shader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ impl ProcessedShader {
source: match self {
ProcessedShader::Wgsl(source) => {
#[cfg(debug_assertions)]
// This isn't neccessary, but catches errors early during hot reloading of invalid wgsl shaders.
// Eventually, wgpu will have features that will make this unneccessary like compilation info
// or error scopes, but until then parsing the shader twice during development the easiest solution.
// Parse and validate the shader early, so that (e.g. while hot reloading) we can
// display nicely formatted error messages instead of relying on just displaying the error string
// returned by wgpu upon creating the shader module.
let _ = self.reflect()?;

ShaderSource::Wgsl(source.clone())
Expand Down