Guard against CAMetalDrawable with invalid pixel format.

- Calling nextDrawable may result in a nil drawable, or a drawable with no
  pixel format. Attempt several times to retrieve a drawable with a valid
  pixel format, and if unsuccessful, return an error from vkQueuePresentKHR()
  and vkAcquireNextImageKHR(), to force swapchain to be re-created.
- Reorganize MVKQueuePresentSurfaceSubmission::execute() to detect drawable
  with invalid format, attach MTLCommandBuffer completion handler just before
  commit, and delay enqueuing MTLCommandBuffer until commit.
- Refactor mvkOSVersionIsAtLeast() for clarity (unrelated).
This commit is contained in:
Bill Hollings 2023-09-06 16:16:11 -04:00
parent 781a834663
commit 7fe4963985
5 changed files with 42 additions and 26 deletions

View File

@ -39,27 +39,30 @@ static const MVKOSVersion kMVKOSVersionUnsupported = std::numeric_limits<MVKOSVe
MVKOSVersion mvkOSVersion(); MVKOSVersion mvkOSVersion();
/** Returns a MVKOSVersion built from the version components. */ /** Returns a MVKOSVersion built from the version components. */
inline MVKOSVersion mvkMakeOSVersion(uint32_t major, uint32_t minor, uint32_t patch) { static inline MVKOSVersion mvkMakeOSVersion(uint32_t major, uint32_t minor, uint32_t patch) {
return (float)major + ((float)minor / 100.0f) + ((float)patch / 10000.0f); return (float)major + ((float)minor / 100.0f) + ((float)patch / 10000.0f);
} }
/** Returns whether the operating system version is at least minVer. */ /** Returns whether the operating system version is at least minVer. */
inline bool mvkOSVersionIsAtLeast(MVKOSVersion minVer) { return mvkOSVersion() >= minVer; } static inline bool mvkOSVersionIsAtLeast(MVKOSVersion minVer) { return mvkOSVersion() >= minVer; }
/** /**
* Returns whether the operating system version is at least the appropriate min version. * Returns whether the operating system version is at least the appropriate min version.
* The constant kMVKOSVersionUnsupported can be used for either value to cause the test * The constant kMVKOSVersionUnsupported can be used for any of the values to cause the test
* to always fail on that OS, which is useful for indidicating functionalty guarded by * to always fail on that OS, which is useful for indidicating that functionalty guarded by
* this test is not supported on that OS. * this test is not supported on that OS.
*/ */
inline bool mvkOSVersionIsAtLeast(MVKOSVersion macOSMinVer, MVKOSVersion iOSMinVer, MVKOSVersion visionOSMinVer) { static inline bool mvkOSVersionIsAtLeast(MVKOSVersion macOSMinVer,
MVKOSVersion iOSMinVer,
MVKOSVersion visionOSMinVer) {
#if MVK_MACOS #if MVK_MACOS
return mvkOSVersionIsAtLeast(macOSMinVer); return mvkOSVersionIsAtLeast(macOSMinVer);
#endif #endif
#if MVK_IOS_OR_TVOS
return mvkOSVersionIsAtLeast(iOSMinVer);
#endif
#if MVK_VISIONOS #if MVK_VISIONOS
return mvkOSVersionIsAtLeast(visionOSMinVer); return mvkOSVersionIsAtLeast(visionOSMinVer);
#elif MVK_IOS_OR_TVOS
return mvkOSVersionIsAtLeast(iOSMinVer);
#endif #endif
} }

View File

@ -20,6 +20,7 @@ Released TBD
- Fix rare case where vertex attribute buffers are not bound to Metal - Fix rare case where vertex attribute buffers are not bound to Metal
when no other bindings change between pipelines. when no other bindings change between pipelines.
- Fix case where a `CAMetalDrawable` with invalid pixel format causes onscreen flickering.
- Improve behavior of swapchain image presentation stalls caused by Metal regression. - Improve behavior of swapchain image presentation stalls caused by Metal regression.
- Add several additional performance trackers, available via logging, or the `mvk_private_api.h` API. - Add several additional performance trackers, available via logging, or the `mvk_private_api.h` API.
- Update `MVK_CONFIGURATION_API_VERSION` and `MVK_PRIVATE_API_VERSION` to `38`. - Update `MVK_CONFIGURATION_API_VERSION` and `MVK_PRIVATE_API_VERSION` to `38`.

View File

