BatchNode modification proposal

In another thread, that is dead long out of answer, I investigated strange things about BatchNode.

To make the story short, Node are not unbatched when removed, while Geometry are.

Here is my proposal to solve this bug : just add these few lines to BatchNode.

[java]
@Override
public Spatial detachChildAt(int index) {
Spatial s = super.detachChildAt(index);
if (s instanceof Node) {
if (isBatchedNode((Node)s)) {
setNeedsFullRebatch(true);
}
}
return s;
}

protected boolean isBatchedNode(Node n) {
  Spatial s = children.get(0);
  while (s instanceof Node) {
    s = ((Node)s).getChild(0); // descend into the child node
  }
  if (s instanceof Geometry) {
    return ((Geometry)s).isBatched();
  } else
    return false;
}

[/java]

The isBatchedNode utility function is only there to check if the node contains some batched geometry.
If a node can contain something which is not a Node nor a Geometry, descending into first child may be not enough… We should then iterate over all children until a geometry is found.

2 Likes

oh… I never got into this into, but I don’t remember testing it, so I guess i overlooked it :stuck_out_tongue:
Thanks for the patch. Only thing that bothers me is that sub geometries should be unbatched too.
unbatch just reset the geometry state and clean up variables used only for batching, and set the batchNode to full rebatch, then the batchNode handles the real batching.
Right now a geometry is unbatched when it’s detached from its parent, so here the sub geometries to this node are still marked as batched and still hold areference too the batchNode. This can have undesired effects later.
I’ll look into this.

1 Like

I committed a fix, tell me if you have any issue.
And thanks for reporting this

1 Like

Well. If keeping the sub-meshes batched is a trouble, I think we could recursively remove them. Maybe it would be even cleaner…

@yang71 said: Well. If keeping the sub-meshes batched is a trouble, I think we could recursively remove them. Maybe it would be even cleaner...
that's what i did. I used the detachChildAt you posted, but instead of calling needFullRebatch() I inspect the sub graph for batched geoms and call unbatch on them. Unbatch call needFullRebatch on the batchNode http://code.google.com/p/jmonkeyengine/source/diff?spec=svn10112&r=10112&format=side&path=/trunk/engine/src/core/com/jme3/scene/BatchNode.java
1 Like

Ok, nice. Not available right now, but maybe I should wait for the next nightly built…

Thanks.