Fix SMaterialLayer operator!= and optimize operator=

SMaterialLayers are now identical when both have identity matrices.
Before it didn't consider them identical when one layer had set the identity matrix explicitly and the other didn't.
Generally didn't matter, just caused very rarely some extra state switches in the drivers. And just as rarely had a cheaper comparison. Just seems more correct this way.

operator= no longer releases texture memory which was allocated at one point. 
Unless explicitly requested such memory is now always released later in the destructor.
This can avoid quite a few memory allocations/released in the driver. Usually not a noticeable performance difference on most platforms. But it can help avoid memory fragmentation.
We instead use an extra bool now to tell if the texture memory is used. So slight increase in SMaterialLayer and SMaterial size. But I did a quick performance test and this had no negative influence here, while it did improve speed in the case where it switched between material layers using/not using texture matrices a bit.

git-svn-id: svn://svn.code.sf.net/p/irrlicht/code/trunk@6488 dfc29bdd-3216-0410-991c-e03cc46cb475

changes.txt

@@ -1,6 +1,10 @@
 --------------------------
 Changes in 1.9 (not yet released)
 
+- Bugfix: SMaterialLayer::operator!= no longer returns true when comparing a layer without texture matrix with one with a set identity texture matrix. Those are the same.
+- SMaterialLayer no longer releases allocated texture memory before destructor unless explicitly requested.
+  This can avoid constantly allocation/releasing memory in the active driver material when setting materials.
+  Note that it does not copy texture memory when it's unused. This is still optimized.
 - Add IMeshSceneNode::setNodeRegistration to allow registering MeshSceneNodes to the SceneManager per buffer instead of per node
 - Add SMaterialLayer::hasSetTextureMatrix and SMaterialLayer::resetTextureMatrix
 - Add IShaderConstantSetCallBack::OnCreate to allow earlier access to IMaterialRendererServices

include/SMaterialLayer.h

@@ -51,7 +51,8 @@ namespace video
 	public:
 		//! Default constructor
 		SMaterialLayer() : Texture(0), TextureWrapU(ETC_REPEAT), TextureWrapV(ETC_REPEAT), TextureWrapW(ETC_REPEAT),
-			BilinearFilter(true), TrilinearFilter(false), AnisotropicFilter(0), LODBias(0), TextureMatrix(0)
+			BilinearFilter(true), TrilinearFilter(false), AnisotropicFilter(0), LODBias(0), 
+			TextureMatrix(0), TextureMatrixUsed(false)
 		{
 		}
 
@@ -84,26 +85,22 @@ namespace video
 				return *this;
 
 			Texture = other.Texture;
-			if (TextureMatrix)
+			if (other.TextureMatrixUsed)
 			{
-				if (other.TextureMatrix)
-					*TextureMatrix = *other.TextureMatrix;
-				else
+				if (TextureMatrix)
 				{
-					MatrixAllocator.destruct(TextureMatrix);
-					MatrixAllocator.deallocate(TextureMatrix);
-					TextureMatrix = 0;
+					*TextureMatrix = *other.TextureMatrix;
 				}
-			}
-			else
-			{
-				if (other.TextureMatrix)
+				else
 				{
 					TextureMatrix = MatrixAllocator.allocate(1);
 					MatrixAllocator.construct(TextureMatrix,*other.TextureMatrix);
 				}
-				else
-					TextureMatrix = 0;
+				TextureMatrixUsed = true;
+			}
+			else
+			{
+				TextureMatrixUsed = false;
 			}
 			TextureWrapU = other.TextureWrapU;
 			TextureWrapV = other.TextureWrapV;
@@ -124,6 +121,12 @@ namespace video
 			{
 				TextureMatrix = MatrixAllocator.allocate(1);
 				MatrixAllocator.construct(TextureMatrix,core::IdentityMatrix);
+				TextureMatrixUsed = true;
+			}
+			else if ( !TextureMatrixUsed )
+			{
+				*TextureMatrix = core::IdentityMatrix;
+				TextureMatrixUsed = true;
 			}
 			return *TextureMatrix;
 		}
