When BatchNode is cloned, bad things are happening - duplicated object is created at 0,0,0 coordinates which is not reacting to the translation. Quick workaround was to add
but I suppose that there will be a nice way to check it rather than tmpFloat.
In any case, I’m bit worried about the duplication. Are we sure that currently it is not drawing two instances of same geometry in same place and they are just not visible as duplication?
Don’t use the BatchNode in this case, it really makes no sense. Its meant to be used when you add and remove meshes often. Cloning wouldn’t work then anyway. Use the GeometryBatchFactory to make a normal Geometry and Mesh and clone that.
I’m using BatchNode to to optimize part-based animations (NWN models). In such case, things are not added/removed ever, but transformed independent of each other in each frame due to animations.
Previously, I was creating models from scratch each time, so batching was working. Now, I’m trying to reuse asset manager cloning/weak ref cache - I have made all the controls properly cloneable, but there is still an issue with batched nodes as above.
I’m not doing it at the moment, but what is wrong with cloning the object and then adding/removing few children independent in each clone? I thought that entire hierarchy is being duplicated on clone and nothing except meshes is being shared?
@abies said:
I'm using BatchNode to to optimize part-based animations (NWN models). In such case, things are not added/removed ever, but transformed independent of each other in each frame due to animations.
Previously, I was creating models from scratch each time, so batching was working. Now, I’m trying to reuse asset manager cloning/weak ref cache - I have made all the controls properly cloneable, but there is still an issue with batched nodes as above.
I’m not doing it at the moment, but what is wrong with cloning the object and then adding/removing few children independent in each clone? I thought that entire hierarchy is being duplicated on clone and nothing except meshes is being shared?
Its one big mesh, if you add something to one its not the same as the other and can’t be re-used anyway.
Even though cloning itself should work (maybe make a test case for @nehon) there can not be any “sharing” going on. You change the mesh by transforming the single geometries and thus the meshes differ and cannot be shared anymore, just like animated meshes cannot be shared.
Sounds like an issue yes.
The idea behind the batchnode is to make one mesh from several meshes and still be able to move parts independently.
What Normen says is that cloning the batchNode won’t result in a shared batched mesh between all the same model instance. Because you wouldn’t be able to have them have different animations (They’d all animate in sync).
So BatchNode.clone() should clone underlying geometries, and rebatch to create a new mesh for each one.
So that’s exactly the same as cloning the unbatched models and batch them with a batchNode. What you should cache is the model, and not the batched model. Idk if that makes sense.
about that :
@abies said:
In any case, I'm bit worried about the duplication. Are we sure that currently it is not drawing two instances of same geometry in same place and they are just not visible as duplication?
BatchNode is keeping underlying geom meshes, and set them to cullMode.always, so they are never rendered.
They are kept for 2 reasons : picking, and they act as a bind pose like the one made for bone animation.
The fact is using BatchNode doubles your mem requirements for a mesh. it's a trade of against speed.
I agree that there cannot be sharing of resulting batch-generated meshes, nor of children geometries. But is there a reason why static children meshes cannot be shared (one which are being processed by a batch itself)?
BTW, there is a bug in the way I’m doing the clone right now - batches and batchesByGeom need to be updated after clone with new references. I will try to find a proper solution today evening and post it here.
Ok, final code which seems to work is
[java] @Override
public Node clone(boolean cloneMaterials) {
BatchNode clone = (BatchNode)super.clone(cloneMaterials);
if ( batches.size() > 0) {
for ( Batch b : batches ) {
for ( int i =0; i < clone.children.size(); i++ ) {
if ( clone.children.get(i).getName().equals(b.geometry.getName())) {
clone.children.remove(i);
break;
}
}
}
clone.needsFullRebatch = true;
clone.batches.clear();
clone.batchesByGeom.clear();
clone.batch();
}
return clone;
}
[/java]
Of course, as you both said, it probably doesn’t make a real sense to do it this way - better to have non-batched version in cache and batch after cloning. BUT… current implementation breaks in very unexpected way. If not a fix above, I would suggests something like
[java] @Override
public Node clone(boolean cloneMaterials) {
BatchNode clone = (BatchNode)super.clone(cloneMaterials);
if ( batches.size() > 0) {
throw new DontDoThatYouDumbUserException(“Cloning of batched nodes not supported”);
}
return clone;
}
[/java]
Yes, I know it is very bad - but I have not found any better way without actually rewritting half of spatial/node cloning logic. Children which are being removed have autogenerated names from BatchNode like
[java]
name + “-batch” + batches.size();
[/java]
So yes, if somebody has a BatchNode named XYZModel and directly below that he puts a subnode called like XYZModel-batch0 or XYZModel-batch3, then clone could possibly break.
Other option would be possible to remove batched nodes from original before cloning (this could be done by reference), then cloning, then readding the nodes directly to the children list.
In any case, I already know what was wrong (I had a major puzzle when two copies of object appeared, one non-moving at 0,0,0 coordinates), so I can work around that. Just wanted to suggest that some kind of either fix or throwing exception would be nice, so other people won’t hit same problem without warning.