KhronosGroup / MoltenVK

MoltenVK is a Vulkan Portability implementation. It layers a subset of the high-performance, industry-standard Vulkan graphics and compute API over Apple's Metal graphics framework, enabling Vulkan applications to run on macOS, iOS and tvOS.
Apache License 2.0
4.63k stars 402 forks source link

Vulkan examples regression: descriptor indexing and oit #2216

Closed SRSaunders closed 1 month ago

SRSaunders commented 2 months ago

I have been working on updating macOS and iOS support and fixing a few defects that I found in Sascha Willems' Vulkan examples repository. See https://github.com/SaschaWillems/Vulkan/pull/1117 and https://github.com/SaschaWillems/Vulkan/pull/1119. During this work I observed two examples that expose what appear to regressions across MoltenVK releases:

• The descriptorindexing example runs well on Vulkan SDK 1.3.250.1/MoltenVK 1.2.4, but fails on any later release with:

[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.
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.

• The oit example runs well up to SDK 1.3.275.0/MoltenVK 1.2.7, but fails on the current release (1.3.280.0/1):

-[MTLTextureDescriptorInternal validateWithDevice:]:1344: failed assertion `Texture Descriptor Validation
MTLTextureUsage has unknown bits 0x20.

The first of these may be a duplicate of #2180, but I just wanted to report them here to make sure the MoltenVK team is aware.

SRSaunders commented 2 months ago

After a bit of debugging, I believe the first one is a SPIRVCross defect. I patched in the old SPIRVCross (from MoltenVK 1.2.4) at commit 12542fc6fc05000e04742daf93892a0b10edbe80 to the most recent head of MoltenVK 1.2.9. I had to comment out a few newer msl options, but this did not seem to affect things for the descriptorindexing example. I now have a working example with this mashup. Note the newest SPRIVCross for SDK 1.3.283/MVK 1.2.9 does not fix it.

I have not done any research on the second issue above (oit example). Using the older SPIRVCross does not fix it. Screenshot 2024-05-05 at 11 22 08 AM

SRSaunders commented 2 months ago

Issue one (descriptorindexing) reported with additional data at the existing https://github.com/KhronosGroup/SPIRV-Cross/issues/2198

Issue two (oit) still not understood.

cdavis5e commented 2 months ago

The oit example runs well up to SDK 1.3.275.0/MoltenVK 1.2.7, but fails on the current release (1.3.280.0/1):

-[MTLTextureDescriptorInternal validateWithDevice:]:1344: failed assertion `Texture Descriptor Validation
MTLTextureUsage has unknown bits 0x20.

What OS? More specifically, what version?

SRSaunders commented 2 months ago

OS is Ventura 13.6.6 (on x86) with Xcode 15.2. GPU is AMD Radeon 6600 (Metal Tier 2).

cdavis5e commented 2 months ago

OS is Ventura 13.6.6 (on x86) with Xcode 15.2. GPU is AMD Radeon 6600 (Metal Tier 2).

The bit 0x20 is MTLTextureUsageShaderAtomic, which wasn't introduced until Sonoma. We shouldn't be using that bit prior to Sonoma.

billhollings commented 2 months ago

The descriptorindexing example runs well on Vulkan SDK 1.3.250.1/MoltenVK 1.2.4, but fails on any later release... /clip/ ...The first of these may be a duplicate of https://github.com/KhronosGroup/MoltenVK/issues/2180, but I just wanted to report them here to make sure the MoltenVK team is aware.

Thanks for submitting this as an issue affecting a known sample app! It made it much easier to track down!

PR #2231 fixes this part of this issue.

billhollings commented 2 months ago

OS is Ventura 13.6.6 (on x86) with Xcode 15.2. GPU is AMD Radeon 6600 (Metal Tier 2).

The bit 0x20 is MTLTextureUsageShaderAtomic, which wasn't introduced until Sonoma. We shouldn't be using that bit prior to Sonoma.

@SRSaunders Can you try changing:

https://github.com/KhronosGroup/MoltenVK/blob/1abae612ae940f69b82a5e67bec3a76a811f57a7/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm#L1084

to include , _shouldSupportAtomics && getPhysicalDevice()->useNativeTextureAtomics()); (and maybe put a couple of line-returns as needed to make the line not so far off to the right), and then retest, please?

SRSaunders commented 2 months ago

Fantastic! Both issues are solved for me now.

For issue one (arg buffer padding), I checked out https://github.com/KhronosGroup/SPIRV-Cross/commit/e41d79fe8fae3b950253e4da6527c34bc93b13dd and rebuilt the External Dependencies project, followed by the MoltenVK project. The descriptorindexing example is now working fine.

For issue two (MTLTextureUsageShaderAtomic), I added your additional code and rebuilt MoltenVK. The oit example is now working again.

Thanks for jumping on these and solving so quickly.

billhollings commented 2 months ago

For issue two (MTLTextureUsageShaderAtomic), I added your additional code and rebuilt MoltenVK. The oit example is now working again.

Wonderful! Thanks for testing.

I'll make that simple change here and run CTS to see if it breaks anything, then I'll issue a PR.

billhollings commented 1 month ago

For issue two (MTLTextureUsageShaderAtomic), I added your additional code and rebuilt MoltenVK. The oit example is now working again.

Wonderful! Thanks for testing.

I'll make that simple change here and run CTS to see if it breaks anything, then I'll issue a PR.

The updated PR #2231 now also fixes this part of this issue.

SRSaunders commented 1 month ago

I have now tested against the new merged PR and the Vulkan examples work well. Thanks again.

Closing issue.