[vulkan] Fixed data race in object manager

10 files changed, 65 insertions, 51 deletions

diff --git a/drape/vulkan/vulkan_gpu_buffer_impl.cpp b/drape/vulkan/vulkan_gpu_buffer_impl.cpp
index 4672ae075e..0f7ef241b8 100644
--- a/drape/vulkan/vulkan_gpu_buffer_impl.cpp
+++ b/drape/vulkan/vulkan_gpu_buffer_impl.cpp
@@ -111,19 +111,11 @@ void VulkanGPUBuffer::Resize(ref_ptr<VulkanBaseContext> context, void const * da
   BufferBase::Resize(elementCount);
 
   m_objectManager = context->GetObjectManager();
-
   uint32_t const sizeInBytes = GetCapacity() * GetElementSize();
 
   m_geometryBuffer = m_objectManager->CreateBuffer(VulkanMemoryManager::ResourceType::Geometry,
                                                    sizeInBytes, 0 /* batcherHash */);
-  void * gpuPtr = m_objectManager->Map(m_geometryBuffer);
-  if (data != nullptr)
-  {
-    memcpy(gpuPtr, data, sizeInBytes);
-    m_objectManager->Flush(m_geometryBuffer);
-  }
-
-  m_objectManager->Unmap(m_geometryBuffer);
+  m_objectManager->Fill(m_geometryBuffer, data, sizeInBytes);
 
   // If we have already set up data, we have to call SetDataSize.
   if (data != nullptr)
diff --git a/drape/vulkan/vulkan_mesh_object_impl.cpp b/drape/vulkan/vulkan_mesh_object_impl.cpp
index e08052a4e7..0d4571694e 100644
--- a/drape/vulkan/vulkan_mesh_object_impl.cpp
+++ b/drape/vulkan/vulkan_mesh_object_impl.cpp
@@ -55,10 +55,7 @@ public:
                                                      sizeof(m_mesh->m_buffers[i].m_data[0]));
       m_geometryBuffers[i] = m_objectManager->CreateBuffer(VulkanMemoryManager::ResourceType::Geometry,
                                                            sizeInBytes, 0 /* batcherHash */);
-      void * gpuPtr = m_objectManager->Map(m_geometryBuffers[i]);
-      memcpy(gpuPtr, m_mesh->m_buffers[i].m_data.data(), sizeInBytes);
-      m_objectManager->Flush(m_geometryBuffers[i]);
-      m_objectManager->Unmap(m_geometryBuffers[i]);
+      m_objectManager->Fill(m_geometryBuffers[i], m_mesh->m_buffers[i].m_data.data(), sizeInBytes);
 
       m_bindingInfo[i] = dp::BindingInfo(static_cast<uint8_t>(m_mesh->m_buffers[i].m_attributes.size()),
                                          static_cast<uint8_t>(i));
diff --git a/drape/vulkan/vulkan_object_manager.cpp b/drape/vulkan/vulkan_object_manager.cpp
index 18e7504059..dcd2fde8a0 100644
--- a/drape/vulkan/vulkan_object_manager.cpp
+++ b/drape/vulkan/vulkan_object_manager.cpp
@@ -99,7 +99,8 @@ VulkanObject VulkanObjectManager::CreateBuffer(VulkanMemoryManager::ResourceType
 
   result.m_allocation = m_memoryManager.Allocate(resourceType, memReqs, batcherHash);
 
-  CHECK_VK_CALL(vkBindBufferMemory(m_device, result.m_buffer, result.GetMemory(), result.GetAlignedOffset()));
+  CHECK_VK_CALL(vkBindBufferMemory(m_device, result.m_buffer, result.GetMemory(),
+                                   result.GetAlignedOffset()));
 
   return result;
 }
@@ -276,12 +277,11 @@ void VulkanObjectManager::CollectObjects()
   m_descriptorsToDestroy.clear();
 }
 
-uint8_t * VulkanObjectManager::Map(VulkanObject object)
+uint8_t * VulkanObjectManager::MapUnsafe(VulkanObject object)
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
   CHECK(!object.m_allocation->m_memoryBlock->m_isBlocked, ());
 
