Revert "Removing tests which depend on wrap modes, in preparation for removing the wrap modes themselves."

This reverts commit 917d3b8c953c7fa6ceb3ab232dab54b78d57751c.

tests/TestAStarPathfinder.cpp

@@ -40,7 +40,7 @@ bool testVoxelValidator(const VolumeType* volData, const Vector3DInt32& v3dPos)
 		return false;
 	}
 
-	typename VolumeType::VoxelType voxel = volData->getVoxel(v3dPos);
+	typename VolumeType::VoxelType voxel = volData->getVoxel(v3dPos, WrapModes::Validate);  // FIXME use templatised version of getVoxel(), but watch out for Linux compile issues.
 	if(voxel != 0)
 	{
 		return false;

tests/testvolume.cpp

@@ -42,16 +42,22 @@ inline int32_t cantorTupleFunction(int32_t previousResult, int32_t value)
 /*
  * Funtions for testing iteration in a forwards direction
  */
+
+// We allow user provided offset in this function so we can test the case when all samples are inside a volume and also the case when some samples are outside.
+// This is important because samplers are often slower when outside the volume as they have to fall back on directly accessing the volume data.
 template <typename VolumeType>
-int32_t testDirectAccessWithWrappingForwards(const VolumeType* volume)
+int32_t testDirectAccessWithWrappingForwards(const VolumeType* volume, int lowXOffset, int lowYOffset, int lowZOffset, int highXOffset, int highYOffset, int highZOffset)
 {
 	int32_t result = 0;
 
-	for(int z = volume->getEnclosingRegion().getLowerZ() + 1; z < volume->getEnclosingRegion().getUpperZ(); z++)
+	// If we know that we are only iterating over voxels internal to the volume then we can avoid calling the 'wrapping' function. This should be faster.
+	bool bAllVoxelsInternal = (lowXOffset > 0) && (lowYOffset > 0) && (lowZOffset > 0) && (highXOffset < 0) && (highYOffset < 0) && (highZOffset < 0);
+
+	for(int z = volume->getEnclosingRegion().getLowerZ() + lowZOffset; z <= volume->getEnclosingRegion().getUpperZ() + highZOffset; z++)
 	{
-		for(int y = volume->getEnclosingRegion().getLowerY() + 1; y < volume->getEnclosingRegion().getUpperY(); y++)
+		for(int y = volume->getEnclosingRegion().getLowerY() + lowYOffset; y <= volume->getEnclosingRegion().getUpperY() + highYOffset; y++)
 		{
-			for(int x = volume->getEnclosingRegion().getLowerX() + 1; x < volume->getEnclosingRegion().getUpperX(); x++)
+			for(int x = volume->getEnclosingRegion().getLowerX() + lowXOffset; x <= volume->getEnclosingRegion().getUpperX() + highXOffset; x++)
 			{
 				//Three level loop now processes 27 voxel neighbourhood
 				for(int innerZ = -1; innerZ <=1; innerZ++)
@@ -60,7 +66,16 @@ int32_t testDirectAccessWithWrappingForwards(const VolumeType* volume)
 					{
 						for(int innerX = -1; innerX <=1; innerX++)
 						{
-							result = cantorTupleFunction(result, volume->getVoxel(x + innerX, y + innerY, z + innerZ));
+							// Deeply nested 'if', but this is just a unit test and we should still
+							// see some performance improvement by skipping the wrapping versions.
+							if(bAllVoxelsInternal)
+							{
+								result = cantorTupleFunction(result, volume->getVoxel(x + innerX, y + innerY, z + innerZ));
+							}
+							else
+							{
+								result = cantorTupleFunction(result, volume->getVoxel(x + innerX, y + innerY, z + innerZ, WrapModes::Border, 3));
+							}
 						}
 					}
 				}	
@@ -73,7 +88,7 @@ int32_t testDirectAccessWithWrappingForwards(const VolumeType* volume)
 }
 
 template <typename VolumeType>
-int32_t testSamplersWithWrappingForwards(VolumeType* volume)
+int32_t testSamplersWithWrappingForwards(VolumeType* volume, int lowXOffset, int lowYOffset, int lowZOffset, int highXOffset, int highYOffset, int highZOffset)
 {
 	int32_t result = 0;
 
@@ -86,14 +101,14 @@ int32_t testSamplersWithWrappingForwards(VolumeType* volume)
 	ySampler.setWrapMode(WrapModes::Border, 3);
 	zSampler.setWrapMode(WrapModes::Border, 3);
 
-	zSampler.setPosition(volume->getEnclosingRegion().getLowerX() + 1, volume->getEnclosingRegion().getLowerY() + 1, volume->getEnclosingRegion().getLowerZ() + 1);
+	zSampler.setPosition(volume->getEnclosingRegion().getLowerX() + lowXOffset, volume->getEnclosingRegion().getLowerY() + lowYOffset, volume->getEnclosingRegion().getLowerZ() + lowZOffset);
-	for(int z = volume->getEnclosingRegion().getLowerZ() + 1; z < volume->getEnclosingRegion().getUpperZ(); z++)
+	for(int z = volume->getEnclosingRegion().getLowerZ() + lowZOffset; z <= volume->getEnclosingRegion().getUpperZ() + highZOffset; z++)
 	{
 		ySampler = zSampler;
-		for(int y = volume->getEnclosingRegion().getLowerY() + 1; y < volume->getEnclosingRegion().getUpperY(); y++)
+		for(int y = volume->getEnclosingRegion().getLowerY() + lowYOffset; y <= volume->getEnclosingRegion().getUpperY() + highYOffset; y++)
 		{
 			xSampler = ySampler;
-			for(int x = volume->getEnclosingRegion().getLowerX() + 1; x < volume->getEnclosingRegion().getUpperX(); x++)
+			for(int x = volume->getEnclosingRegion().getLowerX() + lowXOffset; x <= volume->getEnclosingRegion().getUpperX() + highXOffset; x++)
 			{
 				xSampler.setPosition(x, y, z); // HACK - Accessing a volume through multiple samplers currently breaks the PagedVolume.
 
@@ -140,16 +155,22 @@ int32_t testSamplersWithWrappingForwards(VolumeType* volume)
 /*
  * Funtions for testing iteration in a backwards direction
  */
+
+// We allow user provided offset in this function so we can test the case when all samples are inside a volume and also the case when some samples are outside.
+// This is important because samplers are often slower when outside the volume as they have to fall back on directly accessing the volume data.
 template <typename VolumeType>
-int32_t testDirectAccessWithWrappingBackwards(const VolumeType* volume)
+int32_t testDirectAccessWithWrappingBackwards(const VolumeType* volume, int lowXOffset, int lowYOffset, int lowZOffset, int highXOffset, int highYOffset, int highZOffset)
 {
 	int32_t result = 0;
 
-	for(int z = volume->getEnclosingRegion().getUpperZ() - 1; z > volume->getEnclosingRegion().getLowerZ(); z--)
+	// If we know that we are only iterating over voxels internal to the volume then we can avoid calling the 'wrapping' function. This should be faster.
+	bool bAllVoxelsInternal = (lowXOffset > 0) && (lowYOffset > 0) && (lowZOffset > 0) && (highXOffset < 0) && (highYOffset < 0) && (highZOffset < 0);
+
+	for(int z = volume->getEnclosingRegion().getUpperZ() + highZOffset; z >= volume->getEnclosingRegion().getLowerZ() + lowZOffset; z--)
 	{
-		for(int y = volume->getEnclosingRegion().getUpperY() - 1; y > volume->getEnclosingRegion().getLowerY(); y--)
+		for(int y = volume->getEnclosingRegion().getUpperY() + highYOffset; y >= volume->getEnclosingRegion().getLowerY() + lowYOffset; y--)
 		{
-			for(int x = volume->getEnclosingRegion().getUpperX() - 1; x > volume->getEnclosingRegion().getLowerX(); x--)
+			for(int x = volume->getEnclosingRegion().getUpperX() + highXOffset; x >= volume->getEnclosingRegion().getLowerX() + lowXOffset; x--)
 			{
 				//Three level loop now processes 27 voxel neighbourhood
 				for(int innerZ = -1; innerZ <=1; innerZ++)
@@ -158,7 +179,16 @@ int32_t testDirectAccessWithWrappingBackwards(const VolumeType* volume)
 					{
 						for(int innerX = -1; innerX <=1; innerX++)
 						{
-							result = cantorTupleFunction(result, volume->getVoxel(x + innerX, y + innerY, z + innerZ));
+							// Deeply nested 'if', but this is just a unit test and we should still
+							// see some performance improvement by skipping the wrapping versions.
+							if(bAllVoxelsInternal)
+							{
+								result = cantorTupleFunction(result, volume->getVoxel(x + innerX, y + innerY, z + innerZ));
+							}
+							else
+							{
+								result = cantorTupleFunction(result, volume->getVoxel(x + innerX, y + innerY, z + innerZ, WrapModes::Border, 3));
+							}
 						}
 					}
 				}	
@@ -171,7 +201,7 @@ int32_t testDirectAccessWithWrappingBackwards(const VolumeType* volume)
 }
 
 template <typename VolumeType>
-int32_t testSamplersWithWrappingBackwards(VolumeType* volume)
+int32_t testSamplersWithWrappingBackwards(VolumeType* volume, int lowXOffset, int lowYOffset, int lowZOffset, int highXOffset, int highYOffset, int highZOffset)
 {
 	int32_t result = 0;
 
@@ -184,14 +214,14 @@ int32_t testSamplersWithWrappingBackwards(VolumeType* volume)
 	ySampler.setWrapMode(WrapModes::Border, 3);
 	zSampler.setWrapMode(WrapModes::Border, 3);
 
-	zSampler.setPosition(volume->getEnclosingRegion().getUpperX() - 1, volume->getEnclosingRegion().getUpperY() - 1, volume->getEnclosingRegion().getUpperZ() - 1);
+	zSampler.setPosition(volume->getEnclosingRegion().getUpperX() + highXOffset, volume->getEnclosingRegion().getUpperY() + highYOffset, volume->getEnclosingRegion().getUpperZ() + highZOffset);
-	for (int z = volume->getEnclosingRegion().getUpperZ() - 1; z > volume->getEnclosingRegion().getLowerZ(); z--)
+	for(int z = volume->getEnclosingRegion().getUpperZ() + highZOffset; z >= volume->getEnclosingRegion().getLowerZ() + lowZOffset; z--)
 	{
 		ySampler = zSampler;
-		for (int y = volume->getEnclosingRegion().getUpperY() - 1; y > volume->getEnclosingRegion().getLowerY(); y--)
+		for(int y = volume->getEnclosingRegion().getUpperY() + highYOffset; y >= volume->getEnclosingRegion().getLowerY() + lowYOffset; y--)
 		{
 			xSampler = ySampler;
-			for (int x = volume->getEnclosingRegion().getUpperX() - 1; x > volume->getEnclosingRegion().getLowerX(); x--)
+			for(int x = volume->getEnclosingRegion().getUpperX() + highXOffset; x >= volume->getEnclosingRegion().getLowerX() + lowXOffset; x--)
 			{
 				xSampler.setPosition(x, y, z); // HACK - Accessing a volume through multiple samplers currently breaks the PagedVolume.
 
@@ -280,9 +310,9 @@ void TestVolume::testRawVolumeDirectAccessAllInternalForwards()
 
 	QBENCHMARK
 	{
-		result = testDirectAccessWithWrappingForwards(m_pRawVolume);
+		result = testDirectAccessWithWrappingForwards(m_pRawVolume, 4, 2, 2, -3, -1, -2);
 	}
-	QCOMPARE(result, static_cast<int32_t>(199594219));
+	QCOMPARE(result, static_cast<int32_t>(1004598054));
 }
 
 void TestVolume::testRawVolumeSamplersAllInternalForwards()
@@ -291,9 +321,31 @@ void TestVolume::testRawVolumeSamplersAllInternalForwards()
 
 	QBENCHMARK
 	{
-		result = testSamplersWithWrappingForwards(m_pRawVolume);
+		result = testSamplersWithWrappingForwards(m_pRawVolume, 4, 2, 2, -3, -1, -2);
 	}
-	QCOMPARE(result, static_cast<int32_t>(199594219));
+	QCOMPARE(result, static_cast<int32_t>(1004598054));
+}
+
+void TestVolume::testRawVolumeDirectAccessWithExternalForwards()
+{
+	int32_t result = 0;
+
+	QBENCHMARK
+	{
+		result = testDirectAccessWithWrappingForwards(m_pRawVolume, -1, -3, -2, 2, 5, 4);
+	}
+	QCOMPARE(result, static_cast<int32_t>(-928601007));
+}
+
+void TestVolume::testRawVolumeSamplersWithExternalForwards()
+{
+	int32_t result = 0;
+
+	QBENCHMARK
+	{
+		result = testSamplersWithWrappingForwards(m_pRawVolume, -1, -3, -2, 2, 5, 4);
+	}
+	QCOMPARE(result, static_cast<int32_t>(-928601007));
 }
 
 void TestVolume::testRawVolumeDirectAccessAllInternalBackwards()
@@ -302,9 +354,9 @@ void TestVolume::testRawVolumeDirectAccessAllInternalBackwards()
 
 	QBENCHMARK
 	{
-		result = testDirectAccessWithWrappingBackwards(m_pRawVolume);
+		result = testDirectAccessWithWrappingBackwards(m_pRawVolume, 4, 2, 2, -3, -1, -2);
 	}
-	QCOMPARE(result, static_cast<int32_t>(-960618300));
+	QCOMPARE(result, static_cast<int32_t>(-269366578));
 }
 
 void TestVolume::testRawVolumeSamplersAllInternalBackwards()
@@ -313,9 +365,31 @@ void TestVolume::testRawVolumeSamplersAllInternalBackwards()
 
 	QBENCHMARK
 	{
-		result = testSamplersWithWrappingBackwards(m_pRawVolume);
+		result = testSamplersWithWrappingBackwards(m_pRawVolume, 4, 2, 2, -3, -1, -2);
 	}
-	QCOMPARE(result, static_cast<int32_t>(-960618300));
+	QCOMPARE(result, static_cast<int32_t>(-269366578));
+}
+
+void TestVolume::testRawVolumeDirectAccessWithExternalBackwards()
+{
+	int32_t result = 0;
+
+	QBENCHMARK
+	{
+		result = testDirectAccessWithWrappingBackwards(m_pRawVolume, -1, -3, -2, 2, 5, 4);
+	}
+	QCOMPARE(result, static_cast<int32_t>(-769775893));
+}
+
+void TestVolume::testRawVolumeSamplersWithExternalBackwards()
+{
+	int32_t result = 0;
+
+	QBENCHMARK
+	{
+		result = testSamplersWithWrappingBackwards(m_pRawVolume, -1, -3, -2, 2, 5, 4);
+	}
+	QCOMPARE(result, static_cast<int32_t>(-769775893));
 }
 
 /*
@@ -327,9 +401,9 @@ void TestVolume::testPagedVolumeDirectAccessAllInternalForwards()
 	int32_t result = 0;
 	QBENCHMARK
 	{
-		result = testDirectAccessWithWrappingForwards(m_pPagedVolume);
+		result = testDirectAccessWithWrappingForwards(m_pPagedVolume, 4, 2, 2, -3, -1, -2);
 	}
-	QCOMPARE(result, static_cast<int32_t>(199594219));
+	QCOMPARE(result, static_cast<int32_t>(1004598054));
 }
 
 void TestVolume::testPagedVolumeSamplersAllInternalForwards()
@@ -337,9 +411,29 @@ void TestVolume::testPagedVolumeSamplersAllInternalForwards()
 	int32_t result = 0;
 	QBENCHMARK
 	{
-		result = testSamplersWithWrappingForwards(m_pPagedVolume);
+		result = testSamplersWithWrappingForwards(m_pPagedVolume, 4, 2, 2, -3, -1, -2);
 	}
-	QCOMPARE(result, static_cast<int32_t>(199594219));
+	QCOMPARE(result, static_cast<int32_t>(1004598054));
+}
+
+void TestVolume::testPagedVolumeDirectAccessWithExternalForwards()
+{
+	int32_t result = 0;
+	QBENCHMARK
+	{
+		result = testDirectAccessWithWrappingForwards(m_pPagedVolume, -1, -3, -2, 2, 5, 4);
+	}
+	QCOMPARE(result, static_cast<int32_t>(-928601007));
+}
+
+void TestVolume::testPagedVolumeSamplersWithExternalForwards()
+{
+	int32_t result = 0;
+	QBENCHMARK
+	{
+		result = testSamplersWithWrappingForwards(m_pPagedVolume, -1, -3, -2, 2, 5, 4);
+	}
+	QCOMPARE(result, static_cast<int32_t>(-928601007));
 }
 
 void TestVolume::testPagedVolumeDirectAccessAllInternalBackwards()
@@ -347,9 +441,9 @@ void TestVolume::testPagedVolumeDirectAccessAllInternalBackwards()
 	int32_t result = 0;
 	QBENCHMARK
 	{
-		result = testDirectAccessWithWrappingBackwards(m_pPagedVolume);
+		result = testDirectAccessWithWrappingBackwards(m_pPagedVolume, 4, 2, 2, -3, -1, -2);
 	}
-	QCOMPARE(result, static_cast<int32_t>(-960618300));
+	QCOMPARE(result, static_cast<int32_t>(-269366578));
 }
 
 void TestVolume::testPagedVolumeSamplersAllInternalBackwards()
@@ -357,9 +451,29 @@ void TestVolume::testPagedVolumeSamplersAllInternalBackwards()
 	int32_t result = 0;
 	QBENCHMARK
 	{
-		result = testSamplersWithWrappingBackwards(m_pPagedVolume);
+		result = testSamplersWithWrappingBackwards(m_pPagedVolume, 4, 2, 2, -3, -1, -2);
 	}
-	QCOMPARE(result, static_cast<int32_t>(-960618300));
+	QCOMPARE(result, static_cast<int32_t>(-269366578));
+}
+
+void TestVolume::testPagedVolumeDirectAccessWithExternalBackwards()
+{
+	int32_t result = 0;
+	QBENCHMARK
+	{
+		result = testDirectAccessWithWrappingBackwards(m_pPagedVolume, -1, -3, -2, 2, 5, 4);
+	}
+	QCOMPARE(result, static_cast<int32_t>(-769775893));
+}
+
+void TestVolume::testPagedVolumeSamplersWithExternalBackwards()
+{
+	int32_t result = 0;
+	QBENCHMARK
+	{
+		result = testSamplersWithWrappingBackwards(m_pPagedVolume, -1, -3, -2, 2, 5, 4);
+	}
+	QCOMPARE(result, static_cast<int32_t>(-769775893));
 }
 
 QTEST_MAIN(TestVolume)

tests/testvolume.h

@@ -39,13 +39,21 @@ public:
 private slots:
 	void testRawVolumeDirectAccessAllInternalForwards();
 	void testRawVolumeSamplersAllInternalForwards();
+	void testRawVolumeDirectAccessWithExternalForwards();
+	void testRawVolumeSamplersWithExternalForwards();
 	void testRawVolumeDirectAccessAllInternalBackwards();
 	void testRawVolumeSamplersAllInternalBackwards();
+	void testRawVolumeDirectAccessWithExternalBackwards();
+	void testRawVolumeSamplersWithExternalBackwards();
 
 	void testPagedVolumeDirectAccessAllInternalForwards();
 	void testPagedVolumeSamplersAllInternalForwards();
+	void testPagedVolumeDirectAccessWithExternalForwards();
+	void testPagedVolumeSamplersWithExternalForwards();
 	void testPagedVolumeDirectAccessAllInternalBackwards();
 	void testPagedVolumeSamplersAllInternalBackwards();
+	void testPagedVolumeDirectAccessWithExternalBackwards();
+	void testPagedVolumeSamplersWithExternalBackwards();
 
 private:
 	PolyVox::FilePager<int32_t>* m_pFilePager;