gfx-rs/wgpu-rs

Potentially Incorrect shader validation?

Nehliin opened this issue · 4 comments

I ran into an issue when creating a new render pass which uses the same type of resources which works fine in previous render passes. Basically it seems like my previous render passes isn't parsed correctly
by naga
and thus are never validated. However in this new render pass they are indeed parsed correctly and gives me the following validation error:

wgpu error: Validation Error

Caused by:
    In Device::create_render_pipeline
      note: label = `Debuglines pipeline`
    error in stage VERTEX
    error matching the stage input at 0 against the previous stage outputs: input type is not compatible with the provided

I am a bit unsure if it's an actual error because it seems to me like it's a false positive?
Anyways here is the code:
debug_lines.vert:

#version 450

layout(location = 0) in mat4 model;
layout(location = 0) out vec3 fragment_position;

layout(set=0, binding=0, std140)
uniform CameraUniforms {
    mat4 view;
    mat4 projection;
    vec3 view_pos;
};

void main() {
    fragment_position = vec3(model * vec4(1.0));
    gl_Position = projection * view * vec4(fragment_position, 1.0);
}

Render pipeline:

#[repr(C)]
struct InstanceData {
    model_matrix: [[f32;4]; 4]
}
const ROW_SIZE: wgpu::BufferAddress = (std::mem::size_of::<f32>() * 4) as wgpu::BufferAddress;

let render_pipeline_layout =
            device.create_pipeline_layout(&wgpu::PipelineLayoutDescriptor {
                label: Some("Debug lines pass pipeline layout"),
                bind_group_layouts: &[&camera_bind_group_layout],
                push_constant_ranges: &[],
            });
        let render_pipeline = device.create_render_pipeline(&wgpu::RenderPipelineDescriptor {
            label: Some("Debuglines pipeline"),
            layout: Some(&render_pipeline_layout),
            vertex: wgpu::VertexState {
                module: &vs_module,
                entry_point: "main",
                buffers: &[
                     wgpu::VertexBufferLayout {
                        array_stride: std::mem::size_of::<InstanceData>() as wgpu::BufferAddress,
                        step_mode: wgpu::InputStepMode::Instance,
                        attributes: &[
                            wgpu::VertexAttribute {
                                offset: 0,
                                format: wgpu::VertexFormat::Float4,
                                shader_location: 0,
                            },
                            wgpu::VertexAttribute {
                                offset: ROW_SIZE,
                                format: wgpu::VertexFormat::Float4,
                                shader_location: 1,
                            },
                            wgpu::VertexAttribute {
                                offset: ROW_SIZE * 2,
                                format: wgpu::VertexFormat::Float4,
                                shader_location: 2,
                            },
                            wgpu::VertexAttribute {
                                offset: ROW_SIZE * 3,
                                format: wgpu::VertexFormat::Float4,
                                shader_location: 3,
                            },
                        ],
                    },
                ],
            },
            primitive: wgpu::PrimitiveState {
                cull_mode: wgpu::CullMode::None,
                polygon_mode: wgpu::PolygonMode::Line,
                topology: wgpu::PrimitiveTopology::LineList,
                ..Default::default()
            },
            // details omitted...
        });
kvark commented

We don't have any support for vertex attributes that are matrices, so the validation error is correct.

@kvark thanks for the quick reply, when you say "we" do you mean naga or wgpu in general because I use the exact same vertex attribute in the other shader (it's my instance buffer) without problems. Does that mean it's not portable and I'm just lucky?

kvark commented

By "we" I meant WebGPU group. I think the reason it may work in other shaders is that something else in them was confusing the Naga's SPIR-V parser, so that validation wasn't even happening (and you should see the RUST_LOG messages about this).
I recommend just sending the transform as 3 vectors (perspective not needed), or even as displacement with rotation (2 vectors). Perhaps if the group comes back to this, we may open the path in the future.

@kvark Thanks for the response, yes I could confirm that the other shader was never validated like mentioned in the original post. I was mainly concerned that the implicit conversion from 4 vectors to a mat4 in glsl could break on certain backends or that I was relying on undefined behavior in general. But I'll stop relying on that and instead use the vectors directly in the shader like you suggested 👍