Subtle memory leak bug with LWJGLTextureState (and solution)

[Originally posted under troubleshooting, here: http://www.jmonkeyengine.com/jmeforum/index.php?topic=11718.new;topicseen]



Hey guys, I just spent the last two days tracking down a major memory leak which turned out to be indirectly caused by LWJGLTextureState. 



Take a look at the following code from the LWJGL Texture State, in the load (int) method:


        if (texture.getTextureKey() != null) {

            Texture cached = TextureManager.findCachedTexture(texture

                    .getTextureKey());

            if (cached == null) {

                TextureManager.addToCache(texture);

            } else if (cached.getTextureId() != 0) {

                texture.setTextureId(cached.getTextureId());

                GL11.glBindTexture(getGLType(type), cached.getTextureId());

                if (Debug.stats) {

                    StatCollector.addStat(StatType.STAT_TEXTURE_BINDS, 1);

                }

                if (record != null)

                    record.units[unit].boundTexture = texture.getTextureId();

                return;

            }

        }



   IntBuffer id = BufferUtils.createIntBuffer(1);

        id.clear();

        GL11.glGenTextures(id);

        texture.setTextureId(id.get(0));



As written now, LWJGLTextureState assumes that all textures are using JME's TextureManager. If a texture is not in the TextureManager's cache, then it adds it to the cache and reloads the texture - without checking to see if the texture has already been loaded (as indicated by texture.getTextureId being non-zero).

This works fine as long as the user is using JME's TextureManager for every texture, which may be true for many users, but it is entirely possible in JME to load a texutre without using the TextureManager, and in particular to create and use a texture with now TextureKey. In this case, TextureState will always load a new instance of the texture when load is run, leading to a big memory leak.

All you need to do to fix this problem is check whether the textureID is non-zero, and avoid creating a new texture in that case.

Heres the patch:

Index: LWJGLTextureState.java
===================================================================
--- LWJGLTextureState.java   (revision 4531)
+++ LWJGLTextureState.java   (working copy)
@@ -296,8 +296,15 @@
                     record.units[unit].boundTexture = texture.getTextureId();
                 return;
             }
+        }else if (texture.getTextureId()!=0)
+        {  
+            GL11.glBindTexture(GL11.GL_TEXTURE_2D, texture.getTextureId());
+            if (record != null)
+               record.units[unit].boundTexture = texture.getTextureId();
+            return;
         }
 
+
         IntBuffer id = BufferUtils.createIntBuffer(1);
         id.clear();
         GL11.glGenTextures(id);

Great job!  :-o

I was suffering some kind of memory leak, but I couldn't pick it out.

I'll try your patch.