Subtle memory leak bug with LWJGLTextureState (and solution)

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:

        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;

            }

        }else if (texture.getTextureId()!=0)

        {  

            GL11.glBindTexture(GL11.GL_TEXTURE_2D, texture.getTextureId());

            if (record != null)

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

            return;

        }




This may not effect most users, but for those who are not using the build in TextureManager, this bug is both really important, and very easy to miss.

nice find, could you submit it as a patch in the contribution depot?