From 6229582887d35b5365b35af4102b34b8d74b258a Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Thu, 2 Apr 2020 10:57:06 -0400 Subject: [PATCH] Move tracking of swapchain image availability from MVKSwapchain to MVKSwapchainImage. --- MoltenVK/MoltenVK/GPUObjects/MVKImage.h | 26 +++ MoltenVK/MoltenVK/GPUObjects/MVKImage.mm | 114 ++++++++++++- MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.h | 38 +---- MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm | 161 ++++--------------- 4 files changed, 168 insertions(+), 171 deletions(-) diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.h b/MoltenVK/MoltenVK/GPUObjects/MVKImage.h index 8ae0a5c5d..7e2c1637a 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKImage.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKImage.h @@ -274,6 +274,19 @@ class MVKImage : public MVKResource { #pragma mark - #pragma mark MVKSwapchainImage +/** 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; +} MVKSwapchainImageAvailability; + +/** Tracks a semaphore and fence for later signaling. */ +typedef std::pair MVKSwapchainSignaler; + + /** Represents a Vulkan image used as a rendering destination within a swapchain. */ class MVKSwapchainImage : public MVKImage { @@ -315,14 +328,27 @@ class MVKSwapchainImage : public MVKImage { ~MVKSwapchainImage() override; protected: + friend MVKSwapchain; + id newMTLTexture() override; id getCAMetalDrawable(); void resetMetalDrawable(); + MVKSwapchainImageAvailability getAvailability(); + void makeAvailable(); + void signalWhenAvailable(MVKSemaphore* semaphore, MVKFence* fence); + void signal(MVKSwapchainSignaler& signaler, id mtlCmdBuff); + void signalPresentationSemaphore(id mtlCmdBuff); + static void markAsTracked(MVKSwapchainSignaler& signaler); + static void unmarkAsTracked(MVKSwapchainSignaler& signaler); void renderWatermark(id mtlCmdBuff); MVKSwapchain* _swapchain; uint32_t _swapchainIndex; id _mtlDrawable; + MVKSwapchainImageAvailability _availability; + MVKVectorInline _availabilitySignalers; + MVKSwapchainSignaler _preSignaler; + std::mutex _availabilityLock; }; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm index ba6e3cf21..cfcdb1a9a 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm @@ -820,6 +820,112 @@ 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 lock(_availabilityLock); + + return _availability; +} + +// Makes an image available for acquisition by the app. +// If any semaphores are waiting to be signaled when this image becomes available, the +// earliest semaphore is signaled, and this image remains unavailable for other uses. +void MVKSwapchainImage::makeAvailable() { + lock_guard lock(_availabilityLock); + + // Mark when this event happened, relative to that of other images + _availability.acquisitionID = _swapchain->getNextAcquisitionID(); + + // Mark this image as available if no semaphores or fences are waiting to be signaled. + _availability.isAvailable = _availabilitySignalers.empty(); + + MVKSwapchainSignaler signaler; + if (_availability.isAvailable) { + // If this image is available, signal the semaphore and fence that were associated + // with the last time this image was acquired while available. This is a workaround for + // when an app uses a single semaphore or fence for more than one swapchain image. + // Becuase the semaphore or fence will be signaled by more than one image, it will + // get out of sync, and the final use of the image would not be signaled as a result. + signaler = _preSignaler; + } else { + // If this image is not yet available, extract and signal the first semaphore and fence. + auto sigIter = _availabilitySignalers.begin(); + signaler = *sigIter; + _availabilitySignalers.erase(sigIter); + } + + // Signal the semaphore and fence, and let them know they are no longer being tracked. + signal(signaler, nil); + unmarkAsTracked(signaler); + +// 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) { + lock_guard lock(_availabilityLock); + + auto signaler = make_pair(semaphore, fence); + if (_availability.isAvailable) { + _availability.isAvailable = false; + + // If signalling through a MTLEvent, use an ephemeral MTLCommandBuffer. + // Another option would be to use MTLSharedEvent in MVKSemaphore, but that might + // impose unacceptable performance costs to handle this particular case. + @autoreleasepool { + MVKSemaphore* mvkSem = signaler.first; + id mtlCmdBuff = (mvkSem && mvkSem->isUsingCommandEncoding() + ? [_device->getAnyQueue()->getMTLCommandQueue() commandBufferWithUnretainedReferences] + : nil); + signal(signaler, mtlCmdBuff); + [mtlCmdBuff commit]; + } + + _preSignaler = signaler; + } else { + _availabilitySignalers.push_back(signaler); + } + markAsTracked(signaler); + +// MVKLogDebug("%s swapchain image %p semaphore %p in acquire with %lu other semaphores.", (_availability.isAvailable ? "Signaling" : "Tracking"), this, semaphore, _availabilitySignalers.size()); +} + +// If present, signal the semaphore for the first waiter for the given image. +void MVKSwapchainImage::signalPresentationSemaphore(id mtlCmdBuff) { + lock_guard lock(_availabilityLock); + + if ( !_availabilitySignalers.empty() ) { + MVKSemaphore* mvkSem = _availabilitySignalers.front().first; + if (mvkSem) { mvkSem->encodeSignal(mtlCmdBuff); } + } +} + +// Signal either or both of the semaphore and fence in the specified tracker pair. +void MVKSwapchainImage::signal(MVKSwapchainSignaler& signaler, id mtlCmdBuff) { + if (signaler.first) { signaler.first->encodeSignal(mtlCmdBuff); } + if (signaler.second) { signaler.second->signal(); } +} + +// Tell the semaphore and fence that they are being tracked for future signaling. +void MVKSwapchainImage::markAsTracked(MVKSwapchainSignaler& signaler) { + if (signaler.first) { signaler.first->retain(); } + if (signaler.second) { signaler.second->retain(); } +} + +// Tell the semaphore and fence that they are no longer being tracked for future signaling. +void MVKSwapchainImage::unmarkAsTracked(MVKSwapchainSignaler& signaler) { + if (signaler.first) { signaler.first->release(); } + if (signaler.second) { signaler.second->release(); } +} + #pragma mark Metal @@ -853,11 +959,11 @@ if (scName) { mvkPopDebugGroup(mtlCmdBuff); } resetMetalDrawable(); - _swapchain->signalPresentationSemaphore(_swapchainIndex, mtlCmdBuff); + signalPresentationSemaphore(mtlCmdBuff); retain(); // Ensure this image is not destroyed while awaiting MTLCommandBuffer completion [mtlCmdBuff addCompletedHandler: ^(id mcb) { - _swapchain->makeAvailable(_swapchainIndex); + makeAvailable(); release(); }]; } @@ -879,6 +985,10 @@ _swapchain = swapchain; _swapchainIndex = swapchainIndex; _mtlDrawable = nil; + + _availability.acquisitionID = _swapchain->getNextAcquisitionID(); + _availability.isAvailable = true; + _preSignaler = make_pair(nullptr, nullptr); } MVKSwapchainImage::MVKSwapchainImage(MVKDevice* device, diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.h b/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.h index d18e5e47d..8b88c6441 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.h @@ -27,21 +27,9 @@ class MVKWatermark; @class MVKBlockObserver; -/** 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; -} MVKSwapchainImageAvailability; - - +#pragma mark - #pragma mark MVKSwapchain -/** Tracks a semaphore and fence for later signaling. */ -typedef std::pair MVKSwapchainSignaler; - /** Represents a Vulkan swapchain. */ class MVKSwapchain : public MVKVulkanAPIDeviceObject { @@ -54,10 +42,10 @@ class MVKSwapchain : public MVKVulkanAPIDeviceObject { VkDebugReportObjectTypeEXT getVkDebugReportObjectType() override { return VK_DEBUG_REPORT_OBJECT_TYPE_SWAPCHAIN_KHR_EXT; } /** Returns the number of images in this swapchain. */ - uint32_t getImageCount(); + inline uint32_t getImageCount() { return (uint32_t)_surfaceImages.size(); } /** Returns the image at the specified index. */ - MVKSwapchainImage* getImage(uint32_t index); + inline MVKSwapchainImage* getImage(uint32_t index) { return _surfaceImages[index]; } /** * Returns the array of presentable images associated with this swapchain. @@ -93,13 +81,6 @@ class MVKSwapchain : public MVKVulkanAPIDeviceObject { /** Adds HDR metadata to this swapchain. */ void setHDRMetadataEXT(const VkHdrMetadataEXT& metadata); - /** - * Registers a semaphore and/or fence that will be signaled when the image at the given index becomes available. - * This function accepts both a semaphore and a fence, and either none, one, or both may be provided. - * If this image is available already, the semaphore and fence are immediately signaled. - */ - void signalWhenAvailable(uint32_t imageIndex, MVKSemaphore* semaphore, MVKFence* fence); - #pragma mark Construction @@ -110,12 +91,6 @@ class MVKSwapchain : public MVKVulkanAPIDeviceObject { protected: friend class MVKSwapchainImage; - struct Availability { - MVKSwapchainImageAvailability status; - MVKVectorInline signalers; - MVKSwapchainSignaler preSignaled; - }; - void propogateDebugName() override; void initCAMetalLayer(const VkSwapchainCreateInfoKHR* pCreateInfo, uint32_t imgCnt); void initSurfaceImages(const VkSwapchainCreateInfoKHR* pCreateInfo, uint32_t imgCnt); @@ -125,17 +100,10 @@ class MVKSwapchain : public MVKVulkanAPIDeviceObject { void willPresentSurface(id mtlTexture, id mtlCmdBuff); void renderWatermark(id mtlTexture, id mtlCmdBuff); void markFrameInterval(); - void signal(MVKSwapchainSignaler& signaler, id mtlCmdBuff); - void signalPresentationSemaphore(uint32_t imgIdx, id mtlCmdBuff); - static void markAsTracked(MVKSwapchainSignaler& signaler); - static void unmarkAsTracked(MVKSwapchainSignaler& signaler); - void makeAvailable(uint32_t imgIdx); CAMetalLayer* _mtlLayer; MVKWatermark* _licenseWatermark; MVKVectorInline _surfaceImages; - MVKVectorInline _imageAvailability; - std::mutex _availabilityLock; std::atomic _currentAcquisitionID; CGSize _mtlLayerOrigDrawSize; MVKSwapchainPerformance _performanceStatistics; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm b/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm index c6da2900a..9bc255f83 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm @@ -35,18 +35,9 @@ using namespace std; +#pragma mark - #pragma mark MVKSwapchain -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; -} - void MVKSwapchain::propogateDebugName() { if (_debugName) { size_t imgCnt = _surfaceImages.size(); @@ -58,10 +49,6 @@ } } -uint32_t MVKSwapchain::getImageCount() { return (uint32_t)_imageAvailability.size(); } - -MVKSwapchainImage* MVKSwapchain::getImage(uint32_t index) { return _surfaceImages[index]; } - VkResult MVKSwapchain::getImages(uint32_t* pCount, VkImage* pSwapchainImages) { // Get the number of surface images @@ -86,29 +73,33 @@ } VkResult MVKSwapchain::acquireNextImageKHR(uint64_t timeout, - VkSemaphore semaphore, - VkFence fence, + VkSemaphore semaphore, + VkFence fence, uint32_t deviceMask, - uint32_t* pImageIndex) { + uint32_t* pImageIndex) { - if ( getIsSurfaceLost() ) { return VK_ERROR_SURFACE_LOST_KHR; } + if ( getIsSurfaceLost() ) { return VK_ERROR_SURFACE_LOST_KHR; } - // Find the image that has the smallest availability measure - uint32_t minWaitIndex = 0; - MVKSwapchainImageAvailability minAvailability = { .acquisitionID = kMVKUndefinedLargeUInt64, + // Find the image that has the smallest availability measure + MVKSwapchainImage* minWaitImage = nullptr; + MVKSwapchainImageAvailability minAvailability = { .acquisitionID = kMVKUndefinedLargeUInt64, .waitCount = kMVKUndefinedLargeUInt32, .isAvailable = false }; - for (uint32_t imgIdx = 0; imgIdx < _imageAvailability.size(); imgIdx++) { - const Availability& avail = _imageAvailability[imgIdx]; - if (avail.status < minAvailability) { - minAvailability = avail.status; - minWaitIndex = imgIdx; - } - } + uint32_t imgCnt = getImageCount(); + for (uint32_t imgIdx = 0; imgIdx < imgCnt; imgIdx++) { + MVKSwapchainImage* img = getImage(imgIdx); + auto imgAvail = img->getAvailability(); + if (imgAvail < minAvailability) { + minAvailability = imgAvail; + minWaitImage = img; + } + } + + // Return the index of the image with the shortest wait and signal the semaphore and fence when it's available + *pImageIndex = minWaitImage->_swapchainIndex; + minWaitImage->signalWhenAvailable((MVKSemaphore*)semaphore, (MVKFence*)fence); - *pImageIndex = minWaitIndex; // Return the index of the image with the shortest wait - signalWhenAvailable(minWaitIndex, (MVKSemaphore*)semaphore, (MVKFence*)fence); - return getHasSurfaceSizeChanged() ? VK_ERROR_OUT_OF_DATE_KHR : VK_SUCCESS; + return getHasSurfaceSizeChanged() ? VK_ERROR_OUT_OF_DATE_KHR : VK_SUCCESS; } bool MVKSwapchain::getHasSurfaceSizeChanged() { @@ -117,104 +108,10 @@ uint64_t MVKSwapchain::getNextAcquisitionID() { return ++_currentAcquisitionID; } -/** - * Releases any surfaces that are not currently being displayed, - * so they can be used by a different swapchain. - */ +// Releases any surfaces that are not currently being displayed, +// so they can be used by a different swapchain. void MVKSwapchain::releaseUndisplayedSurfaces() {} -// Makes an image available for acquisition by the app. -// If any semaphores are waiting to be signaled when this image becomes available, the -// earliest semaphore is signaled, and this image remains unavailable for other uses. -void MVKSwapchain::makeAvailable(uint32_t imgIdx) { - lock_guard lock(_availabilityLock); - auto& availability = _imageAvailability[imgIdx].status; - - // Mark when this event happened, relative to that of other images - availability.acquisitionID = getNextAcquisitionID(); - - // Mark this image as available if no semaphores or fences are waiting to be signaled. - availability.isAvailable = _imageAvailability[imgIdx].signalers.empty(); - - MVKSwapchainSignaler signaler; - if (availability.isAvailable) { - // If this image is available, signal the semaphore and fence that were associated - // with the last time this image was acquired while available. This is a workaround for - // when an app uses a single semaphore or fence for more than one swapchain image. - // Becuase the semaphore or fence will be signaled by more than one image, it will - // get out of sync, and the final use of the image would not be signaled as a result. - signaler = _imageAvailability[imgIdx].preSignaled; - } else { - // If this image is not yet available, extract and signal the first semaphore and fence. - auto& imgSignalers = _imageAvailability[imgIdx].signalers; - auto sigIter = imgSignalers.begin(); - signaler = *sigIter; - imgSignalers.erase(sigIter); - } - - // Signal the semaphore and fence, and let them know they are no longer being tracked. - signal(signaler, nil); - unmarkAsTracked(signaler); - -// MVKLogDebug("Signaling%s swapchain image %p semaphore %p from present, with %lu remaining semaphores.", (_availability.isAvailable ? " pre-signaled" : ""), this, signaler.first, _availabilitySignalers.size()); -} - -void MVKSwapchain::signalWhenAvailable(uint32_t imageIndex, MVKSemaphore* semaphore, MVKFence* fence) { - lock_guard lock(_availabilityLock); - auto signaler = make_pair(semaphore, fence); - auto& availability = _imageAvailability[imageIndex].status; - if (availability.isAvailable) { - availability.isAvailable = false; - - // If signalling through a MTLEvent, use an ephemeral MTLCommandBuffer. - // Another option would be to use MTLSharedEvent in MVKSemaphore, but that might - // impose unacceptable performance costs to handle this particular case. - @autoreleasepool { - MVKSemaphore* mvkSem = signaler.first; - id mtlCmdBuff = (mvkSem && mvkSem->isUsingCommandEncoding() - ? [_device->getAnyQueue()->getMTLCommandQueue() commandBufferWithUnretainedReferences] - : nil); - signal(signaler, mtlCmdBuff); - [mtlCmdBuff commit]; - } - - _imageAvailability[imageIndex].preSignaled = signaler; - } else { - _imageAvailability[imageIndex].signalers.push_back(signaler); - } - markAsTracked(signaler); - -// MVKLogDebug("%s swapchain image %p semaphore %p in acquire with %lu other semaphores.", (_availability.isAvailable ? "Signaling" : "Tracking"), this, semaphore, _availabilitySignalers.size()); -} - -// Signal either or both of the semaphore and fence in the specified tracker pair. -void MVKSwapchain::signal(MVKSwapchainSignaler& signaler, id mtlCmdBuff) { - if (signaler.first) { signaler.first->encodeSignal(mtlCmdBuff); } - if (signaler.second) { signaler.second->signal(); } -} - -// If present, signal the semaphore for the first waiter for the given image. -void MVKSwapchain::signalPresentationSemaphore(uint32_t imgIdx, id mtlCmdBuff) { - lock_guard lock(_availabilityLock); - auto& imgSignalers = _imageAvailability[imgIdx].signalers; - if ( !imgSignalers.empty() ) { - MVKSemaphore* mvkSem = imgSignalers.front().first; - if (mvkSem) { mvkSem->encodeSignal(mtlCmdBuff); } - } -} - -// Tell the semaphore and fence that they are being tracked for future signaling. -void MVKSwapchain::markAsTracked(MVKSwapchainSignaler& signaler) { - if (signaler.first) { signaler.first->retain(); } - if (signaler.second) { signaler.second->retain(); } -} - -// Tell the semaphore and fence that they are no longer being tracked for future signaling. -void MVKSwapchain::unmarkAsTracked(MVKSwapchainSignaler& signaler) { - if (signaler.first) { signaler.first->release(); } - if (signaler.second) { signaler.second->release(); } -} - #pragma mark Rendering @@ -485,13 +382,9 @@ static inline CIE1931XY VkXYColorEXTToCIE1931XY(VkXYColorEXT xy) { mvkEnableFlags(imgInfo.flags, VK_IMAGE_CREATE_SPLIT_INSTANCE_BIND_REGIONS_BIT); } - _imageAvailability.resize(imgCnt); - for (uint32_t imgIdx = 0; imgIdx < imgCnt; imgIdx++) { - _surfaceImages.push_back(_device->createSwapchainImage(&imgInfo, this, imgIdx, NULL)); - _imageAvailability[imgIdx].status.acquisitionID = getNextAcquisitionID(); - _imageAvailability[imgIdx].status.isAvailable = true; - _imageAvailability[imgIdx].preSignaled = make_pair(nullptr, nullptr); - } + for (uint32_t imgIdx = 0; imgIdx < imgCnt; imgIdx++) { + _surfaceImages.push_back(_device->createSwapchainImage(&imgInfo, this, imgIdx, NULL)); + } MVKLogInfo("Created %d swapchain images with initial size (%d, %d).", imgCnt, imgExtent.width, imgExtent.height); }