Skip to content

Commit

Permalink
Set naga capabilities corresponding to wgpu features (bevyengine#4824)
Browse files Browse the repository at this point in the history
# Objective

At the moment all extra capabilities are disabled when validating shaders with naga:
https://github.com/bevyengine/bevy/blob/c7c08f95cb784afc366eb2dcedd21d9d40e72d32/crates/bevy_render/src/render_resource/shader.rs#L146-L149
This means these features can't be used even if the corresponding wgpu features are active.

## Solution

With these changes capabilities are now set corresponding to `RenderDevice::features`.

---

I have validated these changes for push constants with a project I am currently working on. Though bevy does not support creating pipelines with push constants yet, so I was only able to see that shaders are validated and compiled as expected.
  • Loading branch information
MDeiml authored and james7132 committed Oct 28, 2022
1 parent 5c1629c commit 3f793d8
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 10 deletions.
4 changes: 3 additions & 1 deletion crates/bevy_render/src/render_resource/pipeline_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ impl ShaderCache {
&self.shaders,
&self.import_path_shaders,
)?;
let module_descriptor = match processed.get_module_descriptor() {
let module_descriptor = match processed
.get_module_descriptor(render_device.features())
{
Ok(module_descriptor) => module_descriptor,
Err(err) => {
return Err(PipelineCacheError::AsModuleDescriptorError(err, processed));
Expand Down
35 changes: 26 additions & 9 deletions crates/bevy_render/src/render_resource/shader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ use bevy_asset::{AssetLoader, Handle, LoadContext, LoadedAsset};
use bevy_reflect::{TypeUuid, Uuid};
use bevy_utils::{tracing::error, BoxedFuture, HashMap};
use naga::back::wgsl::WriterFlags;
use naga::valid::Capabilities;
use naga::{valid::ModuleInfo, Module};
use once_cell::sync::Lazy;
use regex::Regex;
use std::{
borrow::Cow, collections::HashSet, marker::Copy, ops::Deref, path::PathBuf, str::FromStr,
};
use thiserror::Error;
use wgpu::Features;
use wgpu::{util::make_spirv, ShaderModuleDescriptor, ShaderSource};

#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
Expand Down Expand Up @@ -125,7 +127,7 @@ impl ProcessedShader {
}
}

pub fn reflect(&self) -> Result<ShaderReflection, ShaderReflectError> {
pub fn reflect(&self, features: Features) -> Result<ShaderReflection, ShaderReflectError> {
let module = match &self {
// TODO: process macros here
ProcessedShader::Wgsl(source) => naga::front::wgsl::parse_str(source)?,
Expand All @@ -143,19 +145,34 @@ impl ProcessedShader {
},
)?,
};
let module_info = naga::valid::Validator::new(
naga::valid::ValidationFlags::default(),
naga::valid::Capabilities::default(),
)
.validate(&module)?;
const CAPABILITIES: &[(Features, Capabilities)] = &[
(Features::PUSH_CONSTANTS, Capabilities::PUSH_CONSTANT),
(Features::SHADER_FLOAT64, Capabilities::FLOAT64),
(
Features::SHADER_PRIMITIVE_INDEX,
Capabilities::PRIMITIVE_INDEX,
),
];
let mut capabilities = Capabilities::empty();
for (feature, capability) in CAPABILITIES {
if features.contains(*feature) {
capabilities |= *capability;
}
}
let module_info =
naga::valid::Validator::new(naga::valid::ValidationFlags::default(), capabilities)
.validate(&module)?;

Ok(ShaderReflection {
module,
module_info,
})
}

pub fn get_module_descriptor(&self) -> Result<ShaderModuleDescriptor, AsModuleDescriptorError> {
pub fn get_module_descriptor(
&self,
features: Features,
) -> Result<ShaderModuleDescriptor, AsModuleDescriptorError> {
Ok(ShaderModuleDescriptor {
label: None,
source: match self {
Expand All @@ -164,12 +181,12 @@ impl ProcessedShader {
// Parse and validate the shader early, so that (e.g. while hot reloading) we can
// display nicely formatted error messages instead of relying on just displaying the error string
// returned by wgpu upon creating the shader module.
let _ = self.reflect()?;
let _ = self.reflect(features)?;

ShaderSource::Wgsl(source.clone())
}
ProcessedShader::Glsl(_source, _stage) => {
let reflection = self.reflect()?;
let reflection = self.reflect(features)?;
// TODO: it probably makes more sense to convert this to spirv, but as of writing
// this comment, naga's spirv conversion is broken
let wgsl = reflection.get_wgsl()?;
Expand Down

0 comments on commit 3f793d8

Please sign in to comment.