Skip to content

Commit 18949b1

Browse files
committed
remove SIntendedSubmitInfo from UI::render, we cannot overflow while recording non-dynamic renderpass - bring back command buffer + add semaphore wait info for deferred memory deallocation, clean more code, add comments regarding linear allocator TODO, update examples_tests submodule
1 parent ebb16fb commit 18949b1

File tree

3 files changed

+43
-44
lines changed

3 files changed

+43
-44
lines changed

examples_tests

include/nbl/ext/ImGui/ImGui.h

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ class UI final : public core::IReferenceCounted
2323
EBC_COUNT,
2424
};
2525

26-
//! composed buffer layout is [EBC_DRAW_INDIRECT_STRUCTURES] [EBC_ELEMENT_STRUCTURES] [EBC_INDEX_BUFFERS] [EBC_VERTEX_BUFFERS]
27-
nbl::core::smart_refctd_ptr<typename COMPOSE_T> streamingTDBufferST;
26+
nbl::core::smart_refctd_ptr<typename COMPOSE_T> streamingTDBufferST; //! composed buffer layout is [EBC_DRAW_INDIRECT_STRUCTURES] [EBC_ELEMENT_STRUCTURES] [EBC_INDEX_BUFFERS] [EBC_VERTEX_BUFFERS]
27+
28+
static constexpr auto MDI_BUFFER_REQUIRED_ALLOCATE_FLAGS = nbl::core::bitflag<nbl::video::IDeviceMemoryAllocation::E_MEMORY_ALLOCATE_FLAGS>(nbl::video::IDeviceMemoryAllocation::EMAF_DEVICE_ADDRESS_BIT); //! required flags
29+
static constexpr auto MDI_BUFFER_REQUIRED_USAGE_FLAGS = nbl::core::bitflag(nbl::asset::IBuffer::EUF_INDIRECT_BUFFER_BIT) | nbl::asset::IBuffer::EUF_INDEX_BUFFER_BIT | nbl::asset::IBuffer::EUF_VERTEX_BUFFER_BIT | nbl::asset::IBuffer::EUF_SHADER_DEVICE_ADDRESS_BIT; //! required flags
2830
};
2931

3032
struct S_CREATION_PARAMETERS
@@ -59,7 +61,7 @@ class UI final : public core::IReferenceCounted
5961
//! what we pass to ImGuiIO::AddMousePosEvent
6062
nbl::hlsl::float32_t2 mousePosition,
6163

62-
//! main display size in pixels (generally == GetMainViewport()->Size)
64+
//! main display size in pixels
6365
displaySize;
6466

6567
//! Nabla events you want to be handled with the backend
@@ -75,14 +77,14 @@ class UI final : public core::IReferenceCounted
7577
UI(S_CREATION_PARAMETERS&& params);
7678
~UI() override;
7779

78-
//! Nabla ImGUI backend reserves this index for font atlas, any attempt to hook user defined texture within the index will cause runtime error [TODO: could have a setter & getter to control the default & currently hooked font texture ID and init 0u by default]
79-
_NBL_STATIC_INLINE_CONSTEXPR auto NBL_FONT_ATLAS_TEX_ID = 0u;
80+
//! Nabla ImGUI backend reserves this index for font atlas, any attempt to hook user defined texture within the index will result in undefined behaviour
81+
static constexpr auto NBL_FONT_ATLAS_TEX_ID = 0u;
8082

81-
//! update ImGuiIO & record ImGUI *cpu* draw command lists, call it before .render
83+
//! updates ImGuiIO & records ImGUI *cpu* draw command lists, you have to call it before .render
8284
bool update(const S_UPDATE_PARAMETERS& params);
8385

