GPUUploadManagerTest: allow initialization with null device context

Author: TheMostDiligent

Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp

@@ -110,14 +110,6 @@ GPUUploadManagerImpl::Page::Page(IRenderDevice*  pDevice,
     Desc.CPUAccessFlags = CPU_ACCESS_WRITE;
     pDevice->CreateBuffer(Desc, nullptr, &m_pStagingBuffer);
     VERIFY_EXPR(m_pStagingBuffer != nullptr);
-
-    RENDER_DEVICE_TYPE DeviceType = pDevice->GetDeviceInfo().Type;
-
-    MAP_FLAGS MapFlags = (DeviceType == RENDER_DEVICE_TYPE_D3D12 || DeviceType == RENDER_DEVICE_TYPE_VULKAN) ?
-        MAP_FLAG_DO_NOT_WAIT :
-        MAP_FLAG_NONE;
-    pContext->MapBuffer(m_pStagingBuffer, MAP_WRITE, MapFlags, m_pData);
-    VERIFY_EXPR(m_pData != nullptr);
 }
 
 GPUUploadManagerImpl::Page::Writer GPUUploadManagerImpl::Page::TryBeginWriting()
@@ -276,12 +268,12 @@ void GPUUploadManagerImpl::Page::Reset(IDeviceContext* pContext)
     m_Enqueued.store(false);
     m_FenceValue = 0;
 
-    if (pContext != nullptr)
+    if (pContext != nullptr && m_pData == nullptr)
     {
-        if (!m_PersistentMapped)
-        {
-            pContext->MapBuffer(m_pStagingBuffer, MAP_WRITE, MAP_FLAG_NONE, m_pData);
-        }
+        const MAP_FLAGS MapFlags = m_PersistentMapped ?
+            MAP_FLAG_DO_NOT_WAIT :
+            MAP_FLAG_NONE;
+        pContext->MapBuffer(m_pStagingBuffer, MAP_WRITE, MapFlags, m_pData);
         VERIFY_EXPR(m_pData != nullptr);
     }
 }
@@ -375,16 +367,31 @@ GPUUploadManagerImpl::GPUUploadManagerImpl(IReferenceCounters* pRefCounters, con
     m_pDevice->CreateFence(Desc, &m_pFence);
     VERIFY_EXPR(m_pFence != nullptr);
 
-    // Create at least one page.
-    m_pCurrentPage.store(CreatePage(CI.pContext), std::memory_order_release);
+    if (CI.pContext != nullptr)
+    {
+        // Create at least one page.
+        m_pCurrentPage.store(CreatePage(CI.pContext), std::memory_order_release);
+    }
 
     // Create additional pages if requested.
     const Uint32 InitialPageCount = CI.MaxPageCount > 0 ?
         std::min(CI.InitialPageCount, CI.MaxPageCount) :
         CI.InitialPageCount;
     while (m_Pages.size() < InitialPageCount)
     {
-        m_FreePages.Push(CreatePage(CI.pContext));
+        Page* pPage = CreatePage(CI.pContext);
+        if (CI.pContext != nullptr)
+        {
+            // If a context is provided, we can immediately map the staging buffer and
+            // prepare the page for use, so we push it to the free list.
+            m_FreePages.Push(pPage);
+        }
+        else
+        {
+            // If no context is provided, the page needs to be mapped in Reset(),
+            // so we add it to the list of in-flight pages.
+            m_InFlightPages.emplace_back(pPage);
+        }
     }
 }
 
