Regression in Vulkan descriptorindexing sample

[SRSaunders]

Starting with Vulkan SDK 1.3.296, Sascha Willems' Vulkan descriptorindexing sample is broken. Unfortunately it remains broken with MoltenVK's current main development head with updated SPIRV-Cross. The error is:

[mvk-error] SPIR-V to MSL conversion error: Argument buffer resource base type could not be determined. When padding argument buffer elements, all descriptor set resources must be supplied with a base type by the app.
[mvk-error] VK_ERROR_INVALID_SHADER_NV: Fragment shader function could not be compiled into pipeline. See previous logged error.
ERROR: [0][mvk-error] : VK_ERROR_INVALID_SHADER_NV: Fragment shader function could not be compiled into pipeline. See previous logged error.

Fatal : VkResult is "ERROR_INVALID_SHADER_NV" in /Users/steve/XcodeProjects/Vulkan/examples/descriptorindexing/descriptorindexing.cpp at line 346
Assertion failed: (res == VK_SUCCESS), function preparePipelines, file descriptorindexing.cpp, line 346.
Error, result code = -6

This looks like a reoccurance of a previously solved issue, or potentially a dup of a currently open issue. I will review open PRs to check and update any found references here. Likely a dup of #2377 and #2374.

[SRSaunders]

Ok, I have read through #2374 and the descriptorindexing sample has the same situation with a combined image sampler and variable descriptor count. So the problem seems to be the same.

What is the likelihood of this deficiency being addressed in the near term? I am wondering whether it would be worthwhile to propose a MoltenVK-specific workaround in the descriptorindexing sample to solve until this is fixed.

UPDATE: It turns out this is very easy to work around in the descriptorindexing sample. I will likely propose a PR with this change to the samples repo, and monitor for any future developments on the MoltenVK side.

[billhollings]

@SRSaunders

UPDATE: It turns out this is very easy to work around in the descriptorindexing sample. I will likely propose a PR with this change to the samples repo, and monitor for any future developments on the MoltenVK side.

Thanks for triaging and debugging this.

Do you mean a PR to MoltenVK, or a PR to the sample app?

Are you able to give a quick description of what the issue was?

[SRSaunders]

@billhollings

Do you mean a PR to MoltenVK, or a PR to the sample app?

I mean a PR for the sample app, contained here: SaschaWillems/Vulkan#1192

Are you able to give a quick description of what the issue was?

The descriptorindexing sample app uses a combined image sampler with a variable descriptor count, which according to your previous answers in #2374, is not supported by MoltenVK and SPIRV-Cross at the moment. I was able to easily disable the variable descriptor aspect in the sample for macOS and iOS only - in this sample it is not really needed, but seems to be there mainly for illustrative purposes. My PR is not merged yet, so please review and comment if you have feedback.

See commits SaschaWillems/Vulkan@0ab8f8d and SaschaWillems/Vulkan@39a1320 which are part of the PR linked above.

Also, I can close off this issue here since I believe it is a dup of #2374. Please let me know.

[dboyan]

It seems a few recent issues (also including #2453 besides the ones mentioned above) are related to the variable descriptor count support. I'm not an expert here but I've been thinking if we can possibly solve the issue by transforming the spirv program before it is fed in spirv-cross and make some arrangement when constructing the pipeline. The goal is to make variable-sized array fixed on metal side by inserting a specialized array size, initialized at pipeline creation.

To be clear, what I'm thinking is:

• When we see we are using argument buffer and having multiple variable-sized arrays in the argument buffer, create a specialization constant to serve as the size of arrays and remember their constant ids. (or maybe just do it unconditionally)
• At pipeline creation time, apply specialization on the shader according to descriptor set layouts (along with the actual specialization provided by the user).

Note: with #2441, we support specialization constant as array dimension via macro.

Not sure if this route is feasible, or if there is something else missing I am not aware of. If this is indeed feasible, it would be interesting to see what else can be done if we have an IR (spirv) manipulation pass. It would be conceptually similar to the "lowering" passes mesa done in nir, and could be used to support features like VK_EXT_custom_border_color. Unfortunately I don't have extra bandwidth to realize such an idea as of now. Just throwing it in to see if someone would want to give it a shot.

[billhollings]

Note: with #2441, we support specialization constant as array dimension via macro.

While a good suggestion, my preference would be not to have to recompile a shader for each length of a variable-length array.

This issue and the others mentioned here are caused by the use of variable-length COMBINED_IMAGE_SAMPLER arrays. It would be much better to fix the problem in SPIRV-Cross as I describe in that issue. It will be a bit of a headache, which is why it hasn't been fixed yet, but probably no more than the changes you're suggesting.

However, if you have a good handle on what you want to do, and feel it will be relatively straightforward, and want to submit PRs here and at SPIRV-Cross, at least having your suggestion in place would change the suggestion I made to one of optimization instead of bug-fix.

[billhollings]

Also, I can close off this issue here since I believe it is a dup of #2374. Please let me know.

Thanks. Closing here.