-  CHECK(object.m_buffer != 0 || object.m_image != 0, ());
+  CHECK(object.m_buffer != VK_NULL_HANDLE || object.m_image != VK_NULL_HANDLE, ());
   uint8_t * ptr = nullptr;
   CHECK_VK_CALL(vkMapMemory(m_device, object.GetMemory(), object.GetAlignedOffset(),
                             object.GetAlignedSize(), 0, reinterpret_cast<void **>(&ptr)));
@@ -289,10 +289,8 @@ uint8_t * VulkanObjectManager::Map(VulkanObject object)
   return ptr;
 }
 
-void VulkanObjectManager::Flush(VulkanObject object, uint32_t offset, uint32_t size)
+void VulkanObjectManager::FlushUnsafe(VulkanObject object, uint32_t offset, uint32_t size)
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
-
   if (object.m_allocation->m_memoryBlock->m_isCoherent)
     return;
 
@@ -309,28 +307,44 @@ void VulkanObjectManager::Flush(VulkanObject object, uint32_t offset, uint32_t s
   CHECK_VK_CALL(vkFlushMappedMemoryRanges(m_device, 1, &mappedRange));
 }
 
-void VulkanObjectManager::Unmap(VulkanObject object)
+void VulkanObjectManager::UnmapUnsafe(VulkanObject object)
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
   CHECK(object.m_allocation->m_memoryBlock->m_isBlocked, ());
   vkUnmapMemory(m_device, object.GetMemory());
   object.m_allocation->m_memoryBlock->m_isBlocked = false;
 }
 