@@ -406,7 +413,15 @@ GPUUploadManagerImpl::~GPUUploadManagerImpl()
 
 void GPUUploadManagerImpl::RenderThreadUpdate(IDeviceContext* pContext)
 {
-    DEV_CHECK_ERR(pContext == m_pContext, "The context passed to RenderThreadUpdate must be the same as the one used to create the GPUUploadManagerImpl");
+    if (!m_pContext)
+    {
+        // If no context was provided at creation, we can accept any context in RenderThreadUpdate, but it must be the same across calls.
+        m_pContext = pContext;
+    }
+    else
+    {
+        DEV_CHECK_ERR(pContext == m_pContext, "The context provided to RenderThreadUpdate must be the same as the one used to create the GPUUploadManagerImpl");
+    }
 
     SealAndSwapCurrentPage(pContext);
     ReclaimCompletedPages(pContext);
@@ -473,7 +488,14 @@ void GPUUploadManagerImpl::ScheduleBufferUpdate(IDeviceContext*               pC
 
     while (!AbortUpdate && !m_Stopping.load(std::memory_order_acquire))
     {
-        Page*        P      = m_pCurrentPage.load(std::memory_order_acquire);
+        Page* P = m_pCurrentPage.load(std::memory_order_acquire);
+        if (P == nullptr)
+        {
+            // No current page, wait for a page to become available
+            UpdatePendingSizeAndTryRotate(P);
+            continue;
+        }
+
         Page::Writer Writer = P->TryBeginWriting();
         if (!Writer)
         {
@@ -531,6 +553,10 @@ GPUUploadManagerImpl::Page* GPUUploadManagerImpl::CreatePage(IDeviceContext* pCo
     std::unique_ptr<Page> NewPage = std::make_unique<Page>(m_pDevice, pContext, PageSize);
 
     Page* P = NewPage.get();
+    if (pContext != nullptr)
+    {
+        P->Reset(pContext);
+    }
     m_Pages.emplace_back(std::move(NewPage));
 
     return P;
@@ -548,7 +574,7 @@ bool GPUUploadManagerImpl::SealAndSwapCurrentPage(IDeviceContext* pContext)
     Page* pOld = m_pCurrentPage.exchange(pFreshPage, std::memory_order_acq_rel);
 
     // Seal old page and enqueue if no writers; otherwise last writer will enqueue it
-    if (pOld->TrySeal() == Page::SealStatus::Ready)
+    if (pOld != nullptr && pOld->TrySeal() == Page::SealStatus::Ready)
     {
         TryEnqueuePage(pOld);
     }
@@ -572,7 +598,7 @@ bool GPUUploadManagerImpl::TryRotatePage(IDeviceContext* pContext, Page* Expecte
     }
 
     // We won: seal and enqueue if no writers
-    if (ExpectedCurrent && ExpectedCurrent->TrySeal() == Page::SealStatus::Ready)
+    if (ExpectedCurrent != nullptr && ExpectedCurrent->TrySeal() == Page::SealStatus::Ready)
         TryEnqueuePage(ExpectedCurrent);
 
     m_PageRotatedSignal.Tick();

Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp

@@ -255,21 +255,21 @@ TEST(GPUUploadManagerTest, DestroyWhileUpdatesAreRunning)
     const size_t             kNumThreads = 4;
     std::vector<std::thread> Threads;
     std::atomic<Uint32>      NumUpdatesRunning{0};
+    Threading::Signal        AllThreadsRunningSignal;
     for (size_t t = 0; t < kNumThreads; ++t)
     {
         Threads.emplace_back(
             [&]() {
-                NumUpdatesRunning.fetch_add(1);
+                if (NumUpdatesRunning.fetch_add(1) == kNumThreads - 1)
+                {
+                    AllThreadsRunningSignal.Trigger();
+                }
                 pUploadManager->ScheduleBufferUpdate(nullptr, nullptr, 0, 2048, nullptr);
                 NumUpdatesRunning.fetch_sub(1);
             });
     }
 
-    // Wait until all threads are running updates
-    while (NumUpdatesRunning.load() < kNumThreads)
-    {
-        std::this_thread::yield();
-    }
+    AllThreadsRunningSignal.Wait();
 
     std::this_thread::sleep_for(10ms);
     EXPECT_EQ(NumUpdatesRunning.load(), kNumThreads) << "All threads should be running updates because RenderThreadUpdate() was not called";
@@ -284,4 +284,97 @@ TEST(GPUUploadManagerTest, DestroyWhileUpdatesAreRunning)
     }
 }
 
+
+TEST(GPUUploadManagerTest, CreateWithNullContext)
+{
+    GPUTestingEnvironment* pEnv     = GPUTestingEnvironment::GetInstance();
+    IRenderDevice*         pDevice  = pEnv->GetDevice();
+    IDeviceContext*        pContext = pEnv->GetDeviceContext();
+
+    GPUTestingEnvironment::ScopedReset AutoReset;
+
+    RefCntAutoPtr<IGPUUploadManager> pUploadManager;
+    GPUUploadManagerCreateInfo       CreateInfo{pDevice};
+    CreateInfo.PageSize         = 1024;
+    CreateInfo.InitialPageCount = 8;
+    CreateGPUUploadManager(CreateInfo, &pUploadManager);
+    ASSERT_TRUE(pUploadManager != nullptr);
+
+    const size_t kNumThreads = std::max(2u, std::thread::hardware_concurrency() - 1);
+    LOG_INFO_MESSAGE("Number of threads: ", kNumThreads);
+
+    constexpr size_t kNumUpdatesPerThread = 16;
+    constexpr size_t kUpdateSize          = 2048;
+
+    std::vector<Uint8> BufferData(kNumUpdatesPerThread * kUpdateSize * kNumThreads);
+    for (size_t i = 0; i < BufferData.size(); ++i)
+    {
+        BufferData[i] = static_cast<Uint8>(i % 256);
+    }
+
+    BufferDesc Desc;
+    Desc.Name      = "GPUUploadManagerTest buffer";
+    Desc.Size      = BufferData.size();
+    Desc.Usage     = USAGE_DEFAULT;
+    Desc.BindFlags = BIND_VERTEX_BUFFER;
+
+    RefCntAutoPtr<IBuffer> pBuffer;
+    pDevice->CreateBuffer(Desc, nullptr, &pBuffer);
+    ASSERT_TRUE(pBuffer);
+
+    std::vector<std::thread> Threads;
+    std::atomic<Uint32>      NumUpdatesRunning{0};
+    Threading::Signal        AllThreadsRunningSignal;
+
+    std::atomic<Uint32> CurrOffset{0};
+    for (size_t t = 0; t < kNumThreads; ++t)
+    {
+        Threads.emplace_back(
+            [&]() {
+                if (NumUpdatesRunning.fetch_add(1) == kNumThreads - 1)
+                {
+                    AllThreadsRunningSignal.Trigger();
+                }
+                for (size_t i = 0; i < kNumUpdatesPerThread; ++i)
+                {
+                    Uint32 Offset = CurrOffset.fetch_add(kUpdateSize);
+                    pUploadManager->ScheduleBufferUpdate(nullptr, pBuffer, Offset, kUpdateSize, &BufferData[Offset]);
+                }
+                NumUpdatesRunning.fetch_sub(1);
+            });
+    }
+
+    AllThreadsRunningSignal.Wait();
+
+    std::this_thread::sleep_for(10ms);
+    EXPECT_EQ(NumUpdatesRunning.load(), kNumThreads) << "All threads should be running updates because RenderThreadUpdate() was not called";
+
+    while (NumUpdatesRunning.load() > 0)
+    {
+        pUploadManager->RenderThreadUpdate(pContext);
+        pContext->Flush();
+        pContext->FinishFrame();
+        std::this_thread::sleep_for(10ms);
+    }
+
+    pUploadManager->RenderThreadUpdate(pContext);
+
+    for (std::thread& thread : Threads)
+    {
+        thread.join();
+    }
+
+    VerifyBufferContents(pBuffer, BufferData);
+
+    GPUUploadManagerStats Stats;
+    pUploadManager->GetStats(Stats);
+    LOG_INFO_MESSAGE("GPU Upload Manager Stats:"
+                     "\n    NumPages                   ",
+                     Stats.NumPages,
+                     "\n    NumFreePages               ", Stats.NumFreePages,
+                     "\n    NumInFlightPages           ", Stats.NumInFlightPages,
+                     "\n    PeakTotalPendingUpdateSize ", Stats.PeakTotalPendingUpdateSize,
+                     "\n    PeakUpdateSize             ", Stats.PeakUpdateSize);
+}
+
 } // namespace