Skip to content

Commit ceb270b

Browse files
Erfan-AhmadiErfan-Ahmadi
committed
OpenGLQueryPool Fixes and Validations for Active Queries within CommandBuffer
1 parent e97e017 commit ceb270b

File tree

6 files changed

+62
-20
lines changed

6 files changed

+62
-20
lines changed

‎include/nbl/video/IGPUCommandBuffer.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ class IGPUCommandBuffer :
100100
uint32_t getQueueFamilyIndex() const { return m_cmdpool->getQueueFamilyIndex(); }
101101

102102
IGPUCommandPool* getPool() const { return m_cmdpool.get(); }
103-
104103

105104
bool regenerateMipmaps(IGPUImage* img, uint32_t lastReadyMip, asset::IImage::E_ASPECT_FLAGS aspect) override
106105
{

‎include/nbl/video/IQueryPool.h

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,18 @@ class IQueryPool : public core::IReferenceCounted, public IBackendObject
1111
{
1212

1313
public:
14-
enum E_QUERY_TYPE
14+
enum E_QUERY_TYPE : uint32_t
1515
{
16-
EQT_OCCLUSION = 0,
17-
EQT_PIPELINE_STATISTICS = 1,
18-
EQT_TIMESTAMP = 2,
19-
EQT_PERFORMANCE_QUERY = 1000116000, // VK_KHR_performance_query // TODO: We don't support this fully yet -> needs Acquire/ReleaseProfilingLock + Counters Information report from physical device
20-
EQT_ACCELERATION_STRUCTURE_COMPACTED_SIZE = 1000150000, // VK_KHR_acceleration_structure
21-
EQT_ACCELERATION_STRUCTURE_SERIALIZATION_SIZE = 1000150001, // VK_KHR_acceleration_structure
16+
EQT_OCCLUSION = 0x00000001,
17+
EQT_PIPELINE_STATISTICS = 0x00000002,
18+
EQT_TIMESTAMP = 0x00000004,
19+
EQT_PERFORMANCE_QUERY = 0x00000008, // VK_KHR_performance_query // TODO: We don't support this fully yet -> needs Acquire/ReleaseProfilingLock + Counters Information report from physical device
20+
EQT_ACCELERATION_STRUCTURE_COMPACTED_SIZE = 0x00000010, // VK_KHR_acceleration_structure
21+
EQT_ACCELERATION_STRUCTURE_SERIALIZATION_SIZE = 0x00000020, // VK_KHR_acceleration_structure
22+
EQT_COUNT = 6u,
2223
};
2324

24-
enum E_PIPELINE_STATISTICS_FLAGS
25+
enum E_PIPELINE_STATISTICS_FLAGS : uint32_t
2526
{
2627
EPSF_NONE = 0,
2728
EPSF_INPUT_ASSEMBLY_VERTICES_BIT = 0x00000001,
@@ -37,7 +38,7 @@ class IQueryPool : public core::IReferenceCounted, public IBackendObject
3738
EPSF_COMPUTE_SHADER_INVOCATIONS_BIT = 0x00000400,
3839
};
3940

40-
enum E_QUERY_RESULTS_FLAGS
41+
enum E_QUERY_RESULTS_FLAGS : uint32_t
4142
{
4243
EQRF_NONE = 0,
4344
EQRF_64_BIT = 0x00000001,

‎src/nbl/video/COpenGLCommandBuffer.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -927,15 +927,26 @@ COpenGLCommandBuffer::~COpenGLCommandBuffer()
927927
{
928928
auto& c = cmd.get<impl::ECT_BEGIN_QUERY>();
929929
const COpenGLQueryPool* qp = static_cast<const COpenGLQueryPool*>(c.queryPool.get());
930-
qp->beginQuery(gl, ctxid, c.query, c.flags.value);
930+
auto currentQuery = core::bitflag(qp->getCreationParameters().queryType);
931+
if(!queriesUsed.hasValue(currentQuery))
932+
{
933+
qp->beginQuery(gl, ctxid, c.query, c.flags.value);
934+
queriesUsed |= currentQuery;
935+
}
936+
else
937+
{
938+
assert(false); // There is an active query with the same query type.
939+
}
931940
}
932941
break;
933942
case impl::ECT_END_QUERY:
934943
{
935944
// TODO: set last queue to use
936945
auto& c = cmd.get<impl::ECT_END_QUERY>();
937946
const COpenGLQueryPool* qp = static_cast<const COpenGLQueryPool*>(c.queryPool.get());
947+
auto currentQuery = core::bitflag(qp->getCreationParameters().queryType);
938948
qp->endQuery(gl, ctxid, c.query);
949+
queriesUsed &= ~currentQuery;
939950
}
940951
break;
941952
case impl::ECT_COPY_QUERY_POOL_RESULTS:
@@ -1024,6 +1035,9 @@ COpenGLCommandBuffer::~COpenGLCommandBuffer()
10241035
const uint32_t lastQueueToUse = qp->getLastQueueToUseForQuery(queryIdx);
10251036
GLuint query = qp->getQueryAt(lastQueueToUse, queryIdx);
10261037

1038+
if(query == GL_NONE)
1039+
continue;
1040+
10271041
GLenum pname;
10281042
if(waitForAllResults)
10291043
{

‎src/nbl/video/COpenGLCommandBuffer.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,19 +533,29 @@ class COpenGLCommandBuffer final : public IGPUCommandBuffer
533533
}
534534
core::vector<SCommand> m_commands; // TODO: embed in the command pool via the use of linked list
535535
const COpenGLFeatureMap* m_features;
536+
mutable core::bitflag<IQueryPool::E_QUERY_TYPE> queriesUsed;
537+
mutable std::tuple<IQueryPool*,uint32_t/*query ix*/,renderpass_t*,uint32_t/*subpass ix*/> currentlyRecordingQuery[IQueryPool::EQT_COUNT];
536538

537539
public:
538540
void executeAll(IOpenGL_FunctionTable* gl, SOpenGLContextLocalCache* ctxlocal, uint32_t ctxid) const;
539541

540-
541542
COpenGLCommandBuffer(core::smart_refctd_ptr<const ILogicalDevice>&& dev, E_LEVEL lvl, core::smart_refctd_ptr<IGPUCommandPool>&& _cmdpool, system::logger_opt_smart_ptr&& logger, const COpenGLFeatureMap* _features);
542543

543544
inline bool begin(uint32_t _flags) override final
544545
{
545546
reset(_flags);
546547
return IGPUCommandBuffer::begin(_flags);
547548
}
549+
550+
inline bool begin(uint32_t _flags, const SInheritanceInfo* inheritanceInfo = nullptr) override final
551+
{
552+
return IGPUCommandBuffer::begin(_flags, inheritanceInfo);
553+
}
548554

555+
inline bool end() override final
556+
{
557+
return IGPUCommandBuffer::end();
558+
}
549559
bool reset(uint32_t _flags) override final;
550560

551561

‎src/nbl/video/COpenGLQueryPool.h

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,16 @@ class COpenGLQueryPool final : public IQueryPool
4747

4848
inline core::SRange<const GLuint> getQueries(uint32_t queueIdx) const
4949
{
50-
assert(queueIdx < IOpenGLPhysicalDeviceBase::MaxQueues);
51-
auto queryArray = m_queries[queueIdx].get();
52-
if(queryArray)
53-
return core::SRange<const GLuint>(queryArray->begin(), queryArray->begin() + queryArray->size());
50+
auto ret = core::SRange<const GLuint>(nullptr, nullptr);
51+
if(queueIdx < IOpenGLPhysicalDeviceBase::MaxQueues)
52+
{
53+
auto queryArray = m_queries[queueIdx].get();
54+
if(queryArray)
55+
ret = core::SRange<const GLuint>(queryArray->begin(), queryArray->begin() + queryArray->size());
56+
}
5457
else
55-
return core::SRange<const GLuint>(nullptr, nullptr);
58+
assert(false);
59+
return ret;
5660
}
5761

5862
inline GLuint getQueryAt(uint32_t queueIdx, uint32_t query) const
@@ -65,7 +69,7 @@ class COpenGLQueryPool final : public IQueryPool
6569
else
6670
{
6771
assert(false);
68-
return 0u;
72+
return GL_NONE;
6973
}
7074
}
7175

@@ -153,8 +157,19 @@ class COpenGLQueryPool final : public IQueryPool
153157
inline bool resetQueries(IOpenGL_FunctionTable* gl, uint32_t ctxid, uint32_t query, uint32_t queryCount)
154158
{
155159
// NOTE: There is no Reset Queries on OpenGL
156-
// NOOP
157-
return true;
160+
// NOOP, ONLY set last queue used.
161+
// TODO: validations about reseting queries and unavailable/available state
162+
if(query + queryCount <= params.queryCount)
163+
{
164+
for(uint32_t i = 0; i < queryCount; ++i)
165+
lastQueueToUseArray[query + i] = ctxid;
166+
return true;
167+
}
168+
else
169+
{
170+
assert(false);
171+
return false;
172+
}
158173
}
159174

160175
};

‎src/nbl/video/COpenGL_LogicalDevice.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,9 @@ class COpenGL_LogicalDevice : public IOpenGL_LogicalDevice
531531
const uint32_t lastQueueToUse = qp->getLastQueueToUseForQuery(queryIdx);
532532
GLuint query = qp->getQueryAt(lastQueueToUse, queryIdx);
533533

534+
if(query == GL_NONE)
535+
continue;
536+
534537
GLenum pname;
535538

536539
if(waitForAllResults)

0 commit comments

Comments
 (0)