From 51e4927428443b71dba7b33e114da5340e2169dc Mon Sep 17 00:00:00 2001
From: Bryan Bernhart <bryan.bernhart@intel.com>
Date: Wed, 4 May 2022 15:35:20 -0700
Subject: [PATCH] Return S_FALSE when re-inserting residency set.

Also, add documentation to ResidencySet.
---
 src/gpgmm/d3d12/ResidencySetD3D12.cpp         |  2 +-
 src/gpgmm/d3d12/ResidencySetD3D12.h           | 18 ++++++++++++--
 .../end2end/D3D12ResidencyManagerTests.cpp    | 24 +++++++++++++++++++
 3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/src/gpgmm/d3d12/ResidencySetD3D12.cpp b/src/gpgmm/d3d12/ResidencySetD3D12.cpp
index 2d859bc21..148a20aa0 100644
--- a/src/gpgmm/d3d12/ResidencySetD3D12.cpp
+++ b/src/gpgmm/d3d12/ResidencySetD3D12.cpp
@@ -22,7 +22,7 @@ namespace gpgmm { namespace d3d12 {
         if (mSet.insert(heap).second) {
             return S_OK;
         }
-        return E_FAIL;
+        return S_FALSE;
     }
 
     HRESULT ResidencySet::Reset() {
diff --git a/src/gpgmm/d3d12/ResidencySetD3D12.h b/src/gpgmm/d3d12/ResidencySetD3D12.h
index df2802d7b..73b7e232f 100644
--- a/src/gpgmm/d3d12/ResidencySetD3D12.h
+++ b/src/gpgmm/d3d12/ResidencySetD3D12.h
@@ -24,13 +24,27 @@ namespace gpgmm { namespace d3d12 {
 
     class Heap;
 
-    // Represents a set of heaps which are referenced by a command list.
-    // The set must be updated to ensure each heap is made resident for execution.
+    /** \brief Represents a set of heaps which are referenced by a command list.
+
+    The set must be updated to ensure each heap is made resident for execution.
+    */
     class GPGMM_EXPORT ResidencySet {
       public:
+        /** \brief  Create a residency set or collection of heaps to manage together for residency.
+         */
         ResidencySet() = default;
 
+        /** \brief  Insert heap into this residency set.
+
+        @param heap A pointer to Heap about to be inserted.
+        \return S_OK if heap was inserted or S_FALSE if heap already exists, else error.
+         */
         HRESULT Insert(Heap* heap);
+
+        /** \brief  Reset this residency set.
+
+        Removes all heaps in the set so the set can be re-used.
+        */
         HRESULT Reset();
 
         std::set<Heap*>::iterator begin() const;
diff --git a/src/tests/end2end/D3D12ResidencyManagerTests.cpp b/src/tests/end2end/D3D12ResidencyManagerTests.cpp
index 8d2853648..1fe3b1f27 100644
--- a/src/tests/end2end/D3D12ResidencyManagerTests.cpp
+++ b/src/tests/end2end/D3D12ResidencyManagerTests.cpp
@@ -29,6 +29,30 @@ class D3D12ResidencyManagerTests : public D3D12TestBase, public ::testing::Test
     }
 };
 
+TEST_F(D3D12ResidencyManagerTests, CreateResidencySet) {
+    ComPtr<ResourceAllocator> resourceAllocator;
+    ASSERT_SUCCEEDED(ResourceAllocator::CreateAllocator(CreateBasicAllocatorDesc(),
+                                                        &resourceAllocator, nullptr));
+
+    ComPtr<ResourceAllocation> allocation;
+    ASSERT_SUCCEEDED(resourceAllocator->CreateResource(
+        {}, CreateBasicBufferDesc(1), D3D12_RESOURCE_STATE_COMMON, nullptr, &allocation));
+
+    // Inserting a non-existant heap should always fail
+    {
+        ResidencySet set;
+        Heap* invalid = nullptr;
+        ASSERT_FAILED(set.Insert(invalid));
+    }
+
+    // Inserting from a valid allocation should always succeed.
+    {
+        ResidencySet set;
+        ASSERT_SUCCEEDED(set.Insert(allocation->GetMemory()));
+        ASSERT_SUCCEEDED(set.Insert(allocation->GetMemory()));
+    }
+}
+
 TEST_F(D3D12ResidencyManagerTests, CreateResidencyManager) {
     ComPtr<ResidencyManager> residencyManager;
     ComPtr<ResourceAllocator> resourceAllocator;