From 0061d54d2114904ec8738bde5d70698d37476942 Mon Sep 17 00:00:00 2001 From: wayne warren Date: Tue, 16 Jan 2024 23:26:36 -0700 Subject: [PATCH] naga/wgpu/wgpu-core: improve spirv parse error handling --- Cargo.lock | 2 ++ naga/src/front/spv/error.rs | 25 +++++++++++++++++++++++++ wgpu-core/Cargo.toml | 4 ++++ wgpu-core/src/device/global.rs | 6 ++++++ wgpu-core/src/device/resource.rs | 13 +++++++++++++ wgpu-core/src/pipeline.rs | 12 ++++++++++++ wgpu/src/backend/wgpu_core.rs | 4 +--- 7 files changed, 63 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d317e1476b..91c89582a14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4006,6 +4006,7 @@ dependencies = [ "web-sys", "wgpu-hal", "wgpu-types", + "zerocopy", ] [[package]] @@ -4635,6 +4636,7 @@ version = "0.7.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74d4d3961e53fa4c9a25a8637fc2bfaf2595b3d3ae34875568a5cf64787716be" dependencies = [ + "byteorder", "zerocopy-derive", ] diff --git a/naga/src/front/spv/error.rs b/naga/src/front/spv/error.rs index 2f9bf2d1bca..af025636c0e 100644 --- a/naga/src/front/spv/error.rs +++ b/naga/src/front/spv/error.rs @@ -1,5 +1,9 @@ use super::ModuleState; use crate::arena::Handle; +use codespan_reporting::diagnostic::Diagnostic; +use codespan_reporting::files::SimpleFile; +use codespan_reporting::term; +use termcolor::{NoColor, WriteColor}; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -127,3 +131,24 @@ pub enum Error { )] NonBindingArrayOfImageOrSamplers, } + +impl Error { + pub fn emit_to_writer(&self, writer: &mut impl WriteColor, source: &str) { + self.emit_to_writer_with_path(writer, source, "glsl"); + } + + pub fn emit_to_writer_with_path(&self, writer: &mut impl WriteColor, source: &str, path: &str) { + let path = path.to_string(); + let files = SimpleFile::new(path, source); + let config = codespan_reporting::term::Config::default(); + let diagnostic = Diagnostic::error().with_message(format!("{self:?}")); + + term::emit(writer, &config, &files, &diagnostic).expect("cannot write error"); + } + + pub fn emit_to_string(&self, source: &str) -> String { + let mut writer = NoColor::new(Vec::new()); + self.emit_to_writer(&mut writer, source); + String::from_utf8(writer.into_inner()).unwrap() + } +} diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index 26985692721..63620c6bce3 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -62,6 +62,9 @@ wgsl = ["naga/wgsl-in"] ## Enable `ShaderModuleSource::Glsl` glsl = ["naga/glsl-in"] +## Enable `ShaderModuleSource::SpirV` +spirv = ["naga/spv-in", "dep:zerocopy"] + ## Implement `Send` and `Sync` on Wasm, but only if atomics are not enabled. ## ## WebGL/WebGPU objects can not be shared between threads. @@ -109,6 +112,7 @@ rustc-hash = "1.1" serde = { version = "1", features = ["serde_derive"], optional = true } smallvec = "1" thiserror = "1" +zerocopy = { version = "0.7", optional = true } [dependencies.naga] path = "../naga" diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index 352946e79ec..c1db399277b 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -22,6 +22,8 @@ use crate::{ use arrayvec::ArrayVec; use hal::Device as _; use parking_lot::RwLock; +#[cfg(feature = "spirv")] +use zerocopy::AsBytes; use wgt::{BufferAddress, TextureFormat}; @@ -1193,6 +1195,10 @@ impl Global { pipeline::ShaderModuleSource::Glsl(ref code, _) => { trace.make_binary("glsl", code.as_bytes()) } + #[cfg(feature = "spirv")] + pipeline::ShaderModuleSource::SpirV(ref code, _) => { + trace.make_binary("spirv", code.as_bytes()) + } pipeline::ShaderModuleSource::Naga(ref module) => { let string = ron::ser::to_string_pretty(module, ron::ser::PrettyConfig::default()) diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 61374a215e6..6e20b95327e 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -1319,6 +1319,19 @@ impl Device { })?; (Cow::Owned(module), code.into_owned()) } + #[cfg(feature = "spirv")] + pipeline::ShaderModuleSource::SpirV(spv, options) => { + let parser = naga::front::spv::Frontend::new(spv.iter().cloned(), &options); + profiling::scope!("naga::front::spv::Frontend"); + let module = parser.parse().map_err(|inner| { + pipeline::CreateShaderModuleError::ParsingSpirV(pipeline::ShaderError { + source: String::new(), + label: desc.label.as_ref().map(|l| l.to_string()), + inner: Box::new(inner), + }) + })?; + (Cow::Owned(module), String::new()) + } #[cfg(feature = "glsl")] pipeline::ShaderModuleSource::Glsl(code, options) => { let mut parser = naga::front::glsl::Frontend::default(); diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index 5b82bafe1e4..4fcaba3fe82 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -28,6 +28,8 @@ pub enum ShaderModuleSource<'a> { Wgsl(Cow<'a, str>), #[cfg(feature = "glsl")] Glsl(Cow<'a, str>, naga::front::glsl::Options), + #[cfg(feature = "spirv")] + SpirV(Cow<'a, [u32]>, naga::front::spv::Options), Naga(Cow<'static, naga::Module>), /// Dummy variant because `Naga` doesn't have a lifetime and without enough active features it /// could be the last one active. @@ -112,6 +114,13 @@ impl fmt::Display for ShaderError { write!(f, "\nShader '{label}' parsing {string}") } } +impl fmt::Display for ShaderError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let label = self.label.as_deref().unwrap_or_default(); + let string = self.inner.emit_to_string(&self.source); + write!(f, "\nShader '{label}' parsing {string}") + } +} impl fmt::Display for ShaderError> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use codespan_reporting::{ @@ -163,6 +172,9 @@ pub enum CreateShaderModuleError { #[cfg(feature = "glsl")] #[error(transparent)] ParsingGlsl(#[from] ShaderError), + #[cfg(feature = "spirv")] + #[error(transparent)] + ParsingSpirV(#[from] ShaderError), #[error("Failed to generate the backend-specific code")] Generation, #[error(transparent)] diff --git a/wgpu/src/backend/wgpu_core.rs b/wgpu/src/backend/wgpu_core.rs index 38297f53b06..a2c3f449234 100644 --- a/wgpu/src/backend/wgpu_core.rs +++ b/wgpu/src/backend/wgpu_core.rs @@ -844,9 +844,7 @@ impl crate::Context for ContextWgpuCore { strict_capabilities: true, block_ctx_dump_prefix: None, }; - let parser = naga::front::spv::Frontend::new(spv.iter().cloned(), &options); - let module = parser.parse().unwrap(); - wgc::pipeline::ShaderModuleSource::Naga(Owned(module)) + wgc::pipeline::ShaderModuleSource::SpirV(Borrowed(spv), options) } #[cfg(feature = "glsl")] ShaderSource::Glsl {