Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ Top level categories:

Bottom level categories:

- naga
- General
- naga
- Validation
- DX12
- Vulkan
- Metal
Expand All @@ -50,14 +51,20 @@ Bottom level categories:
#### General

- BREAKING: Migrated from the `maxInterStageShaderComponents` limit to `maxInterStageShaderVariables`, which changes validation in a way that should not affect most programs. This follows the latest changes of the WebGPU spec. By @ErichDonGubler in [#8652](https://github.com/gfx-rs/wgpu/pull/8652), [#8792](https://github.com/gfx-rs/wgpu/pull/8792).
- Fixed validation of the texture format in GPUDepthStencilState when neither depth nor stencil is actually enabled. By @andyleiserson in [#8766](https://github.com/gfx-rs/wgpu/pull/8766).
- Tracing support has been restored. By @andyleiserson in [#8429](https://github.com/gfx-rs/wgpu/pull/8429).
- Allow using a shader that defines I/O for dual-source blending in a pipeline that does not make use of it. By @andyleiserson in [#8856](https://github.com/gfx-rs/wgpu/pull/8856).

#### naga

- The validator checks that override-sized arrays have a positive size, if overrides have been resolved. By @andyleiserson in [#8822](https://github.com/gfx-rs/wgpu/pull/8822).
- Fix some cases where f16 constants were not working. By @andyleiserson in [#8816](https://github.com/gfx-rs/wgpu/pull/8816).

#### Validation

- Fixed validation of the texture format in GPUDepthStencilState when neither depth nor stencil is actually enabled. By @andyleiserson in [#8766](https://github.com/gfx-rs/wgpu/pull/8766).
- Check that depth bias is not used with non-triangle topologies. By @andyleiserson in [#8856](https://github.com/gfx-rs/wgpu/pull/8856).
- Check that if the shader outputs `frag_depth`, then the pipeline must have a depth attachment. By @andyleiserson in [#8856](https://github.com/gfx-rs/wgpu/pull/8856).

#### GLES

- `DisplayHandle` should now be passed to `InstanceDescriptor` for correct EGL initialization on Wayland. By @MarijnS95 in [#8012](https://github.com/gfx-rs/wgpu/pull/8012)
Expand Down
3 changes: 3 additions & 0 deletions cts_runner/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,11 @@ webgpu:api,validation,render_pass,render_pass_descriptor:attachments,*
webgpu:api,validation,render_pass,render_pass_descriptor:color_attachments,*
webgpu:api,validation,render_pass,render_pass_descriptor:resolveTarget,*
webgpu:api,validation,render_pass,resolve:resolve_attachment:*
webgpu:api,validation,render_pipeline,depth_stencil_state:depth_bias:*
webgpu:api,validation,render_pipeline,depth_stencil_state:depth_write,frag_depth:*
webgpu:api,validation,render_pipeline,depth_stencil_state:format:*
webgpu:api,validation,render_pipeline,depth_stencil_state:stencil_write:*
webgpu:api,validation,render_pipeline,fragment_state:dual_source_blending,use_blend_src:*
webgpu:api,validation,render_pipeline,inter_stage:max_shader_variable_location:isAsync=false;*
//FAIL: webgpu:api,validation,render_pipeline,inter_stage:max_shader_variable_location:isAsync=true;*
// https://github.com/gfx-rs/wgpu/pull/8712
Expand Down
42 changes: 19 additions & 23 deletions tests/tests/wgpu-gpu/dual_source_blending.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,30 +182,26 @@ async fn dual_source_blending_enabled(ctx: TestingContext) {
..render_pipeline_descriptor_template.clone()
});

// Failure mode:
// Happy path:
// blend operator dual source: no
// shader handling dual source: yes
fail(
&ctx.device,
|| {
let _ = ctx
.device
.create_render_pipeline(&wgpu::RenderPipelineDescriptor {
fragment: Some(wgpu::FragmentState {
module: &fragment_shader_with_dual_source_blending,
entry_point: None,
targets: &[Some(wgpu::ColorTargetState {
format: wgpu::TextureFormat::Rgba8Unorm,
blend: None,
write_mask: wgpu::ColorWrites::all(),
})],
compilation_options: Default::default(),
}),
..render_pipeline_descriptor_template.clone()
});
},
Some("Shader entry point expects the pipeline to make use of dual-source blending."),
);
// (It is okay for the shader to define dual-source I/O that the pipeline
// does not use.)
let _ = ctx
.device
.create_render_pipeline(&wgpu::RenderPipelineDescriptor {
fragment: Some(wgpu::FragmentState {
module: &fragment_shader_with_dual_source_blending,
entry_point: None,
targets: &[Some(wgpu::ColorTargetState {
format: wgpu::TextureFormat::Rgba8Unorm,
blend: None,
write_mask: wgpu::ColorWrites::all(),
})],
compilation_options: Default::default(),
}),
..render_pipeline_descriptor_template.clone()
});

// Failure mode:
// blend operator dual source: yes
Expand All @@ -229,6 +225,6 @@ async fn dual_source_blending_enabled(ctx: TestingContext) {
..render_pipeline_descriptor_template.clone()
});
},
Some("Pipeline expects the shader entry point to make use of dual-source blending."),
Some("Pipeline uses dual-source blending, but the shader does not support it"),
);
}
47 changes: 20 additions & 27 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3896,14 +3896,12 @@ impl Device {
let mut vertex_steps;
let mut vertex_buffers;
let mut total_attributes;
let mut shader_expects_dual_source_blending = false;
let mut pipeline_expects_dual_source_blending = false;
let mut dual_source_blending = false;
let mut has_depth_attachment = false;
if let pipeline::RenderPipelineVertexProcessor::Vertex(ref vertex) = desc.vertex {
vertex_steps = Vec::with_capacity(vertex.buffers.len());
vertex_buffers = Vec::with_capacity(vertex.buffers.len());
total_attributes = 0;
shader_expects_dual_source_blending = false;
pipeline_expects_dual_source_blending = false;
for (i, vb_state) in vertex.buffers.iter().enumerate() {
// https://gpuweb.github.io/gpuweb/#abstract-opdef-validating-gpuvertexbufferlayout

Expand Down Expand Up @@ -4123,7 +4121,7 @@ impl Device {
if factor.ref_second_blend_source() {
self.require_features(wgt::Features::DUAL_SOURCE_BLENDING)?;
if i == 0 {
pipeline_expects_dual_source_blending = true;
dual_source_blending = true;
break;
} else {
return Err(pipeline::CreateRenderPipelineError
Expand Down Expand Up @@ -4169,7 +4167,9 @@ impl Device {
}

let aspect = hal::FormatAspects::from(ds.format);
if ds.is_depth_enabled() && !aspect.contains(hal::FormatAspects::DEPTH) {
if aspect.contains(hal::FormatAspects::DEPTH) {
has_depth_attachment = true;
} else if ds.is_depth_enabled() {
break 'error Some(pipeline::DepthStencilStateError::FormatNotDepth(ds.format));
}
if ds.stencil.is_enabled() && !aspect.contains(hal::FormatAspects::STENCIL) {
Expand Down Expand Up @@ -4205,6 +4205,16 @@ impl Device {
if ds.bias.clamp != 0.0 {
self.require_downlevel_flags(wgt::DownlevelFlags::DEPTH_BIAS_CLAMP)?;
}

if (ds.bias.is_enabled() || ds.bias.clamp != 0.0)
&& !desc.primitive.topology.is_triangles()
{
return Err(pipeline::CreateRenderPipelineError::DepthStencilState(
pipeline::DepthStencilStateError::DepthBiasWithIncompatibleTopology(
desc.primitive.topology,
),
));
}
}

if !target_specified {
Expand Down Expand Up @@ -4378,7 +4388,10 @@ impl Device {
let fragment_entry_point_name;
let fragment_stage = match desc.fragment {
Some(ref fragment_state) => {
let stage = validation::ShaderStageForValidation::Fragment;
let stage = validation::ShaderStageForValidation::Fragment {
dual_source_blending,
has_depth_attachment,
};
let stage_bit = stage.to_wgt_bit();

let shader_module = &fragment_state.stage.module;
Expand Down Expand Up @@ -4413,15 +4426,6 @@ impl Device {
validated_stages |= stage_bit;
}

if let Some(ref interface) = shader_module.interface {
shader_expects_dual_source_blending = interface
.fragment_uses_dual_source_blending(&fragment_entry_point_name)
.map_err(|error| pipeline::CreateRenderPipelineError::Stage {
stage: stage_bit,
error,
})?;
}

Some(hal::ProgrammableStage {
module: shader_module.raw(),
entry_point: &fragment_entry_point_name,
Expand All @@ -4434,17 +4438,6 @@ impl Device {
None => None,
};

if !pipeline_expects_dual_source_blending && shader_expects_dual_source_blending {
return Err(
pipeline::CreateRenderPipelineError::ShaderExpectsPipelineToUseDualSourceBlending,
);
}
if pipeline_expects_dual_source_blending && !shader_expects_dual_source_blending {
return Err(
pipeline::CreateRenderPipelineError::PipelineExpectsShaderToUseDualSourceBlending,
);
}

if validated_stages.contains(wgt::ShaderStages::FRAGMENT) {
for (i, output) in io.varyings.iter() {
match color_targets.get(*i as usize) {
Expand Down
8 changes: 2 additions & 6 deletions wgpu-core/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,8 @@ pub enum DepthStencilStateError {
FormatNotStencil(wgt::TextureFormat),
#[error("Sample count {0} is not supported by format {1:?} on this device. The WebGPU spec guarantees {2:?} samples are supported by this format. With the TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES feature your device supports {3:?}.")]
InvalidSampleCount(u32, wgt::TextureFormat, Vec<u32>, Vec<u32>),
#[error("Depth bias is not compatible with non-triangle topology {0:?}")]
DepthBiasWithIncompatibleTopology(wgt::PrimitiveTopology),
}

#[derive(Clone, Debug, Error)]
Expand Down Expand Up @@ -702,10 +704,6 @@ pub enum CreateRenderPipelineError {
factor: wgt::BlendFactor,
target: u32,
},
#[error("Pipeline expects the shader entry point to make use of dual-source blending.")]
PipelineExpectsShaderToUseDualSourceBlending,
#[error("Shader entry point expects the pipeline to make use of dual-source blending.")]
ShaderExpectsPipelineToUseDualSourceBlending,
#[error("{}", concat!(
"At least one color attachment or depth-stencil attachment was expected, ",
"but no render target for the pipeline was specified."
Expand Down Expand Up @@ -742,8 +740,6 @@ impl WebGpuError for CreateRenderPipelineError {
| Self::Stage { .. }
| Self::UnalignedShader { .. }
| Self::BlendFactorOnUnsupportedTarget { .. }
| Self::PipelineExpectsShaderToUseDualSourceBlending
| Self::ShaderExpectsPipelineToUseDualSourceBlending
| Self::NoTargetSpecified
| Self::PipelineConstants { .. }
| Self::VertexAttributeStrideTooLarge { .. } => return ErrorType::Validation,
Expand Down
39 changes: 34 additions & 5 deletions wgpu-core/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,10 @@ pub enum StageError {
MissingPrimitiveIndex,
#[error("DrawId cannot be used in the same pipeline as a task shader")]
DrawIdError,
#[error("Pipeline uses dual-source blending, but the shader does not support it")]
InvalidDualSourceBlending,
#[error("Fragment shader writes depth, but pipeline does not have a depth attachment")]
MissingFragDepthAttachment,
}

impl WebGpuError for StageError {
Expand Down Expand Up @@ -428,7 +432,9 @@ impl WebGpuError for StageError {
| Self::TaskPayloadMustMatch { .. }
| Self::InvalidPrimitiveIndex
| Self::MissingPrimitiveIndex
| Self::DrawIdError => return ErrorType::Validation,
| Self::DrawIdError
| Self::InvalidDualSourceBlending
| Self::MissingFragDepthAttachment => return ErrorType::Validation,
};
e.webgpu_error_type()
}
Expand Down Expand Up @@ -1563,7 +1569,10 @@ impl Interface {
});
}
}
ShaderStageForValidation::Fragment => {
ShaderStageForValidation::Fragment {
dual_source_blending,
has_depth_attachment,
} => {
let mut max_fragment_shader_input_variables =
self.limits.max_inter_stage_shader_variables;

Expand Down Expand Up @@ -1631,6 +1640,23 @@ impl Interface {
});
}
}

// If the pipeline uses dual-source blending, then the shader
// must configure appropriate I/O, but it is not an error to
// use a shader that defines the I/O in a pipeline that only
// uses one blend source.
if dual_source_blending && !entry_point.dual_source_blending {
return Err(StageError::InvalidDualSourceBlending);
}

if entry_point
.outputs
.iter()
.any(|varying| matches!(varying, Varying::BuiltIn(naga::BuiltIn::FragDepth)))
&& !has_depth_attachment
{
return Err(StageError::MissingFragDepthAttachment);
}
}
_ => (),
}
Expand Down Expand Up @@ -1753,7 +1779,10 @@ pub enum ShaderStageForValidation {
compare_function: Option<wgt::CompareFunction>,
},
Mesh,
Fragment,
Fragment {
dual_source_blending: bool,
has_depth_attachment: bool,
},
Compute,
Task,
}
Expand All @@ -1763,7 +1792,7 @@ impl ShaderStageForValidation {
match self {
Self::Vertex { .. } => naga::ShaderStage::Vertex,
Self::Mesh => naga::ShaderStage::Mesh,
Self::Fragment => naga::ShaderStage::Fragment,
Self::Fragment { .. } => naga::ShaderStage::Fragment,
Self::Compute => naga::ShaderStage::Compute,
Self::Task => naga::ShaderStage::Task,
}
Expand All @@ -1772,7 +1801,7 @@ impl ShaderStageForValidation {
pub fn to_wgt_bit(&self) -> wgt::ShaderStages {
match self {
Self::Vertex { .. } => wgt::ShaderStages::VERTEX,
Self::Mesh { .. } => wgt::ShaderStages::MESH,
Self::Mesh => wgt::ShaderStages::MESH,
Self::Fragment { .. } => wgt::ShaderStages::FRAGMENT,
Self::Compute => wgt::ShaderStages::COMPUTE,
Self::Task => wgt::ShaderStages::TASK,
Expand Down
9 changes: 9 additions & 0 deletions wgpu-types/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,15 @@ impl PrimitiveTopology {
Self::LineStrip | Self::TriangleStrip => true,
}
}

/// Returns true for triangle topologies.
#[must_use]
pub fn is_triangles(&self) -> bool {
match *self {
Self::TriangleList | Self::TriangleStrip => true,
Self::PointList | Self::LineList | Self::LineStrip => false,
}
}
}

/// Vertex winding order which classifies the "front" face of a triangle.
Expand Down