-
Notifications
You must be signed in to change notification settings - Fork 434
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
SPIR-V to MSL conversion error: Argument buffer resource base type could not be determined #2271
Comments
It's likely that PR #2260 and followups may have fixed this. Please build from the latest MoltenVK, and try again. Or you can wait for the next SDK, which should be released in a couple of weeks. With this updated code, |
It seems that reducing my descriptor counts to something very small (and therby my constexpr uint32_t maxBindlessResourceCount = 100; // Stops crashing if I change this to something like 25
constexpr uint32_t maxSamplerCount = 2;
std::array<VkDescriptorPoolSize, 2> bindlessDescriptorPoolSizes {{
{ VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, maxBindlessResourceCount},
{ VK_DESCRIPTOR_TYPE_SAMPLER, maxSamplerCount}
}};
VkDescriptorPoolCreateInfo poolCreateInfo = {
.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO,
.pNext = nullptr,
.flags = VK_DESCRIPTOR_POOL_CREATE_UPDATE_AFTER_BIND_BIT_EXT,
.maxSets = maxBindlessResourceCount * static_cast<uint32_t>(bindlessDescriptorPoolSizes.size()),
.poolSizeCount = static_cast<uint32_t>(bindlessDescriptorPoolSizes.size()),
.pPoolSizes = bindlessDescriptorPoolSizes.data()
}; Although this seems too small? |
Just pulled and built @ edbdcf0 Still getting the same crash unfortunately
Lowering I've verified everything renders correctly on a Windows system (Nvidia). |
Thanks for testing. Can you test again with Metal validation enabled, and report any validation errors that are logged, or that trip an assertion, please? You can do this using the following environment variable:
To avoid the assertion, and just log all Metal validation errors, you can also add the following environment variables:
|
Ah I forgot I am using |
I'm wondering if there's something disconnecting within MoltenVK between your descriptor set allocations and the descriptor pool. Can you run your app with the following env vars set, please?
You should seem some entries in the logs, similar to the following:
and then copy them and post them here. Or you can just ZIP up a copy of the log file and post it here as a file. And can you confirm that you have updated the descriptors with the full set of resources that are accessed statically or dynamically from the shader? Finally, is there any way you can post a minimal app that demonstrates the problem, so we can evaluate it here? |
@billhollings The log file with those 2 env vars enabled is attached:
If I understand your question correctly, yes. I do my
The repo for this app is public on my profile |
I'm seeing this in Godot when setting
|
There have been a number of MoltenVK updates around this since SDK Can you try building the latest MoltenVK from this repo, and retesting? If not, a new SDK will be released at the end of September that will have the latest MoltenVK in it. |
@billhollings thanks for the feedback – once I get further along with my work in Godot, that requires these improvements, I'll either compile from source or pull down the official release. |
Thanks for this. It was very helpful in getting to the bottom of this. The crash is being caused by a bug in the The issue of textures not being used correctly is due to your app treating the sampler descriptor binding as an array of length 2 (only one of which is used), but the SPIR-V shaders treating the descriptor binding as a single sampler (not an array). If this is working correctly on other platforms, then we should patch SPIRV-Cross to deal with extra padding in the descriptors. In the meantime, in your app, if you change the value of BTW...with |
Thank you for the information, and with the patch I'm able to work with the large texture count now :) Just to be extra clear for my own understanding, my usage of only 1 sampler in the array despite it being of length 2 is not incorrect usage is it? I'm not too familiar with things once they get down to SPIR-V. It does work properly on my Windows PC. |
The crash is being caused by a bug in the PR #2355 fixes the crash.
I believe what you are doing is correct, particularly if it is working on other platforms. GPU's have indexed descriptors. Unfortunately, despite the In this case, we'll have to add padding in the MSL structure after a SPIR-V scalar that is actually the first element of a fixed-length array. I'm going to leave this issue open, to track that fix. |
I did end up filling that other "slot" with another sampler s.t. I now have 2 samplers in the array: patrick-han/magic-red@d3ea2d7 But I'm still getting incorrect rendering on my Mac, where it's working on my PC. From what I understand the issue with the padding should only occur if I try to "leave a gap" though, right? No validation errors to report either. |
When I test patrick-han/magic-red@d3ea2d7, I am still seeing the same mismatch between the descriptor set layouts declaring an array of 2 samplers, and the SPIR-V variable definitions declaring a single scalar sampler:
|
PR #2409 fixes the remaining issue, where a shader reads the first element of an array as a scalar. |
This should also fix #2374, right? |
Sadly, no. The issue there is the use of a runtime array of Fixing this will require an intrusive update to SPIRV-Cross, as explained in this comment. |
I see --- I just thought the two issues were related, as they had the same error messages. |
This seems to have cropped up at various points before, but unsure if it's related. I have the latest VulkanSDK as of yesterday (1.3.283.0) so in theory I have the fix for these issues:
#2216
#2016
After enabling
MVK_CONFIG_USE_METAL_ARGUMENT_BUFFERS
programmatically I also disabled shader validation at the advice of one of the recent Vulkanised 2024 talks.The specific descriptor indexing features I enabled are:
descriptorBindingSampledImageUpdateAfterBind
,descriptorBindingPartiallyBound
,runtimeDescriptorArray
with the appropriate pool/binding/layout flags:VK_DESCRIPTOR_POOL_CREATE_UPDATE_AFTER_BIND_BIT_EXT
,VK_DESCRIPTOR_BINDING_PARTIALLY_BOUND_BIT_EXT
,VK_DESCRIPTOR_BINDING_UPDATE_AFTER_BIND_BIT_EXT
,VK_DESCRIPTOR_SET_LAYOUT_CREATE_UPDATE_AFTER_BIND_POOL_BIT_EXT
Although in my fragment shader I'm still just declaring my texture normally:
The text was updated successfully, but these errors were encountered: