report shader processing errors in RenderPipelineCache (#3289)

### Problem
- shader processing errors are not displayed
- during hot reloading when encountering a shader with errors, the whole app crashes

### Solution
- log `error!`s for shader processing errors 
- when `cfg(debug_assertions)` is enabled (i.e. you're running in `debug` mode), parse shaders before passing them to wgpu. This lets us handle errors early.
This commit is contained in:
Jakob Hellermann 2021-12-22 22:16:42 +00:00
parent 6c479649bf
commit 081350916c
3 changed files with 83 additions and 10 deletions

View file

@ -38,6 +38,7 @@ image = { version = "0.23.12", default-features = false }
# misc
wgpu = { version = "0.12.0", features = ["spirv"] }
codespan-reporting = "0.11.0"
naga = { version = "0.8.0", features = ["glsl-in", "spv-in", "spv-out", "wgsl-in", "wgsl-out"] }
serde = { version = "1", features = ["derive"] }
bitflags = "1.2.1"

View file

@ -2,7 +2,7 @@ use crate::{
render_resource::{
AsModuleDescriptorError, BindGroupLayout, BindGroupLayoutId, ProcessShaderError,
RawFragmentState, RawRenderPipelineDescriptor, RawVertexState, RenderPipeline,
RenderPipelineDescriptor, Shader, ShaderImport, ShaderProcessor,
RenderPipelineDescriptor, Shader, ShaderImport, ShaderProcessor, ShaderReflectError,
},
renderer::RenderDevice,
RenderWorld,
@ -10,11 +10,13 @@ use crate::{
use bevy_app::EventReader;
use bevy_asset::{AssetEvent, Assets, Handle};
use bevy_ecs::system::{Res, ResMut};
use bevy_utils::{HashMap, HashSet};
use bevy_utils::{tracing::error, HashMap, HashSet};
use std::{collections::hash_map::Entry, hash::Hash, ops::Deref, sync::Arc};
use thiserror::Error;
use wgpu::{PipelineLayoutDescriptor, ShaderModule, VertexBufferLayout};
use super::ProcessedShader;
#[derive(Default)]
pub struct ShaderData {
pipelines: HashSet<CachedPipelineId>,
@ -52,7 +54,16 @@ impl ShaderCache {
.get(handle)
.ok_or_else(|| RenderPipelineError::ShaderNotLoaded(handle.clone_weak()))?;
let data = self.data.entry(handle.clone_weak()).or_default();
if shader.imports().len() != data.resolved_imports.len() {
let n_asset_imports = shader
.imports()
.filter(|import| matches!(import, ShaderImport::AssetPath(_)))
.count();
let n_resolved_asset_imports = data
.resolved_imports
.keys()
.filter(|import| matches!(import, ShaderImport::AssetPath(_)))
.count();
if n_asset_imports != n_resolved_asset_imports {
return Err(RenderPipelineError::ShaderImportNotYetAvailable);
}
@ -68,7 +79,12 @@ impl ShaderCache {
&self.shaders,
&self.import_path_shaders,
)?;
let module_descriptor = processed.get_module_descriptor()?;
let module_descriptor = match processed.get_module_descriptor() {
Ok(module_descriptor) => module_descriptor,
Err(err) => {
return Err(RenderPipelineError::AsModuleDescriptorError(err, processed));
}
};
entry.insert(Arc::new(
render_device.create_shader_module(&module_descriptor),
))
@ -206,8 +222,8 @@ pub enum RenderPipelineError {
ShaderNotLoaded(Handle<Shader>),
#[error(transparent)]
ProcessShaderError(#[from] ProcessShaderError),
#[error(transparent)]
AsModuleDescriptorError(#[from] AsModuleDescriptorError),
#[error("{0}")]
AsModuleDescriptorError(AsModuleDescriptorError, ProcessedShader),
#[error("Shader import not yet available.")]
ShaderImportNotYetAvailable,
}
@ -274,9 +290,13 @@ impl RenderPipelineCache {
match err {
RenderPipelineError::ShaderNotLoaded(_)
| RenderPipelineError::ShaderImportNotYetAvailable => { /* retry */ }
RenderPipelineError::ProcessShaderError(_)
| RenderPipelineError::AsModuleDescriptorError(_) => {
// shader could not be processed ... retrying won't help
// shader could not be processed ... retrying won't help
RenderPipelineError::ProcessShaderError(err) => {
error!("failed to process shader: {}", err);
continue;
}
RenderPipelineError::AsModuleDescriptorError(err, source) => {
log_shader_error(source, err);
continue;
}
}
@ -386,3 +406,47 @@ impl RenderPipelineCache {
}
}
}
fn log_shader_error(source: &ProcessedShader, err: &AsModuleDescriptorError) {
use codespan_reporting::{
diagnostic::{Diagnostic, Label},
files::SimpleFile,
term,
};
if let ProcessedShader::Wgsl(source) = source {
if let AsModuleDescriptorError::ShaderReflectError(err) = err {
match err {
ShaderReflectError::WgslParse(parse) => {
let msg = parse.emit_to_string(source);
error!("failed to process shader:\n{}", msg);
}
ShaderReflectError::Validation(error) => {
let files = SimpleFile::new("wgsl", &source);
let config = term::Config::default();
let mut writer = term::termcolor::Ansi::new(Vec::new());
let diagnostic = Diagnostic::error().with_labels(
error
.spans()
.map(|(span, desc)| {
Label::primary((), span.to_range().unwrap())
.with_message(desc.to_owned())
})
.collect(),
);
term::emit(&mut writer, &config, &files, &diagnostic)
.expect("cannot write error");
let msg = writer.into_inner();
let msg = String::from_utf8_lossy(&msg);
error!("failed to process shader: \n{}", msg);
}
ShaderReflectError::GlslParse(_) => {}
ShaderReflectError::SpirVParse(_) => {}
}
}
}
}

View file

@ -156,7 +156,15 @@ impl ProcessedShader {
Ok(ShaderModuleDescriptor {
label: None,
source: match self {
ProcessedShader::Wgsl(source) => ShaderSource::Wgsl(source.clone()),
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.
let _ = self.reflect()?;
ShaderSource::Wgsl(source.clone())
}
ProcessedShader::Glsl(_source, _stage) => {
let reflection = self.reflect()?;
// TODO: it probably makes more sense to convert this to spirv, but as of writing