[fixed]Memory leak

[hybrid]

I did some valgrinding using the dwarf mesh and couldn't find any leaked mem besides the usual X11/OpenGL stuff. I'll check with b3d as well (thre had been some mem leaks in previous versions, do you use the latest SVN trunk?)

[Iaro]

No. I'm using 1.4. I'll also try the SVN version. I did some more tests.

1. normal b3d mesh, 3000 iterations - did not stabilize.
2. a very simple b3d mesh - 3 textures and no lightmaps, 3000 iterations - same problem.
3. the mesh from 2 in x format - no problem (at least regarding memory, the mesh wasn't correct after exporting from Blender).

Here you can get an archive with a simple b3d file for testing, the code i use for testing and the output data for the three cases above.

Many thanks for everyone's help so far.

[hybrid]

I just did a test with another (quite large) b3d model and had no problems at all. Will try your mesh as well, though.
Edit: No memleaks with your application and model, so it's either the 1.4.x SDK, or it's just a wrong memory report.

[Iaro]

I downloaded the SVN build 1471 from the nightly build server, and now it's working fine .
Some more test I did:

1. very simple mesh, no lightmaps , 3000 iterations - stable
2. normal b3d (textures+lightmaps), 1000 iterations - stable after 4 iterations.
3. alternating between loading two meshes, 500 iterations stable after 3 iterations.
4. alternating between a normal mesh and a huge mesh with hundreds of textures and lightmaps, 250 iterations - stable

Again many thanks for everyone's help.

[rogerborg]

I got Fortify working after a fashion, and .x and .md2 do seem fine, but there does appear to be issue with (at least some) b3d files, e.g.:

https://first-king.svn.sf.net/svnroot/f ... /ninja.b3d
(and its associated textures in that directory).

Four textures are being loaded in CB3DMeshFileLoader::readChunkTEXS() that aren't being cleaned up successfully by this code:

Code: Select all

        animatedMesh = node->getMesh();
        IMesh * mesh = animatedMesh->getMesh(0);

        u32 meshBufferCount = mesh->getMeshBufferCount();

        for (u32 mb = 0; mb < meshBufferCount; mb++)
        {
            IMeshBuffer * meshBuffer = mesh->getMeshBuffer(mb);

            for(int texture = 0; texture < MATERIAL_MAX_TEXTURES; ++texture)
            {
                ITexture * toRemove = meshBuffer->getMaterial().getTexture(texture);
                if(toRemove)
                    driver->removeTexture(toRemove);
            }
        }

        for(u32 materials = 0; materials < node->getMaterialCount(); ++materials)
        {
            SMaterial & thisMaterial = node->getMaterial(materials);
            for(u32 textures = 0; textures < MATERIAL_MAX_TEXTURES; ++textures)
            {
                ITexture * texture = thisMaterial.getTexture(textures);
                if(texture)
                    driver->removeTexture(texture);
            }
        }

They are eventually cleaned up CNullDriver::deleteAllTextures() on exit.

They're only being 'leaked' once per load of that mesh, no matter how many times it's loaded / removed, so it's probably not major, but it is annoying me, so I'll keep picking at it.

[hybrid]

Yes, the b3d loader loads textures directly, i.e. at the moment it encounters the filename. Could be delayed until the first material uses this "brush" to avoid unnecessary loading (which will also use up video memory...). Would you provide a patch

[rogerborg]

Bah, I'd just come to the same conclusion. No problem, although real life beckons first.

[rogerborg]

Works For Me. Thanks for taunting us into looking at this, Iaro.

Code: Select all

Index: source/Irrlicht/CB3DMeshFileLoader.cpp
===================================================================
--- source/Irrlicht/CB3DMeshFileLoader.cpp	(revision 1477)
+++ source/Irrlicht/CB3DMeshFileLoader.cpp	(working copy)
@@ -228,7 +228,29 @@
 	return true;
 }
 
