Apply reviews & merge swapchain_maintenance2

deprilula28 · deprilula28 · commit 102367fe4404 · 2022-08-01T08:26:24.000-03:00
diff --git a/include/nbl/video/COpenGLImage.h b/include/nbl/video/COpenGLImage.h
@@ -19,8 +19,6 @@ class IOpenGL_FunctionTable;
 
 class COpenGLImage final : public IGPUImage, public IOpenGLMemoryAllocation
 {
-	friend COpenGLSwapchain;
-	friend COpenGLESSwapchain;
 	protected:
 		virtual ~COpenGLImage();
 
diff --git a/include/nbl/video/ISwapchain.h b/include/nbl/video/ISwapchain.h
@@ -109,10 +109,10 @@ class NBL_API ISwapchain : public core::IReferenceCounted, public IBackendObject
         IGPUImage::SCreationParams m_imgCreationParams;
 
         friend class CCleanupSwapchainReference;
-        void freeImageExists(uint32_t ix) { m_imageExists.fetch_and(~(1U << ix)); }
+        void freeImageExists(uint32_t ix) { std::cout << "freeImageExists(" << ix << ")\n;"; m_imageExists.fetch_and(~(1U << ix)); }
 
         // Returns false if the image already existed
-        bool setImageExists(uint32_t ix) { return (m_imageExists.fetch_or(1U << ix) & (1U << ix)) == 0; }
+        bool setImageExists(uint32_t ix) { std::cout << "setImageExists(" << ix << ")\n;"; return (m_imageExists.fetch_or(1U << ix) & (1U << ix)) == 0; }
 
     public:
         static inline constexpr uint32_t MaxImages = sizeof(m_imageExists) * 8u;
diff --git a/src/nbl/video/COpenGLBuffer.cpp b/src/nbl/video/COpenGLBuffer.cpp
@@ -10,7 +10,7 @@ namespace nbl::video
 COpenGLBuffer::~COpenGLBuffer()
 {
     preDestroyStep();
-    if (!m_cachedCreationParams.importedHandle)
+    if (!m_cachedCreationParams.skipHandleDestroy)
     {
         auto* device = static_cast<IOpenGL_LogicalDevice*>(const_cast<ILogicalDevice*>(getOriginDevice()));
         device->destroyBuffer(BufferName);
diff --git a/src/nbl/video/COpenGLImage.cpp b/src/nbl/video/COpenGLImage.cpp
@@ -61,7 +61,7 @@ COpenGLImage::~COpenGLImage()
         fbohash = COpenGLFramebuffer::getHashColorImage(this);
     device->destroyFramebuffer(fbohash);
     // destroy only if not observing (we own)
-    if (!m_cachedCreationParams.importedHandle)
+    if (!m_cachedCreationParams.skipHandleDestroy)
         device->destroyTexture(name);
 }
 
diff --git a/src/nbl/video/COpenGL_Queue.h b/src/nbl/video/COpenGL_Queue.h
@@ -230,7 +230,7 @@ class COpenGL_Queue final : public IGPUQueue
                     }
                     
                     // need to possibly wait for master context (object creation, and buffer mapping and flushing)
-                    m_masterContextCallsWaited = m_device->waitOnMasterContext(gl,m_masterContextCallsWaited);
+                    m_masterContextCallsWaited = m_device->waitOnMasterContext(&gl,m_masterContextCallsWaited);
 
                     if (barrierBits)
                         gl.glSync.pglMemoryBarrier(barrierBits);
diff --git a/src/nbl/video/COpenGL_Swapchain.cpp b/src/nbl/video/COpenGL_Swapchain.cpp
@@ -36,7 +36,6 @@ class COpenGL_SwapchainThreadHandler final : public system::IThreadHandler<COpen
 
     const egl::CEGL* egl;
     ISurface::E_PRESENT_MODE m_presentMode;
-    const COpenGLFeatureMap* features;
     uint32_t imageCount;
     uint32_t imgWidth, imgHeight;
     core::smart_refctd_ptr<COpenGLSync> syncs[ISwapchain::MaxImages];
@@ -46,7 +45,7 @@ class COpenGL_SwapchainThreadHandler final : public system::IThreadHandler<COpen
     asset::E_FORMAT m_ourFormat;
     GLenum m_glFormat;
 
-    bool needToBlit = false;
+    std::atomic_bool needToBlit = false;
 
     EGLBoolean m_makeCurrentRes = EGL_FALSE;
     //std::condition_variable m_ctxCreatedCvar;
@@ -55,15 +54,13 @@ class COpenGL_SwapchainThreadHandler final : public system::IThreadHandler<COpen
         IOpenGL_LogicalDevice* dev,
         ISurface::E_PRESENT_MODE presentMode,
         uint32_t _imgCount, uint32_t _imgWidth, uint32_t _imgHeight,
-        const COpenGLFeatureMap* _features,
         egl::CEGL::Context _glctx,
         COpenGLDebugCallback* _dbgCb,
         asset::E_FORMAT format
     ) : m_device(dev), m_masterContextCallsWaited(0),
         egl(_egl),
         m_presentMode(presentMode),
         glctx(_glctx),
