Patch in Mesh.updateCounts()

i downloaded the nighty build,

wow after 3 weeks still not patched.



http://hub.jmonkeyengine.org/groups/general-2/forum/topic/potential-bug-in-mesh-updatecounts/



is it so hard change line :



690 : vertCount = pb.getData().capacity() / pb.getNumComponents();



to :



690 : vertCount = pb.getData().limit() / pb.getNumComponents();



thank you,

You’ve got a contributor tag mate, commit it. If the guys don’t like it, they can revert it. :slight_smile:

that seems cool, i havent done it before, any guide on how to do it ?

i also noticed that :



elementCount = computeNumElements(ib.getData().capacity());



also uses capacity method instead of limit(), so i dont want to do “hasty changes”

fix only 1 bug, risking for the same type of bug to persist in future versions.



Cant a buffer expert take a look at this ?



I will create a test case to help detect the bug.

@madjack said:
You've got a contributor tag mate

....oops, and you too again now :)
@normen said:
....oops, and you too again now :)


Thanks! :D

@tralala said:
i also noticed that :

elementCount = computeNumElements(ib.getData().capacity());

also uses capacity method instead of limit(), so i dont want to do "hasty changes"
fix only 1 bug, risking for the same type of bug to persist in future versions.

Cant a buffer expert take a look at this ?

I will create a test case to help detect the bug.


The best way to look at it is to make those changes locally and test if the behavior is the right one. If everything is fine post a test case including a diff at the end.

That way the core members will be able to take a look at your result and your fix and either tell you to commit or will commit it themselves or will tell you why it's not working, etc.
@tralala said:
that seems cool, i havent done it before, any guide on how to do it ?


I'm pretty sure there was a wiki on the subject but I can't find it. :'(

Test case showing the problem before the patch and after the patch.



Before :

http://i.imgur.com/zyLhi.png





After:

http://i.imgur.com/NBjZN.png



Patch:

[java]

public void updateCounts(){

if (getBuffer(Type.InterleavedData) != null)

throw new IllegalStateException(“Should update counts before interleave”);



VertexBuffer pb = getBuffer(Type.Position);

VertexBuffer ib = getBuffer(Type.Index);

if (pb != null){

vertCount = pb.getData().limit() / pb.getNumComponents();

}

if (ib != null){

elementCount = computeNumElements(ib.getData().limit());

}else{

elementCount = computeNumElements(vertCount);

}

}

[/java]



Test Case

[java]

package tests.jme;



import com.jme3.app.SimpleApplication;

import com.jme3.material.Material;

import com.jme3.material.RenderState.BlendMode;

import com.jme3.math.ColorRGBA;

import com.jme3.math.Vector3f;

import com.jme3.scene.Geometry;

import com.jme3.scene.Mesh;

import com.jme3.scene.VertexBuffer;

import com.jme3.scene.VertexBuffer.Format;

import com.jme3.scene.VertexBuffer.Usage;

import com.jme3.texture.Texture;

import com.jme3.util.BufferUtils;



import java.lang.reflect.Field;

import java.nio.Buffer;

import java.nio.ByteBuffer;

import java.nio.FloatBuffer;

import java.util.logging.Level;

import java.util.logging.Logger;



