Merge pull request #1890 from billhollings/fix-display-timing-race-co…

…ndition Fix race condition when updating values in VkPastPresentationTimingGOOGLE.
KhronosGroup · May 5, 2023 · dfc0af8 · dfc0af8
2 parents af9544d + 9e46669
commit dfc0af8
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 6 deletions.
diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md
@@ -24,6 +24,7 @@ Released TBD
 - Support separate depth and stencil attachments during dynamic rendering.
 - Deprecate the obsolete and non-standard `VK_MVK_moltenvk` extension.
 - Fix memory leak when waiting on timeline semaphores.
+- Fix race condition when updating values in `VkPastPresentationTimingGOOGLE`.
 - Ensure shaders that use `PhysicalStorageBufferAddresses` encode the use of the associated `MTLBuffer`.
 - Disable pipeline cache compression prior to macOS 10.15 and iOS/tvOS 13.0.
 - Add `MVK_ENABLE_EXPLICIT_LOD_WORKAROUND` environment variable to selectively 

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.h b/MoltenVK/MoltenVK/GPUObjects/MVKImage.h
@@ -449,7 +449,7 @@ class MVKPresentableSwapchainImage : public MVKSwapchainImage {
 #pragma mark Metal
 
  /** Presents the contained drawable to the OS. */
- void presentCAMetalDrawable(id<MTLCommandBuffer> mtlCmdBuff, MVKImagePresentInfo& presentInfo);
+ void presentCAMetalDrawable(id<MTLCommandBuffer> mtlCmdBuff, MVKImagePresentInfo presentInfo);
 
 
 #pragma mark Construction
@@ -463,7 +463,7 @@ class MVKPresentableSwapchainImage : public MVKSwapchainImage {
  friend MVKSwapchain;
 
  id<CAMetalDrawable> getCAMetalDrawable() override;
- void addPresentedHandler(id<CAMetalDrawable> mtlDrawable, MVKImagePresentInfo& presentInfo);
+ void addPresentedHandler(id<CAMetalDrawable> mtlDrawable, MVKImagePresentInfo presentInfo);
  void releaseMetalDrawable();
  MVKSwapchainImageAvailability getAvailability();
  void makeAvailable(const MVKSwapchainSignaler& signaler);

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
@@ -1294,8 +1294,9 @@ static void signalAndUnmarkAsTracked(const MVKSwapchainSignaler& signaler) {
 }
 
 // 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.
 void MVKPresentableSwapchainImage::presentCAMetalDrawable(id<MTLCommandBuffer> mtlCmdBuff,
- MVKImagePresentInfo& presentInfo) {
+ MVKImagePresentInfo presentInfo) {
  lock_guard<mutex> lock(_availabilityLock);
 
  _swapchain->willPresentSurface(getMTLTexture(0), mtlCmdBuff);
@@ -1357,8 +1358,9 @@ static void signalAndUnmarkAsTracked(const MVKSwapchainSignaler& signaler) {
  signalPresentationSemaphore(signaler, mtlCmdBuff);
 }
 
+// Pass MVKImagePresentInfo by value because it may not exist when the callback runs.
 void MVKPresentableSwapchainImage::addPresentedHandler(id<CAMetalDrawable> mtlDrawable,
- MVKImagePresentInfo& presentInfo) {
+ MVKImagePresentInfo presentInfo) {
 #if !MVK_OS_SIMULATOR
  if ([mtlDrawable respondsToSelector: @selector(addPresentedHandler:)]) {
  retain(); // Ensure this image is not destroyed while awaiting presentation

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.h b/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.h
@@ -121,7 +121,7 @@ class MVKSwapchain : public MVKVulkanAPIDeviceObject {
  void willPresentSurface(id<MTLTexture> mtlTexture, id<MTLCommandBuffer> mtlCmdBuff);
  void renderWatermark(id<MTLTexture> mtlTexture, id<MTLCommandBuffer> mtlCmdBuff);
  void markFrameInterval();
- void recordPresentTime(MVKImagePresentInfo& presentInfo, uint64_t actualPresentTime = 0);
+ void recordPresentTime(const MVKImagePresentInfo& presentInfo, uint64_t actualPresentTime = 0);
 
  CAMetalLayer* _mtlLayer = nil;
  MVKWatermark* _licenseWatermark = nullptr;

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm b/MoltenVK/MoltenVK/GPUObjects/MVKSwapchain.mm
@@ -557,7 +557,7 @@ static CALayerContentsGravity getCALayerContentsGravity(VkSwapchainPresentScalin
  return res;
 }
 
-void MVKSwapchain::recordPresentTime(MVKImagePresentInfo& presentInfo, uint64_t actualPresentTime) {
+void MVKSwapchain::recordPresentTime(const MVKImagePresentInfo& presentInfo, uint64_t actualPresentTime) {
  std::lock_guard<std::mutex> lock(_presentHistoryLock);
  if (_presentHistoryCount < kMaxPresentationHistory) {
  _presentHistoryCount++;