[fixed]TextureMatrix bug

[hybrid]

There is already material.getTextureMatrix(n)

[BlindSide]

But that one is an inline function that causes it to get allocated to the wrong heap. I thought we were on the same page here?

Ok, here is a realistic idea:

Use irrAllocator to allocate memory that can be used across dll boundaries.

[hybrid]

I just didn't knew what you were after. Also, the last time this issue came up, it was said that putting it into a separate compilation unit wouldn't help. But honestly, I'm probably also the wrong person to come up with a working solution, because Linux usually doesn't suffer from these problems. But I'm open to any solution which helps others to also make use of texture matrices.

[vitek]

I can't reproduce the problem if I build both the Irrlicht.dll and the executable with /MD.

Travis

[hybrid]

It seems that this is not enough for some people. And it might even make sense to put the really huge SMaterial methods into a separately compiled cpp file. I'm just not sure if this would help.

[BlindSide]

Yes! I fixed it. Tell me if there are any complaints with this method. The irrAllocator class has no member variables so it shouldn't actually take up any space inside SMaterial right?

Code: Select all

// Copyright (C) 2002-2007 Nikolaus Gebhardt
// This file is part of the "Irrlicht Engine".
// For conditions of distribution and use, see copyright notice in irrlicht.h

#ifndef __S_MATERIAL_LAYER_H_INCLUDED__
#define __S_MATERIAL_LAYER_H_INCLUDED__

#include "matrix4.h"
#include "irrAllocator.h"

namespace irr
{
namespace video
{
	class ITexture;

	//! Texture coord clamp mode outside [0.0, 1.0]
	enum E_TEXTURE_CLAMP
	{
		//! Texture repeats
		ETC_REPEAT = 0,
		//! Texture is clamped to the last pixel
		ETC_CLAMP,
		//! Texture is clamped to the edge pixel
		ETC_CLAMP_TO_EDGE,
		//! Texture is clamped to the border pixel (if exists)
		ETC_CLAMP_TO_BORDER,
		//! Texture is alternatingly mirrored (0..1..0..1..0..)
		ETC_MIRROR
	};
	static const char* const aTextureClampNames[] = {
			"texture_clamp_repeat",
			"texture_clamp_clamp",
			"texture_clamp_clamp_to_edge",
			"texture_clamp_clamp_to_border",
			"texture_clamp_mirror", 0};

	//! struct for holding material parameters which exist per texture layer
	class SMaterialLayer
	{
	private:
		irr::core::irrAllocator<irr::core::matrix4> MatrixAllocator;

	public:
		//! default constructor
		SMaterialLayer()
			: Texture(0), TextureMatrix(0),
				TextureWrap(ETC_REPEAT),
				BilinearFilter(true),
				TrilinearFilter(false),
				AnisotropicFilter(false)
			{}

		//! copy constructor
		SMaterialLayer(const SMaterialLayer& other)
		{
			// This pointer is checked during assignment
			TextureMatrix = 0;
			*this = other;
		}

		//! destructor
		~SMaterialLayer()
		{
			MatrixAllocator.destruct(TextureMatrix);
			MatrixAllocator.deallocate(TextureMatrix);
		}

		//! Assignment operator
		SMaterialLayer& operator=(const SMaterialLayer& other)
		{
			Texture = other.Texture;
			if (TextureMatrix)
			{
				if (other.TextureMatrix)
					*TextureMatrix = *other.TextureMatrix;
				else
				{
					MatrixAllocator.destruct(TextureMatrix);
					MatrixAllocator.deallocate(TextureMatrix);
					TextureMatrix = 0;
				}
			}
			else
			{
				if (other.TextureMatrix)
				{
					TextureMatrix = MatrixAllocator.allocate(1);
					MatrixAllocator.construct(TextureMatrix,irr::core::matrix4());
					TextureMatrix->setM(other.TextureMatrix->pointer());
				}
				else
					TextureMatrix = 0;
			}
			TextureWrap = other.TextureWrap;
			BilinearFilter = other.BilinearFilter;
			TrilinearFilter = other.TrilinearFilter;
			AnisotropicFilter = other.AnisotropicFilter;

			return *this;
		}

		//! Texture
		ITexture* Texture;

		//! Texture Matrix
		//! Do not access this element directly as the internal
		//! ressource management has to cope with Null pointers etc.
		core::matrix4* TextureMatrix;

		//! Texture Clamp Mode
		E_TEXTURE_CLAMP TextureWrap;

		//! Is bilinear filtering enabled? Default: true
		bool BilinearFilter;

		//! Is trilinear filtering enabled? Default: false
		/** If the trilinear filter flag is enabled,
		the bilinear filtering flag is ignored. */
		bool TrilinearFilter;

		//! Is anisotropic filtering enabled? Default: false
		/** In Irrlicht you can use anisotropic texture filtering
		    in conjunction with bilinear or trilinear texture
		    filtering to improve rendering results. Primitives
		    will look less blurry with this flag switched on. */
		bool AnisotropicFilter;

		//! Gets the texture transformation matrix
		core::matrix4& getTextureMatrix()
		{
			if (!TextureMatrix)
			{
				TextureMatrix = MatrixAllocator.allocate(1);
				MatrixAllocator.construct(TextureMatrix,irr::core::matrix4());
			}
			return *TextureMatrix;
		}

		//! Gets the immutable texture transformation matrix
		const core::matrix4& getTextureMatrix() const
		{
			if (TextureMatrix)
				return *TextureMatrix;
			else
				return core::IdentityMatrix;
		}

