More validations for InheritanceInfo and ActiveQueries within renderpasses

Author: Erfan-Ahmadi

include/nbl/video/IGPUCommandBuffer.h

@@ -84,6 +84,9 @@ class IGPUCommandBuffer :
             return false;
         }
 
+        if (inheritanceInfo != nullptr)
+            m_cachedInheritanceInfo = *inheritanceInfo;
+
         return base_t::begin(_flags);
     }
 
@@ -166,6 +169,11 @@ class IGPUCommandBuffer :
         return true;
     }
 
+    SInheritanceInfo getCachedInheritanceInfo() const
+    {
+        return m_cachedInheritanceInfo;
+    }
+
 protected:
     friend class IGPUQueue;
 
@@ -175,7 +183,7 @@ class IGPUCommandBuffer :
     virtual ~IGPUCommandBuffer() = default;
 
     core::smart_refctd_ptr<IGPUCommandPool> m_cmdpool;
-    
+    SInheritanceInfo m_cachedInheritanceInfo;
 
     inline bool validate_updateBuffer(IGPUBuffer* dstBuffer, size_t dstOffset, size_t dataSize, const void* pData)
     {

src/nbl/video/COpenGLCommandBuffer.cpp

@@ -864,14 +864,16 @@ COpenGLCommandBuffer::~COpenGLCommandBuffer()
             {
                 auto& c = cmd.get<impl::ECT_BEGIN_RENDERPASS>();
                 auto framebuf = core::smart_refctd_ptr_static_cast<const COpenGLFramebuffer>(c.renderpassBegin.framebuffer);
-
+                
                 ctxlocal->nextState.framebuffer.hash = framebuf->getHashValue();
                 ctxlocal->nextState.framebuffer.fbo = std::move(framebuf);
                 ctxlocal->flushStateGraphics(gl, SOpenGLContextLocalCache::GSB_FRAMEBUFFER, ctxid);
 
                 GLuint fbo = ctxlocal->currentState.framebuffer.GLname;
                 if (fbo)
                     beginRenderpass_clearAttachments(gl, ctxlocal, ctxid, c.renderpassBegin, fbo, m_logger.getOptRawPtr());
+
+                currentlyRecordingRenderPass = c.renderpassBegin.renderpass.get();
             }
             break;
             case impl::ECT_NEXT_SUBPASS:
@@ -887,6 +889,7 @@ COpenGLCommandBuffer::~COpenGLCommandBuffer()
                 ctxlocal->nextState.framebuffer.hash = SOpenGLState::NULL_FBO_HASH;
                 ctxlocal->nextState.framebuffer.GLname = 0u;
                 ctxlocal->nextState.framebuffer.fbo = nullptr;
+                currentlyRecordingRenderPass = nullptr;
             }
             break;
             case impl::ECT_SET_DEVICE_MASK:
@@ -928,10 +931,18 @@ COpenGLCommandBuffer::~COpenGLCommandBuffer()
                 auto& c = cmd.get<impl::ECT_BEGIN_QUERY>();
                 const COpenGLQueryPool* qp = static_cast<const COpenGLQueryPool*>(c.queryPool.get());
                 auto currentQuery = core::bitflag(qp->getCreationParameters().queryType);