@ -454,7 +454,7 @@ public:
#pragma mark Metal #pragma mark Metal
/** Presents the contained drawable to the OS. */ /** Presents the contained drawable to the OS. */
void presentCAMetalDrawable(id<MTLCommandBuffer> mtlCmdBuff, MVKImagePresentInfo presentInfo); VkResult presentCAMetalDrawable(id<MTLCommandBuffer> mtlCmdBuff, MVKImagePresentInfo presentInfo);
/** Called when the presentation begins. */ /** Called when the presentation begins. */
void beginPresentation(const MVKImagePresentInfo& presentInfo); void beginPresentation(const MVKImagePresentInfo& presentInfo);

View File

@ -1258,7 +1258,6 @@ VkResult MVKPresentableSwapchainImage::acquireAndSignalWhenAvailable(MVKSemaphor
// This is not done earlier so the texture is retained for any post-processing such as screen captures, etc. // This is not done earlier so the texture is retained for any post-processing such as screen captures, etc.
releaseMetalDrawable(); releaseMetalDrawable();
VkResult rslt = VK_SUCCESS;
auto signaler = MVKSwapchainSignaler{fence, semaphore, semaphore ? semaphore->deferSignal() : 0}; auto signaler = MVKSwapchainSignaler{fence, semaphore, semaphore ? semaphore->deferSignal() : 0};
if (_availability.isAvailable) { if (_availability.isAvailable) {
_availability.isAvailable = false; _availability.isAvailable = false;
@ -1271,7 +1270,7 @@ VkResult MVKPresentableSwapchainImage::acquireAndSignalWhenAvailable(MVKSemaphor
id<MTLCommandBuffer> mtlCmdBuff = nil; id<MTLCommandBuffer> mtlCmdBuff = nil;
if (mvkSem && mvkSem->isUsingCommandEncoding()) { if (mvkSem && mvkSem->isUsingCommandEncoding()) {
mtlCmdBuff = _device->getAnyQueue()->getMTLCommandBuffer(kMVKCommandUseAcquireNextImage); mtlCmdBuff = _device->getAnyQueue()->getMTLCommandBuffer(kMVKCommandUseAcquireNextImage);
if ( !mtlCmdBuff ) { rslt = VK_ERROR_OUT_OF_POOL_MEMORY; } if ( !mtlCmdBuff ) { setConfigurationResult(VK_ERROR_OUT_OF_POOL_MEMORY); }
} }
signal(signaler, mtlCmdBuff); signal(signaler, mtlCmdBuff);
[mtlCmdBuff commit]; [mtlCmdBuff commit];
@ -1283,19 +1282,29 @@ VkResult MVKPresentableSwapchainImage::acquireAndSignalWhenAvailable(MVKSemaphor
} }
markAsTracked(signaler); markAsTracked(signaler);
return rslt; return getConfigurationResult();
} }
// Calling nextDrawable may result in a nil drawable, or a drawable with no pixel format.
// Attempt several times to retrieve a good drawable, and set an error to trigger the
// swapchain to be re-established if one cannot be retrieved.
id<CAMetalDrawable> MVKPresentableSwapchainImage::getCAMetalDrawable() { id<CAMetalDrawable> MVKPresentableSwapchainImage::getCAMetalDrawable() {
if ( !_mtlDrawable ) { if ( !_mtlDrawable ) {
@autoreleasepool { @autoreleasepool {
bool hasInvalidFormat = false;
uint32_t attemptCnt = _swapchain->getImageCount() * 2; // Attempt a resonable number of times uint32_t attemptCnt = _swapchain->getImageCount() * 2; // Attempt a resonable number of times
for (uint32_t attemptIdx = 0; !_mtlDrawable && attemptIdx < attemptCnt; attemptIdx++) { for (uint32_t attemptIdx = 0; !_mtlDrawable && attemptIdx < attemptCnt; attemptIdx++) {
uint64_t startTime = _device->getPerformanceTimestamp(); uint64_t startTime = _device->getPerformanceTimestamp();
_mtlDrawable = [_swapchain->_surface->getCAMetalLayer().nextDrawable retain]; // retained _mtlDrawable = [_swapchain->_surface->getCAMetalLayer().nextDrawable retain]; // retained
_device->addPerformanceInterval(_device->_performanceStatistics.queue.retrieveCAMetalDrawable, startTime); _device->addPerformanceInterval(_device->_performanceStatistics.queue.retrieveCAMetalDrawable, startTime);
hasInvalidFormat = _mtlDrawable && !_mtlDrawable.texture.pixelFormat;
if (hasInvalidFormat) { releaseMetalDrawable(); }
}
if (hasInvalidFormat) {
setConfigurationResult(reportError(VK_ERROR_OUT_OF_DATE_KHR, "CAMetalDrawable with valid format could not be acquired after %d attempts.", attemptCnt));
} else if ( !_mtlDrawable ) {
setConfigurationResult(reportError(VK_ERROR_OUT_OF_POOL_MEMORY, "CAMetalDrawable could not be acquired after %d attempts.", attemptCnt));
} }
if ( !_mtlDrawable ) { reportError(VK_ERROR_OUT_OF_POOL_MEMORY, "CAMetalDrawable could not be acquired after %d attempts.", attemptCnt); }
} }
} }
return _mtlDrawable; return _mtlDrawable;
@ -1303,8 +1312,8 @@ id<CAMetalDrawable> MVKPresentableSwapchainImage::getCAMetalDrawable() {
// Present the drawable and make myself available only once the command buffer has completed. // Present the drawable and make myself available only once the command buffer has completed.
// Pass MVKImagePresentInfo by value because it may not exist when the callback runs. // Pass MVKImagePresentInfo by value because it may not exist when the callback runs.
void MVKPresentableSwapchainImage::presentCAMetalDrawable(id<MTLCommandBuffer> mtlCmdBuff, VkResult MVKPresentableSwapchainImage::presentCAMetalDrawable(id<MTLCommandBuffer> mtlCmdBuff,
MVKImagePresentInfo presentInfo) { MVKImagePresentInfo presentInfo) {
lock_guard<mutex> lock(_availabilityLock); lock_guard<mutex> lock(_availabilityLock);
_swapchain->renderWatermark(getMTLTexture(0), mtlCmdBuff); _swapchain->renderWatermark(getMTLTexture(0), mtlCmdBuff);
@ -1363,6 +1372,8 @@ void MVKPresentableSwapchainImage::presentCAMetalDrawable(id<MTLCommandBuffer> m
}]; }];
signalPresentationSemaphore(signaler, mtlCmdBuff); signalPresentationSemaphore(signaler, mtlCmdBuff);
return getConfigurationResult();
} }
// Pass MVKImagePresentInfo by value because it may not exist when the callback runs. // Pass MVKImagePresentInfo by value because it may not exist when the callback runs.

View File

@ -623,12 +623,6 @@ MVKQueueFullCommandBufferSubmission<N>::MVKQueueFullCommandBufferSubmission(MVKQ
// The semaphores know what to do. // The semaphores know what to do.
VkResult MVKQueuePresentSurfaceSubmission::execute() { VkResult MVKQueuePresentSurfaceSubmission::execute() {
id<MTLCommandBuffer> mtlCmdBuff = _queue->getMTLCommandBuffer(kMVKCommandUseQueuePresent); id<MTLCommandBuffer> mtlCmdBuff = _queue->getMTLCommandBuffer(kMVKCommandUseQueuePresent);
[mtlCmdBuff enqueue];
// Add completion handler that will destroy this submission only once the MTLCommandBuffer
// is finished with the resources retained here, including the wait semaphores.
// Completion handlers are also added in presentCAMetalDrawable() to retain the swapchain images.
[mtlCmdBuff addCompletedHandler: ^(id<MTLCommandBuffer> mtlCB) { this->finish(); }];
for (auto& ws : _waitSemaphores) { for (auto& ws : _waitSemaphores) {
auto& sem4 = ws.first; auto& sem4 = ws.first;
@ -637,15 +631,22 @@ VkResult MVKQueuePresentSurfaceSubmission::execute() {
} }
for (int i = 0; i < _presentInfo.size(); i++ ) { for (int i = 0; i < _presentInfo.size(); i++ ) {
_presentInfo[i].presentableImage->presentCAMetalDrawable(mtlCmdBuff, _presentInfo[i]); setConfigurationResult(_presentInfo[i].presentableImage->presentCAMetalDrawable(mtlCmdBuff, _presentInfo[i]));
} }
[mtlCmdBuff commit]; if ( !mtlCmdBuff ) { setConfigurationResult(VK_ERROR_OUT_OF_POOL_MEMORY); } // Check after images may set error.
// If an error occurred and the MTLCommandBuffer was not created, call finish() directly. // Add completion callback to the MTLCommandBuffer to call finish(),
if ( !mtlCmdBuff ) { finish(); } // or if the MTLCommandBuffer could not be created, call finish() directly.
// Retrieve the result first, because finish() will destroy this instance.
return mtlCmdBuff ? VK_SUCCESS : VK_ERROR_OUT_OF_POOL_MEMORY; VkResult rslt = getConfigurationResult();
if (mtlCmdBuff) {
[mtlCmdBuff addCompletedHandler: ^(id<MTLCommandBuffer> mtlCB) { this->finish(); }];
[mtlCmdBuff commit];
} else {
finish();
}
return rslt;
} }
void MVKQueuePresentSurfaceSubmission::finish() { void MVKQueuePresentSurfaceSubmission::finish() {