diff --git a/changes.txt b/changes.txt index 10027554..66e641bf 100644 --- a/changes.txt +++ b/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 diff --git a/include/SMaterialLayer.h b/include/SMaterialLayer.h index 48698cd5..c8006d01 100644 --- a/include/SMaterialLayer.h +++ b/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 diff --git a/tests/material.cpp b/tests/material.cpp index 2a6dc1ae..d5b7e3bf 100644 --- a/tests/material.cpp +++ b/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; } diff --git a/tests/tests-last-passed-at.txt b/tests/tests-last-passed-at.txt index f7f59afa..a9989971 100644 --- a/tests/tests-last-passed-at.txt +++ b/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