The problem stems from the constructor for SharedMesh taking a TriMesh to use as the mesh to copy. That in itself isn’t a problem, but the constructor then goes on to copy the target meshes trans/rot/scale which is where the problem lies. These values in the target mesh object are used as a scratchpad for various calculations (picking being the one that showed the problem in this case) and as such cannot be relied upon as actually holding any values of meaning if you are using such functionality - and as such should not be used when creating new shared objects.
I mentioned this in one of the threads above, but there were no followups, so now im adding this here as a potential problem for others to review because im not totally sure of the implications. Basically why would you want to clone the positional info of a targetmesh as the point of cloning it is to show the same mesh at a different location. If this is to ease showing it at a location relative to the original, then this will break that, but the onlyclean way of doing this would be to add a flag to say “use current targets coords”.
- this.localRotation.set(target.getLocalRotation());
- this.localScale.set(target.getLocalScale());
- this.localTranslation.set(target.getLocalTranslation());
+ // Commented out the below as the "target"s local[RotTransScale] values may not be the values it was originally given.
+ // The values in the target object are used as temporary storage when calculating things for all other shared
+ // objects and therefore cannot be considered a reliable value of the original objects positions and must be
+ // set independently by the creating process on each new sharedmesh.
+// this.localRotation.set(target.getLocalRotation());
+// this.localScale.set(target.getLocalScale());
+// this.localTranslation.set(target.getLocalTranslation());
}
+
/**
* <code>getTarget</code> returns the mesh that is being shared by this
* object.
@@ -169,7 +175,24 @@
return target;
}
+
/**
+ * As a sharedmesh can be created from another sharedmesh, there is a possibility of the target
+ * being nested more than one level down (sharedmesh->sharedmesh->sharedmesh->target).
+ * Adding this check will find the actual target in these cases, otherwise creating a
+ * sharedmesh from a shared mesh should be disallowed.
+ *
+ * @return the base mesh being shared.
+ */
+ public TriMesh getDeepTarget() {
+ if( target instanceof SharedMesh )
+ return ((SharedMesh)target).getTarget();
+ return target;
+ }
+
+
+
+ /**
* <code>reconstruct</code> is not supported in SharedMesh.
*
* @param vertices
Damn… yes it probably should! The patch as it is fixed the original persons problem, but for completeness sake it should be properly recursive (although not basing a shared mesh on another shared mesh would still be the more sensible approach).
I will make the change, and thanks for pointing it out
I don't believe it's solely down to SharedMeshes being constructed from other SharedMeshes, we have similar problems with just one 'target' TriMesh when picking is added to the mix:
This issue seems to me to be due to the following methods in SharedMesh:
hasTriangleCollision()
findTriangleCollision()
findTrianglePick()
All the above change the targets local translation/rotation/scale as a side-effect of picking - some of the repliers to this topic have sorted it out by making temporary copies (can also confirm that this sorts out the problem).
Surely by default SharedMesh shouldn't be fiddling with the properties of it's target?
I've just checked the latest code but this hasn't been fixed as yet (assumed it is indeed a bug) - so is it a genuine bug or a feature? ;)
It was a while since I looked at this, and I dont have a good memory so might be remembering it wrong
The problem with making temporary copies of the translation/rotation/scale attributes is that you are having to create potentially huge amounts of objects and throwing them away - for maybe a lot of objects.
Instead of doing this, the shared meshes use the attributes from the target mesh as a calculation space for things like picking etc.
That is also the reason the target mesh should never be added to the scene (as obviously its not gonna stay where you put it :)), as the comment from SharedMesh.java pasted below points out . . . .
<b>Important:</b> It is highly recommended that the Target mesh is NOT
placed into the scenegraph, as its translation, rotation and scale are
replaced by the shared meshes using it before they are rendered. <br>
That is also the reason the target mesh should never be added to the scene (as obviously its not gonna stay where you put it :)), as the comment from SharedMesh.java pasted below points out . . . .
* <b>Important:</b> It is highly recommended that the Target mesh is NOT
* placed into the scenegraph, as its translation, rotation and scale are
* replaced by the shared meshes using it before they are rendered. <br>
* <b>Note:</b> Special thanks to Kevin Glass.
Hmmm.
Although the target isn't rendered itself, it's used as a template for new instances, so it's the same issue unfortunately.
I think the design is wrong here - there shouldn't be side-effects like this. Obviously if there's a performance issue then obviously something needs to be bastardised, but not this way. However I guess it can't be fixed now as many people will be using it as it stands.
The only way for me to get around this is to frig the SharedMesh code which is not at all palatale. Is there any other way around this?
Note that I can't simply extend SharedMesh as I'm using the JME loaders to create the targets.
As an aside...
JOC said:
The problem with making temporary copies of the translation/rotation/scale attributes is that you are having to create potentially huge amounts of objects and throwing them away - for maybe a lot of objects.
Instead of doing this, the shared meshes use the attributes from the target mesh as a calculation space for things like picking etc.
Is the above valid? I've altered SharedMesh to use local temporaries (as suggested) to prevent it trashing the target scene-graph. I measured loading and rendering performance before/after and there was no measureable difference (on a scene-graph of ~25K nodes, picking EVERY frame).
Well every object you create has to be garbage collected sooner or later, and creating and destroying many in a tight loop is where its assumed the performance and memory hit comes. There are some threads around here where the major developers of JME talked about this in other areas. Admittedly my knowlege is based on the positively ancient 1.4 JVM and there have been huge increases in performance for both object creation and object destruction, and I guess as JME is somewhat single threaded on a multicore processor you have a whole cpu just for GC ? - just guessing here - probably need to brush up on this.
With regard to your main issue tho, this does seem to be working the way it was designed to work. If there is a particular problem you are having here, can you post a small example showing it in your original thread rather than this thread.
With regard to your main issue tho, this does seem to be working the way it was designed to work. If there is a particular problem you are having here, can you post a small example showing it in your original thread rather than this thread.
Yes I realise it's working as intended, I just don't believe that's the way is should work ;)
Not to worry, I'll just have to work around it.
Thanks for the input though.
).