-        features(_features),
         imageCount(_imgCount),
         imgWidth(_imgWidth),
         imgHeight(_imgHeight),
@@ -75,7 +72,7 @@ class COpenGL_SwapchainThreadHandler final : public system::IThreadHandler<COpen
     {
         auto raii_handler = base_t::createRAIIDispatchHandler();
 
-        needToBlit = true;
+        needToBlit.store(true);
         request.imgIx = _imgIx;
         request.semCount = semCount;
         request.sems = core::make_refctd_dynamic_array<core::smart_refctd_dynamic_array<core::smart_refctd_ptr<COpenGLSemaphore>>>(semCount);
@@ -141,9 +138,9 @@ class COpenGL_SwapchainThreadHandler final : public system::IThreadHandler<COpen
         }
 
         if (m_device->getAPIType() == video::EAT_OPENGL)
-            new (state_ptr) COpenGLFunctionTable(egl, features, core::smart_refctd_ptr<system::ILogger>(m_dbgCb->getLogger()));
+            new (state_ptr) COpenGLFunctionTable(egl, m_device->getGlFeatures(), core::smart_refctd_ptr<system::ILogger>(m_dbgCb->getLogger()));
         else if (m_device->getAPIType() == video::EAT_OPENGL_ES)
-            new (state_ptr) COpenGLESFunctionTable(egl, features, core::smart_refctd_ptr<system::ILogger>(m_dbgCb->getLogger()));
+            new (state_ptr) COpenGLESFunctionTable(egl, m_device->getGlFeatures(), core::smart_refctd_ptr<system::ILogger>(m_dbgCb->getLogger()));
         else assert(false);
         auto gl = getFunctionTable(m_device->getAPIType(), state_ptr);
 
@@ -195,7 +192,7 @@ class COpenGL_SwapchainThreadHandler final : public system::IThreadHandler<COpen
     void work(typename base_t::lock_t& lock, typename base_t::internal_state_t& state)
     {
         auto gl = getFunctionTable(m_device->getAPIType(), &state);
-        needToBlit = false;
+        needToBlit.store(false);
 
         const uint32_t imgix = request.imgIx;
         const uint32_t w = imgWidth;
@@ -226,8 +223,8 @@ class COpenGL_SwapchainThreadHandler final : public system::IThreadHandler<COpen
         egl->call.peglDestroySurface(egl->display, glctx.surface);
     }
 
-    bool wakeupPredicate() const { return needToBlit; }
-    bool continuePredicate() const { return needToBlit; }
+    bool wakeupPredicate() const { return needToBlit.load(); }
+    bool continuePredicate() const { return needToBlit.load(); }
 };
 
 template <typename FunctionTableType>
