GPUUploadManagerImpl::Page: add debug asserts; clarify debug-only method names

Author: TheMostDiligent

Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp

@@ -141,13 +141,13 @@ class GPUUploadManagerImpl final : public ObjectBase<IGPUUploadManager>
         Uint32 GetSize() const { return m_Size; }
 
         // Returns the number of pending operations. This is used for testing and debugging purposes.
-        size_t GetNumPendingOps() const { return m_NumPendingOps.load(std::memory_order_relaxed); }
+        size_t DbgGetNumPendingOps() const { return m_NumPendingOps.load(std::memory_order_relaxed); }
 
         // Returns the number of active writers. This is used for testing and debugging purposes.
-        Uint32 GetWriterCount() const { return m_State.load(std::memory_order_acquire) & WRITER_MASK; }
+        Uint32 DbgGetWriterCount() const { return m_State.load(std::memory_order_relaxed) & WRITER_MASK; }
 
         // Returns true if the page is sealed for new writes. This is used for testing and debugging purposes.
-        bool IsSealed() const { return (m_State.load(std::memory_order_acquire) & SEALED_BIT) != 0; }
+        bool DbgIsSealed() const { return (m_State.load(std::memory_order_relaxed) & SEALED_BIT) != 0; }
 
     private:
         // Schedules a buffer update operation on the page.

Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp

@@ -46,6 +46,8 @@ GPUUploadManagerImpl::Page::Writer& GPUUploadManagerImpl::Page::Writer::operator
 {
     if (this != &Other)
     {
+        VERIFY(m_pPage == nullptr, "Assigning to a non-empty writer. End writing before assignment or use move assignment operator only for empty writers.");
+
         m_pPage       = Other.m_pPage;
         Other.m_pPage = nullptr;
     }
@@ -187,7 +189,7 @@ bool GPUUploadManagerImpl::Page::ScheduleBufferUpdate(IBuffer*
                                                       GPUUploadEnqueuedCallbackType Callback,
                                                       void*                         pCallbackData)
 {
-    VERIFY_EXPR(GetWriterCount() > 0);
+    VERIFY_EXPR(DbgGetWriterCount() > 0);
 
     // Note that the page may be sealed for new writes at this point,
     // but we can still schedule the update since we have an active writer
@@ -228,18 +230,19 @@ bool GPUUploadManagerImpl::Page::ScheduleBufferUpdate(IBuffer*
 
 void GPUUploadManagerImpl::Page::ExecutePendingOps(IDeviceContext* pContext, Uint64 FenceValue)
 {
-    VERIFY(IsSealed(), "Page must be sealed before executing pending operations");
-    VERIFY(GetWriterCount() == 0, "All writers must finish before executing pending operations");
+    VERIFY(DbgIsSealed(), "Page must be sealed before executing pending operations");
+    VERIFY(DbgGetWriterCount() == 0, "All writers must finish before executing pending operations");
 
-    if (pContext != nullptr)
+    if (m_pData != nullptr)
     {
+        VERIFY_EXPR(pContext != nullptr);
         pContext->UnmapBuffer(m_pStagingBuffer, MAP_WRITE);
         m_pData = nullptr;
     }
 
     for (PendingOp Op; m_PendingOps.Dequeue(Op);)
     {
-        if (pContext != nullptr)
+        if (pContext != nullptr && Op.NumBytes > 0)
         {
             pContext->CopyBuffer(m_pStagingBuffer, Op.SrcOffset, RESOURCE_STATE_TRANSITION_MODE_TRANSITION,
                                  Op.pDstBuffer, Op.DstOffset, Op.NumBytes, RESOURCE_STATE_TRANSITION_MODE_TRANSITION);
@@ -256,13 +259,14 @@ void GPUUploadManagerImpl::Page::ExecutePendingOps(IDeviceContext* pContext, Uin
 
 void GPUUploadManagerImpl::Page::Reset(IDeviceContext* pContext)
 {
-    VERIFY(GetWriterCount() == 0, "All writers must finish before resetting the page");
+    VERIFY(DbgGetWriterCount() == 0, "All writers must finish before resetting the page");
     VERIFY(m_PendingOps.IsEmpty(), "All pending operations must be executed before resetting the page");
 
     m_Offset.store(0);
     m_State.store(0);
     m_NumPendingOps.store(0);
     m_Enqueued.store(false);
+    m_FenceValue = 0;
 
     if (pContext != nullptr)
     {
@@ -273,8 +277,8 @@ void GPUUploadManagerImpl::Page::Reset(IDeviceContext* pContext)
 
 bool GPUUploadManagerImpl::Page::TryEnqueue()
 {
-    VERIFY(IsSealed(), "Page must be sealed before it can be enqueued for execution");
-    VERIFY(GetWriterCount() == 0, "All writers must finish before the page can be enqueued");
+    VERIFY(DbgIsSealed(), "Page must be sealed before it can be enqueued for execution");
+    VERIFY(DbgGetWriterCount() == 0, "All writers must finish before the page can be enqueued");
 
     bool Expected = false;
     return m_Enqueued.compare_exchange_strong(Expected, true, std::memory_order_acq_rel);

Tests/DiligentCoreTest/src/GraphicsTools/GPUUploadManagerPageTest.cpp

@@ -83,12 +83,12 @@ TEST(GPUUploadManagerPageTest, States)
         EXPECT_TRUE(Writer.ScheduleBufferUpdate(nullptr, 0, 512, nullptr, nullptr, nullptr));
         EXPECT_TRUE(Writer.ScheduleBufferUpdate(nullptr, 512, 512, nullptr, nullptr, nullptr));
         EXPECT_FALSE(Writer.ScheduleBufferUpdate(nullptr, 1024, 512, nullptr, nullptr, nullptr)) << "Should not be able to schedule an update that exceeds the page size";
-        EXPECT_EQ(Page.GetNumPendingOps(), size_t{2});
+        EXPECT_EQ(Page.DbgGetNumPendingOps(), size_t{2});
         EXPECT_TRUE(Writer.EndWriting() == GPUUploadManagerImpl::Page::WritingStatus::NotSealed) << "Page should not be sealed";
         EXPECT_EQ(Page.TrySeal(), GPUUploadManagerImpl::Page::SealStatus::Ready) << "Page with no active writers should be ready immediately";
-        EXPECT_EQ(Page.GetNumPendingOps(), size_t{2});
+        EXPECT_EQ(Page.DbgGetNumPendingOps(), size_t{2});
         Page.ExecutePendingOps(nullptr, 0);
-        EXPECT_EQ(Page.GetNumPendingOps(), size_t{0});
+        EXPECT_EQ(Page.DbgGetNumPendingOps(), size_t{0});
     }
 
     {
@@ -272,7 +272,7 @@ TEST(GPUUploadManagerPageTest, ScheduleBufferUpdateParallel)
     for (auto& thread : threads)
         thread.join();
 
-    EXPECT_EQ(Page.GetNumPendingOps(), kNumUpdates);
+    EXPECT_EQ(Page.DbgGetNumPendingOps(), kNumUpdates);
     EXPECT_EQ(UpdatesScheduled.load(), kNumUpdates) << "Should be able to schedule updates until the page size is reached";
     EXPECT_EQ(Page.TrySeal() == GPUUploadManagerImpl::Page::SealStatus::Ready, true) << "Page should be ready for sealing after all updates are scheduled";
     Page.ExecutePendingOps(nullptr, 0);