+void VulkanObjectManager::Fill(VulkanObject object, void const * data, uint32_t sizeInBytes)
+{
+  if (data == nullptr)
+    return;
+
+  std::lock_guard<std::mutex> lock(m_mutex);
+  void * gpuPtr = MapUnsafe(object);
+  memcpy(gpuPtr, data, sizeInBytes);
+  FlushUnsafe(object);
+  UnmapUnsafe(object);
+}
+
 void VulkanObjectManager::CreateDescriptorPool()
 {
+  // Maximum uniform buffers descriptors count per frame.
+  uint32_t constexpr kMaxUniformBufferDescriptorsCount = 500;
+  // Maximum textures descriptors count per frame.
+  uint32_t constexpr kMaxImageSamplerDescriptorsCount = 1000;
+
   std::vector<VkDescriptorPoolSize> poolSizes =
-    {
-      {VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, 500},  // Maximum uniform buffers count per draw call.
-      {VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1000},  // Maximum textures count per draw call.
-    };
+  {
+    {VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, kMaxUniformBufferDescriptorsCount},
+    {VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, kMaxImageSamplerDescriptorsCount},
+  };
 
   VkDescriptorPoolCreateInfo descriptorPoolInfo = {};
   descriptorPoolInfo.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO;
   descriptorPoolInfo.flags = VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT;
   descriptorPoolInfo.poolSizeCount = static_cast<uint32_t>(poolSizes.size());
   descriptorPoolInfo.pPoolSizes = poolSizes.data();
-  descriptorPoolInfo.maxSets = 1500;  // Approximately equal to doubled maximum VAO per frame.
+  descriptorPoolInfo.maxSets = kMaxUniformBufferDescriptorsCount + kMaxImageSamplerDescriptorsCount;
 
   VkDescriptorPool descriptorPool;
   CHECK_VK_CALL(vkCreateDescriptorPool(m_device, &descriptorPoolInfo, nullptr, &descriptorPool));
diff --git a/drape/vulkan/vulkan_object_manager.hpp b/drape/vulkan/vulkan_object_manager.hpp
index e9e41aacc9..f50022f2a3 100644
--- a/drape/vulkan/vulkan_object_manager.hpp
+++ b/drape/vulkan/vulkan_object_manager.hpp
@@ -64,9 +64,12 @@ public:
   DescriptorSetGroup CreateDescriptorSetGroup(ref_ptr<VulkanGpuProgram> program,
                                               std::vector<ParamDescriptor> const & descriptors);
 
-  uint8_t * Map(VulkanObject object);
-  void Flush(VulkanObject object, uint32_t offset = 0, uint32_t size = 0);
-  void Unmap(VulkanObject object);
+  // Use unsafe function ONLY if an object exists on the only thread, otherwise
+  // use safe Fill function.
+  uint8_t * MapUnsafe(VulkanObject object);
+  void FlushUnsafe(VulkanObject object, uint32_t offset = 0, uint32_t size = 0);
+  void UnmapUnsafe(VulkanObject object);
+  void Fill(VulkanObject object, void const * data, uint32_t sizeInBytes);
 
   ref_ptr<VulkanStagingBuffer> GetDefaultStagingBuffer() const;
   void FlushDefaultStagingBuffer();
diff --git a/drape/vulkan/vulkan_staging_buffer.cpp b/drape/vulkan/vulkan_staging_buffer.cpp
index 6408fdfb2e..fe563fbb2b 100644
--- a/drape/vulkan/vulkan_staging_buffer.cpp
+++ b/drape/vulkan/vulkan_staging_buffer.cpp
@@ -24,17 +24,18 @@ VulkanStagingBuffer::VulkanStagingBuffer(ref_ptr<VulkanObjectManager> objectMana
   vkGetBufferMemoryRequirements(device, m_object.m_buffer, &memReqs);
   m_sizeAlignment = mm.GetSizeAlignment(memReqs);
   m_offsetAlignment = mm.GetOffsetAlignment(kStagingBuffer);
-  m_pointer = m_objectManager->Map(m_object);
+  m_pointer = m_objectManager->MapUnsafe(m_object);
 }
 
 VulkanStagingBuffer::~VulkanStagingBuffer()
 {
-  m_objectManager->Unmap(m_object);
+  m_objectManager->UnmapUnsafe(m_object);
   m_objectManager->DestroyObject(m_object);
 }
 
 bool VulkanStagingBuffer::HasEnoughSpace(uint32_t sizeInBytes) const
 {
+  CHECK_THREAD_CHECKER(m_threadChecker, ());
   auto const & mm = m_objectManager->GetMemoryManager();
   auto const alignedSize = mm.GetAligned(sizeInBytes, m_sizeAlignment);
   return m_offset + alignedSize <= m_sizeInBytes;
@@ -42,6 +43,7 @@ bool VulkanStagingBuffer::HasEnoughSpace(uint32_t sizeInBytes) const
 
 VulkanStagingBuffer::StagingData VulkanStagingBuffer::Reserve(uint32_t sizeInBytes)
 {
+  CHECK_THREAD_CHECKER(m_threadChecker, ());
   CHECK(m_offset % m_offsetAlignment == 0, ());
   CHECK(HasEnoughSpace(sizeInBytes), ());
   auto const & mm = m_objectManager->GetMemoryManager();
@@ -64,6 +66,7 @@ VulkanStagingBuffer::StagingData VulkanStagingBuffer::Reserve(uint32_t sizeInByt
 
 uint32_t VulkanStagingBuffer::ReserveWithId(uint32_t sizeInBytes, StagingData & data)
 {
+  CHECK_THREAD_CHECKER(m_threadChecker, ());
   data = Reserve(sizeInBytes);
   m_reservation.push_back(data);
   return static_cast<uint32_t>(m_reservation.size()) - 1;
@@ -71,21 +74,24 @@ uint32_t VulkanStagingBuffer::ReserveWithId(uint32_t sizeInBytes, StagingData &
 
 VulkanStagingBuffer::StagingData const & VulkanStagingBuffer::GetReservationById(uint32_t id) const
 {
+  CHECK_THREAD_CHECKER(m_threadChecker, ());
   CHECK_LESS(id, m_reservation.size(), ());
   return m_reservation[id];
 }
 
 void VulkanStagingBuffer::Flush()
 {
+  CHECK_THREAD_CHECKER(m_threadChecker, ());
   if (m_offset == 0)
     return;
 
   auto const size = m_offset;
-  m_objectManager->Flush(m_object, 0 /* offset */, size);
+  m_objectManager->FlushUnsafe(m_object, 0 /* offset */, size);
 }
 
 void VulkanStagingBuffer::Reset()
 {
+  CHECK_THREAD_CHECKER(m_threadChecker, ());
   m_offset = 0;
   m_reservation.clear();
 }
diff --git a/drape/vulkan/vulkan_staging_buffer.hpp b/drape/vulkan/vulkan_staging_buffer.hpp
index e45b5df8dc..70710714af 100644
--- a/drape/vulkan/vulkan_staging_buffer.hpp
+++ b/drape/vulkan/vulkan_staging_buffer.hpp
@@ -3,6 +3,8 @@
 #include "drape/pointers.hpp"
 #include "drape/vulkan/vulkan_object_manager.hpp"
 
+#include "base/thread_checker.hpp"
+
 #include <vulkan_wrapper.h>
 #include <vulkan/vulkan.h>
 
@@ -47,6 +49,7 @@ private:
   uint8_t * m_pointer = nullptr;
   uint32_t m_offset = 0;
   std::vector<StagingData> m_reservation;
+  ThreadChecker m_threadChecker;
 };
 }  // namespace vulkan
 }  // namespace dp
diff --git a/drape/vulkan/vulkan_texture.cpp b/drape/vulkan/vulkan_texture.cpp
index 30b754689d..29a8675c72 100644
--- a/drape/vulkan/vulkan_texture.cpp
+++ b/drape/vulkan/vulkan_texture.cpp
@@ -81,22 +81,6 @@ VkBufferImageCopy BufferCopyRegion(uint32_t x, uint32_t y, uint32_t width, uint3
 }
 }  // namespace
 
-VkFormat UnpackFormat(TextureFormat format)
-{
-  switch (format)
-  {
-    case TextureFormat::RGBA8: return VK_FORMAT_R8G8B8A8_UNORM;
-    case TextureFormat::Alpha: return VK_FORMAT_R8_UNORM;
-    case TextureFormat::RedGreen: return VK_FORMAT_R8G8_UNORM;
-    case TextureFormat::DepthStencil: return VK_FORMAT_D24_UNORM_S8_UINT;
-    case TextureFormat::Depth: return VK_FORMAT_D32_SFLOAT;
-    case TextureFormat::Unspecified:
-      CHECK(false, ());
-      return VK_FORMAT_UNDEFINED;
-  }
-  CHECK(false, ());
-}
-
 drape_ptr<HWTexture> VulkanTextureAllocator::CreateTexture(ref_ptr<dp::GraphicsContext> context)
 {
   return make_unique_dp<VulkanTexture>(make_ref<VulkanTextureAllocator>(this));
diff --git a/drape/vulkan/vulkan_texture.hpp b/drape/vulkan/vulkan_texture.hpp
index cd6949e66c..bf453f5a49 100644
--- a/drape/vulkan/vulkan_texture.hpp
+++ b/drape/vulkan/vulkan_texture.hpp
@@ -50,7 +50,5 @@ private:
   uint32_t m_reservationId = 0;
   bool m_isMutable = false;
 };
-
-VkFormat UnpackFormat(TextureFormat format);
 }  // namespace vulkan
 }  // namespace dp
