Fixing swapchain for use with triple buffering

Some platforms are using triple buffering, but the
SwapchainBuffers only allocate 2. Changed to ensure that
the buffer count matches at least the minimum number of
images returned on swapchain creation.

Fixed coverity error in CreateSwapchainForSurface /
ReplaceSwapchainForSurface.

Change-Id: I6292daec0bf6e568f6818751ef1994c5ce87b24b
Signed-off-by: David Steele <david.steele@samsung.com>

diff --git a/dali/internal/graphics/vulkan-impl/vulkan-swapchain-impl.cpp b/dali/internal/graphics/vulkan-impl/vulkan-swapchain-impl.cpp
index 7bb6c504a0e1b6c3031dac03eac35f7832fae678..62bcbfb00987b9af9837012ebf219b3573db1aeb 100644 (file)
--- a/dali/internal/graphics/vulkan-impl/vulkan-swapchain-impl.cpp
+++ b/dali/internal/graphics/vulkan-impl/vulkan-swapchain-impl.cpp
@@ -32,10 +32,6 @@ extern Debug::Filter* gVulkanFilter;
 
 namespace Dali::Graphics::Vulkan
 {
-namespace
-{
-const auto MAX_SWAPCHAIN_RESOURCE_BUFFERS = 2u;
-}
 
 /**
  * SwapchainBuffer stores all per-buffer data
@@ -144,7 +140,7 @@ void Swapchain::CreateVkSwapchain(
 
   // Determine the number of images
   if(surfaceCapabilities.minImageCount > 0 &&
-     bufferCount > surfaceCapabilities.minImageCount)
+     bufferCount != surfaceCapabilities.minImageCount)
   {
     bufferCount = surfaceCapabilities.minImageCount;
   }
@@ -164,7 +160,8 @@ void Swapchain::CreateVkSwapchain(
   auto presentModes = surface->GetSurfacePresentModes();
   auto found        = std::find_if(presentModes.begin(),
                             presentModes.end(),
-                            [&](vk::PresentModeKHR mode) {
+                            [&](vk::PresentModeKHR mode)
+                            {
                               return presentMode == mode;
                             });
 
@@ -271,7 +268,8 @@ void Swapchain::CreateFramebuffers(FramebufferAttachmentHandle depthAttachment)
                            depthAttachment,
                            mSwapchainCreateInfoKHR.imageExtent.width,
                            mSwapchainCreateInfoKHR.imageExtent.height),
-      [](FramebufferImpl* framebuffer1) {
+      [](FramebufferImpl* framebuffer1)
+      {
         framebuffer1->Destroy();
         delete framebuffer1;
       });
@@ -315,7 +313,7 @@ FramebufferImpl* Swapchain::AcquireNextFramebuffer(bool shouldCollectGarbageNow)
   // on swapchain first create sync primitives if not created yet
   if(mSwapchainBuffers.empty())
   {
-    mSwapchainBuffers.resize(MAX_SWAPCHAIN_RESOURCE_BUFFERS);
+    mSwapchainBuffers.resize(mSwapchainImages.size());
     for(auto& buffer : mSwapchainBuffers)
     {
       buffer.reset(new SwapchainBuffer(mGraphicsDevice));

diff --git a/dali/internal/graphics/vulkan/vulkan-device.cpp b/dali/internal/graphics/vulkan/vulkan-device.cpp
index a93d8f70a88da8c1bc6a8d38450953e1daa4a187..1e70fe8cb28a997b99484ab41687049dac18496d 100644 (file)
--- a/dali/internal/graphics/vulkan/vulkan-device.cpp
+++ b/dali/internal/graphics/vulkan/vulkan-device.cpp
@@ -305,62 +305,41 @@ void Device::DestroySurface(Dali::Graphics::SurfaceId surfaceId)
   }
 }
 
-Swapchain* Device::CreateSwapchainForSurface(SurfaceImpl* surface)
+void Device::CreateSwapchainForSurface(SurfaceId surfaceId)
 {
-  DALI_ASSERT_DEBUG(surface && "Surface ptr must be allocated");
-
-  auto surfaceCapabilities = surface->GetCapabilities();
-
-  // TODO: propagate the format and presentation mode to higher layers to allow for more control?
-  //  @todo - for instance, Graphics::RenderTargetCreateInfo?!
-  Swapchain* swapchain = CreateSwapchain(surface,
-                                         vk::Format::eB8G8R8A8Unorm,
-                                         vk::PresentModeKHR::eFifo,
-                                         surfaceCapabilities.minImageCount,
-                                         nullptr);
-
   // store swapchain in the correct pair
-  for(auto&& val : mSurfaceMap)
+  if(auto match = mSurfaceMap.find(surfaceId); match != mSurfaceMap.end())
   {
-    if(val.second.surface == surface)
-    {
-      val.second.swapchain = swapchain;
-      break;
-    }
+    match->second.swapchain = CreateSwapchain(match->second.surface,
+                                              vk::Format::eB8G8R8A8Unorm,
+                                              vk::PresentModeKHR::eFifo,
+                                              nullptr);
+  }
+  else
+  {
+    DALI_LOG_ERROR("Can't find surface: %d\n", surfaceId);
   }
-
-  return swapchain;
 }
 
-Swapchain* Device::ReplaceSwapchainForSurface(SurfaceImpl* surface, Swapchain*&& oldSwapchain)
+Swapchain* Device::ReplaceSwapchainForSurface(SurfaceId surfaceId, Swapchain*&& oldSwapchain)
 {
-  auto surfaceCapabilities = surface->GetCapabilities();
-
-  mSurfaceResized = false;
-
-  auto swapchain = CreateSwapchain(surface,
-                                   vk::Format::eB8G8R8A8Unorm,
-                                   vk::PresentModeKHR::eFifo,
-                                   surfaceCapabilities.minImageCount,
-                                   std::move(oldSwapchain));
-
-  // store swapchain in the correct pair
-  for(auto&& val : mSurfaceMap)
+  if(auto match = mSurfaceMap.find(surfaceId); match != mSurfaceMap.end())
   {
-    if(val.second.surface == surface)
-    {
-      val.second.swapchain = swapchain;
-      break;
-    }
+    mSurfaceResized         = false;
+    match->second.swapchain = CreateSwapchain(match->second.surface,
+                                              vk::Format::eB8G8R8A8Unorm,
+                                              vk::PresentModeKHR::eFifo,
+                                              std::move(oldSwapchain));
+    return match->second.swapchain;
   }
 
-  return swapchain;
+  DALI_LOG_ERROR("Can't find surface: %d\n", surfaceId);
+  return nullptr;
 }
 
 Swapchain* Device::CreateSwapchain(SurfaceImpl*       surface,
                                    vk::Format         requestedFormat,
                                    vk::PresentModeKHR presentMode,
-                                   uint32_t&          bufferCount,
                                    Swapchain*&&       oldSwapchain)
 {
   auto newSwapchain = Swapchain::NewSwapchain(*this, GetPresentQueue(), oldSwapchain ? oldSwapchain->GetVkHandle() : nullptr, surface, requestedFormat, presentMode, mBufferCount);
@@ -379,8 +358,6 @@ Swapchain* Device::CreateSwapchain(SurfaceImpl*       surface,
 
   if(oldSwapchain)
   {
-    // prevent destroying the swapchain as it is handled automatically
-    // during replacing the swapchain
     auto khr = oldSwapchain->GetVkHandle();
     oldSwapchain->SetVkHandle(nullptr);
     oldSwapchain->Destroy();
@@ -389,7 +366,6 @@ Swapchain* Device::CreateSwapchain(SurfaceImpl*       surface,
     mLogicalDevice.destroySwapchainKHR(khr, *mAllocator);
   }
 
-  // @todo: Only create framebuffers if no "external" render passes.
   FramebufferAttachmentHandle empty;
   newSwapchain->CreateFramebuffers(empty); // Note, this may destroy vk swapchain if invalid.
   return newSwapchain;
@@ -410,8 +386,7 @@ void Device::AcquireNextImage(SurfaceId surfaceId)
       DeviceWaitIdle();
 
       // replace swapchain (only once)
-      swapchain = ReplaceSwapchainForSurface(swapchain->GetSurface(), std::move(swapchain));
-
+      swapchain = ReplaceSwapchainForSurface(surfaceId, std::move(swapchain));
       // get new valid framebuffer
       if(swapchain)
       {
@@ -489,8 +464,6 @@ Image* Device::CreateImageFromExternal(vk::Image externalImage, vk::Format image
                            .setMipLevels(1);
 
   return new Image(*this, imageCreateInfo, externalImage);
-
-  return nullptr;
 }
 
 // -------------------------------------------------------------------------------------------------------
@@ -521,9 +494,7 @@ Swapchain* Device::GetSwapchainForSurfaceId(Graphics::SurfaceId surfaceId)
 {
   if(surfaceId == 0)
   {
-    return mSurfaceMap.begin()
-      ->second
-      .swapchain;
+    return mSurfaceMap.begin()->second.swapchain;
   }
   return mSurfaceMap[surfaceId].swapchain;
 }

diff --git a/dali/internal/graphics/vulkan/vulkan-device.h b/dali/internal/graphics/vulkan/vulkan-device.h
index 2b9c108fb6b323b157e596cb17cd8a0ea31c35c9..48671473efa64dbfdfa04aa6a8488d6c3b34c352 100644 (file)
--- a/dali/internal/graphics/vulkan/vulkan-device.h
+++ b/dali/internal/graphics/vulkan/vulkan-device.h
@@ -69,11 +69,9 @@ public: // Create methods
 
   void DestroySurface(Dali::Graphics::SurfaceId surfaceId);
 
-  Swapchain* CreateSwapchainForSurface(SurfaceImpl* surface);
+  void CreateSwapchainForSurface(Dali::Graphics::SurfaceId surfaceId);
 
-  Swapchain* ReplaceSwapchainForSurface(SurfaceImpl* surface, Swapchain*&& oldSwapchain);
-
-  Swapchain* CreateSwapchain(SurfaceImpl* surface, vk::Format requestedFormat, vk::PresentModeKHR presentMode, uint32_t& bufferCount, Swapchain*&& oldSwapchain);
+  Swapchain* ReplaceSwapchainForSurface(Dali::Graphics::SurfaceId surfaceId, Swapchain*&& oldSwapchain);
 
   /**
    * Ensures that the next available image is retrieved for drawing onto.
@@ -160,6 +158,8 @@ private: // Methods
 
   void ReleaseCommandPools();
 
+  Swapchain* CreateSwapchain(SurfaceImpl* surface, vk::Format requestedFormat, vk::PresentModeKHR presentMode, Swapchain*&& oldSwapchain);
+
 private: // Members
   vk::PhysicalDevice mPhysicalDevice;
   vk::Device         mLogicalDevice;

diff --git a/dali/internal/graphics/vulkan/vulkan-graphics-impl.cpp b/dali/internal/graphics/vulkan/vulkan-graphics-impl.cpp
index b54e6660f5a5a85ea0a4d980bc89210447cdc91f..aea6dd5a2669707f03b4e3ad750224b59a874e88 100644 (file)
--- a/dali/internal/graphics/vulkan/vulkan-graphics-impl.cpp
+++ b/dali/internal/graphics/vulkan/vulkan-graphics-impl.cpp
@@ -93,8 +93,7 @@ Graphics::SurfaceId VulkanGraphics::CreateSurface(
   auto surfaceId           = mGraphicsDevice.CreateSurface(*surfaceFactory, createInfo);
 
   // create swapchain for surface
-  auto surface = mGraphicsDevice.GetSurface(surfaceId);
-  mGraphicsDevice.CreateSwapchainForSurface(surface);
+  mGraphicsDevice.CreateSwapchainForSurface(surfaceId);
 
   return surfaceId;
 }