I wanted to suggest a "hasType" method for SceneElement to ease checking element types. This would remove the need for bitwise & checks. Here's the code:
/**
* Returns true if any of passed types are present in this SceneElement.
* Comparisons are done via bitwise &.
*
* @param checkTypes the SceneElement types to check
* @return true if one or more of the passed types are present
*/
public boolean hasType(int... checkTypes) {
int currentType = getType();
for (int checkType : checkTypes) {
if ((currentType & checkType) != 0) {
return true;
}
}
return false;
}
So code that current looks like this:
if ((spat.getType() & SceneElement.TRIMESH) != 0) {
...
}
Would now look like this:
if (spat.hasType(SceneElement.TRIMESH)) {
...
}
And since the parameter is defined as "int..." you can pass in multiple element types to check:
if (spat.hasType(SceneElement.TERRAINBLOCK, SceneElement.TERRAINPAGE)) {
Note: From what I saw, no current JME code would benefit from the ability to pass in multiple types to check so I couldn't determine whether it would be better to return true if
any types are found, or only return true if
all types are found. A flag param could easily be added to adjust the behavior also, or the multi-type check could be changed to just checking a single type.
If you find this to be useful, I'd be willing to switch all of the current bitwise & checks to use hasType and put a patch file here.
Thanks!
I like the idea of collapsing the check into a simple method call, but wonder about the performance implications of having an ellipses parameter set. This means that every call an array of ints will be created, hasType can be called in tight loops of the main rendering system, and I worry about the garbage creation.
mojomonk said:
I like the idea of collapsing the check into a simple method call, but wonder about the performance implications of having an ellipses parameter set. This means that every call an array of ints will be created, hasType can be called in tight loops of the main rendering system, and I worry about the garbage creation.
Good point, I'm not sure how smart the compiler and VM are about optimizing that stuff but it's safest to assume it would create an array each time. As I mentioned the ellipses parameter isn't needed by any JME code, I checked and every call to getType is only interested in one type check.
So something like this:
/**
* Returns true if a given type is present in this SceneElement.
* Comparisons are done via bitwise &.
*
* @param type the SceneElement type to check for
* @return true if the SceneElement has the type
* @see #getType()
*/
public boolean hasType(int type) {
return (getType() & type) != 0;
}
Heh, down to one line of code ;) Anyway, as I mentioned I'm willing to create a patch for the getType/hasType conversion if you'd like. it would clean up some of my code too.
The hasType thought came about while messing with ideas to eliminate the need for TriMesh, etc typecasts in my code. I'm not sure I like it, but the easiest solution that crossed my mind is to add "as*" methods for the different SceneElement types. The biggest downside of this would be the addition of 12 new methods to SceneElement (one for each type). Here's the code for asTriMesh:
/**
* Returns this SceneElement as a {@link TriMesh} if the interface is
* supported. If not, <code>null</code> is returned.
*
* @return this SceneElement as a {@link TriMesh}, or null if not supported
* @see #hasType(int)
* @see #getType()
*/
public TriMesh asTriMesh() {
if (hasType(SceneElement.TRIMESH)) {
return (TriMesh) this;
}
return null;
}
So instead of:
if (spat.hasType(SceneElement.TRIMESH)) {
TriMesh mesh = (TriMesh) spatial
....
}
The as* way would look like this:
if (spat.hasType(SceneElement.TRIMESH)) {
TriMesh mesh = spatial.asTriMesh();
....
}
or:
TriMesh mesh = spatial.asTriMesh();
if (mesh != null) {
....
}
I know this type of change has got to be at the absolute bottom of the priority list, but I wanted to post it as a small piece of food for thought.
Late here, but with recent changes to the speed and efficiency of the instanceof operation in Java, it seems like we would be better served simply dumping the type flag.
renanse said:
Late here, but with recent changes to the speed and efficiency of the instanceof operation in Java, it seems like we would be better served simply dumping the type flag.
I have a gut feeling there's got a be a non-casting functional way of working with meshes. That said, I don't have the time to look into it atm, and I'm sure you guys have many bigger fish to fry ;) I guess since we have to cast anyway, instanceof may be a bit more clear paired with the typecast instead of a .getType/typecast pair.