Merge pull request #1677 from billhollings/fix-missing-metal-buffer-binding

Fix occasional missing Metal buffer binding when only offset changes.

Docs/Whats_New.md

@@ -811,6 +811,7 @@ Released 2020/04/05
 - Fix memory estimates for iOS 13+.
 - Broaden conditions for host read sync for image memory barriers on macOS.
 - Fix issue of reseting `CAMetalDrawable` and `MTLTexture` of peer swapchain images.
+- Fix occasional missing Metal buffer binding when only offset changes.
 - Fix the `make install` build command to overwrite the existing framework in the system
   framework library, and update `README.md` to clarify the instructions for using `make install`. 
 - Update the `README.md` and `MoltenVK_Runtime_UserGuide.md` documents to clarify that 

MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm

@@ -478,8 +478,6 @@ void MVKCmdBlitImage<N>::encode(MVKCommandEncoder* cmdEncoder, MVKCommandUse com
             bool isBlittingStencil = mvkIsAnyFlagEnabled(blitKey.srcAspect, (VK_IMAGE_ASPECT_STENCIL_BIT));
             id<MTLDepthStencilState> mtlDSS = cmdEncoder->getCommandEncodingPool()->getMTLDepthStencilState(isBlittingDepth, isBlittingStencil);
             
-            uint32_t vtxBuffIdx = cmdEncoder->getDevice()->getMetalBufferIndexForVertexAttributeBinding(kMVKVertexContentBufferIndex);
-            
             mtlColorAttDesc.level = mvkIBR.region.dstSubresource.mipLevel;
             mtlDepthAttDesc.level = mvkIBR.region.dstSubresource.mipLevel;
             mtlStencilAttDesc.level = mvkIBR.region.dstSubresource.mipLevel;
@@ -540,7 +538,8 @@ void MVKCmdBlitImage<N>::encode(MVKCommandEncoder* cmdEncoder, MVKCommandUse com
                 [mtlRendEnc pushDebugGroup: @"vkCmdBlitImage"];
                 [mtlRendEnc setRenderPipelineState: mtlRPS];
                 [mtlRendEnc setDepthStencilState: mtlDSS];
-                cmdEncoder->setVertexBytes(mtlRendEnc, mvkIBR.vertices, sizeof(mvkIBR.vertices), vtxBuffIdx);
+                cmdEncoder->setVertexBytes(mtlRendEnc, mvkIBR.vertices, sizeof(mvkIBR.vertices),
+										   cmdEncoder->getDevice()->getMetalBufferIndexForVertexAttributeBinding(kMVKVertexContentBufferIndex));
                 if (isLayeredBlit) {
                     cmdEncoder->setVertexBytes(mtlRendEnc, &zIncr, sizeof(zIncr), 0);
                 }
@@ -1250,7 +1249,6 @@ void MVKCmdClearAttachments<N>::encode(MVKCommandEncoder* cmdEncoder) {
 
 	MVKPixelFormats* pixFmts = cmdEncoder->getPixelFormats();
     MVKRenderSubpass* subpass = cmdEncoder->getSubpass();
-    uint32_t vtxBuffIdx = cmdEncoder->getDevice()->getMetalBufferIndexForVertexAttributeBinding(kMVKVertexContentBufferIndex);
 
     // Populate the render pipeline state attachment key with info from the subpass and framebuffer.
 	_rpsKey.mtlSampleCount = mvkSampleCountFromVkSampleCountFlagBits(subpass->getSampleCount());
@@ -1304,9 +1302,10 @@ void MVKCmdClearAttachments<N>::encode(MVKCommandEncoder* cmdEncoder) {
     [mtlRendEnc setViewport: {0, 0, (double) fbExtent.width, (double) fbExtent.height, 0.0, 1.0}];
     [mtlRendEnc setScissorRect: {0, 0, fbExtent.width, fbExtent.height}];
 
-    cmdEncoder->setVertexBytes(mtlRendEnc, clearColors, sizeof(clearColors), 0);
+    cmdEncoder->setVertexBytes(mtlRendEnc, clearColors, sizeof(clearColors), 0, true);
-    cmdEncoder->setFragmentBytes(mtlRendEnc, clearColors, sizeof(clearColors), 0);
+    cmdEncoder->setFragmentBytes(mtlRendEnc, clearColors, sizeof(clearColors), 0, true);
-    cmdEncoder->setVertexBytes(mtlRendEnc, vertices, vtxCnt * sizeof(vertices[0]), vtxBuffIdx);
+    cmdEncoder->setVertexBytes(mtlRendEnc, vertices, vtxCnt * sizeof(vertices[0]),
+							   cmdEncoder->getDevice()->getMetalBufferIndexForVertexAttributeBinding(kMVKVertexContentBufferIndex), true);
     [mtlRendEnc drawPrimitives: MTLPrimitiveTypeTriangle vertexStart: 0 vertexCount: vtxCnt];
     [mtlRendEnc popDebugGroup];
 
@@ -1334,7 +1333,6 @@ void MVKCmdClearAttachments<N>::encode(MVKCommandEncoder* cmdEncoder) {
     cmdEncoder->_depthBiasState.markDirty();
     cmdEncoder->_viewportState.markDirty();
     cmdEncoder->_scissorState.markDirty();
-	cmdEncoder->_graphicsResourcesState.beginMetalRenderPass();
 }
 
 template class MVKCmdClearAttachments<1>;

MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h

@@ -354,14 +354,29 @@ public:
 	/** Returns the push constants associated with the specified shader stage. */
 	MVKPushConstantsCommandEncoderState* getPushConstants(VkShaderStageFlagBits shaderStage);
 
-    /** Copy bytes into the Metal encoder at a Metal vertex buffer index. */
+    /**
-    void setVertexBytes(id<MTLRenderCommandEncoder> mtlEncoder, const void* bytes, NSUInteger length, uint32_t mtlBuffIndex);
+	 * Copy bytes into the Metal encoder at a Metal vertex buffer index, and optionally indicate
+	 * that this binding might override a desriptor binding. If so, the descriptor binding will
+	 * be marked dirty so that it will rebind before the next usage.
+	 */
+    void setVertexBytes(id<MTLRenderCommandEncoder> mtlEncoder, const void* bytes,
+						NSUInteger length, uint32_t mtlBuffIndex, bool descOverride = false);
 
-    /** Copy bytes into the Metal encoder at a Metal fragment buffer index. */
+	/**
-    void setFragmentBytes(id<MTLRenderCommandEncoder> mtlEncoder, const void* bytes, NSUInteger length, uint32_t mtlBuffIndex);
+	 * Copy bytes into the Metal encoder at a Metal fragment buffer index, and optionally indicate
+	 * that this binding might override a desriptor binding. If so, the descriptor binding will
+	 * be marked dirty so that it will rebind before the next usage.
+	 */
+    void setFragmentBytes(id<MTLRenderCommandEncoder> mtlEncoder, const void* bytes,
+						  NSUInteger length, uint32_t mtlBuffIndex, bool descOverride = false);
 
-    /** Copy bytes into the Metal encoder at a Metal compute buffer index. */
+	/**
-    void setComputeBytes(id<MTLComputeCommandEncoder> mtlEncoder, const void* bytes, NSUInteger length, uint32_t mtlBuffIndex);
+	 * Copy bytes into the Metal encoder at a Metal compute buffer index, and optionally indicate
+	 * that this binding might override a desriptor binding. If so, the descriptor binding will
+	 * be marked dirty so that it will rebind before the next usage.
+	 */
+    void setComputeBytes(id<MTLComputeCommandEncoder> mtlEncoder, const void* bytes,
+						 NSUInteger length, uint32_t mtlBuffIndex, bool descOverride = false);
 
     /** Get a temporary MTLBuffer that will be returned to a pool after the command buffer is finished. */
     const MVKMTLBufferAllocation* getTempMTLBuffer(NSUInteger length, bool isPrivate = false, bool isDedicated = false);

MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm

@@ -836,37 +836,52 @@ MVKPushConstantsCommandEncoderState* MVKCommandEncoder::getPushConstants(VkShade
 void MVKCommandEncoder::setVertexBytes(id<MTLRenderCommandEncoder> mtlEncoder,
                                        const void* bytes,
                                        NSUInteger length,
-                                       uint32_t mtlBuffIndex) {
+									   uint32_t mtlBuffIndex,
+									   bool descOverride) {
     if (_pDeviceMetalFeatures->dynamicMTLBufferSize && length <= _pDeviceMetalFeatures->dynamicMTLBufferSize) {
         [mtlEncoder setVertexBytes: bytes length: length atIndex: mtlBuffIndex];
     } else {
         const MVKMTLBufferAllocation* mtlBuffAlloc = copyToTempMTLBufferAllocation(bytes, length);
         [mtlEncoder setVertexBuffer: mtlBuffAlloc->_mtlBuffer offset: mtlBuffAlloc->_offset atIndex: mtlBuffIndex];
     }
+
+	if (descOverride) {
+		_graphicsResourcesState.markBufferIndexDirty(kMVKShaderStageVertex, mtlBuffIndex);
+	}
 }
 
 void MVKCommandEncoder::setFragmentBytes(id<MTLRenderCommandEncoder> mtlEncoder,
                                          const void* bytes,
                                          NSUInteger length,
-                                         uint32_t mtlBuffIndex) {
+										 uint32_t mtlBuffIndex,
+										 bool descOverride) {
     if (_pDeviceMetalFeatures->dynamicMTLBufferSize && length <= _pDeviceMetalFeatures->dynamicMTLBufferSize) {
         [mtlEncoder setFragmentBytes: bytes length: length atIndex: mtlBuffIndex];
     } else {
         const MVKMTLBufferAllocation* mtlBuffAlloc = copyToTempMTLBufferAllocation(bytes, length);
         [mtlEncoder setFragmentBuffer: mtlBuffAlloc->_mtlBuffer offset: mtlBuffAlloc->_offset atIndex: mtlBuffIndex];
     }
+
+	if (descOverride) {
+		_graphicsResourcesState.markBufferIndexDirty(kMVKShaderStageFragment, mtlBuffIndex);
+	}
 }
 
 void MVKCommandEncoder::setComputeBytes(id<MTLComputeCommandEncoder> mtlEncoder,
                                         const void* bytes,
                                         NSUInteger length,
-                                        uint32_t mtlBuffIndex) {
+                                        uint32_t mtlBuffIndex,
+										bool descOverride) {
     if (_pDeviceMetalFeatures->dynamicMTLBufferSize && length <= _pDeviceMetalFeatures->dynamicMTLBufferSize) {
         [mtlEncoder setBytes: bytes length: length atIndex: mtlBuffIndex];
     } else {
         const MVKMTLBufferAllocation* mtlBuffAlloc = copyToTempMTLBufferAllocation(bytes, length);
         [mtlEncoder setBuffer: mtlBuffAlloc->_mtlBuffer offset: mtlBuffAlloc->_offset atIndex: mtlBuffIndex];
     }
+
+	if (descOverride) {
+		_computeResourcesState.markBufferIndexDirty(mtlBuffIndex);
+	}
 }
 
 // Return the MTLBuffer allocation to the pool once the command buffer is done with it

MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h

@@ -200,8 +200,7 @@ public:
     /** Sets the index of the Metal buffer used to hold the push constants. */
     void setMTLBufferIndex(uint32_t mtlBufferIndex, bool pipelineStageUsesPushConstants);
 
-    /** Constructs this instance for the specified command encoder. */
+	MVKPushConstantsCommandEncoderState(MVKCommandEncoder* cmdEncoder,
-    MVKPushConstantsCommandEncoderState(MVKCommandEncoder* cmdEncoder,
                                         VkShaderStageFlagBits shaderStage)
         : MVKCommandEncoderState(cmdEncoder), _shaderStage(shaderStage) {}
 
@@ -364,11 +363,12 @@ public:
 												   MTLResourceUsage mtlUsage,
 												   MTLRenderStages mtlStages) = 0;
 
+	void markDirty() override;
+
     MVKResourcesCommandEncoderState(MVKCommandEncoder* cmdEncoder) :
 		MVKCommandEncoderState(cmdEncoder), _boundDescriptorSets{} {}
 
 protected:
-	void markDirty() override;
 
     // Template function that marks both the vector and all binding elements in the vector as dirty.
     template<class T>
@@ -377,25 +377,40 @@ protected:
         bindingsDirtyFlag = true;
     }
 
+	// Template function to find and mark dirty the binding that uses the index.
+	template<class T>
+	void markIndexDirty(T& bindings, bool& bindingsDirtyFlag, uint32_t index) {
+		for (auto& b : bindings) {
+			if (b.index == index) {
+				b.markDirty();
+				bindingsDirtyFlag = true;
+				MVKCommandEncoderState::markDirty();
+				return;
+			}
+		}
+	}
+
     // Template function that updates an existing binding or adds a new binding to a vector
     // of bindings, and marks the binding, the vector, and this instance as dirty
     template<class T, class V>
     void bind(const T& b, V& bindings, bool& bindingsDirtyFlag) {
-
         if ( !b.mtlResource ) { return; }
 
-        MVKCommandEncoderState::markDirty();
+        for (auto& rb : bindings) {
-        bindingsDirtyFlag = true;
+			if (rb.index == b.index) {
-
+                rb.update(b);
-        for (auto iter = bindings.begin(), end = bindings.end(); iter != end; ++iter) {
+				if (rb.isDirty) {
-            if (iter->index == b.index) {
+					bindingsDirtyFlag = true;
-                iter->update(b);
+					MVKCommandEncoderState::markDirty();
+				}
                 return;
             }
         }
 
         bindings.push_back(b);
         bindings.back().markDirty();
+		bindingsDirtyFlag = true;
+		MVKCommandEncoderState::markDirty();
     }
 
 	// For texture bindings, we also keep track of whether any bindings need a texture swizzle
@@ -533,6 +548,11 @@ public:
 	/** Offset all buffers for vertex attribute bindings with zero divisors by the given number of strides. */
 	void offsetZeroDivisorVertexBuffers(MVKGraphicsStage stage, MVKGraphicsPipeline* pipeline, uint32_t firstInstance);
 
+	/** Marks dirty the buffer binding using the index. */
+	void markBufferIndexDirty(MVKShaderStage stage, uint32_t mtlBufferIndex);
+
+	void markDirty() override;
+
 #pragma mark Construction
     
     /** Constructs this instance for the specified command encoder. */
@@ -540,7 +560,6 @@ public:
 
 protected:
     void encodeImpl(uint32_t stage) override;
-    void markDirty() override;
 	void bindMetalArgumentBuffer(MVKShaderStage stage, MVKMTLBufferBinding& buffBind) override;
 
     ResourceBindings<8> _shaderStageResourceBindings[4];
@@ -581,6 +600,9 @@ public:
 										   MTLResourceUsage mtlUsage,
 										   MTLRenderStages mtlStages) override;
 
+	/** Marks dirty the buffer binding using the index. */
+	void markBufferIndexDirty(uint32_t mtlBufferIndex);
+
     void markDirty() override;
 
 #pragma mark Construction

MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm

@@ -194,13 +194,13 @@ void MVKPushConstantsCommandEncoderState::encodeImpl(uint32_t stage) {
                 _cmdEncoder->setComputeBytes(_cmdEncoder->getMTLComputeEncoder(kMVKCommandUseTessellationVertexTessCtl),
                                              _pushConstants.data(),
                                              _pushConstants.size(),
-                                             _mtlBufferIndex);
+                                             _mtlBufferIndex, true);
 				_isDirty = false;	// Okay, I changed the encoder
 			} else if (!isTessellating() && stage == kMVKGraphicsStageRasterization) {
                 _cmdEncoder->setVertexBytes(_cmdEncoder->_mtlRenderEncoder,
                                             _pushConstants.data(),
                                             _pushConstants.size(),
-                                            _mtlBufferIndex);
+                                            _mtlBufferIndex, true);
 				_isDirty = false;	// Okay, I changed the encoder
             }
             break;
@@ -209,7 +209,7 @@ void MVKPushConstantsCommandEncoderState::encodeImpl(uint32_t stage) {
                 _cmdEncoder->setComputeBytes(_cmdEncoder->getMTLComputeEncoder(kMVKCommandUseTessellationVertexTessCtl),
                                              _pushConstants.data(),
                                              _pushConstants.size(),
-                                             _mtlBufferIndex);
+                                             _mtlBufferIndex, true);
 				_isDirty = false;	// Okay, I changed the encoder
             }
             break;
@@ -218,7 +218,7 @@ void MVKPushConstantsCommandEncoderState::encodeImpl(uint32_t stage) {
                 _cmdEncoder->setVertexBytes(_cmdEncoder->_mtlRenderEncoder,
                                             _pushConstants.data(),
                                             _pushConstants.size(),
-                                            _mtlBufferIndex);
+                                            _mtlBufferIndex, true);
 				_isDirty = false;	// Okay, I changed the encoder
             }
             break;
@@ -227,7 +227,7 @@ void MVKPushConstantsCommandEncoderState::encodeImpl(uint32_t stage) {
                 _cmdEncoder->setFragmentBytes(_cmdEncoder->_mtlRenderEncoder,
                                               _pushConstants.data(),
                                               _pushConstants.size(),
-                                              _mtlBufferIndex);
+                                              _mtlBufferIndex, true);
 				_isDirty = false;	// Okay, I changed the encoder
             }
             break;
