Skip to content

Comments

fix validation of dual source blending structs#8939

Open
vxpm wants to merge 1 commit intogfx-rs:trunkfrom
vxpm:dual-src-validation
Open

fix validation of dual source blending structs#8939
vxpm wants to merge 1 commit intogfx-rs:trunkfrom
vxpm:dual-src-validation

Conversation

@vxpm
Copy link

@vxpm vxpm commented Jan 27, 2026

Description
validation for structs that contain a blend_src attribute in one of their members is wrong. the WebGPU spec says:

For each structure type S defined in a WGSL module (not just those used in shader stage inputs or outputs), let members be the set of members of S that have location attributes.

If any entry in members specifies a blend_src attribute, members must use dual source blending, which means:

  • members must contain exactly 2 entries, one with @location(0) @blend_src(0) and one with @location(0) @blend_src(1).
  • All the members must have same data type.

Otherwise, members must not contain two entries with the same location value.

(source)

the part where naga is wrong is this one:

let members be the set of members of S that have location attributes

currently, naga uses all the members of S during validation, instead of just the members with location attributes. this means you can't use any members with builtin attributes (such as frag_depth).

this PR fixes this by only checking members with location attributes.

related PR: #8856

Copy link
Contributor

@andyleiserson andyleiserson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to implement the validation of blend_src structs in valid/type.rs. The spec says to do the validation:

for each structure type S defined in a WGSL module (not just those used in shader stage inputs or outputs)

Then, I think we could remove some of the interface validation related to blend_src and rely on the type validation.

There are some CTS tests related to dual-source blending that we may be able to pass after this change (there's information on running CTS tests in docs/testing.md):

  • webgpu:api,validation,render_pipeline,fragment_state:dual_source_blending,*
  • webgpu:shader,validation,extension,dual_source_blending:*

The only other check I'm aware of that's missing is something like this, with the color target validation in create_render_pipeline:

if dual_source_blending && color_targets.len() > 1 {
    return Err(
        pipeline::CreateRenderPipelineError::DualSourceBlendingWithMultipleColorTargets {
            count: color_targets.len() as u32,
        },
    );
}

Comment on lines +664 to +669

// Also, all of them must have the same type.
if location_members[0].ty != location_members[1].ty {
return Err(VaryingError::BlendSrcOutputTypeMismatch {
blend_src_0_type: members[0].ty,
blend_src_1_type: members[1].ty,
blend_src_0_type: location_members[0].ty,
blend_src_1_type: location_members[1].ty,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message text uses the labels blend_src(0) and blend_src(1), but location_members could have them in the opposite order if they were declared that way in the source.

I think it would be possible to do one pass over the members and save the types of the blend_src members in an array [Option<Handle<Type>>; 2] along the way using the actual index values, so that they are identified correctly in the error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants