Bug in GeometryBatchFactory?

I think i found a bug in this class, while trying to narrow down the reasons for my memory leak

Code:
/* * Copyright (c) 2009-2010 jMonkeyEngine * All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are * met: * * * Redistributions of source code must retain the above copyright * notice, this list of conditions and the following disclaimer. * * * Redistributions in binary form must reproduce the above copyright * notice, this list of conditions and the following disclaimer in the * documentation and/or other materials provided with the distribution. * * * Neither the name of 'jMonkeyEngine' nor the names of its contributors * may be used to endorse or promote products derived from this software * without specific prior written permission. * * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */

package jme3test.stress;

import java.util.logging.Level;
import java.util.logging.Logger;

import jme3tools.optimize.GeometryBatchFactory;

import com.jme3.app.SimpleApplication;
import com.jme3.material.Material;
import com.jme3.math.Vector3f;
import com.jme3.scene.Geometry;
import com.jme3.scene.Mesh;
import com.jme3.scene.Node;
import com.jme3.scene.Spatial.CullHint;
import com.jme3.scene.shape.Sphere;
import com.jme3.util.NativeObjectManager;

/**

  • Generates 400 new meshes every frame then leaks them. Notice how memory usage

  • stays constant and OpenGL objects are properly destroyed.
    */
    public class TestLeakingGL extends SimpleApplication {

    private Material solidColor;
    private Sphere original;

    public static void main(final String[] args) {
    final TestLeakingGL app = new TestLeakingGL();
    app.start();
    }

    @Override
    public void simpleInitApp() {
    original = new Sphere(4, 4, 1);
    original.setStatic();
    original.setInterleaved();

     // this will make sure all spheres are rendered always
     rootNode.setCullHint(CullHint.Never);
     solidColor = assetManager.loadMaterial("Common/Materials/RedColor.j3m");
     cam.setLocation(new Vector3f(0, 5, 0));
     cam.lookAt(Vector3f.ZERO, Vector3f.UNIT_Y);
    
     Logger.getLogger(Node.class.getName()).setLevel(Level.WARNING);
     Logger.getLogger(NativeObjectManager.class.getName()).setLevel(Level.WARNING);
    

    }

    @Override
    public void simpleUpdate(final float tpf) {
    rootNode.detachAllChildren();
    final Node unoptimized = new Node();
    for (int y = -15; y < 15; y++) {
    for (int x = -15; x < 15; x++) {
    final Mesh sphMesh = original.deepClone();
    final Geometry sphere = new Geometry("sphere", sphMesh);

     		sphere.setMaterial(solidColor);
     		sphere.setLocalTranslation(x * 1.5f, 0, y * 1.5f);
     		unoptimized.attachChild(sphere);
     	}
     }
    
     rootNode.attachChild(GeometryBatchFactory.optimize(unoptimized));
    

    }
    }


Crasdhes nearly instantly with
Code:
java.lang.IllegalArgumentException: Negative capacity: -10800 at java.nio.Buffer.<init>(Unknown Source) at java.nio.ByteBuffer.<init>(Unknown Source) at java.nio.ByteBuffer.<init>(Unknown Source) at java.nio.MappedByteBuffer.<init>(Unknown Source) at java.nio.DirectByteBuffer.<init>(Unknown Source) at java.nio.ByteBuffer.allocateDirect(Unknown Source) at com.jme3.util.BufferUtils.createFloatBuffer(BufferUtils.java:745) at com.jme3.scene.VertexBuffer.createBuffer(VertexBuffer.java:869) at jme3tools.optimize.GeometryBatchFactory.mergeGeometries(GeometryBatchFactory.java:164) at jme3tools.optimize.GeometryBatchFactory.makeBatches(GeometryBatchFactory.java:301) at jme3tools.optimize.GeometryBatchFactory.optimize(GeometryBatchFactory.java:352) at jme3tools.optimize.GeometryBatchFactory.optimize(GeometryBatchFactory.java:336) at jme3test.stress.TestLeakingGL.simpleUpdate(TestLeakingGL.java:95) at com.jme3.app.SimpleApplication.update(SimpleApplication.java:258) at com.jme3.system.lwjgl.LwjglAbstractDisplay.runLoop(LwjglAbstractDisplay.java:149) at com.jme3.system.lwjgl.LwjglDisplay.runLoop(LwjglDisplay.java:182) at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:223) at java.lang.Thread.run(Unknown Source)

