فهرست منبع

fix problems with array iterator remove and DX12HitGroup memory cleanup

Kolja Strohm 1 هفته پیش
والد
کامیت
c9d84bda8d
4فایلهای تغییر یافته به همراه83 افزوده شده و 39 حذف شده
  1. 10 7
      Array.h
  2. 32 18
      DX12Shader.cpp
  3. 35 14
      Framework Tests/Array.cpp
  4. 6 0
      Framework Tests/Camera3D.cpp

+ 10 - 7
Array.h

@@ -244,7 +244,7 @@ namespace Framework
         bool operator!=(ArrayIterator<TYP>& r)
         {
             return current != r.current
-                || ((!current || !current->set)
+                && ((!current || !current->set)
                     != (!r.current || !r.current->set));
         }
 
@@ -252,7 +252,7 @@ namespace Framework
         {
             return current == r.current
                 || ((!current || !current->set)
-                    && (!r.current || !r.current->set));
+                    == (!r.current || !r.current->set));
         }
 
         void remove() override
@@ -278,14 +278,17 @@ namespace Framework
             }
             ArrayEntry<TYP>* del = current->next;
             if (current->next)
-                current->next = current->next->next;
-            else
             {
-                if (onLastChanged)
+                if (current->next->next)
+                    current->next = current->next->next;
+                else
                 {
-                    (*onLastChanged)(current);
+                    if (onLastChanged)
+                    {
+                        (*onLastChanged)(current);
+                    }
+                    current->next = 0;
                 }
-                current->next = 0;
             }
             if (del)
             {

+ 32 - 18
DX12Shader.cpp

@@ -517,11 +517,15 @@ Framework::DX12ShaderHitGroup::~DX12ShaderHitGroup()
 }
 
 void Framework::DX12ShaderHitGroup::setClosestHitShaderFunction(
-    DX12ShaderFunction* closestHitShaderFunction)
+    DX12ShaderFunction* zClosestHitShaderFunction)
 {
+    if (this->closestHitShaderFunction == zClosestHitShaderFunction)
+    {
+        return;
+    }
     if (anyHitShaderFunction
         && anyHitShaderFunction->zSignature()
-               != closestHitShaderFunction->zSignature())
+               != zClosestHitShaderFunction->zSignature())
     {
         Logging::error()
             << "Any-hit shader function and closest-hit shader "
@@ -530,12 +534,12 @@ void Framework::DX12ShaderHitGroup::setClosestHitShaderFunction(
             << name << "' Any Hit Shader Function: '"
             << anyHitShaderFunction->getFunctionName().getText()
             << "' Closest Hit Shader Function: '"
-            << closestHitShaderFunction->getFunctionName().getText() << "'";
+            << zClosestHitShaderFunction->getFunctionName().getText() << "'";
         throw std::runtime_error("Incompatible root signatures in hit group");
     }
     if (intersectionShaderFunction
         && intersectionShaderFunction->zSignature()
-               != closestHitShaderFunction->zSignature())
+               != zClosestHitShaderFunction->zSignature())
     {
         Logging::error()
             << "Intersection shader function and closest-hit shader "
@@ -544,43 +548,48 @@ void Framework::DX12ShaderHitGroup::setClosestHitShaderFunction(
             << name << "' Intersection Shader Function: '"
             << intersectionShaderFunction->getFunctionName().getText()
             << "' Closest Hit Shader Function: '"
-            << closestHitShaderFunction->getFunctionName().getText() << "'";
+            << zClosestHitShaderFunction->getFunctionName().getText() << "'";
         throw std::runtime_error("Incompatible root signatures in hit group");
     }
 
-    if (this->closestHitShaderFunction)
+    if (closestHitShaderFunction)
     {
-        this->closestHitShaderFunction->release();
+        closestHitShaderFunction->release();
     }
-    this->closestHitShaderFunction = closestHitShaderFunction;
+    closestHitShaderFunction = zClosestHitShaderFunction;
     hitGroupDesc->ClosestHitShaderImport = 0;
     if (closestHitShaderFunction)
     {
         hitGroupDesc->ClosestHitShaderImport
             = closestHitShaderFunction->zExportDesc()->Name;
+        closestHitShaderFunction->getThis();
     }
 }
 
 void Framework::DX12ShaderHitGroup::setAnyHitShaderFunction(
-    DX12ShaderFunction* anyHitShaderFunction)
+    DX12ShaderFunction* zAnyHitShaderFunction)
 {
+    if (this->anyHitShaderFunction == zAnyHitShaderFunction)
+    {
+        return;
+    }
     if (closestHitShaderFunction
         && closestHitShaderFunction->zSignature()
-               != anyHitShaderFunction->zSignature())
+               != zAnyHitShaderFunction->zSignature())
     {
         Logging::error()
             << "Any-hit shader function and closest-hit shader "
                "function must have the same root signature when they are "
                "combined in the same hit group. HitGroup Name: '"
             << name << "' Any Hit Shader Function: '"
-            << anyHitShaderFunction->getFunctionName().getText()
+            << zAnyHitShaderFunction->getFunctionName().getText()
             << "' Closest Hit Shader Function: '"
             << closestHitShaderFunction->getFunctionName().getText() << "'";
         throw std::runtime_error("Incompatible root signatures in hit group");
     }
     if (intersectionShaderFunction
         && intersectionShaderFunction->zSignature()
-               != anyHitShaderFunction->zSignature())
+               != zAnyHitShaderFunction->zSignature())
     {
         Logging::error()
             << "Intersection shader function and any-hit shader "
@@ -589,26 +598,31 @@ void Framework::DX12ShaderHitGroup::setAnyHitShaderFunction(
             << name << "' Intersection Shader Function: '"
             << intersectionShaderFunction->getFunctionName().getText()
             << "' Any Hit Shader Function: '"
-            << anyHitShaderFunction->getFunctionName().getText() << "'";
+            << zAnyHitShaderFunction->getFunctionName().getText() << "'";
         throw std::runtime_error("Incompatible root signatures in hit group");
     }
 
-    if (this->anyHitShaderFunction)
+    if (anyHitShaderFunction)
     {
-        this->anyHitShaderFunction->release();
+        anyHitShaderFunction->release();
     }
-    this->anyHitShaderFunction = anyHitShaderFunction;
+    anyHitShaderFunction = zAnyHitShaderFunction;
     hitGroupDesc->AnyHitShaderImport = 0;
     if (anyHitShaderFunction)
     {
         hitGroupDesc->AnyHitShaderImport
             = anyHitShaderFunction->zExportDesc()->Name;
+        anyHitShaderFunction->getThis();
     }
 }
 
 void Framework::DX12ShaderHitGroup::setIntersectionShaderFunction(
     DX12ShaderFunction* zIntersectionShaderFunction)
 {
+    if (intersectionShaderFunction == zIntersectionShaderFunction)
+    {
+        return;
+    }
     if (closestHitShaderFunction
         && closestHitShaderFunction->zSignature()
                != zIntersectionShaderFunction->zSignature())
@@ -642,13 +656,13 @@ void Framework::DX12ShaderHitGroup::setIntersectionShaderFunction(
     {
         intersectionShaderFunction->release();
     }
-    intersectionShaderFunction = dynamic_cast<DX12ShaderFunction*>(
-        zIntersectionShaderFunction->getThis());
+    intersectionShaderFunction = zIntersectionShaderFunction;
     hitGroupDesc->IntersectionShaderImport = 0;
     if (intersectionShaderFunction)
     {
         hitGroupDesc->IntersectionShaderImport
             = intersectionShaderFunction->zExportDesc()->Name;
+        intersectionShaderFunction->getThis();
     }
 }
 

+ 35 - 14
Framework Tests/Array.cpp

@@ -74,19 +74,31 @@ namespace FrameworkTests
             array.add(0);
             array.remove(3);
             array.remove(1);
-            array.remove(0);
-            Assert::IsTrue(array.getEntryCount() == 1,
-                L"after adding 4 elements and removing 3 elements "
-                L"getEntryCount() should be 1");
+            array.begin().remove();
+            array.add(30);
+            (++array.begin()).remove();
+            array.begin().remove();
+            array.add(20);
+            array.add(40);
+            Assert::IsTrue(array.getEntryCount() == 2,
+                L"after adding 7 elements and removing 5 elements "
+                L"getEntryCount() should be 2");
             Assert::IsTrue(array.get(0) == 20,
                 L"invalid value at index 0 of array after removing elements");
+            Assert::IsTrue(array.get(1) == 40,
+                L"invalid value at index 1 of array after removing elements");
             Assert::IsTrue(array.begin().val() == 20,
                 L"invalid value at index 0 of array after removing elements");
-            Assert::IsFalse(array.begin().hasNext(),
+            Assert::IsTrue(array.begin().next().val() == 40,
+                L"invalid value at index 1 of array after removing elements");
+            Assert::IsFalse(array.begin().next().hasNext(),
                 L"Iterator has to many elements after removing elements");
             auto end = array.end();
-            Assert::IsFalse(array.begin().next() != end,
+            Assert::IsFalse(array.begin().next().next() != end,
                 L"Iterator has to many elements after removing elements");
+            array.clear();
+            Assert::IsTrue(array.getEntryCount() == 0,
+                L"getEntryCount() should be 0 after the array was cleared");
         }
 
         TEST_METHOD (SwapTest)
@@ -259,22 +271,31 @@ namespace FrameworkTests
             array.add(new Test<RCArrayTests>(0, this));
             array.remove(3);
             array.remove(1);
-            array.remove(0);
-            Assert::IsTrue(array.getEntryCount() == 1,
-                L"after adding 4 elements and removing 3 elements "
-                L"getEntryCount() should be 1");
+            array.begin().remove();
+            array.add(new Test<RCArrayTests>(30, this));
+            (++array.begin()).remove();
+            array.begin().remove();
+            array.add(new Test<RCArrayTests>(20, this));
+            array.add(new Test<RCArrayTests>(40, this));
+            Assert::IsTrue(array.getEntryCount() == 2,
+                L"after adding 7 elements and removing 5 elements "
+                L"getEntryCount() should be 2");
             Assert::IsTrue((int)*array.z(0) == 20,
                 L"invalid value at index 0 of array after removing elements");
+            Assert::IsTrue((int)*array.z(1) == 40,
+                L"invalid value at index 1 of array after removing elements");
             Assert::IsTrue(array.begin()->getVal() == 20,
                 L"invalid value at index 0 of array after removing elements");
-            Assert::IsFalse(array.begin().hasNext(),
+            Assert::IsTrue(array.begin().next()->getVal() == 40,
+                L"invalid value at index 1 of array after removing elements");
+            Assert::IsFalse(array.begin().next().hasNext(),
                 L"Iterator has to many elements after removing elements");
             auto end = array.end();
-            Assert::IsFalse(array.begin().next() != end,
+            Assert::IsFalse(array.begin().next().next() != end,
                 L"Iterator has to many elements after removing elements");
-            Assert::IsTrue(deleteCounter == 3, L"Memory leaks detected");
+            Assert::IsTrue(deleteCounter == 5, L"Memory leaks detected");
             array.clear();
-            Assert::IsTrue(deleteCounter == 4, L"Memory leaks detected");
+            Assert::IsTrue(deleteCounter == 7, L"Memory leaks detected");
         }
 
         TEST_METHOD (SwapTest)

+ 6 - 0
Framework Tests/Camera3D.cpp

@@ -1,5 +1,7 @@
 #include "pch.h"
 
+#include <AsynchronCall.h>
+
 #include "Camera3D.h"
 #include "CppUnitTest.h"
 #include "DX12GraphicsApi.h"
@@ -341,6 +343,10 @@ namespace FrameworkTests
             });
 
             rTh.beginn();
+            new Framework::AsynchronCall([&window]() {
+                Sleep(10000);
+                Framework::StopMessageLoop(window.getWindowHandle());
+            });
             Framework::StartMessageLoop();
             rTh.terminate();
         }