@@ -235,7 +235,7 @@ void MVKPushConstantsCommandEncoderState::encodeImpl(uint32_t stage) {
             _cmdEncoder->setComputeBytes(_cmdEncoder->getMTLComputeEncoder(kMVKCommandUseDispatch),
                                          _pushConstants.data(),
                                          _pushConstants.size(),
-                                         _mtlBufferIndex);
+                                         _mtlBufferIndex, true);
 			_isDirty = false;	// Okay, I changed the encoder
             break;
         default:
@@ -982,6 +982,11 @@ void MVKGraphicsResourcesCommandEncoderState::encodeArgumentBufferResourceUsage(
 	}
 }
 
+void MVKGraphicsResourcesCommandEncoderState::markBufferIndexDirty(MVKShaderStage stage, uint32_t mtlBufferIndex) {
+	auto& stageRezBinds = _shaderStageResourceBindings[stage];
+	markIndexDirty(stageRezBinds.bufferBindings, stageRezBinds.areBufferBindingsDirty, mtlBufferIndex);
+}
+
 
 #pragma mark -
 #pragma mark MVKComputeResourcesCommandEncoderState
@@ -1115,6 +1120,10 @@ void MVKComputeResourcesCommandEncoderState::encodeArgumentBufferResourceUsage(M
 	}
 }
 
+void MVKComputeResourcesCommandEncoderState::markBufferIndexDirty(uint32_t mtlBufferIndex) {
+	markIndexDirty(_resourceBindings.bufferBindings, _resourceBindings.areBufferBindingsDirty, mtlBufferIndex);
+}
+
 
 #pragma mark -
 #pragma mark MVKOcclusionQueryCommandEncoderState

MoltenVK/MoltenVK/Commands/MVKMTLResourceBindings.h

@@ -75,12 +75,11 @@ typedef struct MVKMTLBufferBinding {
     inline void markDirty() { justOffset = false; isDirty = true; }
 
     inline void update(const MVKMTLBufferBinding &other) {
-        if (mtlBuffer != other.mtlBuffer || size != other.size || isInline != other.isInline) {
+        if (mtlBuffer != other.mtlBuffer || size != other.size || other.isInline) {
             mtlBuffer = other.mtlBuffer;
             size = other.size;
             isInline = other.isInline;
             offset = other.offset;
-
             justOffset = false;
             isDirty = true;
         } else if (offset != other.offset) {