I wonder if it makes it through even one frame correctly. You say “nearly instantly”… does that mean it tries to display it a few frames or dies instantly?

Why do you use deepClone()? clone(true) should do what you want.

@normen said:
Why do you use deepClone()? clone(true) should do what you want.


He doesn't. It's a JME test:
http://code.google.com/p/jmonkeyengine/source/browse/trunk/engine/src/test/jme3test/stress/TestLeakingGL.java

I use the original code of that application, that treis to create memory leaks, wich si exactly the reason i use it as a test basis.



I cannot see even one Frame, but since its kinda fast I could be missing it,however I should in worst case get a outofmemory but no the one i get there.

@EmpirePhoenix said:
I use the original code of that application, that treis to create memory leaks, wich si exactly the reason i use it as a test basis.

I cannot see even one Frame, but since its kinda fast I could be missing it,however I should in worst case get a outofmemory but no the one i get there.


Well, I was just wondering if you tried stepping in a debugger or something.

If GeometryBatchFactory completes once correctly in this case then it might be the cloning that's bugged somehow. And if it doesn't complete once, then figuring out which mesh(es) has the negative vertex count should be easy (in a debugger).

Well without the batching the aplication runs for hours without problems and is able to cleanup the memory as well, so i assume the error msut be in the batching. (Also can anyone confirm the error on his comuter with the test case?)

Did you try clone(true)?

That may be true but it is not a valid assumption even given the facts. The differences between running with the batching and without are pretty big and cover more than just the geometry batch factory.



The reason the app throws an exception is because some meshes are returning negative vertex counts. It could be that Mesh.getVertexCount() is bugged or that some other process is not calling the method to recalculated the vertex count. Maybe deepClone().

If i only use clone() this happens

Code:
java.lang.IllegalStateException: Should update counts before interleave at com.jme3.scene.Mesh.updateCounts(Mesh.java:688) at com.jme3.scene.Mesh.setBuffer(Mesh.java:942) at jme3tools.optimize.GeometryBatchFactory.mergeGeometries(GeometryBatchFactory.java:169) at jme3tools.optimize.GeometryBatchFactory.makeBatches(GeometryBatchFactory.java:301) at jme3tools.optimize.GeometryBatchFactory.optimize(GeometryBatchFactory.java:352) at jme3tools.optimize.GeometryBatchFactory.optimize(GeometryBatchFactory.java:336) at jme3test.stress.TestLeakingGL.simpleUpdate(TestLeakingGL.java:95) at com.jme3.app.SimpleApplication.update(SimpleApplication.java:258) at com.jme3.system.lwjgl.LwjglAbstractDisplay.runLoop(LwjglAbstractDisplay.java:149) at com.jme3.system.lwjgl.LwjglDisplay.runLoop(LwjglDisplay.java:182) at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:223) at java.lang.Thread.run(Unknown Source)

Hm I just did a quick test with the origninal unmodified version, and saw that the said vertexamount is negative.



This in Mesh-java seems to be the culprit I guess.

Code:
public Mesh deepClone(){ try{ Mesh clone = (Mesh) super.clone(); clone.meshBound = meshBound != null ? meshBound.clone() : null;
        // TODO: Collision tree cloning
        //clone.collisionTree = collisionTree != null ? collisionTree : null;
        clone.collisionTree = null; // it will get re-generated in any case

        clone.buffers = new IntMap&lt;VertexBuffer&gt;();
        clone.buffersList = new SafeArrayList&lt;VertexBuffer&gt;(VertexBuffer.class);
        for (Entry&lt;VertexBuffer&gt; ent : buffers){
            VertexBuffer bufClone = ent.getValue().clone();
            clone.buffers.put(ent.getKey(), bufClone);
            clone.buffersList.add(bufClone);
        }
        
        clone.vertexArrayID = -1;
        clone.vertCount = -1;
        clone.elementCount = -1;
        
        // although this could change
        // if the bone weight/index buffers are modified
        clone.maxNumWeights = maxNumWeights; 
        
        clone.elementLengths = elementLengths != null ? elementLengths.clone() : null;
        clone.modeStart = modeStart != null ? modeStart.clone() : null;
        return clone;
    }catch (CloneNotSupportedException ex){
        throw new AssertionError();
    }
}</div>

Interleaved meshes aren’t supported well by the engine, and this test uses it.

See the javadoc for Mesh.setInterleaved()

Interleaves the data in this mesh. This operation cannot be reversed.
Some GPUs may prefer the data in this format, however it is a good idea
to avoid using this method as it disables some engine features.