/**

  • What this test does.

    *
  • Allocates a buffer of 3000 sprites (capacity).

    *
  • then we fill it with content only 300 sprites. (so we set the limit to 300),
  • because we dont want jme to read the other 2700 spaces that are empty (unspecified) yet.

    *
  • why i need to do that ?
  • so that i can allocate the buffer only once and not need to change its size again later.

    /

    public class TestJmeReadsAfterBufferLimit extends SimpleApplication

    {

    public static class SimpleSprite

    {

    public Vector3f position;

    public float size = 1f;

    public ColorRGBA color;

    public float startX = 1f;

    public float startY = 1f;

    public float endX = 0f;

    public float endY = 0f;

    }

    Mesh mesh;

    SimpleSprite[] sprites = new SimpleSprite[3000]; // i have a big buffer of 3000 sprites allocated but i only use 300 of them.

    private int spritesNumActuallyUsed = 300;

    private boolean displayVertexCount = true;

    private boolean displayLimit = true;



    public static final boolean TEST_FIX_JME_BUG_HACK = false;



    public static void main(String[] args)

    {

    Logger.getLogger("").setLevel(Level.SEVERE);

    TestJmeReadsAfterBufferLimit app = new TestJmeReadsAfterBufferLimit();

    app.start();

    }



    @Override

    public void simpleInitApp()

    {

    getViewPort().setBackgroundColor(new ColorRGBA(0.7f, 0.8f, 1f, 1f));

    mesh = createSpriteMesh(sprites);

    for (int i = 0; i < sprites.length; i++)

    {

    sprites = new SimpleSprite();

    sprites.position = new Vector3f((float)(3
    Math.random()),(float) (3Math.random()),(float) (3Math.random()));

    sprites.color = ColorRGBA.White;

    }

    updateParticleData(mesh, sprites);



    Geometry geom = new Geometry("", mesh);

    Material mat = new Material(assetManager, "Common/MatDefs/Misc/Particle.j3md");

    Texture tex = assetManager.loadTexture("Interface/Logo/Cursor.png");

    tex.setMinFilter(Texture.MinFilter.NearestNoMipMaps);

    tex.setMagFilter(Texture.MagFilter.Nearest);

    geom.setMaterial(mat);

    mat.setTexture("Texture", tex);

    mat.setFloat("Quadratic", 200f);

    mat.setBoolean("PointSprite", true);

    mat.getAdditionalRenderState().setBlendMode(BlendMode.Alpha);

    mat.getAdditionalRenderState().setAlphaFallOff(0.01f);



    rootNode.attachChild(geom);

    }



    @Override

    public void simpleUpdate(float tpf)

    {

    super.simpleUpdate(tpf);

    sprites[(int) (spritesNumActuallyUsed * Math.random())].position.x += tpf; //change a random sprite.

    updateParticleData(mesh, sprites);

    }



    public void updateParticleData(Mesh mesh, SimpleSprite[] particles)

    {

    VertexBuffer pvb = mesh.getBuffer(VertexBuffer.Type.Position);

    FloatBuffer positions = (FloatBuffer) pvb.getData();



    VertexBuffer cvb = mesh.getBuffer(VertexBuffer.Type.Color);

    ByteBuffer colors = (ByteBuffer) cvb.getData();



    VertexBuffer svb = mesh.getBuffer(VertexBuffer.Type.Size);

    FloatBuffer sizes = (FloatBuffer) svb.getData();



    VertexBuffer tvb = mesh.getBuffer(VertexBuffer.Type.TexCoord);

    FloatBuffer texcoords = (FloatBuffer) tvb.getData();



    // update data in vertex buffers

    positions.rewind();

    colors.rewind();

    sizes.rewind();

    texcoords.rewind();

    for (int i = 0; i < spritesNumActuallyUsed; i++) //i modify only the first [spritesNumActuallyUsed] sprites, i leave the rest unmodified.

    {

    SimpleSprite p = particles;

    positions.put(p.position.x).put(p.position.y).put(p.position.z);

    colors.putInt(p.color.asIntABGR());

    sizes.put(p.size);

    texcoords.put(p.startX).put(p.startY).put(p.endX).put(p.endY);

    }



    int oldLimit = sizes.limit();

    if (displayLimit)

    {

    System.out.println("Limit : "+oldLimit);

    displayLimit = !displayLimit;

    }

    positions.flip();

    colors.flip();

    sizes.flip();

    texcoords.flip();

    int newLimit = sizes.limit();

    if (oldLimit != newLimit) System.out.println("Changing buffer limit from " + oldLimit+ " to "+ newLimit );



    // force renderer to re-send data to GPU

    pvb.updateData(positions);

    cvb.updateData(colors);

    svb.updateData(sizes);

    tvb.updateData(texcoords);

    mesh.updateBound();



    int oldVertexCount = mesh.getVertexCount();

    if (displayVertexCount)

    {

    System.out.println("Vertex count : "+oldVertexCount);

    displayVertexCount = !displayVertexCount;

    }

    if (TEST_FIX_JME_BUG_HACK) updateCountsHack(mesh); // JUST A QUICK TEST HACK (DOESNT RUN FULL CODE).

    else mesh.updateCounts(); //Bug : vertex count is 3000, it doesnt become 300 even though i changed the limit, jme should read beyond limit, its against java buffer design.

    int newVertexCount = mesh.getVertexCount();

    if (oldVertexCount != newVertexCount ) System.out.println("Vertex count changed from " + oldVertexCount + " to " + newVertexCount );

    }



    public void updateCountsHack(Mesh mesh)

    {

    VertexBuffer pb = mesh.getBuffer(VertexBuffer.Type.Position);



    try

    {

    Field field = mesh.getClass().getDeclaredField("vertCount");

    field.setAccessible(true);

    field.set(mesh, pb.getData().limit() / pb.getNumComponents());

    }

    catch (Exception e)

    {

    e.printStackTrace();

    }

    }



    public static void initBuffer(Mesh mesh, VertexBuffer.Type type, Buffer pb, Usage usage, int components, Format format, boolean normalise)

    {

    VertexBuffer pvb = new VertexBuffer(type);

    pvb.setupData(usage, components, format, pb);

    if (normalise) pvb.setNormalized(true);



    VertexBuffer buf = mesh.getBuffer(type);

    if (buf != null)

    {

    buf.updateData(pb);

    }

    else

    {

    mesh.setBuffer(pvb);

    }

    }



    public Mesh createSpriteMesh(SimpleSprite[] sprites)

    {

    Mesh spriteMesh = new Mesh();

    int numParticles = sprites.length;

    spriteMesh.setMode(Mesh.Mode.Points);

    initBuffer(spriteMesh, VertexBuffer.Type.Position, BufferUtils.createFloatBuffer(numParticles * 3), Usage.Stream, 3, Format.Float, false);

    initBuffer(spriteMesh, VertexBuffer.Type.Color, BufferUtils.createByteBuffer(numParticles * 4), Usage.Stream, 4, Format.UnsignedByte, true);

    initBuffer(spriteMesh, VertexBuffer.Type.Size, BufferUtils.createFloatBuffer(numParticles), Usage.Stream, 1, Format.Float, false);

    initBuffer(spriteMesh, VertexBuffer.Type.TexCoord, BufferUtils.createFloatBuffer(numParticles * 4), Usage.Stream, 4, Format.Float, false);

    return spriteMesh;

    }

    }

    [/java]
@tralala said:
Test case showing the problem before the patch and after the patch.


Huh, are you saying you actually gained double FPS from that patch or this is unreliable? I go with the latter. ;)

Of course :



Limit is 300.

Capacity is 3000.



Defined sprites [0, 300].

Undefined sprites [ 301, 3000 ]



before the patch it always created 3000 sprites ( ignoring buffer limit and always sending max buffer capacity sprites. It read beyond limit ( which is illegal according to buffer api ), thus sending garbage undefined sprites ( 301 - 3000 ).



after the patch it creates only 300 sprites.

I didn’t check the code itself but your fix might have lots of good things attached to it. Hopefully it’ll be integrated.

This change needs to be tested thoroughly. Seems it works with your particular use case, but what about skeletal animations, batching and everything that is directly using buffer data?

It requires a little bit more work than just changing a line in a file as you seem to think.



Also it’s easy to loose track on a post on the forum, if you are sure it’s a bug in the engine, you can create an issue in the tracker http://code.google.com/p/jmonkeyengine/issues/list?can=2&q=label%3AProduct-jME3



Recently Momoko_Fan made a pass on the issues, we’ll try to do it more often.

1 Like

i agree with nehon it must be tested, thats why i asked the help of a buffer expert.

i will create an issue in the tracker.

i propose you apply it and if within 1-2 weeks something bad happens it means that it is patch’s fault.

@tralala said:
i agree with nehon it must be tested, thats why i asked the help of a buffer expert.
i will create an issue in the tracker.
i propose you apply it and if within 1-2 weeks something bad happens it means that it is patch's fault.

hehehe, we can do soooo many things in 2 weeks that could screw up the engine :p

We cannot apply this patch because if we do so, then we have to change all other places where either buffer upload happens.



I have created an issue regarding buffer range standards:

http://code.google.com/p/jmonkeyengine/issues/detail?id=504

1 Like