+// This isn't called until we're sure that we really need the textures in this
+// material, so that we don't load textures into the cache unnecessarily.
+void CB3DMeshFileLoader::loadB3DMaterialTextures(SB3dMaterial & b3dMaterial)
+{
+	for (u32 layer = 0; layer < video::MATERIAL_MAX_TEXTURES; ++layer)
+	{
+		SB3dTexture * b3dTexture = b3dMaterial.Textures[layer];
+		if (b3dTexture)
+		{
+			video::ITexture * texture = SceneManager->getVideoDriver()->getTexture ( 
+											b3dTexture->TextureName.c_str() );
 
+			b3dMaterial.Material.setTexture(layer, texture);
+			if (b3dTexture->Flags & 0x10) // Clamp U
+				b3dMaterial.Material.TextureLayer[layer].TextureWrap=video::ETC_CLAMP;
+			if (b3dTexture->Flags & 0x20) // Clamp V, TODO: Needs another attribute
+				b3dMaterial.Material.TextureLayer[layer].TextureWrap=video::ETC_CLAMP;
+		}
+	}
+}
+
+
+
 bool CB3DMeshFileLoader::readChunkMESH(CSkinnedMesh::SJoint *InJoint)
 {
 	const s32 vertices_Start=BaseVertices.size(); //B3Ds have Vertex ID's local within the mesh I don't want this
@@ -261,7 +283,10 @@
 			scene::SSkinMeshBuffer *MeshBuffer = AnimatedMesh->createBuffer();
 
 			if (brush_id!=-1)
+			{
+				loadB3DMaterialTextures(Materials[brush_id]);
 				MeshBuffer->Material=Materials[brush_id].Material;
+			}
 
 			if(readChunkTRIS(MeshBuffer,AnimatedMesh->getMeshBuffers().size()-1, vertices_Start)==false)
 				return false;
@@ -424,7 +449,10 @@
 	SB3dMaterial *B3dMaterial;
 
 	if (triangle_brush_id != -1)
+	{
+		loadB3DMaterialTextures(Materials[triangle_brush_id]);
 		B3dMaterial = &Materials[triangle_brush_id];
+	}
 	else
 		B3dMaterial = 0;
 
@@ -633,11 +661,13 @@
 
 	while((B3dStack.getLast().startposition + B3dStack.getLast().length) > B3DFile->getPos()) //this chunk repeats
 	{
-		core::stringc textureName=readString();
-		textureName=stripPathFromString(B3DFile->getFileName(),true) + stripPathFromString(textureName,false);
-
 		SB3dTexture B3dTexture;
 
+		// Store the texture name, but don't load it until we're sure that it's really used.
+		B3dTexture.TextureName = readString();
+		B3dTexture.TextureName = stripPathFromString(B3DFile->getFileName(),true) 
+								+ stripPathFromString(B3dTexture.TextureName,false);
+
 		B3DFile->read(&B3dTexture.Flags, sizeof(s32));
 		B3DFile->read(&B3dTexture.Blend, sizeof(s32));
 #ifdef __BIG_ENDIAN__
@@ -654,7 +684,6 @@
 		// note that mipmaps might be disabled by Flags & 0x8
 		const bool doMipMaps = SceneManager->getVideoDriver()->getTextureCreationFlag(video::ETCF_CREATE_MIP_MAPS);
 		SceneManager->getVideoDriver()->setTextureCreationFlag(video::ETCF_CREATE_MIP_MAPS, (B3dTexture.Flags & 0x8) ? true:false);
-		B3dTexture.Texture=SceneManager->getVideoDriver()->getTexture ( textureName.c_str() );
 		SceneManager->getVideoDriver()->setTextureCreationFlag(video::ETCF_CREATE_MIP_MAPS, doMipMaps);
 		Textures.push_back(B3dTexture);
 	}
@@ -750,17 +779,6 @@
 			B3dMaterial.Textures[1] = 0;
 		}
 
-		for (i=0; i<2; ++i)
-		{
-			if (B3dMaterial.Textures[i] != 0)
-			{
-				B3dMaterial.Material.setTexture(i, B3dMaterial.Textures[i]->Texture);
-				if (B3dMaterial.Textures[i]->Flags & 0x10) // Clamp U
-					B3dMaterial.Material.TextureLayer[i].TextureWrap=video::ETC_CLAMP;
-				if (B3dMaterial.Textures[i]->Flags & 0x20) // Clamp V, TODO: Needs another attribute
-					B3dMaterial.Material.TextureLayer[i].TextureWrap=video::ETC_CLAMP;
-			}
-		}
 
 		//------ Convert blitz flags/blend to irrlicht -------
 
Index: source/Irrlicht/CB3DMeshFileLoader.h
===================================================================
--- source/Irrlicht/CB3DMeshFileLoader.h	(revision 1477)
+++ source/Irrlicht/CB3DMeshFileLoader.h	(working copy)
@@ -66,7 +66,7 @@
 
 	struct SB3dTexture
 	{
-		video::ITexture* Texture;
+		core::stringc TextureName;
 		s32 Flags;
 		s32 Blend;
 		f32 Xpos;
@@ -103,6 +103,7 @@
 	bool readChunkTEXS();
 	bool readChunkBRUS();
 
+    void loadB3DMaterialTextures(SB3dMaterial & b3dMaterial);
 	core::stringc readString();
 	core::stringc stripPathFromString(const core::stringc& string, bool returnPath=false) const;
 	void readFloats(f32* vec, u32 count);

Test code:

Step into driver->removeTexture(toRemove); and verify that there's only 2 texures in the cache, and that one of them is removed, leaving only the default font texture.

Code: Select all

#include <windows.h>

#include "irrlicht.h"
#pragma comment(lib, "Irrlicht.lib")
using namespace irr;
using namespace core;
using namespace gui;
using namespace io;
using namespace video;
using namespace scene;

int main()
{
    IrrlichtDevice *device;
    IVideoDriver* driver;
    ISceneManager* smgr;

    device = createDevice(EDT_OPENGL, dimension2d<s32>(800,600),32,false, false, false);
    driver = device->getVideoDriver();
    smgr = device->getSceneManager();
    smgr->addCameraSceneNodeFPS();

//    for(int iterations = 0; iterations < 100; ++iterations)
    {
        // Add it...
        IAnimatedMesh * animatedMesh = smgr->getMesh("../../media/ninja.b3d");
        IAnimatedMeshSceneNode * node = smgr->addAnimatedMeshSceneNode(animatedMesh);

        // Take it out...
        animatedMesh = node->getMesh();
        IMesh * mesh = animatedMesh->getMesh(0);
        node->remove();
        node = 0;

        u32 meshBufferCount = mesh->getMeshBufferCount();

        for (u32 mb = 0; mb < meshBufferCount; mb++)
        {
            IMeshBuffer * meshBuffer = mesh->getMeshBuffer(mb);

            for(int texture = 0; texture < MATERIAL_MAX_TEXTURES; ++texture)
            {
                ITexture * toRemove = meshBuffer->getMaterial().getTexture(texture);
                if(toRemove)
                    driver->removeTexture(toRemove);
            }
        }

        smgr->getMeshCache()->removeMesh(animatedMesh);
    }

    device->drop();
    return 0;
}

[Iaro]

No problem . Thanks for writing the patch Rogerborg. The b3d format is really useful for getting data from Blender, and much easier to use than 2 intermediary obj files (for textures and lightmaps) that have to be combined.

[hybrid]

I think the b3d loader in trunk should work ok now.