Fix acquisition ordering of swapchain images.
Remove obsolete MVKSwapchainImageAvailability::waitCount member that was initializing
randomly and corrupting swapchain image acquisition ordering evaluations.
Rename several member functions to clarify purpose.
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.h b/MoltenVK/MoltenVK/GPUObjects/MVKImage.h
index fc1ba81..c8c1c8b 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKImage.h
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKImage.h
@@ -236,8 +236,8 @@
void initSubresources(const VkImageCreateInfo* pCreateInfo);
void initSubresourceLayout(MVKImageSubresource& imgSubRez);
virtual id<MTLTexture> newMTLTexture();
- void resetMTLTexture();
- void resetIOSurface();
+ void releaseMTLTexture();
+ void releaseIOSurface();
MTLTextureDescriptor* newMTLTextureDescriptor();
void updateMTLTextureContent(MVKImageSubresource& subresource, VkDeviceSize offset, VkDeviceSize size);
void getMTLTextureContent(MVKImageSubresource& subresource, VkDeviceSize offset, VkDeviceSize size);
@@ -277,7 +277,6 @@
/** Indicates the relative availability of each image in the swapchain. */
typedef struct MVKSwapchainImageAvailability {
uint64_t acquisitionID; /**< When this image was last made available, relative to the other images in the swapchain. Smaller value is earlier. */
- uint32_t waitCount; /**< The number of semaphores already waiting for this image. */
bool isAvailable; /**< Indicates whether this image is currently available. */
bool operator< (const MVKSwapchainImageAvailability& rhs) const;
@@ -327,10 +326,10 @@
id<MTLTexture> newMTLTexture() override;
id<CAMetalDrawable> getCAMetalDrawable();
- void resetMetalDrawable();
+ void releaseMetalDrawable();
MVKSwapchainImageAvailability getAvailability();
void makeAvailable();
- void signalWhenAvailable(MVKSemaphore* semaphore, MVKFence* fence);
+ void acquireAndSignalWhenAvailable(MVKSemaphore* semaphore, MVKFence* fence);
void signal(MVKSwapchainSignaler& signaler, id<MTLCommandBuffer> mtlCmdBuff);
void signalPresentationSemaphore(id<MTLCommandBuffer> mtlCmdBuff);
static void markAsTracked(MVKSwapchainSignaler& signaler);
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
index 4a226d9..8a16ef2 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
@@ -300,8 +300,8 @@
VkResult MVKImage::setMTLTexture(id<MTLTexture> mtlTexture) {
lock_guard<mutex> lock(_lock);
- resetMTLTexture();
- resetIOSurface();
+ releaseMTLTexture();
+ releaseIOSurface();
_mtlTexture = [mtlTexture retain]; // retained
@@ -349,14 +349,14 @@
}
// Removes and releases the MTLTexture object, and all associated texture views
-void MVKImage::resetMTLTexture() {
+void MVKImage::releaseMTLTexture() {
[_mtlTexture release];
_mtlTexture = nil;
for (auto elem : _mtlTextureViews) { [elem.second release]; }
_mtlTextureViews.clear();
}
-void MVKImage::resetIOSurface() {
+void MVKImage::releaseIOSurface() {
if (_ioSurface) {
CFRelease(_ioSurface);
_ioSurface = nil;
@@ -372,8 +372,8 @@
#if MVK_SUPPORT_IOSURFACE_BOOL
- resetMTLTexture();
- resetIOSurface();
+ releaseMTLTexture();
+ releaseIOSurface();
MVKPixelFormats* pixFmts = getPixelFormats();
@@ -788,14 +788,20 @@
MVKImage::~MVKImage() {
if (_deviceMemory) { _deviceMemory->removeImage(this); }
- resetMTLTexture();
- resetIOSurface();
+ releaseMTLTexture();
+ releaseIOSurface();
}
#pragma mark -
#pragma mark MVKSwapchainImage
+bool MVKSwapchainImageAvailability::operator< (const MVKSwapchainImageAvailability& rhs) const {
+ if ( isAvailable && !rhs.isAvailable) { return true; }
+ if ( !isAvailable && rhs.isAvailable) { return false; }
+ return acquisitionID < rhs.acquisitionID;
+}
+
VkResult MVKSwapchainImage::bindDeviceMemory(MVKDeviceMemory*, VkDeviceSize) {
return VK_ERROR_OUT_OF_DEVICE_MEMORY;
}
@@ -820,16 +826,6 @@
return VK_SUCCESS;
}
-bool MVKSwapchainImageAvailability::operator< (const MVKSwapchainImageAvailability& rhs) const {
- if ( isAvailable && !rhs.isAvailable) { return true; }
- if ( !isAvailable && rhs.isAvailable) { return false; }
-
- if (waitCount < rhs.waitCount) { return true; }
- if (waitCount > rhs.waitCount) { return false; }
-
- return acquisitionID < rhs.acquisitionID;
-}
-
MVKSwapchainImageAvailability MVKSwapchainImage::getAvailability() {
lock_guard<mutex> lock(_availabilityLock);
@@ -870,9 +866,13 @@
// MVKLogDebug("Signaling%s swapchain image %p semaphore %p from present, with %lu remaining semaphores.", (_availability.isAvailable ? " pre-signaled" : ""), this, signaler.first, _availabilitySignalers.size());
}
-void MVKSwapchainImage::signalWhenAvailable(MVKSemaphore* semaphore, MVKFence* fence) {
+void MVKSwapchainImage::acquireAndSignalWhenAvailable(MVKSemaphore* semaphore, MVKFence* fence) {
lock_guard<mutex> lock(_availabilityLock);
+ // Now that this image is being acquired, release the existing drawable and its texture.
+ // This is not done earlier so the texture is retained for any post-processing such as screen captures, etc.
+ releaseMetalDrawable();
+
auto signaler = make_pair(semaphore, fence);
if (_availability.isAvailable) {
_availability.isAvailable = false;
@@ -968,8 +968,8 @@
}
// Resets the MTLTexture and CAMetalDrawable underlying this image.
-void MVKSwapchainImage::resetMetalDrawable() {
- resetMTLTexture(); // Release texture first so drawable will be last to release it
+void MVKSwapchainImage::releaseMetalDrawable() {
+ releaseMTLTexture(); // Release texture first so drawable will be last to release it
[_mtlDrawable release];
_mtlDrawable = nil;
}
@@ -991,7 +991,7 @@
}
MVKSwapchainImage::~MVKSwapchainImage() {
- resetMetalDrawable();
+ releaseMetalDrawable();
}
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm b/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm
index b93d3b9..13041b3 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm
@@ -80,11 +80,9 @@
if ( getIsSurfaceLost() ) { return VK_ERROR_SURFACE_LOST_KHR; }
- // Find the image that has the smallest availability measure
+ // Find the image that has the shortest wait by finding the smallest availability measure.
MVKSwapchainImage* minWaitImage = nullptr;
- MVKSwapchainImageAvailability minAvailability = { .acquisitionID = kMVKUndefinedLargeUInt64,
- .waitCount = kMVKUndefinedLargeUInt32,
- .isAvailable = false };
+ MVKSwapchainImageAvailability minAvailability = { kMVKUndefinedLargeUInt64, false };
uint32_t imgCnt = getImageCount();
for (uint32_t imgIdx = 0; imgIdx < imgCnt; imgIdx++) {
MVKSwapchainImage* img = getImage(imgIdx);
@@ -95,10 +93,10 @@
}
}
- // Return the index of the image with the shortest wait and signal the semaphore and fence when it's available
+ // Return the index of the image with the shortest wait,
+ // and signal the semaphore and fence when it's available
*pImageIndex = minWaitImage->_swapchainIndex;
- minWaitImage->resetMetalDrawable();
- minWaitImage->signalWhenAvailable((MVKSemaphore*)semaphore, (MVKFence*)fence);
+ minWaitImage->acquireAndSignalWhenAvailable((MVKSemaphore*)semaphore, (MVKFence*)fence);
return getHasSurfaceSizeChanged() ? VK_ERROR_OUT_OF_DATE_KHR : VK_SUCCESS;
}
