Skip to content

Commit 6d45f26

Browse files
committed
spot runtime issues - indirect vertex offsets is calculated incorrectly (modulo with the vertex struct size != 0u). Another problem rises, both streaming & linear allocator won't accept alignment which is not PoT - maybe I could hack the imgui vertex struct and just make it PoT in size with IMGUI_OVERRIDE_DRAWVERT_STRUCT_LAYOUT
1 parent 86d9f20 commit 6d45f26

File tree

1 file changed

+15
-3
lines changed

1 file changed

+15
-3
lines changed

src/nbl/ext/ImGui/ImGui.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ namespace nbl::ext::imgui
2929

3030
static constexpr auto InvalidAddress = compose_t::invalid_value;
3131
static constexpr auto MdiAlignments = std::to_array<mdi_size_t>({ alignof(VkDrawIndexedIndirectCommand), alignof(PerObjectData), alignof(ImDrawIdx), alignof(ImDrawVert) });
32+
static constexpr auto MdiSizes = std::to_array<mdi_size_t>({ sizeof(VkDrawIndexedIndirectCommand), sizeof(PerObjectData), sizeof(ImDrawIdx), sizeof(ImDrawVert) });
3233
static constexpr auto MdiMaxAlignment = *std::max_element(MdiAlignments.begin(), MdiAlignments.end());
34+
static constexpr auto MdiMaxSize = *std::max_element(MdiSizes.begin(), MdiSizes.end());
3335

3436
// those must be allocated within single block for content - even though its possible to tell vkCmdDrawIndexedIndirect about stride we cannot guarantee each one will be the same size with our allocation strategy
3537
enum class TightContent : uint16_t
@@ -1113,8 +1115,8 @@ namespace nbl::ext::imgui
11131115
const ImDrawList* commandList = drawData->CmdLists[i];
11141116
limits.totalIndirectDrawCount += commandList->CmdBuffer.Size;
11151117

1116-
allocation.offsets.emplace_back() = InvalidAddress; limits.sizes.emplace_back() = commandList->VtxBuffer.Size * sizeof(ImDrawVert); limits.alignments.emplace_back() = alignof(ImDrawVert); allocation.filled.emplace_back() = false;
1117-
allocation.offsets.emplace_back() = InvalidAddress; limits.sizes.emplace_back() = commandList->IdxBuffer.Size * sizeof(ImDrawIdx); limits.alignments.emplace_back() = alignof(ImDrawIdx); allocation.filled.emplace_back() = false;
1118+
allocation.offsets.emplace_back() = InvalidAddress; limits.sizes.emplace_back() = commandList->VtxBuffer.Size * sizeof(ImDrawVert); limits.alignments.emplace_back() = sizeof(ImDrawVert); allocation.filled.emplace_back() = false;
1119+
allocation.offsets.emplace_back() = InvalidAddress; limits.sizes.emplace_back() = commandList->IdxBuffer.Size * sizeof(ImDrawIdx); limits.alignments.emplace_back() = sizeof(ImDrawIdx); allocation.filled.emplace_back() = false;
11181120
}
11191121

11201122
limits.totalByteSizeRequest += limits.sizes[(uint32_t)TightContent::INDIRECT_STRUCTURES] = (limits.totalIndirectDrawCount * sizeof(VkDrawIndexedIndirectCommand));
@@ -1159,7 +1161,7 @@ namespace nbl::ext::imgui
11591161
else
11601162
{
11611163
constexpr auto AlignOffsetNeeded = 0u;
1162-
SMdiBuffer::suballocator_traits_t::allocator_type fillSubAllocator(mdiData, bigChunkRequestState.offset, AlignOffsetNeeded, MdiMaxAlignment, bigChunkRequestState.size); //! (*) we create linear suballocator to fill the allocated chunk of memory (some of at least)
1164+
SMdiBuffer::suballocator_traits_t::allocator_type fillSubAllocator(mdiData, bigChunkRequestState.offset, AlignOffsetNeeded, MdiMaxSize /* we care about vertex struct which is MdiMaxSize (20) bytes */, bigChunkRequestState.size); //! (*) we create linear suballocator to fill the allocated chunk of memory (some of at least)
11631165
SMdiBuffer::suballocator_traits_t::multi_alloc_addr(fillSubAllocator, allocation.offsets.size(), allocation.offsets.data(), mdiLimits.sizes.data(), mdiLimits.alignments.data()); //! (*) we suballocate memory regions from the allocated chunk with required alignments - multi request all with single traits call
11641166

11651167
auto upload = [&]() -> size_t
@@ -1211,6 +1213,16 @@ namespace nbl::ext::imgui
12111213
return true;
12121214
};
12131215

1216+
auto validateObjectOffsets = [&]() -> bool
1217+
{
1218+
const auto vtxModulo = allocation.offsets[vtxAllocationIx] % sizeof(ImDrawVert);
1219+
const auto ixModulo = allocation.offsets[idxAllocationIx] % sizeof(ImDrawIdx);
1220+
1221+
return (vtxModulo == 0u && ixModulo == 0u); // we must be aligned!
1222+
};
1223+
1224+
assert(validateObjectOffsets()); // debug only
1225+
12141226
// we consider buffers valid if we suballocated them (under the hood filled)
12151227
const auto buffersSuballocated = fillBuffer(vertexBuffer.Data, vtxAllocationIx) && fillBuffer(indexBuffer.Data, idxAllocationIx);
12161228
const auto [vtxGlobalObjectOffset, idxGlobalObjectOffset] = buffersSuballocated ? std::make_tuple(allocation.offsets[vtxAllocationIx] / sizeof(ImDrawVert), allocation.offsets[idxAllocationIx] / sizeof(ImDrawIdx)) : std::make_tuple((size_t)0u, (size_t)0u);

0 commit comments

Comments
 (0)