Skip to content

Commit

Permalink
use error scope to handle errors on shader module creation (bevyengin…
Browse files Browse the repository at this point in the history
…e#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.
  • Loading branch information
jakobhellermann authored and aevyrie committed Jun 7, 2022
1 parent b4ea4d5 commit 1da341a
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 6 deletions.
27 changes: 24 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, .. })) =
bevy_utils::futures::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 Down
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
33 changes: 33 additions & 0 deletions crates/bevy_utils/src/futures.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use std::{
future::Future,
pin::Pin,
task::{Context, Poll, RawWaker, RawWakerVTable, Waker},
};

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

// Safety: `future` is not moved and the original value is shadowed
let future = unsafe { Pin::new_unchecked(&mut 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()) }
}
1 change: 1 addition & 0 deletions crates/bevy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ pub mod prelude {
pub use crate::default;
}

pub mod futures;
pub mod label;

mod default;
Expand Down

0 comments on commit 1da341a

Please sign in to comment.