@@ -236,7 +233,8 @@ const void* COpenGL_Swapchain<FunctionTableType>::getNativeHandle() const { retu
 template <typename FunctionTableType>
 core::smart_refctd_ptr<COpenGL_Swapchain<FunctionTableType>> COpenGL_Swapchain<FunctionTableType>::create(const core::smart_refctd_ptr<ILogicalDevice>&& logicalDevice, ISwapchain::SCreationParams&& params)
 {
-    if (params.surface->getAPIType() != EAT_OPENGL || (params.presentMode == ISurface::EPM_MAILBOX) || (params.presentMode == ISurface::EPM_UNKNOWN))
+    // surfaces are only supported by queue family index 0 on opengl
+    if (!params.surface->isSupportedForPhysicalDevice(logicalDevice->getPhysicalDevice(), 0u) || params.surface->getAPIType() != EAT_OPENGL || (params.presentMode == ISurface::EPM_MAILBOX) || (params.presentMode == ISurface::EPM_UNKNOWN))
         return nullptr;
 
     auto device = core::smart_refctd_ptr_static_cast<IOpenGL_LogicalDevice>(logicalDevice);
@@ -281,7 +279,7 @@ core::smart_refctd_ptr<COpenGL_Swapchain<FunctionTableType>> COpenGL_Swapchain<F
         device.get(), 
         params.presentMode, 
         params.minImageCount, params.width, params.height,
-        device->getGlFeatures(), { ctx, surface }, 
+        { ctx, surface }, 
         static_cast<COpenGLDebugCallback*>(device->getPhysicalDevice()->getDebugCallback()), 
         params.surfaceFormat.format
     ));
@@ -378,7 +376,7 @@ core::smart_refctd_ptr<IGPUImage> COpenGL_Swapchain<FunctionTableType>::createIm
     swapchainRef->m_imageIndex = imageIndex;
 
     imgCreationParams.preDestroyCleanup = std::unique_ptr<ICleanup>(swapchainRef.release());
-    imgCreationParams.importedHandle = true;
+    imgCreationParams.skipHandleDestroy = true;
 
     return m_threadHandler->createImageAsImported(imageIndex, std::move(imgCreationParams));
 }
