From bec073c9470f849bfc5cf364ebb3a9ef5a5d3a8e Mon Sep 17 00:00:00 2001
From: Maciej Babinski <maciej@killer-robot.net>
Date: Tue, 15 May 2018 07:59:19 -0500
Subject: [PATCH] Switch expandRegions to using a rcTempVector of structs
 rather than a blob of ints. Also updates rcVector::clear() to not require the
 contained type to be default-constructible.

---
 Recast/Include/RecastAlloc.h   |  4 +++-
 Recast/Source/RecastRegion.cpp | 24 +++++++++++++++---------
 Tests/Recast/Tests_Recast.cpp  | 15 +++++++++++++++
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/Recast/Include/RecastAlloc.h b/Recast/Include/RecastAlloc.h
index c71c035..a048e28 100644
--- a/Recast/Include/RecastAlloc.h
+++ b/Recast/Include/RecastAlloc.h
@@ -123,10 +123,12 @@ class rcVectorBase {
 
 	void resize(rcSizeType size) { resize_impl(size, NULL); }
 	void resize(rcSizeType size, const T& value) { resize_impl(size, &value); }
+	// Not implemented as resize(0) because resize requires T to be default-constructible.
+	void clear() { destroy_range(0, m_size); m_size = 0; }
 
 	void push_back(const T& value);
 	void pop_back() { rcAssert(m_size > 0); back().~T(); m_size--; }
-	void clear() { resize(0); }
+
 	rcSizeType size() const { return m_size; }
 	rcSizeType capacity() const { return m_cap; }
 	bool empty() const { return size() == 0; }
diff --git a/Recast/Source/RecastRegion.cpp b/Recast/Source/RecastRegion.cpp
index ece2650..7d7c8df 100644
--- a/Recast/Source/RecastRegion.cpp
+++ b/Recast/Source/RecastRegion.cpp
@@ -343,6 +343,14 @@ static bool floodRegion(int x, int y, int i,
 	return count > 0;
 }
 
+// Struct to keep track of entries in the region table that have been changed.
+struct DirtyEntry {
+	DirtyEntry(int index_, unsigned short region_, unsigned short distance2_)
+		: index(index_), region(region_), distance2(distance2_) {}
+	int index;
+	unsigned short region;
+	unsigned short distance2;
+};
 static void expandRegions(int maxIter, unsigned short level,
 					      rcCompactHeightfield& chf,
 					      unsigned short* srcReg, unsigned short* srcDist,
@@ -384,12 +392,12 @@ static void expandRegions(int maxIter, unsigned short level,
 		}
 	}
 
-	rcIntArray dirtyEntries;
+	rcTempVector<DirtyEntry> dirtyEntries;
 	int iter = 0;
 	while (stack.size() > 0)
 	{
 		int failed = 0;
-		dirtyEntries.resize(0);
+		dirtyEntries.clear();
 		
 		for (int j = 0; j < stack.size(); j += 3)
 		{
@@ -425,9 +433,7 @@ static void expandRegions(int maxIter, unsigned short level,
 			if (r)
 			{
 				stack[j+2] = -1; // mark as used
-				dirtyEntries.push(i);
-				dirtyEntries.push(r);
-				dirtyEntries.push(d2);
+				dirtyEntries.push_back(DirtyEntry(i, r, d2));
 			}
 			else
 			{
@@ -436,10 +442,10 @@ static void expandRegions(int maxIter, unsigned short level,
 		}
 		
 		// Copy entries that differ between src and dst to keep them in sync.
-		for (int i = 0; i < dirtyEntries.size(); i+=3) {
-			int idx = dirtyEntries[i];
-			srcReg[idx] = (unsigned short)dirtyEntries[i+1];
-			srcDist[idx] = (unsigned short)dirtyEntries[i+2];
+		for (int i = 0; i < dirtyEntries.size(); i++) {
+			int idx = dirtyEntries[i].index;
+			srcReg[idx] = dirtyEntries[i].region;
+			srcDist[idx] = dirtyEntries[i].distance2;
 		}
 		
 		if (failed*3 == stack.size())
diff --git a/Tests/Recast/Tests_Recast.cpp b/Tests/Recast/Tests_Recast.cpp
index 11def67..7c12bc7 100644
--- a/Tests/Recast/Tests_Recast.cpp
+++ b/Tests/Recast/Tests_Recast.cpp
@@ -892,6 +892,10 @@ struct Copier {
 const int Copier::kAlive = 0x1f;
 const int Copier::kDead = 0xde;
 
+struct NotDefaultConstructible {
+	NotDefaultConstructible(int) {}
+};
+
 TEST_CASE("rcVector")
 {
 	SECTION("Vector basics.")
@@ -1068,6 +1072,17 @@ TEST_CASE("rcVector")
 		REQUIRE(Incrementor::destructions == 0);
 		REQUIRE(Incrementor::copies == 10);
 	}
+
+	SECTION("Type Requirements")
+	{
+		// This section verifies that we don't enforce unnecessary
+		// requirements on the types we hold.
+
+		// Implementing clear as resize(0) will cause this to fail
+		// as resize(0) requires T to be default constructible.
+		rcTempVector<NotDefaultConstructible> v;
+		v.clear();
+	}
 }
 
 // TODO: Implement benchmarking for platforms other than posix.