Merge pull request #1573 from billhollings/push-constant-binding-fix

Fix error where previously bound push constants can override a descriptor buffer binding

Docs/Whats_New.md

@@ -22,6 +22,8 @@ Released TBD
 	- `VK_KHR_portability_enumeration` support added to `MoltenVK_icd.json`, and documentation
 	  updated to indicate the impact of the `VK_KHR_portability_enumeration` extension during 
 	  runtime loading on *macOS* via the *Vulkan Loader*.
+- Fix error where previously bound push constants can override a descriptor buffer binding 
+  used by a subsequent pipeline that does not use push constants.
 
 
 

MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h

@@ -198,7 +198,7 @@ public:
     void setPushConstants(uint32_t offset, MVKArrayRef<char> pushConstants);
 
     /** Sets the index of the Metal buffer used to hold the push constants. */
-    void setMTLBufferIndex(uint32_t mtlBufferIndex);
+    void setMTLBufferIndex(uint32_t mtlBufferIndex, bool pipelineStageUsesPushConstants);
 
     /** Constructs this instance for the specified command encoder. */
     MVKPushConstantsCommandEncoderState(MVKCommandEncoder* cmdEncoder,
@@ -212,6 +212,7 @@ protected:
     MVKSmallVector<char, 128> _pushConstants;
     VkShaderStageFlagBits _shaderStage;
     uint32_t _mtlBufferIndex = 0;
+	bool _pipelineStageUsesPushConstants = false;
 };
 
 

MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm

@@ -166,18 +166,19 @@ void MVKPushConstantsCommandEncoderState:: setPushConstants(uint32_t offset, MVK
     if (pcBuffSize > 0) { markDirty(); }
 }
 
-void MVKPushConstantsCommandEncoderState::setMTLBufferIndex(uint32_t mtlBufferIndex) {
-    if (mtlBufferIndex != _mtlBufferIndex) {
-        _mtlBufferIndex = mtlBufferIndex;
-        markDirty();
-    }
+void MVKPushConstantsCommandEncoderState::setMTLBufferIndex(uint32_t mtlBufferIndex, bool pipelineStageUsesPushConstants) {
+	if ((mtlBufferIndex != _mtlBufferIndex) || (pipelineStageUsesPushConstants != _pipelineStageUsesPushConstants)) {
+		_mtlBufferIndex = mtlBufferIndex;
+		_pipelineStageUsesPushConstants = pipelineStageUsesPushConstants;
+		markDirty();
+	}
 }
 
 // At this point, I have been marked not-dirty, under the assumption that I will make changes to the encoder.
 // However, some of the paths below decide not to actually make any changes to the encoder. In that case,
 // I should remain dirty until I actually do make encoder changes.
 void MVKPushConstantsCommandEncoderState::encodeImpl(uint32_t stage) {
-    if (_pushConstants.empty() ) { return; }
+    if ( !_pipelineStageUsesPushConstants || _pushConstants.empty() ) { return; }
 
 	_isDirty = true;	// Stay dirty until I actually decide to make a change to the encoder
 
@@ -739,7 +740,7 @@ void MVKGraphicsResourcesCommandEncoderState::markDirty() {
 
 void MVKGraphicsResourcesCommandEncoderState::encodeImpl(uint32_t stage) {
 
-    MVKGraphicsPipeline* pipeline = (MVKGraphicsPipeline*)_cmdEncoder->_graphicsPipelineState.getPipeline();
+    MVKGraphicsPipeline* pipeline = (MVKGraphicsPipeline*)getPipeline();
     bool fullImageViewSwizzle = pipeline->fullImageViewSwizzle() || getDevice()->_pMetalFeatures->nativeTextureSwizzle;
     bool forTessellation = pipeline->isTessellationPipeline();
 
@@ -975,7 +976,7 @@ void MVKComputeResourcesCommandEncoderState::encodeImpl(uint32_t) {
 
 	encodeMetalArgumentBuffer(kMVKShaderStageCompute);
 
-    MVKPipeline* pipeline = _cmdEncoder->_computePipelineState.getPipeline();
+    MVKPipeline* pipeline = getPipeline();
 	bool fullImageViewSwizzle = pipeline ? pipeline->fullImageViewSwizzle() : false;
 
     if (_resourceBindings.swizzleBufferBinding.isDirty) {

MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm

@@ -175,8 +175,9 @@ MVKPipelineLayout::~MVKPipelineLayout() {
 
 void MVKPipeline::bindPushConstants(MVKCommandEncoder* cmdEncoder) {
 	for (uint32_t stage = kMVKShaderStageVertex; stage < kMVKShaderStageCount; stage++) {
-		if (cmdEncoder && _stageUsesPushConstants[stage]) {
-			cmdEncoder->getPushConstants(mvkVkShaderStageFlagBitsFromMVKShaderStage(MVKShaderStage(stage)))->setMTLBufferIndex(_pushConstantsBufferIndex.stages[stage]);
+		if (cmdEncoder) {
+			auto* pcState = cmdEncoder->getPushConstants(mvkVkShaderStageFlagBitsFromMVKShaderStage(MVKShaderStage(stage)));
+			pcState->setMTLBufferIndex(_pushConstantsBufferIndex.stages[stage], _stageUsesPushConstants[stage]);
 		}
 	}
 }