PENGUINLIONG / spirq-rs

Light weight SPIR-V reflection library
Apache License 2.0
111 stars 12 forks source link

Access is not correctly calculated for some storage buffers #105

Closed expenses closed 1 year ago

expenses commented 1 year ago

Hi! Thanks for this fantastic crate, it's worked so well for my usecases. I'm running into a storage buffer problem with the attached shader where the descriptor type is StorageBuffer(ReadWrite) when it should be ReadOnly.

moon.spv.zip

I've looked at the visualization from https://www.khronos.org/spir/visualizer/ and inserted some dbg! statements which showed that get_desc_access is being called on id %16 and not %14, which is the thing actually being decorated by NonWritable:

https://github.com/PENGUINLIONG/spirq-rs/blob/f77af02d2f26c47bccebfc23df1460f6ae7affb4/spirq/src/reflect.rs#L1340-L1345

20230526_14h05m26s_grim

expenses commented 1 year ago

In fact, the same problem seems to happen even when I recompile it with the -fspv-target-env=vulkan1.3 hlsl flag which removed BufferBlock:

moon2.spv.zip

PENGUINLIONG commented 1 year ago

Thanks for your report. Will look into this.

PENGUINLIONG commented 1 year ago

It's kinda funny that your shader compiler applies the NonWritable decoration to the structure members instead of the declared variable. What was that? DXC?

PENGUINLIONG commented 1 year ago

Hi, hopefully this issue is addressed by #106. To ensure that this feature doesn't break in the future is it okay that I upload your moon.spv as a test case to this repo?

expenses commented 1 year ago

Hi, hopefully this issue is addressed by #106. To ensure that this feature doesn't break in the future is it okay that I upload your moon.spv as a test case to this repo?

Fantastic, yep that fixes it! You're absolutely welcome to add the shader as a test case. Here's the hlsl source as well (currently just a partially-implemented bindless pbr thing):


struct PushConstant {
    float4x4 combined_matrix;
    float3 camera_pos;
};

static const uint INVALID = 4294967295;

struct MaterialInfo {
    float4 base_color_factor;
    float3 emissive_factor;
    float metallic_factor;
    float roughness_factor;
    uint albedo_texture;
    uint normal_texture;
    uint emissive_texture;
};

[[vk::push_constant]]
PushConstant constant;

struct Varying {
    float4 builtin_position: SV_Position;
    float3 position: POSITION0;
    float2 uv: TEXCOORD0;
    float3 normal: NORMAL0;
    uint material_id: TEXCOORD1;
};

[[vk::binding(0)]] Texture2D<float3> tex[];
[[vk::binding(1)]] SamplerState samp;
[[vk::binding(2)]] StructuredBuffer<MaterialInfo> infos;

float length_squared(float3 vec) {
    return dot(vec, vec);
}

float3x3 compute_cotangent_frame(
    float3 normal,
    float3 position,
    float2 uv
) {
    float3 delta_pos_x = ddx(position);
    float3 delta_pos_y = ddy(position);
    float2 delta_uv_x = ddx(uv);
    float2 delta_uv_y = ddy(uv);

    float3 delta_pos_y_perp = cross(delta_pos_y, normal);
    float3 delta_pos_x_perp = cross(normal, delta_pos_x);

    float3 t = delta_pos_y_perp * delta_uv_x.x + delta_pos_x_perp * delta_uv_y.x;
    float3 b = delta_pos_y_perp * delta_uv_x.y + delta_pos_x_perp * delta_uv_y.y;

    float invmax = 1.0 / sqrt(max(length_squared(t), length_squared(b)));
    return transpose(float3x3(t * invmax, b * invmax, normal));
}

[shader("pixel")]
float4 PSMain(
    Varying varying
): SV_Target0 {
    MaterialInfo info = infos[varying.material_id];

    float3 normal = normalize(varying.normal);

    float3 view_vector = constant.camera_pos - varying.position;

    if (info.normal_texture != INVALID) {
        float3 tex_normal = tex[info.normal_texture].Sample(samp, varying.uv).xyz * 255.0 / 127.0 - 128.0 / 127.0;
        normal = normalize(mul(compute_cotangent_frame(normal, -view_vector, varying.uv), tex_normal));
    }

    float3 sun_dir = normalize(float3(1,1,1));
    float brightness = max(dot(normal, sun_dir), 0.0);

    float3 albedo = info.base_color_factor.rgb;

    if (info.albedo_texture != INVALID) {
        albedo *= tex[info.albedo_texture].Sample(samp, varying.uv).xyz;
    }

    float3 emissive = info.emissive_factor;

    if (info.emissive_texture != INVALID) {
        emissive *= tex[info.emissive_texture].Sample(samp, varying.uv).xyz;
    }

    return float4(albedo * brightness + emissive, 1.0);
}

You can compile it with dxc shader.hlsl -HV 2021 -T ps_6_6 -E PSMain -spirv -Fo shader.spv. dxc --version reports dxcompiler.dll: 1.7(dev;3759-8c9d92be).

PENGUINLIONG commented 1 year ago

Thank you! :)