From 6c3387e76a34526bb96b7bc9e61197b9da979dd0 Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Mon, 13 Jun 2022 15:24:28 -0700 Subject: [PATCH] Use Power-of-Two alignment within resource by default when none specified in resource descriptor. Fixes possible edge-case bug when requesting "alignment=0" could use an alignment much smaller then supported by the allocator. --- src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp | 7 +++- .../end2end/D3D12ResourceAllocatorTests.cpp | 41 ++++++++++++------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp index 41475a178..bd9c7e807 100644 --- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp +++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp @@ -279,6 +279,11 @@ namespace gpgmm { namespace d3d12 { return E_FAIL; } + // Requested resource size should be already aligned with the fixed memory size required + // by the allocator. + ASSERT(allocator->GetMemorySize() == kInvalidSize || + allocator->GetMemorySize() % request.Alignment == 0); + std::unique_ptr allocation = allocator->TryAllocateMemory(request); if (allocation == nullptr) { // NeverAllocate always fails, so suppress it. @@ -813,7 +818,7 @@ namespace gpgmm { namespace d3d12 { // Only constant buffers must be 256B aligned. request.Alignment = (initialResourceState == D3D12_RESOURCE_STATE_GENERIC_READ) ? D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT - : 1; + : NextPowerOfTwo(newResourceDesc.Width); } else { request.Alignment = resourceDescriptor.Alignment; } diff --git a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp index 814acc4fb..1edd89a61 100644 --- a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp +++ b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp @@ -403,7 +403,6 @@ TEST_F(D3D12ResourceAllocatorTests, CreateBufferWithin) { ALLOCATION_DESC desc = {}; desc.Flags = ALLOCATION_FLAG_ALLOW_SUBALLOCATE_WITHIN_RESOURCE; - // Byte-aligned upload buffer within. { ALLOCATION_DESC smallBufferDesc = desc; smallBufferDesc.HeapType = D3D12_HEAP_TYPE_UPLOAD; @@ -416,13 +415,12 @@ TEST_F(D3D12ResourceAllocatorTests, CreateBufferWithin) { EXPECT_EQ(smallBuffer->GetMethod(), gpgmm::AllocationMethod::kSubAllocatedWithin); EXPECT_EQ(smallBuffer->GetSize(), 4u); EXPECT_EQ(smallBuffer->GetOffsetFromResource(), 0u); + EXPECT_EQ(smallBuffer->GetAlignment(), 4u); // Must re-align. EXPECT_EQ(resourceAllocator->GetInfo().UsedBlockCount, 1u); EXPECT_EQ(resourceAllocator->GetInfo().UsedMemoryCount, 1u); EXPECT_EQ(resourceAllocator->GetInfo().UsedBlockUsage, smallBuffer->GetSize()); } - - // Custom-aligned upload buffer within. { ALLOCATION_DESC smallBufferDesc = desc; smallBufferDesc.HeapType = D3D12_HEAP_TYPE_UPLOAD; @@ -435,44 +433,57 @@ TEST_F(D3D12ResourceAllocatorTests, CreateBufferWithin) { EXPECT_EQ(smallBuffer->GetMethod(), gpgmm::AllocationMethod::kSubAllocatedWithin); EXPECT_EQ(smallBuffer->GetSize(), 16u); EXPECT_EQ(smallBuffer->GetOffsetFromResource(), 0u); + EXPECT_EQ(smallBuffer->GetAlignment(), 16u); EXPECT_EQ(resourceAllocator->GetInfo().UsedBlockCount, 1u); EXPECT_EQ(resourceAllocator->GetInfo().UsedMemoryCount, 1u); EXPECT_EQ(resourceAllocator->GetInfo().UsedBlockUsage, smallBuffer->GetSize()); } - - // Constant upload buffer within. { ALLOCATION_DESC smallBufferDesc = desc; smallBufferDesc.HeapType = D3D12_HEAP_TYPE_UPLOAD; ComPtr smallBuffer; ASSERT_SUCCEEDED(resourceAllocator->CreateResource( - smallBufferDesc, CreateBasicBufferDesc(4u, 0), D3D12_RESOURCE_STATE_GENERIC_READ, - nullptr, &smallBuffer)); + smallBufferDesc, CreateBasicBufferDesc(4u), D3D12_RESOURCE_STATE_GENERIC_READ, nullptr, + &smallBuffer)); ASSERT_NE(smallBuffer, nullptr); EXPECT_EQ(smallBuffer->GetMethod(), gpgmm::AllocationMethod::kSubAllocatedWithin); EXPECT_EQ(smallBuffer->GetSize(), 256u); EXPECT_EQ(smallBuffer->GetOffsetFromResource(), 0u); + EXPECT_EQ(smallBuffer->GetAlignment(), 256u); // Re-align EXPECT_EQ(resourceAllocator->GetInfo().UsedBlockCount, 1u); EXPECT_EQ(resourceAllocator->GetInfo().UsedMemoryCount, 1u); EXPECT_EQ(resourceAllocator->GetInfo().UsedBlockUsage, smallBuffer->GetSize()); } - - // Create a read-back buffer within. { ALLOCATION_DESC smallBufferDesc = desc; smallBufferDesc.HeapType = D3D12_HEAP_TYPE_READBACK; - ComPtr smallerBuffer; + ComPtr smallBuffer; ASSERT_SUCCEEDED(resourceAllocator->CreateResource( smallBufferDesc, CreateBasicBufferDesc(4u), D3D12_RESOURCE_STATE_COPY_DEST, nullptr, - &smallerBuffer)); - ASSERT_NE(smallerBuffer, nullptr); - EXPECT_EQ(smallerBuffer->GetMethod(), gpgmm::AllocationMethod::kSubAllocatedWithin); - EXPECT_EQ(smallerBuffer->GetSize(), 4u); - EXPECT_EQ(smallerBuffer->GetOffsetFromResource(), 0u); + &smallBuffer)); + ASSERT_NE(smallBuffer, nullptr); + EXPECT_EQ(smallBuffer->GetMethod(), gpgmm::AllocationMethod::kSubAllocatedWithin); + EXPECT_EQ(smallBuffer->GetSize(), 4u); + EXPECT_EQ(smallBuffer->GetOffsetFromResource(), 0u); + EXPECT_EQ(smallBuffer->GetAlignment(), 4u); + } + { + ALLOCATION_DESC smallBufferDesc = desc; + smallBufferDesc.HeapType = D3D12_HEAP_TYPE_READBACK; + + ComPtr smallBuffer; + ASSERT_SUCCEEDED(resourceAllocator->CreateResource( + smallBufferDesc, CreateBasicBufferDesc(3u), D3D12_RESOURCE_STATE_COPY_DEST, nullptr, + &smallBuffer)); + ASSERT_NE(smallBuffer, nullptr); + EXPECT_EQ(smallBuffer->GetMethod(), gpgmm::AllocationMethod::kSubAllocatedWithin); + EXPECT_EQ(smallBuffer->GetSize(), 4u); + EXPECT_EQ(smallBuffer->GetOffsetFromResource(), 0u); + EXPECT_EQ(smallBuffer->GetAlignment(), 4u); // Re-align } }