Allow unconsumed inputs in fragment shaders (#5531)

* Allow unconsumed inputs in fragment shaders by removing them from vertex
outputs when generating HLSL.

Fixes https://github.com/gfx-rs/wgpu/issues/3748

* Add naga:🔙:hlsl::FragmentEntryPoint for providing information
  about the fragment entry point when generating vertex entry points via
  naga:🔙:hlsl::Writer::write. Vertex outputs not consumed by the
  fragment entry point are omitted in the final output struct.
* Add naga snapshot test for this new feature,
* Remove Features::SHADER_UNUSED_VERTEX_OUTPUT,
  StageError::InputNotConsumed, and associated validation logic.
* Make wgpu dx12 backend pass fragment shader info when generating
  vertex HLSL.
* Add wgpu regression test for allowing unconsumed inputs.

* Address review

* Add note that nesting structs for the inter-stage interface can't
  happen.
* Remove new TODO notes (some addressed and some transferred to an issue
  https://github.com/gfx-rs/wgpu/issues/5577)
* Changed issue that regression test refers to 3748 -> 5553
* Add debug_assert that binding.is_some() in hlsl writer
* Fix typos caught in CI

Also, fix compiling snapshot test when hlsl-out feature is not enabled.
This commit is contained in:
Imbris 2024-07-04 03:08:46 -04:00 committed by GitHub
parent e26d2d7763
commit 3a6814770a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
25 changed files with 357 additions and 69 deletions

View File

@ -137,6 +137,7 @@ By @atlv24 in [#5383](https://github.com/gfx-rs/wgpu/pull/5383)
#### General #### General
- Added `as_hal` for `Buffer` to access wgpu created buffers form wgpu-hal. By @JasondeWolff in [#5724](https://github.com/gfx-rs/wgpu/pull/5724) - Added `as_hal` for `Buffer` to access wgpu created buffers form wgpu-hal. By @JasondeWolff in [#5724](https://github.com/gfx-rs/wgpu/pull/5724)
- Unconsumed vertex outputs are now always allowed. Removed `StageError::InputNotConsumed`, `Features::SHADER_UNUSED_VERTEX_OUTPUT`, and associated validation. By @Imberflur in [#5531](https://github.com/gfx-rs/wgpu/pull/5531)
#### Naga #### Naga
@ -144,6 +145,13 @@ By @atlv24 in [#5383](https://github.com/gfx-rs/wgpu/pull/5383)
- Added type upgrades to SPIR-V atomic support. Added related infrastructure. Tracking issue is [here](https://github.com/gfx-rs/wgpu/issues/4489). By @schell in [#5775](https://github.com/gfx-rs/wgpu/pull/5775). - Added type upgrades to SPIR-V atomic support. Added related infrastructure. Tracking issue is [here](https://github.com/gfx-rs/wgpu/issues/4489). By @schell in [#5775](https://github.com/gfx-rs/wgpu/pull/5775).
- Implement `WGSL`'s `unpack4xI8`,`unpack4xU8`,`pack4xI8` and `pack4xU8`. By @VlaDexa in [#5424](https://github.com/gfx-rs/wgpu/pull/5424) - Implement `WGSL`'s `unpack4xI8`,`unpack4xU8`,`pack4xI8` and `pack4xU8`. By @VlaDexa in [#5424](https://github.com/gfx-rs/wgpu/pull/5424)
- Began work adding support for atomics to the SPIR-V frontend. Tracking issue is [here](https://github.com/gfx-rs/wgpu/issues/4489). By @schell in [#5702](https://github.com/gfx-rs/wgpu/pull/5702). - Began work adding support for atomics to the SPIR-V frontend. Tracking issue is [here](https://github.com/gfx-rs/wgpu/issues/4489). By @schell in [#5702](https://github.com/gfx-rs/wgpu/pull/5702).
- In hlsl-out, allow passing information about the fragment entry point to omit vertex outputs that are not in the fragment inputs. By @Imberflur in [#5531](https://github.com/gfx-rs/wgpu/pull/5531)
```diff
let writer: naga::back::hlsl::Writer = /* ... */;
-writer.write(&module, &module_info);
+writer.write(&module, &module_info, None);
```
#### WebGPU #### WebGPU

View File

@ -308,6 +308,7 @@ fn backends(c: &mut Criterion) {
let _ = writer.write( let _ = writer.write(
input.module.as_ref().unwrap(), input.module.as_ref().unwrap(),
input.module_info.as_ref().unwrap(), input.module_info.as_ref().unwrap(),
None,
); // may fail on unimplemented things ); // may fail on unimplemented things
string.clear(); string.clear();
} }

View File

@ -360,9 +360,6 @@ fn deserialize_features(features: &wgpu_types::Features) -> Vec<&'static str> {
if features.contains(wgpu_types::Features::SHADER_EARLY_DEPTH_TEST) { if features.contains(wgpu_types::Features::SHADER_EARLY_DEPTH_TEST) {
return_features.push("shader-early-depth-test"); return_features.push("shader-early-depth-test");
} }
if features.contains(wgpu_types::Features::SHADER_UNUSED_VERTEX_OUTPUT) {
return_features.push("shader-unused-vertex-output");
}
return_features return_features
} }
@ -648,10 +645,6 @@ impl From<GpuRequiredFeatures> for wgpu_types::Features {
wgpu_types::Features::SHADER_EARLY_DEPTH_TEST, wgpu_types::Features::SHADER_EARLY_DEPTH_TEST,
required_features.0.contains("shader-early-depth-test"), required_features.0.contains("shader-early-depth-test"),
); );
features.set(
wgpu_types::Features::SHADER_UNUSED_VERTEX_OUTPUT,
required_features.0.contains("shader-unused-vertex-output"),
);
features features
} }

View File

@ -811,7 +811,7 @@ fn write_output(
let mut buffer = String::new(); let mut buffer = String::new();
let mut writer = hlsl::Writer::new(&mut buffer, &params.hlsl); let mut writer = hlsl::Writer::new(&mut buffer, &params.hlsl);
writer.write(&module, &info).unwrap_pretty(); writer.write(&module, &info, None).unwrap_pretty();
fs::write(output_path, buffer)?; fs::write(output_path, buffer)?;
} }
"wgsl" => { "wgsl" => {

View File

@ -287,6 +287,35 @@ impl Wrapped {
} }
} }
/// A fragment entry point to be considered when generating HLSL for the output interface of vertex
/// entry points.
///
/// This is provided as an optional parameter to [`Writer::write`].
///
/// If this is provided, vertex outputs will be removed if they are not inputs of this fragment
/// entry point. This is necessary for generating correct HLSL when some of the vertex shader
/// outputs are not consumed by the fragment shader.
pub struct FragmentEntryPoint<'a> {
module: &'a crate::Module,
func: &'a crate::Function,
}
impl<'a> FragmentEntryPoint<'a> {
/// Returns `None` if the entry point with the provided name can't be found or isn't a fragment
/// entry point.
pub fn new(module: &'a crate::Module, ep_name: &'a str) -> Option<Self> {
module
.entry_points
.iter()
.find(|ep| ep.name == ep_name)
.filter(|ep| ep.stage == crate::ShaderStage::Fragment)
.map(|ep| Self {
module,
func: &ep.function,
})
}
}
pub struct Writer<'a, W> { pub struct Writer<'a, W> {
out: W, out: W,
names: crate::FastHashMap<proc::NameKey, String>, names: crate::FastHashMap<proc::NameKey, String>,

View File

@ -4,7 +4,7 @@ use super::{
WrappedZeroValue, WrappedZeroValue,
}, },
storage::StoreValue, storage::StoreValue,
BackendResult, Error, Options, BackendResult, Error, FragmentEntryPoint, Options,
}; };
use crate::{ use crate::{
back::{self, Baked}, back::{self, Baked},
@ -29,6 +29,7 @@ struct EpStructMember {
name: String, name: String,
ty: Handle<crate::Type>, ty: Handle<crate::Type>,
// technically, this should always be `Some` // technically, this should always be `Some`
// (we `debug_assert!` this in `write_interface_struct`)
binding: Option<crate::Binding>, binding: Option<crate::Binding>,
index: u32, index: u32,
} }
@ -200,6 +201,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
&mut self, &mut self,
module: &Module, module: &Module,
module_info: &valid::ModuleInfo, module_info: &valid::ModuleInfo,
fragment_entry_point: Option<&FragmentEntryPoint<'_>>,
) -> Result<super::ReflectionInfo, Error> { ) -> Result<super::ReflectionInfo, Error> {
if !module.overrides.is_empty() { if !module.overrides.is_empty() {
return Err(Error::Override); return Err(Error::Override);
@ -300,7 +302,13 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
// Write all entry points wrapped structs // Write all entry points wrapped structs
for (index, ep) in module.entry_points.iter().enumerate() { for (index, ep) in module.entry_points.iter().enumerate() {
let ep_name = self.names[&NameKey::EntryPoint(index as u16)].clone(); let ep_name = self.names[&NameKey::EntryPoint(index as u16)].clone();
let ep_io = self.write_ep_interface(module, &ep.function, ep.stage, &ep_name)?; let ep_io = self.write_ep_interface(
module,
&ep.function,
ep.stage,
&ep_name,
fragment_entry_point,
)?;
self.entry_point_io.push(ep_io); self.entry_point_io.push(ep_io);
} }
@ -481,6 +489,10 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
write!(self.out, "struct {struct_name}")?; write!(self.out, "struct {struct_name}")?;
writeln!(self.out, " {{")?; writeln!(self.out, " {{")?;
for m in members.iter() { for m in members.iter() {
// Sanity check that each IO member is a built-in or is assigned a
// location. Also see note about nesting in `write_ep_input_struct`.
debug_assert!(m.binding.is_some());
if is_subgroup_builtin_binding(&m.binding) { if is_subgroup_builtin_binding(&m.binding) {
continue; continue;
} }
@ -508,6 +520,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
writeln!(self.out, "}};")?; writeln!(self.out, "}};")?;
writeln!(self.out)?; writeln!(self.out)?;
// See ordering notes on EntryPointInterface fields
match shader_stage.1 { match shader_stage.1 {
Io::Input => { Io::Input => {
// bring back the original order // bring back the original order
@ -539,6 +552,10 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
let mut fake_members = Vec::new(); let mut fake_members = Vec::new();
for arg in func.arguments.iter() { for arg in func.arguments.iter() {
// NOTE: We don't need to handle nesting structs. All members must
// be either built-ins or assigned a location. I.E. `binding` is
// `Some`. This is checked in `VaryingContext::validate`. See:
// https://gpuweb.github.io/gpuweb/wgsl/#input-output-locations
match module.types[arg.ty].inner { match module.types[arg.ty].inner {
TypeInner::Struct { ref members, .. } => { TypeInner::Struct { ref members, .. } => {
for member in members.iter() { for member in members.iter() {
@ -577,10 +594,10 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
result: &crate::FunctionResult, result: &crate::FunctionResult,
stage: ShaderStage, stage: ShaderStage,
entry_point_name: &str, entry_point_name: &str,
frag_ep: Option<&FragmentEntryPoint<'_>>,
) -> Result<EntryPointBinding, Error> { ) -> Result<EntryPointBinding, Error> {
let struct_name = format!("{stage:?}Output_{entry_point_name}"); let struct_name = format!("{stage:?}Output_{entry_point_name}");
let mut fake_members = Vec::new();
let empty = []; let empty = [];
let members = match module.types[result.ty].inner { let members = match module.types[result.ty].inner {
TypeInner::Struct { ref members, .. } => members, TypeInner::Struct { ref members, .. } => members,
@ -590,14 +607,54 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
} }
}; };
for member in members.iter() { // Gather list of fragment input locations. We use this below to remove user-defined
// varyings from VS outputs that aren't in the FS inputs. This makes the VS interface match
// as long as the FS inputs are a subset of the VS outputs. This is only applied if the
// writer is supplied with information about the fragment entry point.
let fs_input_locs = if let (Some(frag_ep), ShaderStage::Vertex) = (frag_ep, stage) {
let mut fs_input_locs = Vec::new();
for arg in frag_ep.func.arguments.iter() {
let mut push_if_location = |binding: &Option<crate::Binding>| match *binding {
Some(crate::Binding::Location { location, .. }) => fs_input_locs.push(location),
Some(crate::Binding::BuiltIn(_)) | None => {}
};
// NOTE: We don't need to handle struct nesting. See note in
// `write_ep_input_struct`.
match frag_ep.module.types[arg.ty].inner {
TypeInner::Struct { ref members, .. } => {
for member in members.iter() {
push_if_location(&member.binding);
}
}
_ => push_if_location(&arg.binding),
}
}
fs_input_locs.sort();
Some(fs_input_locs)
} else {
None
};
let mut fake_members = Vec::new();
for (index, member) in members.iter().enumerate() {
if let Some(ref fs_input_locs) = fs_input_locs {
match member.binding {
Some(crate::Binding::Location { location, .. }) => {
if fs_input_locs.binary_search(&location).is_err() {
continue;
}
}
Some(crate::Binding::BuiltIn(_)) | None => {}
}
}
let member_name = self.namer.call_or(&member.name, "member"); let member_name = self.namer.call_or(&member.name, "member");
let index = fake_members.len() as u32;
fake_members.push(EpStructMember { fake_members.push(EpStructMember {
name: member_name, name: member_name,
ty: member.ty, ty: member.ty,
binding: member.binding.clone(), binding: member.binding.clone(),
index, index: index as u32,
}); });
} }
@ -613,6 +670,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
func: &crate::Function, func: &crate::Function,
stage: ShaderStage, stage: ShaderStage,
ep_name: &str, ep_name: &str,
frag_ep: Option<&FragmentEntryPoint<'_>>,
) -> Result<EntryPointInterface, Error> { ) -> Result<EntryPointInterface, Error> {
Ok(EntryPointInterface { Ok(EntryPointInterface {
input: if !func.arguments.is_empty() input: if !func.arguments.is_empty()
@ -628,7 +686,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
}, },
output: match func.result { output: match func.result {
Some(ref fr) if fr.binding.is_none() && stage == ShaderStage::Vertex => { Some(ref fr) if fr.binding.is_none() && stage == ShaderStage::Vertex => {
Some(self.write_ep_output_struct(module, fr, stage, ep_name)?) Some(self.write_ep_output_struct(module, fr, stage, ep_name, frag_ep)?)
} }
_ => None, _ => None,
}, },

View File

@ -0,0 +1,2 @@
(
)

View File

@ -0,0 +1,13 @@
// Out of order to test sorting.
struct FragmentIn {
@location(1) value: f32,
@location(3) value2: f32,
@builtin(position) position: vec4<f32>,
// @location(0) unused_value: f32,
// @location(2) unused_value2: vec4<f32>,
}
@fragment
fn fs_main(v_out: FragmentIn) -> @location(0) vec4<f32> {
return vec4<f32>(v_out.value, v_out.value, v_out.value2, v_out.value2);
}

View File

@ -0,0 +1,2 @@
(
)

View File

@ -0,0 +1,13 @@
// Out of order to test sorting.
struct VertexOut {
@builtin(position) position: vec4<f32>,
@location(1) value: f32,
@location(2) unused_value2: vec4<f32>,
@location(0) unused_value: f32,
@location(3) value2: f32,
}
@vertex
fn vs_main() -> VertexOut {
return VertexOut(vec4(1.0), 1.0, vec4(2.0), 1.0, 0.5);
}

View File

@ -0,0 +1,17 @@
struct FragmentIn {
float value : LOC1;
float value2_ : LOC3;
float4 position : SV_Position;
};
struct FragmentInput_fs_main {
float value : LOC1;
float value2_ : LOC3;
float4 position : SV_Position;
};
float4 fs_main(FragmentInput_fs_main fragmentinput_fs_main) : SV_Target0
{
FragmentIn v_out = { fragmentinput_fs_main.value, fragmentinput_fs_main.value2_, fragmentinput_fs_main.position };
return float4(v_out.value, v_out.value, v_out.value2_, v_out.value2_);
}

View File

@ -0,0 +1,12 @@
(
vertex:[
],
fragment:[
(
entry_point:"fs_main",
target_profile:"ps_5_1",
),
],
compute:[
],
)

View File

@ -0,0 +1,30 @@
struct VertexOut {
float4 position : SV_Position;
float value : LOC1;
float4 unused_value2_ : LOC2;
float unused_value : LOC0;
float value2_ : LOC3;
};
struct VertexOutput_vs_main {
float value : LOC1;
float value2_ : LOC3;
float4 position : SV_Position;
};
VertexOut ConstructVertexOut(float4 arg0, float arg1, float4 arg2, float arg3, float arg4) {
VertexOut ret = (VertexOut)0;
ret.position = arg0;
ret.value = arg1;
ret.unused_value2_ = arg2;
ret.unused_value = arg3;
ret.value2_ = arg4;
return ret;
}
VertexOutput_vs_main vs_main()
{
const VertexOut vertexout = ConstructVertexOut((1.0).xxxx, 1.0, (2.0).xxxx, 1.0, 0.5);
const VertexOutput_vs_main vertexout_1 = { vertexout.value, vertexout.value2_, vertexout.position };
return vertexout_1;
}

View File

@ -0,0 +1,12 @@
(
vertex:[
(
entry_point:"vs_main",
target_profile:"vs_5_1",
),
],
fragment:[
],
compute:[
],
)

View File

@ -260,13 +260,24 @@ impl Input {
} }
} }
#[cfg(feature = "hlsl-out")]
type FragmentEntryPoint<'a> = naga::back::hlsl::FragmentEntryPoint<'a>;
#[cfg(not(feature = "hlsl-out"))]
type FragmentEntryPoint<'a> = ();
#[allow(unused_variables)] #[allow(unused_variables)]
fn check_targets( fn check_targets(
input: &Input, input: &Input,
module: &mut naga::Module, module: &mut naga::Module,
targets: Targets, targets: Targets,
source_code: Option<&str>, source_code: Option<&str>,
// For testing hlsl generation when fragment shader doesn't consume all vertex outputs.
frag_ep: Option<FragmentEntryPoint>,
) { ) {
if frag_ep.is_some() && !targets.contains(Targets::HLSL) {
panic!("Providing FragmentEntryPoint only makes sense when testing hlsl-out");
}
let params = input.read_parameters(); let params = input.read_parameters();
let name = &input.file_name; let name = &input.file_name;
@ -416,6 +427,7 @@ fn check_targets(
&info, &info,
&params.hlsl, &params.hlsl,
&params.pipeline_constants, &params.pipeline_constants,
frag_ep,
); );
} }
} }
@ -594,6 +606,7 @@ fn write_output_hlsl(
info: &naga::valid::ModuleInfo, info: &naga::valid::ModuleInfo,
options: &naga::back::hlsl::Options, options: &naga::back::hlsl::Options,
pipeline_constants: &naga::back::PipelineConstants, pipeline_constants: &naga::back::PipelineConstants,
frag_ep: Option<naga::back::hlsl::FragmentEntryPoint>,
) { ) {
use naga::back::hlsl; use naga::back::hlsl;
use std::fmt::Write as _; use std::fmt::Write as _;
@ -606,7 +619,9 @@ fn write_output_hlsl(
let mut buffer = String::new(); let mut buffer = String::new();
let mut writer = hlsl::Writer::new(&mut buffer, options); let mut writer = hlsl::Writer::new(&mut buffer, options);
let reflection_info = writer.write(&module, &info).expect("HLSL write failed"); let reflection_info = writer
.write(&module, &info, frag_ep.as_ref())
.expect("HLSL write failed");
input.write_output_file("hlsl", "hlsl", buffer); input.write_output_file("hlsl", "hlsl", buffer);
@ -910,7 +925,7 @@ fn convert_wgsl() {
let input = Input::new(None, name, "wgsl"); let input = Input::new(None, name, "wgsl");
let source = input.read_source(); let source = input.read_source();
match naga::front::wgsl::parse_str(&source) { match naga::front::wgsl::parse_str(&source) {
Ok(mut module) => check_targets(&input, &mut module, targets, None), Ok(mut module) => check_targets(&input, &mut module, targets, None, None),
Err(e) => panic!( Err(e) => panic!(
"{}", "{}",
e.emit_to_string_with_path(&source, input.input_path()) e.emit_to_string_with_path(&source, input.input_path())
@ -932,7 +947,7 @@ fn convert_wgsl() {
// crlf will make the large split output different on different platform // crlf will make the large split output different on different platform
let source = source.replace('\r', ""); let source = source.replace('\r', "");
match naga::front::wgsl::parse_str(&source) { match naga::front::wgsl::parse_str(&source) {
Ok(mut module) => check_targets(&input, &mut module, targets, Some(&source)), Ok(mut module) => check_targets(&input, &mut module, targets, Some(&source), None),
Err(e) => panic!( Err(e) => panic!(
"{}", "{}",
e.emit_to_string_with_path(&source, input.input_path()) e.emit_to_string_with_path(&source, input.input_path())
@ -942,6 +957,36 @@ fn convert_wgsl() {
} }
} }
#[cfg(all(feature = "wgsl-in", feature = "hlsl-out"))]
#[test]
fn unconsumed_vertex_outputs_hlsl_out() {
let load_and_parse = |name| {
// WGSL shaders lives in root dir as a privileged.
let input = Input::new(None, name, "wgsl");
let source = input.read_source();
let module = match naga::front::wgsl::parse_str(&source) {
Ok(module) => module,
Err(e) => panic!(
"{}",
e.emit_to_string_with_path(&source, input.input_path())
),
};
(input, module)
};
// Uses separate wgsl files to make sure the tested code doesn't accidentally rely on
// the fragment entry point being from the same parsed content (e.g. accidentally using the
// wrong `Module` when looking up info). We also don't just create a module from the same file
// twice since everything would probably be stored behind the same keys.
let (input, mut module) = load_and_parse("unconsumed_vertex_outputs_vert");
let (frag_input, mut frag_module) = load_and_parse("unconsumed_vertex_outputs_frag");
let frag_ep = naga::back::hlsl::FragmentEntryPoint::new(&frag_module, "fs_main")
.expect("fs_main not found");
check_targets(&input, &mut module, Targets::HLSL, None, Some(frag_ep));
check_targets(&frag_input, &mut frag_module, Targets::HLSL, None, None);
}
#[cfg(feature = "spv-in")] #[cfg(feature = "spv-in")]
fn convert_spv(name: &str, adjust_coordinate_space: bool, targets: Targets) { fn convert_spv(name: &str, adjust_coordinate_space: bool, targets: Targets) {
let _ = env_logger::try_init(); let _ = env_logger::try_init();
@ -956,7 +1001,7 @@ fn convert_spv(name: &str, adjust_coordinate_space: bool, targets: Targets) {
}, },
) )
.unwrap(); .unwrap();
check_targets(&input, &mut module, targets, None); check_targets(&input, &mut module, targets, None, None);
} }
#[cfg(feature = "spv-in")] #[cfg(feature = "spv-in")]
@ -1022,7 +1067,7 @@ fn convert_glsl_variations_check() {
&source, &source,
) )
.unwrap(); .unwrap();
check_targets(&input, &mut module, Targets::GLSL, None); check_targets(&input, &mut module, Targets::GLSL, None, None);
} }
#[cfg(feature = "glsl-in")] #[cfg(feature = "glsl-in")]

View File

@ -0,0 +1,53 @@
use wgpu_test::{gpu_test, GpuTestConfiguration};
use wgpu::*;
/// Previously, for every user-defined vertex output a fragment shader had to have a corresponding
/// user-defined input. This would generate `StageError::InputNotConsumed`.
///
/// This requirement was removed from the WebGPU spec. Now, when generating hlsl, wgpu will
/// automatically remove any user-defined outputs from the vertex shader that are not present in
/// the fragment inputs. This is necessary for generating correct hlsl:
/// https://github.com/gfx-rs/wgpu/issues/5553
#[gpu_test]
static ALLOW_INPUT_NOT_CONSUMED: GpuTestConfiguration =
GpuTestConfiguration::new().run_async(|ctx| async move {
let module = ctx
.device
.create_shader_module(include_wgsl!("issue_5553.wgsl"));
let pipeline_layout = ctx
.device
.create_pipeline_layout(&PipelineLayoutDescriptor {
label: Some("Pipeline Layout"),
bind_group_layouts: &[],
push_constant_ranges: &[],
});
ctx.device
.create_render_pipeline(&RenderPipelineDescriptor {
label: Some("Pipeline"),
layout: Some(&pipeline_layout),
vertex: VertexState {
module: &module,
entry_point: "vs_main",
compilation_options: Default::default(),
buffers: &[],
},
primitive: PrimitiveState::default(),
depth_stencil: None,
multisample: MultisampleState::default(),
fragment: Some(FragmentState {
module: &module,
entry_point: "fs_main",
compilation_options: Default::default(),
targets: &[Some(ColorTargetState {
format: TextureFormat::Rgba8Unorm,
blend: None,
write_mask: ColorWrites::all(),
})],
}),
multiview: None,
cache: None,
});
});

View File

@ -0,0 +1,23 @@
struct VertexOut {
@builtin(position) position: vec4<f32>,
@location(0) unused_value: f32,
@location(1) value: f32,
}
struct FragmentIn {
@builtin(position) position: vec4<f32>,
// @location(0) unused_value: f32,
@location(1) value: f32,
}
@vertex
fn vs_main() -> VertexOut {
return VertexOut(vec4(1.0), 1.0, 1.0);
}
@fragment
fn fs_main(v_out: FragmentIn) -> @location(0) vec4<f32> {
return vec4<f32>(v_out.value);
}

View File

@ -3,6 +3,7 @@ mod regression {
mod issue_3457; mod issue_3457;
mod issue_4024; mod issue_4024;
mod issue_4122; mod issue_4122;
mod issue_5553;
} }
mod bgra8unorm_storage; mod bgra8unorm_storage;

View File

@ -1603,8 +1603,7 @@ impl<A: HalApi> Device<A> {
}) })
})?; })?;
let interface = let interface = validation::Interface::new(&module, &info, self.limits.clone());
validation::Interface::new(&module, &info, self.limits.clone(), self.features);
let hal_shader = hal::ShaderInput::Naga(hal::NagaShader { let hal_shader = hal::ShaderInput::Naga(hal::NagaShader {
module, module,
info, info,

View File

@ -128,7 +128,6 @@ struct EntryPoint {
#[derive(Debug)] #[derive(Debug)]
pub struct Interface { pub struct Interface {
limits: wgt::Limits, limits: wgt::Limits,
features: wgt::Features,
resources: naga::Arena<Resource>, resources: naga::Arena<Resource>,
entry_points: FastHashMap<(naga::ShaderStage, String), EntryPoint>, entry_points: FastHashMap<(naga::ShaderStage, String), EntryPoint>,
} }
@ -232,8 +231,6 @@ pub enum StageError {
#[source] #[source]
error: InputError, error: InputError,
}, },
#[error("Location[{location}] is provided by the previous stage output but is not consumed as input by this stage.")]
InputNotConsumed { location: wgt::ShaderLocation },
#[error( #[error(
"Unable to select an entry point: no entry point was found in the provided shader module" "Unable to select an entry point: no entry point was found in the provided shader module"
)] )]
@ -838,12 +835,7 @@ impl Interface {
list.push(varying); list.push(varying);
} }
pub fn new( pub fn new(module: &naga::Module, info: &naga::valid::ModuleInfo, limits: wgt::Limits) -> Self {
module: &naga::Module,
info: &naga::valid::ModuleInfo,
limits: wgt::Limits,
features: wgt::Features,
) -> Self {
let mut resources = naga::Arena::new(); let mut resources = naga::Arena::new();
let mut resource_mapping = FastHashMap::default(); let mut resource_mapping = FastHashMap::default();
for (var_handle, var) in module.global_variables.iter() { for (var_handle, var) in module.global_variables.iter() {
@ -921,7 +913,6 @@ impl Interface {
Self { Self {
limits, limits,
features,
resources, resources,
entry_points, entry_points,
} }
@ -1172,27 +1163,6 @@ impl Interface {
} }
} }
// Check all vertex outputs and make sure the fragment shader consumes them.
// This requirement is removed if the `SHADER_UNUSED_VERTEX_OUTPUT` feature is enabled.
if shader_stage == naga::ShaderStage::Fragment
&& !self
.features
.contains(wgt::Features::SHADER_UNUSED_VERTEX_OUTPUT)
{
for &index in inputs.keys() {
// This is a linear scan, but the count should be low enough
// that this should be fine.
let found = entry_point.inputs.iter().any(|v| match *v {
Varying::Local { location, .. } => location == index,
Varying::BuiltIn(_) => false,
});
if !found {
return Err(StageError::InputNotConsumed { location: index });
}
}
}
if shader_stage == naga::ShaderStage::Vertex { if shader_stage == naga::ShaderStage::Vertex {
for output in entry_point.outputs.iter() { for output in entry_point.outputs.iter() {
//TODO: count builtins towards the limit? //TODO: count builtins towards the limit?

View File

@ -206,14 +206,27 @@ impl super::Device {
Ok(()) Ok(())
} }
/// When generating the vertex shader, the fragment stage must be passed if it exists!
/// Otherwise, the generated HLSL may be incorrect since the fragment shader inputs are
/// allowed to be a subset of the vertex outputs.
fn load_shader( fn load_shader(
&self, &self,
stage: &crate::ProgrammableStage<super::Api>, stage: &crate::ProgrammableStage<super::Api>,
layout: &super::PipelineLayout, layout: &super::PipelineLayout,
naga_stage: naga::ShaderStage, naga_stage: naga::ShaderStage,
fragment_stage: Option<&crate::ProgrammableStage<super::Api>>,
) -> Result<super::CompiledShader, crate::PipelineError> { ) -> Result<super::CompiledShader, crate::PipelineError> {
use naga::back::hlsl; use naga::back::hlsl;
let frag_ep = fragment_stage
.map(|fs_stage| {
hlsl::FragmentEntryPoint::new(&fs_stage.module.naga.module, fs_stage.entry_point)
.ok_or(crate::PipelineError::EntryPoint(
naga::ShaderStage::Fragment,
))
})
.transpose()?;
let stage_bit = auxil::map_naga_stage(naga_stage); let stage_bit = auxil::map_naga_stage(naga_stage);
let (module, info) = naga::back::pipeline_constants::process_overrides( let (module, info) = naga::back::pipeline_constants::process_overrides(
@ -240,7 +253,7 @@ impl super::Device {
let reflection_info = { let reflection_info = {
profiling::scope!("naga::back::hlsl::write"); profiling::scope!("naga::back::hlsl::write");
writer writer
.write(&module, &info) .write(&module, &info, frag_ep.as_ref())
.map_err(|e| crate::PipelineError::Linkage(stage_bit, format!("HLSL: {e:?}")))? .map_err(|e| crate::PipelineError::Linkage(stage_bit, format!("HLSL: {e:?}")))?
}; };
@ -1335,12 +1348,16 @@ impl crate::Device for super::Device {
let (topology_class, topology) = conv::map_topology(desc.primitive.topology); let (topology_class, topology) = conv::map_topology(desc.primitive.topology);
let mut shader_stages = wgt::ShaderStages::VERTEX; let mut shader_stages = wgt::ShaderStages::VERTEX;
let blob_vs = let blob_vs = self.load_shader(
self.load_shader(&desc.vertex_stage, desc.layout, naga::ShaderStage::Vertex)?; &desc.vertex_stage,
desc.layout,
naga::ShaderStage::Vertex,
desc.fragment_stage.as_ref(),
)?;
let blob_fs = match desc.fragment_stage { let blob_fs = match desc.fragment_stage {
Some(ref stage) => { Some(ref stage) => {
shader_stages |= wgt::ShaderStages::FRAGMENT; shader_stages |= wgt::ShaderStages::FRAGMENT;
Some(self.load_shader(stage, desc.layout, naga::ShaderStage::Fragment)?) Some(self.load_shader(stage, desc.layout, naga::ShaderStage::Fragment, None)?)
} }
None => None, None => None,
}; };
@ -1523,7 +1540,8 @@ impl crate::Device for super::Device {
&self, &self,
desc: &crate::ComputePipelineDescriptor<super::Api>, desc: &crate::ComputePipelineDescriptor<super::Api>,
) -> Result<super::ComputePipeline, crate::PipelineError> { ) -> Result<super::ComputePipeline, crate::PipelineError> {
let blob_cs = self.load_shader(&desc.stage, desc.layout, naga::ShaderStage::Compute)?; let blob_cs =
self.load_shader(&desc.stage, desc.layout, naga::ShaderStage::Compute, None)?;
let pair = { let pair = {
profiling::scope!("ID3D12Device::CreateComputePipelineState"); profiling::scope!("ID3D12Device::CreateComputePipelineState");

View File

@ -471,7 +471,6 @@ impl super::Adapter {
wgt::Features::SHADER_EARLY_DEPTH_TEST, wgt::Features::SHADER_EARLY_DEPTH_TEST,
supported((3, 1), (4, 2)) || extensions.contains("GL_ARB_shader_image_load_store"), supported((3, 1), (4, 2)) || extensions.contains("GL_ARB_shader_image_load_store"),
); );
features.set(wgt::Features::SHADER_UNUSED_VERTEX_OUTPUT, true);
if extensions.contains("GL_ARB_timer_query") { if extensions.contains("GL_ARB_timer_query") {
features.set(wgt::Features::TIMESTAMP_QUERY, true); features.set(wgt::Features::TIMESTAMP_QUERY, true);
features.set(wgt::Features::TIMESTAMP_QUERY_INSIDE_ENCODERS, true); features.set(wgt::Features::TIMESTAMP_QUERY_INSIDE_ENCODERS, true);

View File

@ -914,7 +914,6 @@ impl super::PrivateCapabilities {
features.set(F::ADDRESS_MODE_CLAMP_TO_ZERO, true); features.set(F::ADDRESS_MODE_CLAMP_TO_ZERO, true);
features.set(F::RG11B10UFLOAT_RENDERABLE, self.format_rg11b10_all); features.set(F::RG11B10UFLOAT_RENDERABLE, self.format_rg11b10_all);
features.set(F::SHADER_UNUSED_VERTEX_OUTPUT, true);
if self.supports_simd_scoped_operations { if self.supports_simd_scoped_operations {
features.insert(F::SUBGROUP | F::SUBGROUP_BARRIER); features.insert(F::SUBGROUP | F::SUBGROUP_BARRIER);

View File

@ -735,7 +735,6 @@ impl PhysicalDeviceFeatures {
| vk::FormatFeatureFlags::COLOR_ATTACHMENT_BLEND, | vk::FormatFeatureFlags::COLOR_ATTACHMENT_BLEND,
); );
features.set(F::RG11B10UFLOAT_RENDERABLE, rg11b10ufloat_renderable); features.set(F::RG11B10UFLOAT_RENDERABLE, rg11b10ufloat_renderable);
features.set(F::SHADER_UNUSED_VERTEX_OUTPUT, true);
features.set( features.set(
F::BGRA8UNORM_STORAGE, F::BGRA8UNORM_STORAGE,

View File

@ -802,14 +802,6 @@ bitflags::bitflags! {
/// ///
/// This is a native only feature. /// This is a native only feature.
const VERTEX_ATTRIBUTE_64BIT = 1 << 45; const VERTEX_ATTRIBUTE_64BIT = 1 << 45;
/// Allows vertex shaders to have outputs which are not consumed
/// by the fragment shader.
///
/// Supported platforms:
/// - Vulkan
/// - Metal
/// - OpenGL
const SHADER_UNUSED_VERTEX_OUTPUT = 1 << 46;
/// Allows for creation of textures of format [`TextureFormat::NV12`] /// Allows for creation of textures of format [`TextureFormat::NV12`]
/// ///
/// Supported platforms: /// Supported platforms: