[Merged by Bors] - report shader processing errors in RenderPipelineCache

crates/bevy_render/Cargo.toml

@@ -37,6 +37,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"

crates/bevy_render/src/render_resource/pipeline_cache.rs

@@ -2,19 +2,21 @@ use crate::{
     render_resource::{
         AsModuleDescriptorError, BindGroupLayout, BindGroupLayoutId, ProcessShaderError,
         RawFragmentState, RawRenderPipelineDescriptor, RawVertexState, RenderPipeline,
-        RenderPipelineDescriptor, Shader, ShaderImport, ShaderProcessor,
+        RenderPipelineDescriptor, Shader, ShaderImport, ShaderProcessor, ShaderReflectError,
     },
     renderer::RenderDevice,
     RenderWorld,
 };
 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(_) => {}
+            }
+        }
+    }
+}

crates/bevy_render/src/render_resource/shader.rs

@@ -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