		//! Sets the texture transformation matrix to mat
		void setTextureMatrix(const core::matrix4& mat)
		{
			if (!TextureMatrix)
			{
				TextureMatrix = MatrixAllocator.allocate(1);
				MatrixAllocator.construct(TextureMatrix,irr::core::matrix4());
				TextureMatrix->setM(mat.pointer());
			}
			else
				*TextureMatrix = mat;
		}

		//! Inequality operator
		inline bool operator!=(const SMaterialLayer& b) const
		{
			bool different = 
				Texture != b.Texture ||
				TextureWrap != b.TextureWrap ||
				BilinearFilter != b.BilinearFilter ||
				TrilinearFilter != b.TrilinearFilter ||
				AnisotropicFilter != b.AnisotropicFilter;
			if (different)
				return true;
			else
				different |= (TextureMatrix != b.TextureMatrix);
			return different;
		}

		//! Equality operator
		inline bool operator==(const SMaterialLayer& b) const
		{ return !(b!=*this); }
	};

} // end namespace video
} // end namespace irr

#endif // __S_MATERIAL_LAYER_H_INCLUDED__

Cheers

[BlindSide]

So what do you think guys? I tested it and there were no problems when compiling with the static runtime. Its really a hassle to be forced to use the dynamic runtime as it requires user to install MSVC redistributable.

If you want they can be put in defines and only used when compiling on windows machines, to save a little performance on linux.

Cheers

[hybrid]

I still don't get where this helps (even though I do believe you that it does). After all, it's still completely inlined, so the new is still in user-code, just as the destructor call.

[BlindSide]

I think that because the irrAllocator uses an "internal_new" method thats private, it cant be inlined. Therefore it gets called at whatever space irrAllocator was initialised. In this case, the users application. Though I'm probably wrong about the reasoning, the comment on the original irrAllocator, the one that uses the internal functions states:

Code: Select all

//! Very simple allocator implementation, containers using it can
//! be used across dll boundaries

Where as the "irrAllocatorFast" comment states:

Code: Select all

//! fast allocator, only to be used in containers inside the same memory heap.
/** Containers using it are NOT able to be used it across dll boundaries. Use this
when using in an internal class or function or when compiled into a static lib */

Its actually more the fact that the Fast ones comment states that it can only be used in the same memory heap that got me thinking that the original may be a viable solution.

the new is still in user-code, just as the destructor call.

Well that fixes our problem then doesn't it? The matrix is both allocated and freed by the same user-code heap, therefore no corruption.

[hybrid]

Sorry, my fault. the meshbuffer is destructed in the library, so destructor for material is called from the library.
And IMHO access attributes don't inhibit inlining, because at that point (when the compiler inlines) access checks are already done.

[BlindSide]

This Microsoft Knowledge base article addresses the issue in depth: http://support.microsoft.com/kb/122675/en-us

Have a look at it, especially this part:

3. If you are using Visual C++ 2.0 or later, try using a template wrapper class. By using this templated class, you are deriving a class "on the fly" from the imported class. The constructor and the destructor for the new class are in the context of the EXE and not in the context of the DLL and the problem will be avoided. Please see the sample code below in the MORE INFORMATION section.

This seems to be what irrAllocator is doing.

Although the article seems to be stressing more on the destructor then the actual memory deallocation, and it stops counting MSVC versions at 5.0, so I'm not 100% if it applies in this context. I'll try and find someone who knows what they are talking about.

[Kriolyth]

Yes! I fixed it. Tell me if there are any complaints with this method. <...>

Your code works perfectly. Thanks!
And just one thing, concerning the same file:

Code: Select all

         if (different) 
            return true; 
         else 
            different |= (TextureMatrix != b.TextureMatrix);

We're comparing pointers that are never equal (due to our own construction and copying), so even same materials will be reported different. Corrected code is

Code: Select all

different |= (*TextureMatrix != *b.TextureMatrix);

Am I not mistaken?

[BlindSide]

Ahh good point, I think that would be the case with the original code too (I don't remember seeing any code to share TextureMatrix between materials.).

I really need more support on this before it can be integrated guys. If anyone has any definite proof that this method works or extensive windows programming (This problem only occurs on MSVC) knowledge please lend me a hand here.

[Kriolyth]

Ahh good point, I think that would be the case with the original code too (I don't remember seeing any code to share TextureMatrix between materials.).

Yes, this is from original code (1.4), so it is a global problem.
And a small correcture, since referencing NULL would cause an AV

Code: Select all

         if (different) 
            return true; 
         else if ( TextureMatrix == b.TextureMatrix )
            return different;
         else
            different |= (*TextureMatrix != *b.TextureMatrix);

I really need more support on this before it can be integrated guys. If anyone has any definite proof that this method works or extensive windows programming (This problem only occurs on MSVC) knowledge please lend me a hand here.

Here's what MSDN tells: http://msdn2.microsoft.com/en-us/library/ms810466.aspx

Notes on Heap Implementation
When applications or DLLs create private heaps, these live in the process space and are accessible process-wide. Any allocation of data made from a given heap should be freed for the same heap. (Allocation from one heap and free to another makes no sense.)

Actually, I don't think it is possible to twiddle switches and have a same heap for DLL and EXE, though I don't know default MSVC libraries well enough to claim taht they each allocate a private heap on load.
BTW, I tried using irrAllocatorFast instead of irrAllocator, and it caused corruption; irrAllocator works fine.

[Dorth]

errr, why not

Code: Select all

         if (different)
            return true;
         else if ( TextureMatrix == b.TextureMatrix )
            return false;
         else
            different |= (*TextureMatrix != *b.TextureMatrix); 

instead, since we already know it isn't true?

or

Code: Select all

         if (different || TextureMatrix == b.TextureMatrix)
            return different;
         else
            different |= (*TextureMatrix != *b.TextureMatrix);