Merge pull request #1677 from billhollings/fix-missing-metal-buffer-binding
Fix occasional missing Metal buffer binding when only offset changes.
diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md
index 9b6287a..ef50c13 100644
--- a/Docs/Whats_New.md
+++ b/Docs/Whats_New.md
@@ -811,6 +811,7 @@
- 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
diff --git a/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm b/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm
index e432115..3253aa9 100644
--- a/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm
+++ b/MoltenVK/MoltenVK/Commands/MVKCmdTransfer.mm
@@ -478,8 +478,6 @@
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 @@
[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 @@
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 @@
[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->setFragmentBytes(mtlRendEnc, clearColors, sizeof(clearColors), 0);
- cmdEncoder->setVertexBytes(mtlRendEnc, vertices, vtxCnt * sizeof(vertices[0]), vtxBuffIdx);
+ cmdEncoder->setVertexBytes(mtlRendEnc, clearColors, sizeof(clearColors), 0, true);
+ cmdEncoder->setFragmentBytes(mtlRendEnc, clearColors, sizeof(clearColors), 0, true);
+ 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 @@
cmdEncoder->_depthBiasState.markDirty();
cmdEncoder->_viewportState.markDirty();
cmdEncoder->_scissorState.markDirty();
- cmdEncoder->_graphicsResourcesState.beginMetalRenderPass();
}
template class MVKCmdClearAttachments<1>;
diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h
index 5502dc2..c5fb691 100644
--- a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h
+++ b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.h
@@ -354,14 +354,29 @@
/** 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);
diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm
index e37a89a..0f4c5f5 100644
--- a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm
+++ b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm
@@ -836,37 +836,52 @@
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
diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h
index 0def2be..166ae74 100644
--- a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h
+++ b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h
@@ -200,8 +200,7 @@
/** 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 @@
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 @@
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();
- bindingsDirtyFlag = true;
-
- for (auto iter = bindings.begin(), end = bindings.end(); iter != end; ++iter) {
- if (iter->index == b.index) {
- iter->update(b);
+ for (auto& rb : bindings) {
+ if (rb.index == b.index) {
+ rb.update(b);
+ if (rb.isDirty) {
+ bindingsDirtyFlag = true;
+ 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 @@
/** 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 @@
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 @@
MTLResourceUsage mtlUsage,
MTLRenderStages mtlStages) override;
+ /** Marks dirty the buffer binding using the index. */
+ void markBufferIndexDirty(uint32_t mtlBufferIndex);
+
void markDirty() override;
#pragma mark Construction
diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
index 1a36927..8cf763a 100644
--- a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
+++ b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
@@ -194,13 +194,13 @@
_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 @@
_cmdEncoder->setComputeBytes(_cmdEncoder->getMTLComputeEncoder(kMVKCommandUseTessellationVertexTessCtl),
_pushConstants.data(),
_pushConstants.size(),
- _mtlBufferIndex);
+ _mtlBufferIndex, true);
_isDirty = false; // Okay, I changed the encoder
}
break;
@@ -218,7 +218,7 @@
_cmdEncoder->setVertexBytes(_cmdEncoder->_mtlRenderEncoder,
_pushConstants.data(),
_pushConstants.size(),
- _mtlBufferIndex);
+ _mtlBufferIndex, true);
_isDirty = false; // Okay, I changed the encoder
}
break;
@@ -227,7 +227,7 @@
_cmdEncoder->setFragmentBytes(_cmdEncoder->_mtlRenderEncoder,
_pushConstants.data(),
_pushConstants.size(),
- _mtlBufferIndex);
+ _mtlBufferIndex, true);
_isDirty = false; // Okay, I changed the encoder
}
break;
@@ -235,7 +235,7 @@
_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::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::markBufferIndexDirty(uint32_t mtlBufferIndex) {
+ markIndexDirty(_resourceBindings.bufferBindings, _resourceBindings.areBufferBindingsDirty, mtlBufferIndex);
+}
+
#pragma mark -
#pragma mark MVKOcclusionQueryCommandEncoderState
diff --git a/MoltenVK/MoltenVK/Commands/MVKMTLResourceBindings.h b/MoltenVK/MoltenVK/Commands/MVKMTLResourceBindings.h
index a0f71d0..f250230 100644
--- a/MoltenVK/MoltenVK/Commands/MVKMTLResourceBindings.h
+++ b/MoltenVK/MoltenVK/Commands/MVKMTLResourceBindings.h
@@ -75,12 +75,11 @@
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) {