@@ -132,7 +135,7 @@ namespace video
 		/** \return Texture matrix of this layer. */
 		const core::matrix4& getTextureMatrix() const
 		{
-			if (TextureMatrix)
+			if (TextureMatrixUsed)
 				return *TextureMatrix;
 			else
 				return core::IdentityMatrix;
@@ -151,25 +154,27 @@ namespace video
 			}
 			else
 				*TextureMatrix = mat;
+			TextureMatrixUsed = true;
 		}
 
 		//! Check if we have set a custom texture matrix
 		//! Note that otherwise we get an IdentityMatrix as default
 		inline bool hasSetTextureMatrix() const
 		{
-			return TextureMatrix != 0;
+			return TextureMatrixUsed;
 		}
 
 		//! Reset texture matrix to identity matrix
-		//! Releases memory, which is expensive, but ver rarely useful for optimizations
-		void resetTextureMatrix()
+		/** \param releaseMemory Releases also texture memory. Otherwise done in destructor */
+		void resetTextureMatrix(bool releaseMemory=true)
 		{
-			if ( TextureMatrix )
+			if ( TextureMatrix && releaseMemory)
 			{
 				MatrixAllocator.destruct(TextureMatrix);
 				MatrixAllocator.deallocate(TextureMatrix);
 				TextureMatrix = 0;
 			}
+			TextureMatrixUsed = false;
 		}
 
 		//! Inequality operator
@@ -189,8 +194,11 @@ namespace video
 			if (different)
 				return true;
 			else
-				different |= (TextureMatrix != b.TextureMatrix) &&
-					(!TextureMatrix || !b.TextureMatrix || (*TextureMatrix != *(b.TextureMatrix)));
+			{
+				different = (TextureMatrixUsed && b.TextureMatrixUsed && (*TextureMatrix != *b.TextureMatrix))
+				         || (TextureMatrixUsed && !b.TextureMatrixUsed && (*TextureMatrix != core::IdentityMatrix))
+				         || (!TextureMatrixUsed && b.TextureMatrixUsed && (core::IdentityMatrix != *b.TextureMatrix));
+			}
 			return different;
 		}
 
@@ -241,6 +249,7 @@ namespace video
 		/** Do not access this element directly as the internal
 		resource management has to cope with Null pointers etc. */
 		core::matrix4* TextureMatrix;
+		bool TextureMatrixUsed;	// TextureMatrix memory stays until destructor even when unused to avoid unnecessary allocation/de-allocations
 	};
 
 } // end namespace video

tests/material.cpp

@@ -71,9 +71,41 @@ static bool polygonOffset(video::E_DRIVER_TYPE type)
     return result;
 }
 
+static bool testSMaterial()
+{
+	irr::video::SMaterial a;
+	irr::video::SMaterial b;
+
+	// Same by default?
+	if ( !(a == b) )
+		return false;
+	if ( a != b )
+		return false;
+
+	// getting (creating) one texture matrix shouldn't change things
+	b.TextureLayer[0].getTextureMatrix();
+	if ( a != b )
+		return false;
+
+	// no longer same now
+	b.TextureLayer[0].getTextureMatrix().setTextureScale(5.f, 0.5f);
+	if ( a == b )
+		return false;
+
+	return true;
+}
+
 bool material()
 {
 	bool result = true;
+
 	TestWithAllDrivers(polygonOffset);
+
+	if ( !testSMaterial() )
+	{
+		logTestString("testSMaterial failed\n\n");
+		result = false;
+	}
+
 	return result;
 }

tests/tests-last-passed-at.txt

@@ -1,4 +1,4 @@
 Tests finished. 72 tests of 72 passed.
 Compiled as DEBUG
-Test suite pass at GMT Fri May 05 18:39:44 2023
+Test suite pass at GMT Sun May 07 14:26:39 2023