Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storage image decorators are not parsed #141

Closed
juliusikkala opened this issue Apr 1, 2022 · 1 comment
Closed

Storage image decorators are not parsed #141

juliusikkala opened this issue Apr 1, 2022 · 1 comment

Comments

@juliusikkala
Copy link

juliusikkala commented Apr 1, 2022

I wanted to check whether a shader has marked a given storage image as readonly. I assume that this would result in SPV_REFLECT_DECORATION_NON_WRITABLE being visible somewhere, but this doesn't seem to be the case. Both SpvReflectDescriptorBinding.block.decoration_flags and SpvReflectDescriptorBinding.type_description->decoration_flags are zero regardless of the memory qualifier on the GLSL side.

I'm not particularly familiar with SPIR-V intricacies, so I don't really know which one of these two would be the right place for the flags for storage images. They're not really blocks, so maybe the type description? But it looks like the only SpvReflectPrvNodes with the correct decorator present are skipped on this line:

if (! p_node->is_type) {

Maybe a SpvReflectDecorationFlags decoration_flags; could be added to SpvReflectImageTraits instead? Or to SpvReflectDescriptorBinding directly?

Also, looks like there are no tests for storage images. Here's a simple shader to reproduce the issue:

#version 450
#pragma shader_stage(compute)

layout (local_size_x = 16, local_size_y = 16, local_size_z = 1) in;

layout(set = 0, binding = 0, rgba32f) uniform readonly image2D input_image;
layout(set = 0, binding = 1, rgba32f) uniform writeonly image2D output_image;

void main() {
    ivec2 p = ivec2(gl_GlobalInvocationID.xy);
    imageStore(output_image, p, imageLoad(input_image, p));
}

With the spirv-reflect program, there's no NON_WRITABLE in sight. I tested with the master branch in its current state.

chaoticbob added a commit that referenced this issue Apr 14, 2022
- Added parsing for SpvDecorationNonReadable decoration.
- Updated parsing to account for decorations at the descriptor binding
  level.
- Added new enum SPV_REFLECT_DECORATION_NON_READABLE.
- Added new member SpvReflectDescriptorBinding::decoration_flags.
- Added test case non_writable_image.glsl which has examples of both
  non_writable and non_readable images.
@chaoticbob
Copy link
Contributor

Hi - added decoration_flags to SpvReflectDescriptorBinding per your suggestion. It seemed like the most logical place to go for SPV_REFLECT_DECORATION_NON_WRITABLE and SPV_REFLECT_DECORATION_NON_READABLE.

I haven't updated spirv-reflect to output NON_WRITABLE and NON_READABLE yet. Need to think of a way to output these without it looking too messy. Also affects the tests which is really a constraint on time right now.

Thanks for the example shader. Let me know if you run into any issues.

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

No branches or pull requests

2 participants