Skip to content

Commit 0813509

Browse files
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.
1 parent 6c47964 commit 0813509

File tree

3 files changed

+83
-10
lines changed

3 files changed

+83
-10
lines changed

crates/bevy_render/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ image = { version = "0.23.12", default-features = false }
3838

3939
# misc
4040
wgpu = { version = "0.12.0", features = ["spirv"] }
41+
codespan-reporting = "0.11.0"
4142
naga = { version = "0.8.0", features = ["glsl-in", "spv-in", "spv-out", "wgsl-in", "wgsl-out"] }
4243
serde = { version = "1", features = ["derive"] }
4344
bitflags = "1.2.1"

crates/bevy_render/src/render_resource/pipeline_cache.rs

Lines changed: 73 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,21 @@ use crate::{
22
render_resource::{
33
AsModuleDescriptorError, BindGroupLayout, BindGroupLayoutId, ProcessShaderError,
44
RawFragmentState, RawRenderPipelineDescriptor, RawVertexState, RenderPipeline,
5-
RenderPipelineDescriptor, Shader, ShaderImport, ShaderProcessor,
5+
RenderPipelineDescriptor, Shader, ShaderImport, ShaderProcessor, ShaderReflectError,
66
},
77
renderer::RenderDevice,
88
RenderWorld,
99
};
1010
use bevy_app::EventReader;
1111
use bevy_asset::{AssetEvent, Assets, Handle};
1212
use bevy_ecs::system::{Res, ResMut};
13-
use bevy_utils::{HashMap, HashSet};
13+
use bevy_utils::{tracing::error, HashMap, HashSet};
1414
use std::{collections::hash_map::Entry, hash::Hash, ops::Deref, sync::Arc};
1515
use thiserror::Error;
1616
use wgpu::{PipelineLayoutDescriptor, ShaderModule, VertexBufferLayout};
1717

18+
use super::ProcessedShader;
19+
1820
#[derive(Default)]
1921
pub struct ShaderData {
2022
pipelines: HashSet<CachedPipelineId>,
@@ -52,7 +54,16 @@ impl ShaderCache {
5254
.get(handle)
5355
.ok_or_else(|| RenderPipelineError::ShaderNotLoaded(handle.clone_weak()))?;
5456
let data = self.data.entry(handle.clone_weak()).or_default();
55-
if shader.imports().len() != data.resolved_imports.len() {
57+
let n_asset_imports = shader
58+
.imports()
59+
.filter(|import| matches!(import, ShaderImport::AssetPath(_)))
60+
.count();
61+
let n_resolved_asset_imports = data
62+
.resolved_imports
63+
.keys()
64+
.filter(|import| matches!(import, ShaderImport::AssetPath(_)))
65+
.count();
66+
if n_asset_imports != n_resolved_asset_imports {
5667
return Err(RenderPipelineError::ShaderImportNotYetAvailable);
5768
}
5869

@@ -68,7 +79,12 @@ impl ShaderCache {
6879
&self.shaders,
6980
&self.import_path_shaders,
7081
)?;
71-
let module_descriptor = processed.get_module_descriptor()?;
82+
let module_descriptor = match processed.get_module_descriptor() {
83+
Ok(module_descriptor) => module_descriptor,
84+
Err(err) => {
85+
return Err(RenderPipelineError::AsModuleDescriptorError(err, processed));
86+
}
87+
};
7288
entry.insert(Arc::new(
7389
render_device.create_shader_module(&module_descriptor),
7490
))
@@ -206,8 +222,8 @@ pub enum RenderPipelineError {
206222
ShaderNotLoaded(Handle<Shader>),
207223
#[error(transparent)]
208224
ProcessShaderError(#[from] ProcessShaderError),
209-
#[error(transparent)]
210-
AsModuleDescriptorError(#[from] AsModuleDescriptorError),
225+
#[error("{0}")]
226+
AsModuleDescriptorError(AsModuleDescriptorError, ProcessedShader),
211227
#[error("Shader import not yet available.")]
212228
ShaderImportNotYetAvailable,
213229
}
@@ -274,9 +290,13 @@ impl RenderPipelineCache {
274290
match err {
275291
RenderPipelineError::ShaderNotLoaded(_)
276292
| RenderPipelineError::ShaderImportNotYetAvailable => { /* retry */ }
277-
RenderPipelineError::ProcessShaderError(_)
278-
| RenderPipelineError::AsModuleDescriptorError(_) => {
279-
// shader could not be processed ... retrying won't help
293+
// shader could not be processed ... retrying won't help
294+
RenderPipelineError::ProcessShaderError(err) => {
295+
error!("failed to process shader: {}", err);
296+
continue;
297+
}
298+
RenderPipelineError::AsModuleDescriptorError(err, source) => {
299+
log_shader_error(source, err);
280300
continue;
281301
}
282302
}
@@ -386,3 +406,47 @@ impl RenderPipelineCache {
386406
}
387407
}
388408
}
409+
410+
fn log_shader_error(source: &ProcessedShader, err: &AsModuleDescriptorError) {
411+
use codespan_reporting::{
412+
diagnostic::{Diagnostic, Label},
413+
files::SimpleFile,
414+
term,
415+
};
416+
417+
if let ProcessedShader::Wgsl(source) = source {
418+
if let AsModuleDescriptorError::ShaderReflectError(err) = err {
419+
match err {
420+
ShaderReflectError::WgslParse(parse) => {
421+
let msg = parse.emit_to_string(source);
422+
error!("failed to process shader:\n{}", msg);
423+
}
424+
ShaderReflectError::Validation(error) => {
425+
let files = SimpleFile::new("wgsl", &source);
426+
let config = term::Config::default();
427+
let mut writer = term::termcolor::Ansi::new(Vec::new());
428+
429+
let diagnostic = Diagnostic::error().with_labels(
430+
error
431+
.spans()
432+
.map(|(span, desc)| {
433+
Label::primary((), span.to_range().unwrap())
434+
.with_message(desc.to_owned())
435+
})
436+
.collect(),
437+
);
438+
439+
term::emit(&mut writer, &config, &files, &diagnostic)
440+
.expect("cannot write error");
441+
442+
let msg = writer.into_inner();
443+
let msg = String::from_utf8_lossy(&msg);
444+
445+
error!("failed to process shader: \n{}", msg);
446+
}
447+
ShaderReflectError::GlslParse(_) => {}
448+
ShaderReflectError::SpirVParse(_) => {}
449+
}
450+
}
451+
}
452+
}

crates/bevy_render/src/render_resource/shader.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,15 @@ impl ProcessedShader {
156156
Ok(ShaderModuleDescriptor {
157157
label: None,
158158
source: match self {
159-
ProcessedShader::Wgsl(source) => ShaderSource::Wgsl(source.clone()),
159+
ProcessedShader::Wgsl(source) => {
160+
#[cfg(debug_assertions)]
161+
// This isn't neccessary, but catches errors early during hot reloading of invalid wgsl shaders.
162+
// Eventually, wgpu will have features that will make this unneccessary like compilation info
163+
// or error scopes, but until then parsing the shader twice during development the easiest solution.
164+
let _ = self.reflect()?;
165+
166+
ShaderSource::Wgsl(source.clone())
167+
}
160168
ProcessedShader::Glsl(_source, _stage) => {
161169
let reflection = self.reflect()?;
162170
// TODO: it probably makes more sense to convert this to spirv, but as of writing

0 commit comments

Comments
 (0)