Fix error where previously bound push constants can override a descriptor

buffer binding used by a subsequent pipeline that does not use push constants.

This error was previously introduced in 2a17f75, where a push constants
binding could override the Metal buffer binding 0 of a subsequent pipeline
that does not use push constants.

When pipeline binding is encoded, track which stages use push constants
and only encode push constants if the pipeline and stage uses them.

(unrelated) Make use of MVKResourcesCommandEncoderState::getPipeline() consistent.
This commit is contained in:
Bill Hollings 2022-04-18 15:32:40 -04:00
parent 023ec50e3c
commit be7a68153f
4 changed files with 16 additions and 11 deletions

View File

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

View File

@ -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;
};

View File

@ -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) {

View File

@ -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]);
}
}
}