Shader validation enum (#17824)

# Objective

Make checked vs unchecked shaders configurable
Fixes #17786 

## Solution

Added `ValidateShaders` enum to `Shader` and added
`create_and_validate_shader_module` to `RenderDevice`

## Testing

I tested the shader examples locally and they all worked. I'd like to
write a few tests to verify but am unsure how to start.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

crates/bevy_render/src/render_resource/pipeline_cache.rs

@@ -318,7 +318,19 @@ impl ShaderCache {
                 render_device
                     .wgpu_device()
                     .push_error_scope(wgpu::ErrorFilter::Validation);
-                let shader_module = render_device.create_shader_module(module_descriptor);
+
+                let shader_module = match shader.validate_shader {
+                    ValidateShader::Enabled => {
+                        render_device.create_and_validate_shader_module(module_descriptor)
+                    }
+                    // SAFETY: we are interfacing with shader code, which may contain undefined behavior,
+                    // such as indexing out of bounds.
+                    // The checks required are prohibitively expensive and a poor default for game engines.
+                    ValidateShader::Disabled => unsafe {
+                        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.

crates/bevy_render/src/render_resource/shader.rs

@@ -21,6 +21,30 @@ pub enum ShaderReflectError {
     #[error(transparent)]
     Validation(#[from] naga::WithSpan<naga::valid::ValidationError>),
 }
+
+/// Describes whether or not to perform runtime checks on shaders.
+/// Runtime checks can be enabled for safety at the cost of speed.
+/// By default no runtime checks will be performed.
+///
+/// # Panics
+/// Because no runtime checks are performed for spirv,
+/// enabling `ValidateShader` for spirv will cause a panic
+#[derive(Clone, Debug, Default)]
+pub enum ValidateShader {
+    #[default]
+    /// No runtime checks for soundness (e.g. bound checking) are performed.
+    ///
+    /// This is suitable for trusted shaders, written by your program or dependencies you trust.
+    Disabled,
+    /// Enable's runtime checks for soundness (e.g. bound checking).
+    ///
+    /// While this can have a meaningful impact on performance,
+    /// this setting should *always* be enabled when loading untrusted shaders.
+    /// This might occur if you are creating a shader playground, running user-generated shaders
+    /// (as in `VRChat`), or writing a web browser in Bevy.
+    Enabled,
+}
+
 /// A shader, as defined by its [`ShaderSource`](wgpu::ShaderSource) and [`ShaderStage`](naga::ShaderStage)
 /// This is an "unprocessed" shader. It can contain preprocessor directives.
 #[derive(Asset, TypePath, Debug, Clone)]
@@ -36,6 +60,10 @@ pub struct Shader {
     // we must store strong handles to our dependencies to stop them
     // from being immediately dropped if we are the only user.
     pub file_dependencies: Vec<Handle<Shader>>,
+    /// Enable or disable runtime shader validation, trading safety against speed.
+    ///
+    /// Please read the [`ValidateShader`] docs for a discussion of the tradeoffs involved.
+    pub validate_shader: ValidateShader,
 }
 
 impl Shader {
@@ -78,6 +106,7 @@ impl Shader {
             additional_imports: Default::default(),
             shader_defs: Default::default(),
             file_dependencies: Default::default(),
+            validate_shader: ValidateShader::Disabled,
         }
     }
 
@@ -108,6 +137,7 @@ impl Shader {
             additional_imports: Default::default(),
             shader_defs: Default::default(),
             file_dependencies: Default::default(),
+            validate_shader: ValidateShader::Disabled,
         }
     }
 
@@ -121,6 +151,7 @@ impl Shader {
             additional_imports: Default::default(),
             shader_defs: Default::default(),
             file_dependencies: Default::default(),
+            validate_shader: ValidateShader::Disabled,
         }
     }
 

crates/bevy_render/src/renderer/render_device.rs

@@ -44,8 +44,18 @@ impl RenderDevice {
     }
 
     /// Creates a [`ShaderModule`](wgpu::ShaderModule) from either SPIR-V or WGSL source code.
+    ///
+    /// # Safety
+    ///
+    /// Creates a shader module with user-customizable runtime checks which allows shaders to
+    /// perform operations which can lead to undefined behavior like indexing out of bounds,
+    /// To avoid UB, ensure any unchecked shaders are sound!
+    /// This method should never be called for user-supplied shaders.
     #[inline]
-    pub fn create_shader_module(&self, desc: wgpu::ShaderModuleDescriptor) -> wgpu::ShaderModule {
+    pub unsafe fn create_shader_module(
+        &self,
+        desc: wgpu::ShaderModuleDescriptor,
+    ) -> wgpu::ShaderModule {
         #[cfg(feature = "spirv_shader_passthrough")]
         match &desc.source {
             wgpu::ShaderSource::SpirV(source)
@@ -64,29 +74,40 @@ impl RenderDevice {
                         })
                 }
             }
-            // SAFETY: we are interfacing with shader code, which may contain undefined behavior,
+            // SAFETY:
-            // such as indexing out of bounds.
+            //
-            // The checks required are prohibitively expensive and a poor default for game engines.
+            // This call passes binary data to the backend as-is and can potentially result in a driver crash or bogus behavior.
-            // TODO: split this method into safe and unsafe variants, and propagate the safety requirements from
+            // No attempt is made to ensure that data is valid SPIR-V.
-            // https://docs.rs/wgpu/latest/wgpu/struct.Device.html#method.create_shader_module_trusted to the unsafe form.
             _ => unsafe {
                 self.device
                     .create_shader_module_trusted(desc, wgpu::ShaderRuntimeChecks::unchecked())
             },
         }
-
         #[cfg(not(feature = "spirv_shader_passthrough"))]
-        // SAFETY: we are interfacing with shader code, which may contain undefined behavior,
+        // SAFETY: the caller is responsible for upholding the safety requirements
-        // such as indexing out of bounds.
-        // The checks required are prohibitively expensive and a poor default for game engines.
-        // TODO: split this method into safe and unsafe variants, and propagate the safety requirements from
-        // https://docs.rs/wgpu/latest/wgpu/struct.Device.html#method.create_shader_module_trusted to the unsafe form.
         unsafe {
             self.device
                 .create_shader_module_trusted(desc, wgpu::ShaderRuntimeChecks::unchecked())
         }
     }
 
+    /// Creates and validates a [`ShaderModule`](wgpu::ShaderModule) from either SPIR-V or WGSL source code.
+    ///
+    /// See [`ValidateShader`](bevy_render::render_resource::ValidateShader) for more information on the tradeoffs involved with shader validation.
+    #[inline]
+    pub fn create_and_validate_shader_module(
+        &self,
+        desc: wgpu::ShaderModuleDescriptor,
+    ) -> wgpu::ShaderModule {
+        #[cfg(feature = "spirv_shader_passthrough")]
+        match &desc.source {
+            wgpu::ShaderSource::SpirV(_source) => panic!("no safety checks are performed for spirv shaders. use `create_shader_module` instead"),
+            _ => self.device.create_shader_module(desc),
+        }
+        #[cfg(not(feature = "spirv_shader_passthrough"))]
+        self.device.create_shader_module(desc)
+    }
+
     /// Check for resource cleanups and mapping callbacks.
     ///
     /// Return `true` if the queue is empty, or `false` if there are more queue