Move command list creation to the Device and indirectly reserve command allocators

examples/D3D12Binding.cpp

@@ -37,7 +37,7 @@ namespace d3d12 {
     void NextSerial(nxtDevice device);
     void ExecuteCommandLists(nxtDevice device, std::initializer_list<ID3D12CommandList*> commandLists);
     void WaitForSerial(nxtDevice device, uint64_t serial);
-    ComPtr<ID3D12CommandAllocator> ReserveCommandAllocator(nxtDevice device);
+    void OpenCommandList(nxtDevice device, ComPtr<ID3D12GraphicsCommandList>* commandList);
 }
 }
 
@@ -130,14 +130,7 @@ class D3D12Binding : public BackendBinding {
 
             // Transition the first frame to be a render target
             {
-                ComPtr<ID3D12CommandAllocator> commandAllocator = backend::d3d12::ReserveCommandAllocator(backendDevice);
+                backend::d3d12::OpenCommandList(backendDevice, &commandList);
-                ASSERT_SUCCESS(d3d12Device->CreateCommandList(
-                    0,
-                    D3D12_COMMAND_LIST_TYPE_DIRECT,
-                    commandAllocator.Get(),
-                    nullptr,
-                    IID_PPV_ARGS(&commandList)
-                ));
 
                 D3D12_RESOURCE_BARRIER resourceBarrier;
                 resourceBarrier.Transition.pResource = renderTargetResources[renderTargetIndex].Get();
@@ -162,8 +155,7 @@ class D3D12Binding : public BackendBinding {
         void SwapBuffers() override {
             // Transition current frame's render target for presenting
             {
-                ComPtr<ID3D12CommandAllocator> commandAllocator = backend::d3d12::ReserveCommandAllocator(backendDevice);
+                backend::d3d12::OpenCommandList(backendDevice, &commandList);
-                ASSERT_SUCCESS(commandList->Reset(commandAllocator.Get(), nullptr));
                 D3D12_RESOURCE_BARRIER resourceBarrier;
                 resourceBarrier.Transition.pResource = renderTargetResources[renderTargetIndex].Get();
                 resourceBarrier.Transition.StateBefore = D3D12_RESOURCE_STATE_RENDER_TARGET;
@@ -181,8 +173,7 @@ class D3D12Binding : public BackendBinding {
 
             // Transition last frame's render target back to being a render target
             {
-                ComPtr<ID3D12CommandAllocator> commandAllocator = backend::d3d12::ReserveCommandAllocator(backendDevice);
+                backend::d3d12::OpenCommandList(backendDevice, &commandList);
-                ASSERT_SUCCESS(commandList->Reset(commandAllocator.Get(), nullptr));
                 D3D12_RESOURCE_BARRIER resourceBarrier;
                 resourceBarrier.Transition.pResource = renderTargetResources[previousRenderTargetIndex].Get();
                 resourceBarrier.Transition.StateBefore = D3D12_RESOURCE_STATE_PRESENT;

src/backend/d3d12/CommandAllocatorManager.h

@@ -30,8 +30,8 @@ namespace d3d12 {
         public:
             CommandAllocatorManager(Device* device);
 
-            // A CommandAllocator that is reserved must be used before the next Device::Tick where the next serial has completed on the GPU
+            // A CommandAllocator that is reserved must be used on the next ExecuteCommandLists
-            // at this time, the CommandAllocator will be reset
+            // otherwise its commands may be reset before execution has completed on the GPU
             ComPtr<ID3D12CommandAllocator> ReserveCommandAllocator();
             void ResetCompletedAllocators(uint64_t lastCompletedSerial);
 

src/backend/d3d12/D3D12Backend.cpp

@@ -68,9 +68,9 @@ namespace d3d12 {
         backendDevice->WaitForSerial(serial);
     }
 
-    ComPtr<ID3D12CommandAllocator> ReserveCommandAllocator(nxtDevice device) {
+    void OpenCommandList(nxtDevice device, ComPtr<ID3D12GraphicsCommandList>* commandList) {
         Device* backendDevice = reinterpret_cast<Device*>(device);
-        return backendDevice->GetCommandAllocatorManager()->ReserveCommandAllocator();
+        return backendDevice->OpenCommandList(commandList);
     }
 
     void ASSERT_SUCCESS(HRESULT hr) {
@@ -81,23 +81,13 @@ namespace d3d12 {
         : d3d12Device(d3d12Device),
           commandAllocatorManager(new CommandAllocatorManager(this)),
           resourceAllocator(new ResourceAllocator(this)),
-          resourceUploader(new ResourceUploader(this)),
+          resourceUploader(new ResourceUploader(this)) {
-          pendingCommands{ commandAllocatorManager->ReserveCommandAllocator() } {
 
         D3D12_COMMAND_QUEUE_DESC queueDesc = {};
         queueDesc.Flags = D3D12_COMMAND_QUEUE_FLAG_NONE;
         queueDesc.Type = D3D12_COMMAND_LIST_TYPE_DIRECT;
         ASSERT_SUCCESS(d3d12Device->CreateCommandQueue(&queueDesc, IID_PPV_ARGS(&commandQueue)));
 
-        ASSERT_SUCCESS(d3d12Device->CreateCommandList(
-            0,
-            D3D12_COMMAND_LIST_TYPE_DIRECT,
-            pendingCommands.commandAllocator.Get(),
-            nullptr,
-            IID_PPV_ARGS(&pendingCommands.commandList)
-        ));
-        pendingCommands.open = true;
-
         ASSERT_SUCCESS(d3d12Device->CreateFence(serial, D3D12_FENCE_FLAG_NONE, IID_PPV_ARGS(&fence)));
         fenceEvent = CreateEvent(nullptr, FALSE, FALSE, nullptr);
         ASSERT(fenceEvent != nullptr);
@@ -114,10 +104,6 @@ namespace d3d12 {
         return commandQueue;
     }
 
-    CommandAllocatorManager* Device::GetCommandAllocatorManager() {
-        return commandAllocatorManager;
-    }
-
     ResourceAllocator* Device::GetResourceAllocator() {
         return resourceAllocator;
     }
@@ -126,11 +112,25 @@ namespace d3d12 {
         return resourceUploader;
     }
 
+    void Device::OpenCommandList(ComPtr<ID3D12GraphicsCommandList>* commandList) {
+        ComPtr<ID3D12GraphicsCommandList> &cmdList = *commandList;
+        if (!cmdList) {
+            ASSERT_SUCCESS(d3d12Device->CreateCommandList(
+                0,
+                D3D12_COMMAND_LIST_TYPE_DIRECT,
+                commandAllocatorManager->ReserveCommandAllocator().Get(),
+                nullptr,
+                IID_PPV_ARGS(&cmdList)
+            ));
+        } else {
+            ASSERT_SUCCESS(cmdList->Reset(commandAllocatorManager->ReserveCommandAllocator().Get(), nullptr));
+        }
+    }
+
     ComPtr<ID3D12GraphicsCommandList> Device::GetPendingCommandList() {
         // Callers of GetPendingCommandList do so to record commands. Only reserve a command allocator when it is needed so we don't submit empty command lists
         if (!pendingCommands.open) {
-            pendingCommands.commandAllocator = commandAllocatorManager->ReserveCommandAllocator();
+            OpenCommandList(&pendingCommands.commandList);
-            ASSERT_SUCCESS(pendingCommands.commandList->Reset(pendingCommands.commandAllocator.Get(), nullptr));
             pendingCommands.open = true;
         }
         return pendingCommands.commandList;

src/backend/d3d12/D3D12Backend.h

@@ -113,10 +113,10 @@ namespace d3d12 {
             ComPtr<ID3D12Device> GetD3D12Device();
             ComPtr<ID3D12CommandQueue> GetCommandQueue();
 
-            CommandAllocatorManager* GetCommandAllocatorManager();
             ResourceAllocator* GetResourceAllocator();
             ResourceUploader* GetResourceUploader();
 
+            void OpenCommandList(ComPtr<ID3D12GraphicsCommandList>* commandList);
             ComPtr<ID3D12GraphicsCommandList> GetPendingCommandList();
 
             D3D12_CPU_DESCRIPTOR_HANDLE GetCurrentRenderTargetDescriptor();
@@ -145,7 +145,6 @@ namespace d3d12 {
             ResourceUploader* resourceUploader;
 
             struct PendingCommandList {
-                ComPtr<ID3D12CommandAllocator> commandAllocator;
                 ComPtr<ID3D12GraphicsCommandList> commandList;
                 bool open = false;
             } pendingCommands;

src/backend/d3d12/QueueD3D12.cpp

@@ -15,7 +15,6 @@
 #include "QueueD3D12.h"
 
 #include "D3D12Backend.h"
-#include "CommandAllocatorManager.h"
 #include "CommandBufferD3D12.h"
 
 namespace backend {
@@ -23,25 +22,12 @@ namespace d3d12 {
 
     Queue::Queue(Device* device, QueueBuilder* builder)
         : QueueBase(builder), device(device) {
-
-        // TODO(enga@google.com): We don't need this allocator, but it's needed for command list initialization. Is there a better way to do this?
-        // Is CommandList creation expensive or can it be done every Queue::Submit?
-        ComPtr<ID3D12CommandAllocator> temporaryCommandAllocator;
-        ASSERT_SUCCESS(device->GetD3D12Device()->CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, IID_PPV_ARGS(&temporaryCommandAllocator)));
-        ASSERT_SUCCESS(device->GetD3D12Device()->CreateCommandList(
-            0,
-            D3D12_COMMAND_LIST_TYPE_DIRECT,
-            temporaryCommandAllocator.Get(),
-            nullptr,
-            IID_PPV_ARGS(&commandList)
-        ));
-        ASSERT_SUCCESS(commandList->Close());
     }
 
     void Queue::Submit(uint32_t numCommands, CommandBuffer* const * commands) {
         device->Tick();
 
-        ASSERT_SUCCESS(commandList->Reset(device->GetCommandAllocatorManager()->ReserveCommandAllocator().Get(), nullptr));
+        device->OpenCommandList(&commandList);
         for (uint32_t i = 0; i < numCommands; ++i) {
             commands[i]->FillCommands(commandList);
         }

src/backend/d3d12/ResourceAllocator.h

@@ -19,8 +19,6 @@
 
 #include "common/SerialQueue.h"
 
-#include <set>
-
 namespace backend {
 namespace d3d12 {