Traverse-Research/rspirv-reflect

Add support for DescriptorType::COMBINED_IMAGE_SAMPLER

dffdff2423 opened this issue · 4 comments

Shader (fragment):

#version 450

layout (location = 0) in vec3 color;
layout (location = 1) in vec2 tex_cord;

layout (location = 0) out vec4 out_color;

layout (set = 0, binding = 0) uniform sampler2D tex;

void main()
{
	out_color = mix(texture(tex, tex_cord), vec4(color, 1.f), 0.2f);
}

Relevant part of the backtrace:

thread 'main' panicked at 'not yet implemented: Instruction { opname: "TypeSampledImage", opcode: TypeSampledImage, capabilities: [], extensions: [], operands: [LogicalOperand { kind: IdResult, quantifier: One }, LogicalOperand { kind: IdRef, quantifier: One }] } Not implemented; untested', /home/$username/.cargo/registry/src/github.com-1ecc6299db9ec823/rspirv-reflect-0.7.0/src/lib.rs:333:17
stack backtrace:
   0: rust_begin_unwind
             at /rustc/420c970cb1edccbf60ff2aeb51ca01e2300b09ef/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/420c970cb1edccbf60ff2aeb51ca01e2300b09ef/library/core/src/panicking.rs:142:14
   2: rspirv_reflect::Reflection::get_descriptor_type
             at /home/$username/.cargo/registry/src/github.com-1ecc6299db9ec823/rspirv-reflect-0.7.0/src/lib.rs:333:17
   3: rspirv_reflect::Reflection::get_descriptor_type_for_var
             at /home/$username/.cargo/registry/src/github.com-1ecc6299db9ec823/rspirv-reflect-0.7.0/src/lib.rs:241:9
   4: rspirv_reflect::Reflection::get_descriptor_type
             at /home/$username/.cargo/registry/src/github.com-1ecc6299db9ec823/rspirv-reflect-0.7.0/src/lib.rs:292:24
   5: rspirv_reflect::Reflection::get_descriptor_type_for_var
             at /home/$username/.cargo/registry/src/github.com-1ecc6299db9ec823/rspirv-reflect-0.7.0/src/lib.rs:241:9
   6: rspirv_reflect::Reflection::get_descriptor_sets
             at /home/$username/.cargo/registry/src/github.com-1ecc6299db9ec823/rspirv-reflect-0.7.0/src/lib.rs:485:21

It looks like the code is already written but is just commented out:
https://github.com/Traverse-Research/rspirv-reflect/blob/main/src/lib.rs#L335

Hi, thanks for reporting this! As you can see the message reads Not implemented; untested, I've simply never hit this bit of code to properly test it. I vaguely remember not trusting the dim match here, but if you have a use-case for it and can test+confirm that would be great to confirm 👍!

rspirv-reflect/src/lib.rs

Lines 333 to 343 in d968ff4

todo!("{:?} Not implemented; untested", type_instruction.class);
// Note that `dim`, `sampled` and `storage` are parsed from TypeImage
// if dim == SpvDimBuffer {
// if sampled {
// DescriptorType::UNIFORM_TEXEL_BUFFER
// } else if storage {
// DescriptorType::STORAGE_TEXEL_BUFFER
// }
// } else {
// DescriptorType::COMBINED_IMAGE_SAMPLER
// }

Looks like I actually handled this a bit different in #5: 3beb355.

Hi, any updates on this issue?

@chyyran I haven't had much feedback on #27 but think I'll just merge it based on the ack from @dffdff2423.