BatchNode Possible Bug

I was looking at batchnode and on line 152, to accesses Geometry → bg.startIndex, startIndex is a protected variable. I can’t wrap my head around how this is compiling? Can someone help me with this. This is not the only example for BatchNode accessing protected variables in this class. There are 3 examples.

What is happening that is allowing to access protected variables from a different class.

Thanks.

BatchNode and Geometry are both in the same package / folder :jmonkeyengine/jme3-core/src/main/java/com/jme3/scene at master · jMonkeyEngine/jmonkeyengine · GitHub

And in java, protected variables can be accessed by any class in the same package. So protected is like a package-wide private variable.

1 Like

Something new. Just weird to code like that. Expanding classes are prevented by not using getters/setters.

Yeah I personally don’t use protected variables much, and only found out it was different than private myself a while back when I first started delving into the jme source code and saw it used a lot.

So I just stick to lots of getters / setters even for variables used solely within their package out of habit, but there’s definitely a few places I could benefit from using protected variables to save me from having to make getters / setters.

For example, when I write a new quest, I split it into a core Quest object and multiple QuestPhase objects that extends a base QuestPhase unique to that quest, and I end up putting lots of getters / setters for variables in the core quest and base questPhase that could just be protected variables instead, since those variables are never used outside the package for that quest and its phases.

But I have definitely found myself in situations with the core engine where it is frustrating trying to extend or tweak some small thing that ends up requiring accessing a protected variable, so you end up having to fork the whole engine to do it a lot of the time.

2 Likes

That is what I was trying is to make a small change, and didn’t want to change the JME-core. but it now requires changing the base to use getters/setters to get access to several variables, or fork JME-core and alter the batchnode to insert my line that is for my game not for the engine.

So I think you may be able to get around forking the whole engine if you are trying to just change a single class if you leave the package com.jme3.scene; package declaration at the top of your version of BatchNode.

Even if it is not in that folder, it will trick the compiler/ide/something to thinking that your class is in that package, and even though that package declaration will be underlined red and say its an error, the app still runs (in my experience in the SDK, both with ant and gradle)

So if you only want to edit BatchNode, copy the whole class into your own BatchNode.java file and don’t update the package delcaration to match its actual location, just leave it as com.jme3.scene; and just run it and it won’t crash even though it tries to tell you there’s a compile error with the package declaration. Not sure why this happens, or if it would cause problems with a distributed version of the app, but this has worked in my experience.

3 Likes

Thanks for the idea, That worked…

2 Likes

donuts takin care of business as usual. glad you got it figured out and thanks for the read!

1 Like

I have another question on what to mean appears to be a bug. But I have a question.

Inside BatchNode class under function “mergeGeometries”. It appears to have a bug from the way I see it. I wanted to make sure.

At the end of the function where it is looping through the vertexBuffer to copy them from the individual Geom to the merge Geom.

It does a check for VertexBuffer.Type.Index, .Position, .Normal and there is an “ELSE” for everything else.

Now the line “inBuf.copyElements(0, outBuf, globalVertIndex, geomVertCount);” copies the vertexBuffer from the inBuf to the outBuf but the inBuf “HAS” to be the same size of the Vertices Count. Which means if you want to create your own vertexBuffer and put in your own buffer, since JME forces you to use a predefined vertexBuffer because you can’t use a “Other”, so you have to use a buffer that is not being used to get your own data down to the shader.

But it is forcing you to have your vertexBuffer to have an array that matches the total number of vertices.
WHY DOES IT FORCE THIS?

Why could you not add

                int numElements = inBuf.getNumElements();
//Change following line to
                inBuf.copyElements(0, outBuf, globalVertIndex, numElements);

This way the vertexBuffer can be anything the user wishes to put into the buffer that they are using for other information.