-                if(!queriesUsed.hasValue(currentQuery))
+                if(!queriesActive.hasValue(currentQuery))
                 {
                     qp->beginQuery(gl, ctxid, c.query, c.flags.value);
-                    queriesUsed |= currentQuery;
+                    queriesActive |= currentQuery;
+
+                    uint32_t queryTypeIndex = std::log2<uint32_t>(currentQuery.value);
+                    currentlyRecordingQueries[queryTypeIndex] = std::make_tuple(
+                        qp,
+                        c.query,
+                        currentlyRecordingRenderPass,
+                        0u
+                    );
                 }
                 else
                 {
@@ -941,12 +952,36 @@ COpenGLCommandBuffer::~COpenGLCommandBuffer()
             break;
             case impl::ECT_END_QUERY:
             {
-                // TODO: set last queue to use
                 auto& c = cmd.get<impl::ECT_END_QUERY>();
                 const COpenGLQueryPool* qp = static_cast<const COpenGLQueryPool*>(c.queryPool.get());
                 auto currentQuery = core::bitflag(qp->getCreationParameters().queryType);
-                qp->endQuery(gl, ctxid, c.query);
-                queriesUsed &= ~currentQuery;
+                if(queriesActive.hasValue(currentQuery))
+                {
+                    uint32_t queryTypeIndex = std::log2<uint32_t>(currentQuery.value);
+                    IQueryPool const * currentQueryPool = std::get<0>(currentlyRecordingQueries[queryTypeIndex]);
+                    uint32_t currentQueryIndex = std::get<1>(currentlyRecordingQueries[queryTypeIndex]);
+                    renderpass_t const * currentQueryRenderpass = std::get<2>(currentlyRecordingQueries[queryTypeIndex]);
+                    uint32_t currentQuerySubpassIndex = std::get<3>(currentlyRecordingQueries[queryTypeIndex]);
+
+                    if(currentQueryPool != c.queryPool.get() || currentQueryIndex != c.query)
+                    {
+                        assert(false); // You must end the same query you began for every query type.
+                        break;
+                    }
+                    if(currentQueryRenderpass != currentlyRecordingRenderPass)
+                    {
+                        assert(false); // Query either starts and ends in the same subpass or starts and ends entirely outside a renderpass
+                        break;
+                    }
+
+                    // currentlyRecordingQuery assert tuple -> same query index and query pool -> same renderpass
+                    qp->endQuery(gl, ctxid, c.query);
+                    queriesActive &= ~currentQuery;
+                }
+                else
+                {
+                    assert(false); // QueryType was not active to end.
+                }
             }
             break;
             case impl::ECT_COPY_QUERY_POOL_RESULTS:
@@ -1259,7 +1294,12 @@ COpenGLCommandBuffer::~COpenGLCommandBuffer()
             case impl::ECT_EXECUTE_COMMANDS:
             {
                 auto& c = cmd.get<impl::ECT_EXECUTE_COMMANDS>();
-
+                auto inheritanceInfo = c.cmdbuf->getCachedInheritanceInfo();
+                if(queriesActive.hasValue(IQueryPool::EQT_OCCLUSION))
+                {
+                    // For a secondary command buffer to be executed while a query is active, it must set the occlusionQueryEnable, queryFlags, and/or pipelineStatistics members of InheritanceInfo to conservative values
+                    assert(inheritanceInfo.occlusionQueryEnable);
+                }
                 static_cast<COpenGLCommandBuffer*>(c.cmdbuf.get())->executeAll(gl, ctxlocal, ctxid);
             }
             break;

src/nbl/video/COpenGLCommandBuffer.h

@@ -533,8 +533,9 @@ class COpenGLCommandBuffer final : public IGPUCommandBuffer
     }
     core::vector<SCommand> m_commands; // TODO: embed in the command pool via the use of linked list
     const COpenGLFeatureMap* m_features;
-    mutable core::bitflag<IQueryPool::E_QUERY_TYPE> queriesUsed;
-    mutable std::tuple<IQueryPool*,uint32_t/*query ix*/,renderpass_t*,uint32_t/*subpass ix*/> currentlyRecordingQuery[IQueryPool::EQT_COUNT];
+    mutable renderpass_t const * currentlyRecordingRenderPass = nullptr;
+    mutable core::bitflag<IQueryPool::E_QUERY_TYPE> queriesActive;
+    mutable std::tuple<IQueryPool const *,uint32_t/*query ix*/,renderpass_t const *,uint32_t/*subpass ix*/> currentlyRecordingQueries[IQueryPool::EQT_COUNT];
 
 public:
     void executeAll(IOpenGL_FunctionTable* gl, SOpenGLContextLocalCache* ctxlocal, uint32_t ctxid) const;
@@ -554,6 +555,7 @@ class COpenGLCommandBuffer final : public IGPUCommandBuffer
 
     inline bool end() override final
     {
+        assert(queriesActive.value == 0u); // No Queries should be active when command buffer ends
         return IGPUCommandBuffer::end();
     }
     bool reset(uint32_t _flags) override final;