Finer control of SkinNode.update()

mulova · December 26, 2008, 4:19am

If I wanna suppress skin update, SkinNode.setExternalControl() provides this functionality.

But if SkinNode consists of multiple TriMesh, selective(partial) mesh update is not possible.

Checking mesh's cull hint before update will make it possible

And logically, culled mesh doesn't need to be updated.

Index: src/com/jme/animation/SkinNode.java =================================================================== --- src/com/jme/animation/SkinNode.java (revision 4079) +++ src/com/jme/animation/SkinNode.java (working copy) @@ -423,6 +423,8 @@ for (int index = cache.length; --index >= 0;) { Geometry geom = (Geometry)skins.getChild(index); + if (geom.getCullHint() == CullHint.Always) + continue; verts = geom.getVertexBuffer(); verts.clear(); norms = geom.getNormalBuffer();

nymon · December 26, 2008, 4:29am

Is it true that a culled mesh doesn't need to be updated, or does culling only exclude it from being rendered? I'm inclined that culling only applies to rendering, not updates.

mulova · December 26, 2008, 5:06am

I agree with you.

Then we need another variable and method to achieve this.

Any other idea?

mulova · December 26, 2008, 5:09am

Geometry has 'enabled' variable, but is not used and not accessible.

Is it reasonable for Geometry to have setEnabled(), isEnabled() method?

nymon · December 26, 2008, 5:36pm

I don't know why geometry has that variable, but whatever changes you need to make should stay in the SkinNode class if possible.

mulova · December 30, 2008, 12:54am

I added skipUpdate variable and

skipUpdate(String, boolean),

skipUpdate(int, boolean) methods to SkinNode

which can specify child geometry not to be updated.

And one more fix.

setSkeleton() method cannot have null parameter.

Sometimes I need to save SkinNode alone (without bone information) by jme Savable type.

(To change skins for one bone)

But currently I can't nullify 'skeleton' variable.

(As a matter of fact, to save SkinNode alone BoneInfluence.bone in cache also should be nullified.)

Index: src/com/jme/animation/SkinNode.java =================================================================== --- src/com/jme/animation/SkinNode.java (revision 4080) +++ src/com/jme/animation/SkinNode.java (working copy) @@ -86,6 +86,7 @@ private final Quaternion tmpRotation = new Quaternion(); private final Vector3f tmpScale = new Vector3f(); + private boolean[] skipUpdate = null; private boolean externalControl = false; /** @@ -246,6 +247,7 @@ @SuppressWarnings("unchecked") public void recreateCache() { cache = new ArrayList[skins.getQuantity()][]; + skipUpdate = new boolean[skins.getQuantity()]; for (int x = 0; x < cache.length; x++) { cache[x] = new ArrayList[skins.getChild(x).getVertexCount()]; } @@ -320,12 +322,13 @@ } public void setSkeleton(Bone b) { - if (skeleton != null) - skeleton.removeBoneListener(this); + if (skeleton != null) { + skeleton.removeBoneListener(this); + b.addBoneListener(this); + newSkeletonAssigned = true; + } skeleton = b; - skeleton.addBoneListener(this); - newSkeletonAssigned = true; } public Bone getSkeleton() { @@ -423,6 +426,8 @@ for (int index = cache.length; --index >= 0;) { Geometry geom = (Geometry)skins.getChild(index); + if (skipUpdate[index]) + continue; verts = geom.getVertexBuffer(); verts.clear(); norms = geom.getNormalBuffer(); @@ -456,7 +461,28 @@ skeleton.getParent().getWorldScale().set(tmpScale); skeleton.updateWorldVectors(true); } - } + } + + /** + * skip updateSkin() for specified child geometry + * @param i geometry index + * @param skip true if skipped + */ + public void skipUpdate(int i, boolean skip) { + skipUpdate[i] = skip; + } + + public void skipUpdate(String geomName, boolean skip) { + if (skins == null || skins.getQuantity() == 0) + return; + for (int i = 0; i < skins.getQuantity(); i++) { + if (geomName.equals(skins.getChild(i))) { + skipUpdate[i] = true; + return; + } + } + } + public ArrayList<BoneInfluence>[][] getCache() { return cache; @@ -464,6 +490,7 @@ public void setCache(ArrayList<BoneInfluence>[][] cache) { this.cache = cache; + this.skipUpdate = new boolean[cache.length]; } public void setBindMatrix(Matrix4f mat) { @@ -487,6 +514,7 @@ cap.write(skeleton, "skeleton", null); cap.writeSavableArrayListArray2D(cache, "cache", null); cap.writeSavableArrayList(connectionPoints, "connectionPoints", null); + cap.write(skipUpdate, "skipUpdate", null); } @SuppressWarnings("unchecked") @@ -498,7 +526,12 @@ skeleton = (Bone)cap.readSavable("skeleton", null); cache = cap.readSavableArrayListArray2D("cache", null); connectionPoints = cap.readSavableArrayList("connectionPoints", null); + skipUpdate = cap.readBooleanArray("skipUpdate", null); + // compatibility for previous version + if (cache != null && skipUpdate == null) { + skipUpdate = new boolean[cache.length]; + } if (skeleton != null) { //skeleton.revertToBind(); skeleton.addBoneListener(this); @@ -543,11 +576,16 @@ @SuppressWarnings("unchecked") public void removeGeometry(int geomIndex) { ArrayList<BoneInfluence>[][] newCache = new ArrayList[skins.getQuantity()][]; + boolean[] newSkipUpdate = new boolean[skins.getQuantity()-1]; for (int x = 0; x < cache.length-1; x++) { - if (x < geomIndex) - newCache[x] = cache[x]; - else - newCache[x] = cache[x+1]; + if (x < geomIndex) { + newCache[x] = cache[x]; + newSkipUpdate[x] = skipUpdate[x]; + } + else { + newCache[x] = cache[x+1]; + newSkipUpdate[x] = skipUpdate[x+1]; + } } cache = newCache; }

basixs · December 30, 2008, 3:16am

are you sure you don't mean this in the setSkeleton method:

public void setSkeleton(Bone b) { if (skeleton != null) { skeleton.removeBoneListener(this); } skeleton = b; if ( skeleton != null ) { skeleton.addBoneListener(this); newSkeletonAssigned = true; } }

As you have it now, if the current skeleton is null the new one won't be assigned a boneListener (regardless of whether its null or not)...
(haven't looked at the source but this jumped out at me)

Also, we (the jME 'crew') might want to make a policy about only having one 'fix' per patch; an error that is introduced with patches that have multiple fixes might be harder to debug/rollback (but I may just be paranoid here)...
(a 'fix' being the scope of the issue the patch is intended to resolve)

mulova · December 30, 2008, 7:13am

Okay I didn't know the policy.

I'll post a new thread for setSkeleton().

And I made a stupid mistake about that, basixs

This is only for the patch for skipUpdate

Index: src/com/jme/animation/SkinNode.java =================================================================== --- src/com/jme/animation/SkinNode.java (revision 4080) +++ src/com/jme/animation/SkinNode.java (working copy) @@ -86,6 +86,7 @@ private final Quaternion tmpRotation = new Quaternion(); private final Vector3f tmpScale = new Vector3f(); + private boolean[] skipUpdate = null; private boolean externalControl = false; /** @@ -246,6 +247,7 @@ @SuppressWarnings("unchecked") public void recreateCache() { cache = new ArrayList[skins.getQuantity()][]; + skipUpdate = new boolean[skins.getQuantity()]; for (int x = 0; x < cache.length; x++) { cache[x] = new ArrayList[skins.getChild(x).getVertexCount()]; } @@ -423,6 +425,8 @@ for (int index = cache.length; --index >= 0;) { Geometry geom = (Geometry)skins.getChild(index); + if (skipUpdate[index]) + continue; verts = geom.getVertexBuffer(); verts.clear(); norms = geom.getNormalBuffer(); @@ -456,7 +460,28 @@ skeleton.getParent().getWorldScale().set(tmpScale); skeleton.updateWorldVectors(true); } - } + } + + /** + * skip updateSkin() for specified child geometry + * @param i geometry index + * @param skip true if skipped + */ + public void skipUpdate(int i, boolean skip) { + skipUpdate[i] = skip; + } + + public void skipUpdate(String geomName, boolean skip) { + if (skins == null || skins.getQuantity() == 0) + return; + for (int i = 0; i < skins.getQuantity(); i++) { + if (geomName.equals(skins.getChild(i))) { + skipUpdate[i] = true; + return; + } + } + } + public ArrayList<BoneInfluence>[][] getCache() { return cache; @@ -464,6 +489,7 @@ public void setCache(ArrayList<BoneInfluence>[][] cache) { this.cache = cache; + this.skipUpdate = new boolean[cache.length]; } public void setBindMatrix(Matrix4f mat) { @@ -487,6 +513,7 @@ cap.write(skeleton, "skeleton", null); cap.writeSavableArrayListArray2D(cache, "cache", null); cap.writeSavableArrayList(connectionPoints, "connectionPoints", null); + cap.write(skipUpdate, "skipUpdate", null); } @SuppressWarnings("unchecked") @@ -498,7 +525,12 @@ skeleton = (Bone)cap.readSavable("skeleton", null); cache = cap.readSavableArrayListArray2D("cache", null); connectionPoints = cap.readSavableArrayList("connectionPoints", null); + skipUpdate = cap.readBooleanArray("skipUpdate", null); + // compatibility for previous version + if (cache != null && skipUpdate == null) { + skipUpdate = new boolean[cache.length]; + } if (skeleton != null) { //skeleton.revertToBind(); skeleton.addBoneListener(this); @@ -543,11 +575,16 @@ @SuppressWarnings("unchecked") public void removeGeometry(int geomIndex) { ArrayList<BoneInfluence>[][] newCache = new ArrayList[skins.getQuantity()][]; + boolean[] newSkipUpdate = new boolean[skins.getQuantity()-1]; for (int x = 0; x < cache.length-1; x++) { - if (x < geomIndex) - newCache[x] = cache[x]; - else - newCache[x] = cache[x+1]; + if (x < geomIndex) { + newCache[x] = cache[x]; + newSkipUpdate[x] = skipUpdate[x]; + } + else { + newCache[x] = cache[x+1]; + newSkipUpdate[x] = skipUpdate[x+1]; + } } cache = newCache; }

basixs · December 30, 2008, 7:32am

And I made a stupid mistake about that, basixs

You don't know how many times I have made them also, wish there was 2 of me so someone would always be around to just to double check my code. :)

Okay I didn't know the policy.

There's no policy (and probably never will be), I was just making a comment to any others that might be interested/concerned about something like that. If no one speaks up then its probably safe to assume no one else feels its an issue (ie, I'm a paranoid SOB)...
(didn't mean to make you feel like it was directed at you or your post)

mulova · December 30, 2008, 11:08am

basixs said:

(didn't mean to make you feel like it was directed at you or your post)

I see. :)
Anyway, one fix for one problem is the clear and right way without doubt.
I say it aloud. ;)

basixs · December 30, 2008, 7:00pm

Do you need someone to commit it for you?

mulova · December 31, 2008, 2:02am

Shamefully, I requested the right for commit and now I have it.

I'll check it in after some more tests.