Am I correct this is an over look of possible usage? Could it be changed to this?

    private void mergeGeometries(Mesh outMesh, List<Geometry> geometries) {
        int[] compsForBuf = new int[VertexBuffer.Type.values().length];
        VertexBuffer.Format[] formatForBuf = new VertexBuffer.Format[compsForBuf.length];
        boolean[] normForBuf = new boolean[VertexBuffer.Type.values().length];

        int totalVerts = 0;
        int totalTris = 0;
        int totalLodLevels = 0;
        int maxWeights = -1;

        Mesh.Mode mode = null;
        float lineWidth = 1f;
        for (Geometry geom : geometries) {
            totalVerts += geom.getVertexCount();
            totalTris += geom.getTriangleCount();
            totalLodLevels = Math.min(totalLodLevels, geom.getMesh().getNumLodLevels());
            if (maxVertCount < geom.getVertexCount()) {
                maxVertCount = geom.getVertexCount();
            }
            Mesh.Mode listMode;
            //float listLineWidth = 1f;
            int components;
            switch (geom.getMesh().getMode()) {
                case Points:
                    listMode = Mesh.Mode.Points;
                    components = 1;
                    break;
                case LineLoop:
                case LineStrip:
                case Lines:
                    listMode = Mesh.Mode.Lines;
                    //listLineWidth = geom.getMesh().getLineWidth();
                    components = 2;
                    break;
                case TriangleFan:
                case TriangleStrip:
                case Triangles:
                    listMode = Mesh.Mode.Triangles;
                    components = 3;
                    break;
                default:
                    throw new UnsupportedOperationException();
            }

            for (VertexBuffer vb : geom.getMesh().getBufferList().getArray()) {
                int currentCompsForBuf = compsForBuf[vb.getBufferType().ordinal()];
                if (vb.getBufferType() != VertexBuffer.Type.Index && currentCompsForBuf != 0 && currentCompsForBuf != vb.getNumComponents()) {
                    throw new UnsupportedOperationException("The geometry " + geom + " buffer " + vb.getBufferType()
                            + " has different number of components than the rest of the meshes "
                            + "(this: " + vb.getNumComponents() + ", expected: " + currentCompsForBuf + ")");
                }
                compsForBuf[vb.getBufferType().ordinal()] = vb.getNumComponents();
                formatForBuf[vb.getBufferType().ordinal()] = vb.getFormat();
                normForBuf[vb.getBufferType().ordinal()] = vb.isNormalized();
            }

            maxWeights = Math.max(maxWeights, geom.getMesh().getMaxNumWeights());

            if (mode != null && mode != listMode) {
                throw new UnsupportedOperationException("Cannot combine different"
                        + " primitive types: " + mode + " != " + listMode);
            }
            mode = listMode;
            compsForBuf[VertexBuffer.Type.Index.ordinal()] = components;
        }

        outMesh.setMaxNumWeights(maxWeights);
        outMesh.setMode(mode);
        //outMesh.setLineWidth(lineWidth);
        if (totalVerts >= 65536) {
            // Make sure we create an UnsignedInt buffer, so we can fit all of the meshes.
            formatForBuf[VertexBuffer.Type.Index.ordinal()] = VertexBuffer.Format.UnsignedInt;
        } else {
            formatForBuf[VertexBuffer.Type.Index.ordinal()] = VertexBuffer.Format.UnsignedShort;
        }

        // generate output buffers based on retrieved info
        for (int i = 0; i < compsForBuf.length; i++) {
            if (compsForBuf[i] == 0) {
                continue;
            }

            Buffer data;
            if (i == VertexBuffer.Type.Index.ordinal()) {
                data = VertexBuffer.createBuffer(formatForBuf[i], compsForBuf[i], totalTris);
            } else {
                data = VertexBuffer.createBuffer(formatForBuf[i], compsForBuf[i], totalVerts);
            }

            VertexBuffer vb = new VertexBuffer(VertexBuffer.Type.values()[i]);
            vb.setupData(VertexBuffer.Usage.Dynamic, compsForBuf[i], formatForBuf[i], data);
            vb.setNormalized(normForBuf[i]);
            outMesh.setBuffer(vb);
        }

        int globalVertIndex = 0;
        int globalTriIndex = 0;

        for (Geometry geom : geometries) {
            Mesh inMesh = geom.getMesh();
            if (!isBatch(geom)) {
                geom.associateWithGroupNode(this, globalVertIndex);
            }

            int geomVertCount = inMesh.getVertexCount();
            int geomTriCount = inMesh.getTriangleCount();

            for (int bufType = 0; bufType < compsForBuf.length; bufType++) {
                VertexBuffer inBuf = inMesh.getBuffer(VertexBuffer.Type.values()[bufType]);

                VertexBuffer outBuf = outMesh.getBuffer(VertexBuffer.Type.values()[bufType]);

                if (outBuf == null) {
                    continue;
                }

                if (VertexBuffer.Type.Index.ordinal() == bufType) {
                    int components = compsForBuf[bufType];

                    IndexBuffer inIdx = inMesh.getIndicesAsList();
                    IndexBuffer outIdx = outMesh.getIndexBuffer();

                    for (int tri = 0; tri < geomTriCount; tri++) {
                        for (int comp = 0; comp < components; comp++) {
                            int idx = inIdx.get(tri * components + comp) + globalVertIndex;
                            outIdx.put((globalTriIndex + tri) * components + comp, idx);
                        }
                    }
                } else if (VertexBuffer.Type.Position.ordinal() == bufType) {
                    FloatBuffer inPos = (FloatBuffer) inBuf.getData();
                    FloatBuffer outPos = (FloatBuffer) outBuf.getData();
                    doCopyBuffer(inPos, globalVertIndex, outPos, 3);
                } else if (VertexBuffer.Type.Normal.ordinal() == bufType || VertexBuffer.Type.Tangent.ordinal() == bufType) {
                    FloatBuffer inPos = (FloatBuffer) inBuf.getData();
                    FloatBuffer outPos = (FloatBuffer) outBuf.getData();
                    doCopyBuffer(inPos, globalVertIndex, outPos, compsForBuf[bufType]);
                    if (VertexBuffer.Type.Tangent.ordinal() == bufType) {
                        useTangents = true;
                    }
                } else {
                    if (inBuf == null) {
                        throw new IllegalArgumentException("Geometry " + geom.getName() + " has no " + outBuf.getBufferType() + " buffer whereas other geoms have. all geometries should have the same types of buffers.\n Try to use GeometryBatchFactory.alignBuffer() on the BatchNode before batching");
                    } else if (outBuf == null) {
                        throw new IllegalArgumentException("Geometry " + geom.getName() + " has a " + outBuf.getBufferType() + " buffer whereas other geoms don't. all geometries should have the same types of buffers.\n Try to use GeometryBatchFactory.alignBuffer() on the BatchNode before batching");
                    } else {
                        inBuf.copyElements(0, outBuf, globalVertIndex, geomVertCount);
                    }
                }
            }

            globalVertIndex += geomVertCount;
            globalTriIndex += geomTriCount;
        }
    }

I don’t think it could work any other way. The purpose of a mesh’s vertex buffers is to store information about each vertex in the geometry, so if there is not a vertex at that index of the buffers, then there is no reason to store any information about that non-existent vertex at the same index in the other buffers.

A mesh’s vertex buffers are like a set of parallell arrays where the array of the mesh’s vertices are the primary data in the first array (or buffer), and all other buffers are extra arrays that exist in parallel to the primary vertex buffer to store extra information unique to each vertex.

So If you have 90 vertices and want to store a 91st value, then that value doesn’t correlate to any one specific vertex and should instead be its own material paramater that gets set to any number of materials as needed, and shouldn’t be put in the mesh buffer at all.

If you’re trying to store that data about the first 45 out of the 90 vertices (and not the whole mesh), then you would just store that value into index 0-44 of your unique buffer.

1 Like