diff --git a/drape/vulkan/vulkan_utils.cpp b/drape/vulkan/vulkan_utils.cpp
index 2e48da845e..ffff85acf3 100644
--- a/drape/vulkan/vulkan_utils.cpp
+++ b/drape/vulkan/vulkan_utils.cpp
@@ -55,6 +55,22 @@ std::string GetVulkanResultString(VkResult result)
   return "Unknown result";
 }
 
+VkFormat UnpackFormat(TextureFormat format)
+{
+  switch (format)
+  {
+  case TextureFormat::RGBA8: return VK_FORMAT_R8G8B8A8_UNORM;
+  case TextureFormat::Alpha: return VK_FORMAT_R8_UNORM;
+  case TextureFormat::RedGreen: return VK_FORMAT_R8G8_UNORM;
+  case TextureFormat::DepthStencil: return VK_FORMAT_D24_UNORM_S8_UINT;
+  case TextureFormat::Depth: return VK_FORMAT_D32_SFLOAT;
+  case TextureFormat::Unspecified:
+    CHECK(false, ());
+    return VK_FORMAT_UNDEFINED;
+  }
+  CHECK(false, ());
+}
+
 SamplerKey::SamplerKey(TextureFilter filter, TextureWrapping wrapSMode, TextureWrapping wrapTMode)
 {
   Set(filter, wrapSMode, wrapTMode);
diff --git a/drape/vulkan/vulkan_utils.hpp b/drape/vulkan/vulkan_utils.hpp
index 5a142a5969..bee3953593 100644
--- a/drape/vulkan/vulkan_utils.hpp
+++ b/drape/vulkan/vulkan_utils.hpp
@@ -15,6 +15,7 @@ namespace dp
 namespace vulkan
 {
 extern std::string GetVulkanResultString(VkResult result);
+extern VkFormat UnpackFormat(TextureFormat format);
 
 struct ParamDescriptor
 {