From bbe87fe2fc9e89ed8713a4292243c8b3b5bf2440 Mon Sep 17 00:00:00 2001
From: Bill Hollings <bill.hollings@brenwill.com>
Date: Wed, 10 Feb 2021 18:51:42 -0500
Subject: [PATCH] Fix Metal validation error of duplicate visibility offsets.

Don't reset MVKOcclusionQueryCommandEncoderState::_mtlVisibilityResultOffset
when ending occlusion query, as subsequent queries need different offsets.

Also, remove MVKOcclusionQueryCommandEncoderState::_needsVisibilityResultMTLBuffer,
as it is always set once from MVKCommandBuffer::_needsVisibilityResultMTLBuffer.

When app setting MVKConfiguration it with a partial copy, start with existing.
---
 MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm |  2 +-
 .../MoltenVK/Commands/MVKCommandEncoderState.h |  7 +------
 .../Commands/MVKCommandEncoderState.mm         | 18 ++++--------------
 MoltenVK/MoltenVK/Vulkan/vk_mvk_moltenvk.mm    |  3 ++-
 4 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm
index 68fe319a..032e5c27 100644
--- a/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm
+++ b/MoltenVK/MoltenVK/Commands/MVKCommandBuffer.mm
@@ -335,7 +335,7 @@ void MVKCommandEncoder::beginMetalRenderPass(bool loadOverride) {
 
     MTLRenderPassDescriptor* mtlRPDesc = [MTLRenderPassDescriptor renderPassDescriptor];
     getSubpass()->populateMTLRenderPassDescriptor(mtlRPDesc, _multiviewPassIndex, _framebuffer, _clearValues.contents(), _isRenderingEntireAttachment, loadOverride);
-    if (_occlusionQueryState.getNeedsVisibilityResultMTLBuffer()) {
+    if (_cmdBuffer->_needsVisibilityResultMTLBuffer) {
         if (!_visibilityResultMTLBuffer) {
             _visibilityResultMTLBuffer = getTempMTLBuffer(_pDeviceMetalFeatures->maxQueryBufferSize, true);
         }
diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h
index db377c5d..d4889326 100644
--- a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h
+++ b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h
@@ -585,17 +585,12 @@ public:
     /** Ends an occlusion query. */
     void endOcclusionQuery(MVKOcclusionQueryPool* pQueryPool, uint32_t query);
 
-    /** Returns whether an MTLBuffer is needed to hold occlusion query results. */
-    bool getNeedsVisibilityResultMTLBuffer();
-
-    /** Constructs this instance for the specified command encoder. */
-    MVKOcclusionQueryCommandEncoderState(MVKCommandEncoder* cmdEncoder);
+	MVKOcclusionQueryCommandEncoderState(MVKCommandEncoder* cmdEncoder) : MVKCommandEncoderState(cmdEncoder) {}
 
 protected:
     void encodeImpl(uint32_t) override;
     void resetImpl() override;
 
-    bool _needsVisibilityResultMTLBuffer = false;
     MTLVisibilityResultMode _mtlVisibilityResultMode = MTLVisibilityResultModeDisabled;
     NSUInteger _mtlVisibilityResultOffset = 0;
 	MVKSmallVector<std::pair<MVKQuerySpec, NSUInteger>> _mtlRenderPassQueries;
diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
index f69b03fe..4919d217 100644
--- a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
+++ b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm
@@ -943,7 +943,7 @@ void MVKOcclusionQueryCommandEncoderState::beginOcclusionQuery(MVKOcclusionQuery
 
     MVKQuerySpec querySpec;
     querySpec.set(pQueryPool, query);
-    NSUInteger offset = _mtlRenderPassQueries.empty() ? 0 : _mtlVisibilityResultOffset + 8;
+    NSUInteger offset = _mtlRenderPassQueries.empty() ? 0 : _mtlVisibilityResultOffset + kMVKQuerySlotSizeInBytes;
     NSUInteger maxOffset = _cmdEncoder->_pDeviceMetalFeatures->maxQueryBufferSize - kMVKQuerySlotSizeInBytes;
     offset = min(offset, maxOffset);
     _mtlRenderPassQueries.push_back(make_pair(querySpec, offset));
@@ -952,17 +952,14 @@ void MVKOcclusionQueryCommandEncoderState::beginOcclusionQuery(MVKOcclusionQuery
     _mtlVisibilityResultMode = shouldCount ? MTLVisibilityResultModeCounting : MTLVisibilityResultModeBoolean;
     _mtlVisibilityResultOffset = offset;
 
-    _needsVisibilityResultMTLBuffer = true;
-
     markDirty();
 }
 
 void MVKOcclusionQueryCommandEncoderState::endOcclusionQuery(MVKOcclusionQueryPool* pQueryPool, uint32_t query) {
-	reset();
+	_mtlVisibilityResultMode = MTLVisibilityResultModeDisabled;
+	markDirty();
 }
 
-bool MVKOcclusionQueryCommandEncoderState::getNeedsVisibilityResultMTLBuffer() { return _needsVisibilityResultMTLBuffer; }
-
 void MVKOcclusionQueryCommandEncoderState::encodeImpl(uint32_t stage) {
 	if (stage != kMVKGraphicsStageRasterization) { return; }
 
@@ -971,14 +968,7 @@ void MVKOcclusionQueryCommandEncoderState::encodeImpl(uint32_t stage) {
 }
 
 void MVKOcclusionQueryCommandEncoderState::resetImpl() {
-    _needsVisibilityResultMTLBuffer = _cmdEncoder->_cmdBuffer->_needsVisibilityResultMTLBuffer;
     _mtlVisibilityResultMode = MTLVisibilityResultModeDisabled;
     _mtlVisibilityResultOffset = 0;
+    _mtlRenderPassQueries.clear();
 }
-
-MVKOcclusionQueryCommandEncoderState::MVKOcclusionQueryCommandEncoderState(MVKCommandEncoder* cmdEncoder)
-        : MVKCommandEncoderState(cmdEncoder) {
-    resetImpl();
-}
-
-
diff --git a/MoltenVK/MoltenVK/Vulkan/vk_mvk_moltenvk.mm b/MoltenVK/MoltenVK/Vulkan/vk_mvk_moltenvk.mm
index 65c1c27d..b2a216e2 100644
--- a/MoltenVK/MoltenVK/Vulkan/vk_mvk_moltenvk.mm
+++ b/MoltenVK/MoltenVK/Vulkan/vk_mvk_moltenvk.mm
@@ -61,7 +61,8 @@ MVK_PUBLIC_SYMBOL VkResult vkSetMoltenVKConfigurationMVK(
 	const MVKConfiguration*                     pConfiguration,
 	size_t*                                     pConfigurationSize) {
 
-	MVKConfiguration mvkConfig = {};	// Ensure initialized in case not fully copied
+	// Start with current config, in case incoming is not fully copied
+	MVKConfiguration mvkConfig = *mvkGetMVKConfiguration();
 	VkResult rslt = mvkCopy(&mvkConfig, pConfiguration, pConfigurationSize);
 	mvkSetMVKConfiguration(&mvkConfig);
 	return rslt;