diff --git a/src/nbl/video/CVulkanBuffer.cpp b/src/nbl/video/CVulkanBuffer.cpp
@@ -8,7 +8,7 @@ namespace nbl::video
 CVulkanBuffer::~CVulkanBuffer()
 {
 	preDestroyStep();
-	if (m_cachedCreationParams.importedHandle)
+	if (m_cachedCreationParams.skipHandleDestroy)
 	{
 		const CVulkanLogicalDevice* vulkanDevice = static_cast<const CVulkanLogicalDevice*>(getOriginDevice());
 		auto* vk = vulkanDevice->getFunctionTable();
diff --git a/src/nbl/video/CVulkanImage.cpp b/src/nbl/video/CVulkanImage.cpp
@@ -10,7 +10,7 @@ CVulkanImage::~CVulkanImage()
 {
     preDestroyStep();
     // don't destroy imported handles
-    if (!m_cachedCreationParams.importedHandle)
+    if (!m_cachedCreationParams.skipHandleDestroy)
     {
         if (m_vkImage != VK_NULL_HANDLE)
         {
diff --git a/src/nbl/video/IOpenGL_LogicalDevice.h b/src/nbl/video/IOpenGL_LogicalDevice.h
@@ -997,19 +997,6 @@ class IOpenGL_LogicalDevice : public ILogicalDevice, protected impl::IOpenGL_Log
     IOpenGL_LogicalDevice(core::smart_refctd_ptr<IAPIConnection>&& api, IPhysicalDevice* physicalDevice, const SCreationParams& params, const egl::CEGL* _egl)
         : ILogicalDevice(std::move(api),physicalDevice,params), m_masterContextCallsInvoked(0u), m_masterContextCallsReturned(0u), m_masterContextSync(nullptr), m_egl(_egl) {}
 
-    template <typename FunctionTableType>
-    inline uint64_t waitOnMasterContext(FunctionTableType& _gl, const uint64_t waitedCallsSoFar)
-    {
-        const uint64_t invokedSoFar = m_masterContextCallsInvoked.load();
-        assert(invokedSoFar >= waitedCallsSoFar); // something went very wrong with causality
-        if (invokedSoFar == waitedCallsSoFar)
-            return waitedCallsSoFar;
-        uint64_t returnedSoFar;
-        while ((returnedSoFar = m_masterContextCallsReturned.load()) < invokedSoFar) {} // waiting on address deadlocks
-        _gl.glSync.pglWaitSync(m_masterContextSync.load(), 0, GL_TIMEOUT_IGNORED);
-        return returnedSoFar;
-    }
-
     template <typename FunctionTableType>
     inline uint64_t waitOnMasterContext(FunctionTableType* _gl, const uint64_t waitedCallsSoFar)
     {

Original file line number	Diff line number	Diff line change
`@@ -19,8 +19,6 @@ class IOpenGL_FunctionTable;`
`19`	`19`
`20`	`20`	`class COpenGLImage final : public IGPUImage, public IOpenGLMemoryAllocation`
`21`	`21`	`{`
`22`		`- friend COpenGLSwapchain;`
`23`		`- friend COpenGLESSwapchain;`
`24`	`22`	`protected:`
`25`	`23`	`virtual ~COpenGLImage();`
`26`	`24`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ namespace nbl::video`
`10`	`10`	`COpenGLBuffer::~COpenGLBuffer()`
`11`	`11`	`{`
`12`	`12`	`preDestroyStep();`
`13`		`- if (!m_cachedCreationParams.importedHandle)`
	`13`	`+ if (!m_cachedCreationParams.skipHandleDestroy)`
`14`	`14`	`{`
`15`	`15`	`auto* device = static_cast<IOpenGL_LogicalDevice>(const_cast<ILogicalDevice>(getOriginDevice()));`
`16`	`16`	`device->destroyBuffer(BufferName);`
Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ COpenGLImage::~COpenGLImage()`
`61`	`61`	`fbohash = COpenGLFramebuffer::getHashColorImage(this);`
`62`	`62`	`device->destroyFramebuffer(fbohash);`
`63`	`63`	`// destroy only if not observing (we own)`
`64`		`- if (!m_cachedCreationParams.importedHandle)`
	`64`	`+ if (!m_cachedCreationParams.skipHandleDestroy)`
`65`	`65`	`device->destroyTexture(name);`
`66`	`66`	`}`
`67`	`67`
Original file line number	Diff line number	Diff line change
`@@ -230,7 +230,7 @@ class COpenGL_Queue final : public IGPUQueue`
`230`	`230`	`}`
`231`	`231`
`232`	`232`	`// need to possibly wait for master context (object creation, and buffer mapping and flushing)`
`233`		`- m_masterContextCallsWaited = m_device->waitOnMasterContext(gl,m_masterContextCallsWaited);`
	`233`	`+ m_masterContextCallsWaited = m_device->waitOnMasterContext(&gl,m_masterContextCallsWaited);`
`234`	`234`
`235`	`235`	`if (barrierBits)`
`236`	`236`	`gl.glSync.pglMemoryBarrier(barrierBits);`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ namespace nbl::video`
`8`	`8`	`CVulkanBuffer::~CVulkanBuffer()`
`9`	`9`	`{`
`10`	`10`	`preDestroyStep();`
`11`		`- if (m_cachedCreationParams.importedHandle)`
	`11`	`+ if (m_cachedCreationParams.skipHandleDestroy)`
`12`	`12`	`{`
`13`	`13`	`const CVulkanLogicalDevice* vulkanDevice = static_cast<const CVulkanLogicalDevice*>(getOriginDevice());`
`14`	`14`	`auto* vk = vulkanDevice->getFunctionTable();`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ CVulkanImage::~CVulkanImage()`
`10`	`10`	`{`
`11`	`11`	`preDestroyStep();`
`12`	`12`	`// don't destroy imported handles`
`13`		`- if (!m_cachedCreationParams.importedHandle)`
	`13`	`+ if (!m_cachedCreationParams.skipHandleDestroy)`
`14`	`14`	`{`
`15`	`15`	`if (m_vkImage != VK_NULL_HANDLE)`
`16`	`16`	`{`