From eb06c9dd5fea6d67481ca8089d02078d7138c013 Mon Sep 17 00:00:00 2001 From: Chip Davis Date: Wed, 5 Sep 2018 13:51:05 -0500 Subject: [PATCH 1/3] vkCmdFillBuffer: Use the older method to dispatch the compute kernel. We're not even running more than one thread, let alone taking advantage of the new method's automatic threadgroup sizing. --- MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm b/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm index b54fc064..68f8759c 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm +++ b/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm @@ -1012,7 +1012,7 @@ void MVKCmdFillBuffer::encode(MVKCommandEncoder* cmdEncoder) { [mtlComputeEnc setComputePipelineState: cmdEncoder->getCommandEncodingPool()->getCmdFillBufferMTLComputePipelineState()]; [mtlComputeEnc setBuffer: dstMTLBuff offset: dstMTLBuffOffset atIndex: 0]; [mtlComputeEnc setBytes: &fillInfo length: sizeof(fillInfo) atIndex: 1]; - [mtlComputeEnc dispatchThreads: MTLSizeMake(1, 1, 1) threadsPerThreadgroup: MTLSizeMake(1, 1, 1)]; + [mtlComputeEnc dispatchThreadgroups: MTLSizeMake(1, 1, 1) threadsPerThreadgroup: MTLSizeMake(1, 1, 1)]; [mtlComputeEnc popDebugGroup]; } From 38825b960087dd070d24efacde5c07a871279ef4 Mon Sep 17 00:00:00 2001 From: Chip Davis Date: Fri, 7 Sep 2018 12:25:40 -0500 Subject: [PATCH 2/3] vkUpdateDescriptorSet: Handle copies of uninitialized descriptors. For a copy, the spec requires only that the source and destination bindings are valid, not that the source has been initialized. If the source binding hasn't been initialized, then we crash attempting to get the Metal resources corresponding to the uninitialized descriptors. So, if the descriptor binding hasn't been initialized, don't try to fetch Metal resources for it. Yes, this causes us to accept writes of NULL descriptors (from templates or otherwise), even though the spec forbids this. I don't know how to solve this without specializing `writeDescriptorSets()` specifically for the `VkCopyDescriptorSet` case (thereby duplicating quite a bit of code). Given the general nature of Vulkan as a framework that does little to no state validation, I wonder if it's even worth it. --- MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm index bd431537..6377ce4e 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm @@ -605,7 +605,7 @@ uint32_t MVKDescriptorBinding::writeBindings(uint32_t srcStartIndex, const auto* pImgInfo = &get(pData, stride, srcStartIndex + i); _imageBindings[dstIdx] = *pImgInfo; if (_hasDynamicSamplers) { - _mtlSamplers[dstIdx] = ((MVKSampler*)pImgInfo->sampler)->getMTLSamplerState(); + _mtlSamplers[dstIdx] = pImgInfo->sampler ? ((MVKSampler*)pImgInfo->sampler)->getMTLSamplerState() : nil; } } break; @@ -615,9 +615,9 @@ uint32_t MVKDescriptorBinding::writeBindings(uint32_t srcStartIndex, uint32_t dstIdx = dstStartIndex + i; const auto* pImgInfo = &get(pData, stride, srcStartIndex + i); _imageBindings[dstIdx] = *pImgInfo; - _mtlTextures[dstIdx] = ((MVKImageView*)pImgInfo->imageView)->getMTLTexture(); + _mtlTextures[dstIdx] = pImgInfo->imageView ? ((MVKImageView*)pImgInfo->imageView)->getMTLTexture() : nil; if (_hasDynamicSamplers) { - _mtlSamplers[dstIdx] = ((MVKSampler*)pImgInfo->sampler)->getMTLSamplerState(); + _mtlSamplers[dstIdx] = pImgInfo->sampler ? ((MVKSampler*)pImgInfo->sampler)->getMTLSamplerState() : nil; } } break; @@ -629,7 +629,7 @@ uint32_t MVKDescriptorBinding::writeBindings(uint32_t srcStartIndex, uint32_t dstIdx = dstStartIndex + i; const auto* pImgInfo = &get(pData, stride, srcStartIndex + i); _imageBindings[dstIdx] = *pImgInfo; - _mtlTextures[dstIdx] = ((MVKImageView*)pImgInfo->imageView)->getMTLTexture(); + _mtlTextures[dstIdx] = pImgInfo->imageView ? ((MVKImageView*)pImgInfo->imageView)->getMTLTexture() : nil; } break; @@ -642,8 +642,8 @@ uint32_t MVKDescriptorBinding::writeBindings(uint32_t srcStartIndex, const auto* pBuffInfo = &get(pData, stride, srcStartIndex + i); _bufferBindings[dstIdx] = *pBuffInfo; MVKBuffer* mtlBuff = (MVKBuffer*)pBuffInfo->buffer; - _mtlBuffers[dstIdx] = mtlBuff->getMTLBuffer(); - _mtlBufferOffsets[dstIdx] = mtlBuff->getMTLBufferOffset() + pBuffInfo->offset; + _mtlBuffers[dstIdx] = mtlBuff ? mtlBuff->getMTLBuffer() : nil; + _mtlBufferOffsets[dstIdx] = mtlBuff ? (mtlBuff->getMTLBufferOffset() + pBuffInfo->offset) : 0; } break; @@ -653,7 +653,7 @@ uint32_t MVKDescriptorBinding::writeBindings(uint32_t srcStartIndex, uint32_t dstIdx = dstStartIndex + i; const auto* pBuffView = &get(pData, stride, srcStartIndex + i); _texelBufferBindings[dstIdx] = *pBuffView; - _mtlTextures[dstIdx] = ((MVKBufferView*)*pBuffView)->getMTLTexture(); + _mtlTextures[dstIdx] = *pBuffView ? ((MVKBufferView*)*pBuffView)->getMTLTexture() : nil; } break; default: From e6fe9c093a42c9c334c5edae98281f9a841d7af2 Mon Sep 17 00:00:00 2001 From: Chip Davis Date: Fri, 7 Sep 2018 14:19:15 -0500 Subject: [PATCH 3/3] Use the older method for vkCmdCopyBuffers(), too. --- MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm b/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm index 68f8759c..d315f367 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm +++ b/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm @@ -594,7 +594,7 @@ void MVKCmdCopyBuffer::encode(MVKCommandEncoder* cmdEncoder) { [mtlComputeEnc setBuffer:srcMTLBuff offset: srcMTLBuffOffset atIndex: 0]; [mtlComputeEnc setBuffer:dstMTLBuff offset: dstMTLBuffOffset atIndex: 1]; [mtlComputeEnc setBytes: ©Info length: sizeof(copyInfo) atIndex: 2]; - [mtlComputeEnc dispatchThreads: MTLSizeMake(1, 1, 1) threadsPerThreadgroup: MTLSizeMake(1, 1, 1)]; + [mtlComputeEnc dispatchThreadgroups: MTLSizeMake(1, 1, 1) threadsPerThreadgroup: MTLSizeMake(1, 1, 1)]; [mtlComputeEnc popDebugGroup]; } else { id mtlBlitEnc = cmdEncoder->getMTLBlitEncoder(kMVKCommandUseCopyBuffer);