84-
//! updates mapped mdi buffer & records *gpu* draw commands, handles overflows for mdi allocation failure cases (pop & submit)
85-
bool render(nbl::video::SIntendedSubmitInfo& info, const nbl::video::IGPUDescriptorSet* const descriptorSet, const std::span<const VkRect2D> scissors = {});
86+
//! updates mapped mdi buffer & records *gpu* draw commands
87+
bool render(nbl::video::IGPUCommandBuffer* commandBuffer, nbl::video::ISemaphore::SWaitInfo waitInfo, const nbl::video::IGPUDescriptorSet* const descriptorSet, const std::span<const VkRect2D> scissors = {});
8688

8789
//! registers lambda listener in which ImGUI calls should be recorded
8890
size_t registerListener(std::function<void()> const& listener);
@@ -91,16 +93,19 @@ class UI final : public core::IReferenceCounted
9193
//! sets ImGUI context, you are supposed to pass valid ImGuiContext* context
9294
void setContext(void* imguiContext);
9395

96+
//! creation parametrs
97+
inline const S_CREATION_PARAMETERS& getCreationParameters() const { return m_creationParams; }
98+
99+
//! ImGUI graphics pipeline
100+
inline nbl::video::IGPUGraphicsPipeline* getPipeline() { return pipeline.get(); }
101+
94102
//! image view default font texture
95103
inline nbl::video::IGPUImageView* getFontAtlasView() { return m_fontAtlasTexture.get(); }
96104

97105
//! mdi streaming buffer
98106
inline typename MDI::COMPOSE_T* getStreamingBuffer() { return m_mdi.streamingTDBufferST.get(); }
99107

100-
//! ImGUI graphics pipeline
101-
inline nbl::video::IGPUGraphicsPipeline* getPipeline() { return pipeline.get(); }
102-
103-
//! ImGUI context getter, you are supposed to cast it, eg. reinterpret_cast<ImGuiContext*>(this->getContext());
108+
//! ImGUI context, you are supposed to cast it, eg. reinterpret_cast<ImGuiContext*>(this->getContext());
104109
void* getContext();
105110
private:
106111
void createPipeline();

src/nbl/ext/ImGui/ImGui.cpp

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -804,8 +804,6 @@ namespace nbl::ext::imgui
804804
void UI::createMDIBuffer()
805805
{
806806
constexpr static uint32_t minStreamingBufferAllocationSize = 32u, maxStreamingBufferAllocationAlignment = 1024u * 64u, mdiBufferDefaultSize = /* 2MB */ 1024u * 1024u * 2u;
807-
constexpr static auto requiredAllocateFlags = core::bitflag<IDeviceMemoryAllocation::E_MEMORY_ALLOCATE_FLAGS>(IDeviceMemoryAllocation::EMAF_DEVICE_ADDRESS_BIT);
808-
constexpr static auto requiredUsageFlags = nbl::core::bitflag(nbl::asset::IBuffer::EUF_INDIRECT_BUFFER_BIT) | nbl::asset::IBuffer::EUF_INDEX_BUFFER_BIT | nbl::asset::IBuffer::EUF_VERTEX_BUFFER_BIT | nbl::asset::IBuffer::EUF_SHADER_DEVICE_ADDRESS_BIT;
809807

810808
auto getRequiredAccessFlags = [&](const core::bitflag<video::IDeviceMemoryAllocation::E_MEMORY_PROPERTY_FLAGS>& properties)
811809
{
@@ -824,15 +822,15 @@ namespace nbl::ext::imgui
824822
else
825823
{
826824
IGPUBuffer::SCreationParams mdiCreationParams = {};
827-
mdiCreationParams.usage = requiredUsageFlags;
825+
mdiCreationParams.usage = MDI::MDI_BUFFER_REQUIRED_USAGE_FLAGS;
828826
mdiCreationParams.size = mdiBufferDefaultSize;
829827

830828
auto buffer = m_creationParams.utilities->getLogicalDevice()->createBuffer(std::move(mdiCreationParams));
831829

832830
auto memoryReqs = buffer->getMemoryReqs();
833831
memoryReqs.memoryTypeBits &= m_creationParams.utilities->getLogicalDevice()->getPhysicalDevice()->getUpStreamingMemoryTypeBits();
834832

835-
auto allocation = m_creationParams.utilities->getLogicalDevice()->allocate(memoryReqs, buffer.get(), requiredAllocateFlags);
833+
auto allocation = m_creationParams.utilities->getLogicalDevice()->allocate(memoryReqs, buffer.get(), MDI::MDI_BUFFER_REQUIRED_ALLOCATE_FLAGS);
836834
{
837835
const bool allocated = allocation.isValid();
838836
assert(allocated);
@@ -851,9 +849,9 @@ namespace nbl::ext::imgui
851849

852850
const auto validation = std::to_array
853851
({
854-
std::make_pair(buffer->getCreationParams().usage.hasFlags(requiredUsageFlags), "MDI buffer must be created with IBuffer::EUF_INDIRECT_BUFFER_BIT | IBuffer::EUF_INDEX_BUFFER_BIT | IBuffer::EUF_VERTEX_BUFFER_BIT | IBuffer::EUF_SHADER_DEVICE_ADDRESS_BIT enabled!"),
852+
std::make_pair(buffer->getCreationParams().usage.hasFlags(MDI::MDI_BUFFER_REQUIRED_USAGE_FLAGS), "MDI buffer must be created with IBuffer::EUF_INDIRECT_BUFFER_BIT | IBuffer::EUF_INDEX_BUFFER_BIT | IBuffer::EUF_VERTEX_BUFFER_BIT | IBuffer::EUF_SHADER_DEVICE_ADDRESS_BIT enabled!"),
855853
std::make_pair(bool(buffer->getMemoryReqs().memoryTypeBits & m_creationParams.utilities->getLogicalDevice()->getPhysicalDevice()->getUpStreamingMemoryTypeBits()), "MDI buffer must have up-streaming memory type bits enabled!"),
856-
std::make_pair(binding.memory->getAllocateFlags().hasFlags(requiredAllocateFlags), "MDI buffer's memory must be allocated with IDeviceMemoryAllocation::EMAF_DEVICE_ADDRESS_BIT enabled!"),
854+
std::make_pair(binding.memory->getAllocateFlags().hasFlags(MDI::MDI_BUFFER_REQUIRED_ALLOCATE_FLAGS), "MDI buffer's memory must be allocated with IDeviceMemoryAllocation::EMAF_DEVICE_ADDRESS_BIT enabled!"),
857855
std::make_pair(binding.memory->isCurrentlyMapped(), "MDI buffer's memory must be mapped!"), // streaming buffer contructor already validates it, but cannot assume user won't unmap its own buffer for some reason (sorry if you have just hit it)
858856
std::make_pair(binding.memory->getCurrentMappingAccess().hasFlags(getRequiredAccessFlags(binding.memory->getMemoryPropertyFlags())), "MDI buffer's memory current mapping access flags don't meet requirements!")
859857
});
@@ -866,17 +864,21 @@ namespace nbl::ext::imgui
866864
}
867865
}
868866

869-
bool UI::render(SIntendedSubmitInfo& info, const IGPUDescriptorSet* const descriptorSet, const std::span<const VkRect2D> scissors)
867+
bool UI::render(nbl::video::IGPUCommandBuffer* commandBuffer, nbl::video::ISemaphore::SWaitInfo waitInfo, const IGPUDescriptorSet* const descriptorSet, const std::span<const VkRect2D> scissors)
870868
{
871-
if (!info.valid())
869+
if (!commandBuffer)
872870
{
873-
m_creationParams.utilities->getLogger()->log("Invalid SIntendedSubmitInfo!", system::ILogger::ELL_ERROR);
871+
m_creationParams.utilities->getLogger()->log("Invalid command buffer!", system::ILogger::ELL_ERROR);
874872
return false;
875873
}
876874

877-
ImGui::Render(); // note it doesn't touch GPU or graphics API at all, its an internal ImGUI call to update & prepare the data for rendering so we can call GetDrawData()
875+
if (commandBuffer->getState() != IGPUCommandBuffer::STATE::RECORDING)
876+
{
877+
m_creationParams.utilities->getLogger()->log("Command buffer is not in recording state!", system::ILogger::ELL_ERROR);
878+
return false;
879+
}
878880

879-
auto* commandBuffer = info.getScratchCommandBuffer();
881+
ImGui::Render(); // note it doesn't touch GPU or graphics API at all, its an internal ImGUI call to update & prepare the data for rendering so we can call GetDrawData()
880882

881883
ImGuiIO& io = ImGui::GetIO();
882884

@@ -983,9 +985,13 @@ namespace nbl::ext::imgui
983985
// calculate upper bound byte size limit for mdi buffer
984986
const auto mdiBufferByteSize = std::reduce(std::begin(multiAllocParams.byteSizes), std::end(multiAllocParams.byteSizes));
985987

988+
/*
989+
TODO: use linear allocator instead, we could try to fill our tightly packed mdi buffer in a loop if full big request is impossible with single allocation call
990+
*/
991+
986992
auto mdiBuffer = smart_refctd_ptr<IGPUBuffer>(m_mdi.streamingTDBufferST->getBuffer());
987993
{
988-
auto timeout(std::chrono::steady_clock::now() + std::chrono::milliseconds(1u));
994+
const auto timeout (std::chrono::steady_clock::now() + std::chrono::milliseconds(1u));
989995

990996
size_t unallocatedSize = m_mdi.streamingTDBufferST->multi_allocate(timeout, MDI_ALLOCATION_COUNT, multiAllocParams.offsets.data(), multiAllocParams.byteSizes.data(), MDI_ALIGNMENTS.data());
991997

@@ -994,11 +1000,9 @@ namespace nbl::ext::imgui
9941000
// retry, second attempt cull frees and execute deferred memory deallocation of offsets no longer in use
9951001
unallocatedSize = m_mdi.streamingTDBufferST->multi_allocate(timeout, MDI_ALLOCATION_COUNT, multiAllocParams.offsets.data(), multiAllocParams.byteSizes.data(), MDI_ALIGNMENTS.data());
9961002

997-
// still no? then we will try suballocating smaller pieces of memory due to possible fragmentation issues & upstream in nice loop (if we get to a point when suballocation could not cover whole 1 sub draw call then we need to submit overflow & rerecord stuff + repeat)
998-
// that's TODO but first a small test for not tightly packed index + vertex buffer
999-
10001003
if (unallocatedSize != 0u)
10011004
{
1005+
#ifdef NBL_IMPL_PRINT_DEBUG_ALLOC_RET_INFO
10021006
m_creationParams.utilities->getLogger()->log("Could not multi alloc mdi buffer!", system::ILogger::ELL_ERROR);
10031007

10041008
auto getOffsetStr = [&](const MDI::COMPOSE_T::value_type offset) -> std::string
@@ -1014,13 +1018,16 @@ namespace nbl::ext::imgui
10141018
m_creationParams.utilities->getLogger()->log("[MDI::EBC_ELEMENT_STRUCTURES offset] = \"%s\" bytes", system::ILogger::ELL_ERROR, getOffsetStr(multiAllocParams.offsets[MDI::EBC_ELEMENT_STRUCTURES]).c_str());
10151019
m_creationParams.utilities->getLogger()->log("[MDI::EBC_INDEX_BUFFERS offset] = \"%s\" bytes", system::ILogger::ELL_ERROR, getOffsetStr(multiAllocParams.offsets[MDI::EBC_INDEX_BUFFERS]).c_str());
10161020
m_creationParams.utilities->getLogger()->log("[MDI::EBC_VERTEX_BUFFERS offset] = \"%s\" bytes", system::ILogger::ELL_ERROR, getOffsetStr(multiAllocParams.offsets[MDI::EBC_VERTEX_BUFFERS]).c_str());
1021+
#endif
1022+
1023+
m_mdi.streamingTDBufferST->multi_deallocate(MDI_ALLOCATION_COUNT, multiAllocParams.offsets.data(), multiAllocParams.byteSizes.data()); // fail, get rid of all requests instantly
1024+
m_mdi.streamingTDBufferST->cull_frees();
10171025

1018-
exit(0x45);
1026+
return false;
10191027
}
10201028
}
10211029

1022-
auto waitInfo = info.getFutureScratchSemaphore();
1023-
m_mdi.streamingTDBufferST->multi_deallocate(MDI_ALLOCATION_COUNT, multiAllocParams.offsets.data(), multiAllocParams.byteSizes.data(), waitInfo);
1030+
m_mdi.streamingTDBufferST->multi_deallocate(MDI_ALLOCATION_COUNT, multiAllocParams.offsets.data(), multiAllocParams.byteSizes.data(), waitInfo); // allocated, latch offsets deallocation
10241031
}
10251032

10261033
const uint32_t drawCount = multiAllocParams.byteSizes[MDI::EBC_DRAW_INDIRECT_STRUCTURES] / sizeof(VkDrawIndexedIndirectCommand);
@@ -1060,11 +1067,6 @@ namespace nbl::ext::imgui
10601067
indirect->firstIndex = pcmd->IdxOffset + cmdListIndexOffset;
10611068
indirect->vertexOffset = pcmd->VtxOffset + cmdListVertexOffset;
10621069

1063-
// TEST: I think this could be illegal in vulkan explaining why I get weird flickering but valid scene still I see (the geometry seems to be OK),
1064-
// could try BDA to get vertex + index instead and this is valid for sure
1065-
// indirect->firstIndex = indexObjectGlobalOffset + pcmd->IdxOffset + cmdListIndexOffset;
1066-
// indirect->vertexOffset = vertexObjectGlobalOffset + pcmd->VtxOffset + cmdListVertexOffset;
1067-
10681070
const auto scissor = clip.getScissor(clipRectangle);
10691071

10701072
auto packSnorm16 = [](float ndc) -> int16_t
@@ -1091,10 +1093,6 @@ namespace nbl::ext::imgui
10911093
cmdListIndexOffset += cmd_list->IdxBuffer.Size;
10921094
cmdListVertexOffset += cmd_list->VtxBuffer.Size;
10931095
}
1094-
1095-
// TEST: flush
1096-
// const ILogicalDevice::MappedMemoryRange memoryRange(binding.memory, 0ull, binding.memory->getAllocationSize());
1097-
// m_creationParams.utilities->getLogicalDevice()->flushMappedMemoryRanges(1u, &memoryRange);
10981096
}
10991097

11001098
auto* rawPipeline = pipeline.get();
@@ -1106,8 +1104,6 @@ namespace nbl::ext::imgui
11061104
const asset::SBufferBinding<const video::IGPUBuffer> binding =
11071105
{
11081106
.offset = multiAllocParams.offsets[MDI::EBC_INDEX_BUFFERS],
1109-
// TEST: start of MDI buffer
1110-
// .offset = offset, // 0u
11111107
.buffer = smart_refctd_ptr(mdiBuffer)
11121108
};
11131109

@@ -1122,8 +1118,6 @@ namespace nbl::ext::imgui
11221118
const asset::SBufferBinding<const video::IGPUBuffer> bindings[] =
11231119
{{
11241120
.offset = multiAllocParams.offsets[MDI::EBC_VERTEX_BUFFERS],
1125-
// TEST: start of MDI buffer
1126-
// .offset = offset, // 0u
11271121
.buffer = smart_refctd_ptr(mdiBuffer)
11281122
}};
11291123

0 commit comments

Comments
 (0)