Skip to content

Commit d93742d

Browse files
authored
solve issue #2076 (software skinning requires vertex normals) (#2155)
* SkinningControl: add null checks to solve issue #2076 * SkeletonControl: add null checks to solve issue #2076 * jme3-examples: add a test for issue #2076
1 parent 3833a2f commit d93742d

File tree

3 files changed

+182
-28
lines changed

3 files changed

+182
-28
lines changed

jme3-core/src/main/java/com/jme3/anim/SkinningControl.java

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2009-2021 jMonkeyEngine
2+
* Copyright (c) 2009-2023 jMonkeyEngine
33
* All rights reserved.
44
*
55
* Redistribution and use in source and binary forms, with or without
@@ -327,15 +327,20 @@ void resetToBind() {
327327
VertexBuffer bindPos = mesh.getBuffer(Type.BindPosePosition);
328328
VertexBuffer bindNorm = mesh.getBuffer(Type.BindPoseNormal);
329329
VertexBuffer pos = mesh.getBuffer(Type.Position);
330-
VertexBuffer norm = mesh.getBuffer(Type.Normal);
331330
FloatBuffer pb = (FloatBuffer) pos.getData();
332-
FloatBuffer nb = (FloatBuffer) norm.getData();
333331
FloatBuffer bpb = (FloatBuffer) bindPos.getData();
334-
FloatBuffer bnb = (FloatBuffer) bindNorm.getData();
335332
pb.clear();
336-
nb.clear();
337333
bpb.clear();
338-
bnb.clear();
334+
335+
// reset bind normals if there is a BindPoseNormal buffer
336+
if (bindNorm != null) {
337+
VertexBuffer norm = mesh.getBuffer(Type.Normal);
338+
FloatBuffer nb = (FloatBuffer) norm.getData();
339+
FloatBuffer bnb = (FloatBuffer) bindNorm.getData();
340+
nb.clear();
341+
bnb.clear();
342+
nb.put(bnb).clear();
343+
}
339344

340345
//resetting bind tangents if there is a bind tangent buffer
341346
VertexBuffer bindTangents = mesh.getBuffer(Type.BindPoseTangent);
@@ -348,9 +353,7 @@ void resetToBind() {
348353
tb.put(btb).clear();
349354
}
350355

351-
352356
pb.put(bpb).clear();
353-
nb.put(bnb).clear();
354357
}
355358
}
356359
}
@@ -583,9 +586,10 @@ private void applySkinningTangents(Mesh mesh, Matrix4f[] offsetMatrices, VertexB
583586

584587
VertexBuffer nb = mesh.getBuffer(Type.Normal);
585588

586-
FloatBuffer fnb = (FloatBuffer) nb.getData();
587-
fnb.rewind();
588-
589+
FloatBuffer fnb = (nb == null) ? null : (FloatBuffer) nb.getData();
590+
if (fnb != null) {
591+
fnb.rewind();
592+
}
589593

590594
FloatBuffer ftb = (FloatBuffer) tb.getData();
591595
ftb.rewind();
@@ -615,7 +619,9 @@ private void applySkinningTangents(Mesh mesh, Matrix4f[] offsetMatrices, VertexB
615619
bufLength = Math.min(posBuf.length, fvb.remaining());
616620
tanLength = Math.min(tanBuf.length, ftb.remaining());
617621
fvb.get(posBuf, 0, bufLength);
618-
fnb.get(normBuf, 0, bufLength);
622+
if (fnb != null) {
623+
fnb.get(normBuf, 0, bufLength);
624+
}
619625
ftb.get(tanBuf, 0, tanLength);
620626
int verts = bufLength / 3;
621627
int idxPositions = 0;
@@ -688,16 +694,20 @@ private void applySkinningTangents(Mesh mesh, Matrix4f[] offsetMatrices, VertexB
688694

689695
fvb.position(fvb.position() - bufLength);
690696
fvb.put(posBuf, 0, bufLength);
691-
fnb.position(fnb.position() - bufLength);
692-
fnb.put(normBuf, 0, bufLength);
697+
if (fnb != null) {
698+
fnb.position(fnb.position() - bufLength);
699+
fnb.put(normBuf, 0, bufLength);
700+
}
693701
ftb.position(ftb.position() - tanLength);
694702
ftb.put(tanBuf, 0, tanLength);
695703
}
696704

697705
vars.release();
698706

699707
vb.updateData(fvb);
700-
nb.updateData(fnb);
708+
if (nb != null) {
709+
nb.updateData(fnb);
710+
}
701711
tb.updateData(ftb);
702712
}
703713

jme3-core/src/main/java/com/jme3/animation/SkeletonControl.java

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2009-2021 jMonkeyEngine
2+
* Copyright (c) 2009-2023 jMonkeyEngine
33
* All rights reserved.
44
*
55
* Redistribution and use in source and binary forms, with or without
@@ -321,15 +321,20 @@ void resetToBind() {
321321
VertexBuffer bindPos = mesh.getBuffer(Type.BindPosePosition);
322322
VertexBuffer bindNorm = mesh.getBuffer(Type.BindPoseNormal);
323323
VertexBuffer pos = mesh.getBuffer(Type.Position);
324-
VertexBuffer norm = mesh.getBuffer(Type.Normal);
325324
FloatBuffer pb = (FloatBuffer) pos.getData();
326-
FloatBuffer nb = (FloatBuffer) norm.getData();
327325
FloatBuffer bpb = (FloatBuffer) bindPos.getData();
328-
FloatBuffer bnb = (FloatBuffer) bindNorm.getData();
329326
pb.clear();
330-
nb.clear();
331327
bpb.clear();
332-
bnb.clear();
328+
329+
// reset bind normals if there is a BindPoseNormal buffer
330+
if (bindNorm != null) {
331+
VertexBuffer norm = mesh.getBuffer(Type.Normal);
332+
FloatBuffer nb = (FloatBuffer) norm.getData();
333+
FloatBuffer bnb = (FloatBuffer) bindNorm.getData();
334+
nb.clear();
335+
bnb.clear();
336+
nb.put(bnb).clear();
337+
}
333338

334339
//reset bind tangents if there is a bind tangent buffer
335340
VertexBuffer bindTangents = mesh.getBuffer(Type.BindPoseTangent);
@@ -343,7 +348,6 @@ void resetToBind() {
343348
}
344349

345350
pb.put(bpb).clear();
346-
nb.put(bnb).clear();
347351
}
348352
}
349353
}
@@ -574,8 +578,10 @@ private void applySkinningTangents(Mesh mesh, Matrix4f[] offsetMatrices, VertexB
574578

575579
VertexBuffer nb = mesh.getBuffer(Type.Normal);
576580

577-
FloatBuffer fnb = (FloatBuffer) nb.getData();
578-
fnb.rewind();
581+
FloatBuffer fnb = (nb == null) ? null : (FloatBuffer) nb.getData();
582+
if (fnb != null) {
583+
fnb.rewind();
584+
}
579585

580586
FloatBuffer ftb = (FloatBuffer) tb.getData();
581587
ftb.rewind();
@@ -603,7 +609,9 @@ private void applySkinningTangents(Mesh mesh, Matrix4f[] offsetMatrices, VertexB
603609
bufLength = Math.min(posBuf.length, fvb.remaining());
604610
tanLength = Math.min(tanBuf.length, ftb.remaining());
605611
fvb.get(posBuf, 0, bufLength);
606-
fnb.get(normBuf, 0, bufLength);
612+
if (fnb != null) {
613+
fnb.get(normBuf, 0, bufLength);
614+
}
607615
ftb.get(tanBuf, 0, tanLength);
608616
int verts = bufLength / 3;
609617
int idxPositions = 0;
@@ -676,16 +684,20 @@ private void applySkinningTangents(Mesh mesh, Matrix4f[] offsetMatrices, VertexB
676684

677685
fvb.position(fvb.position() - bufLength);
678686
fvb.put(posBuf, 0, bufLength);
679-
fnb.position(fnb.position() - bufLength);
680-
fnb.put(normBuf, 0, bufLength);
687+
if (fnb != null) {
688+
fnb.position(fnb.position() - bufLength);
689+
fnb.put(normBuf, 0, bufLength);
690+
}
681691
ftb.position(ftb.position() - tanLength);
682692
ftb.put(tanBuf, 0, tanLength);
683693
}
684694

685695
vars.release();
686696

687697
vb.updateData(fvb);
688-
nb.updateData(fnb);
698+
if (nb != null) {
699+
nb.updateData(fnb);
700+
}
689701
tb.updateData(ftb);
690702
}
691703

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
* Copyright (c) 2023 jMonkeyEngine
3+
* All rights reserved.
4+
*
5+
* Redistribution and use in source and binary forms, with or without
6+
* modification, are permitted provided that the following conditions are
7+
* met:
8+
*
9+
* * Redistributions of source code must retain the above copyright
10+
* notice, this list of conditions and the following disclaimer.
11+
*
12+
* * Redistributions in binary form must reproduce the above copyright
13+
* notice, this list of conditions and the following disclaimer in the
14+
* documentation and/or other materials provided with the distribution.
15+
*
16+
* * Neither the name of 'jMonkeyEngine' nor the names of its contributors
17+
* may be used to endorse or promote products derived from this software
18+
* without specific prior written permission.
19+
*
20+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
21+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
22+
* TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
23+
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
24+
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
25+
* EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
26+
* PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
27+
* PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
28+
* LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
29+
* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
30+
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
31+
*/
32+
package jme3test.animation;
33+
34+
import com.jme3.anim.SkinningControl;
35+
import com.jme3.anim.util.AnimMigrationUtils;
36+
import com.jme3.animation.SkeletonControl;
37+
import com.jme3.app.SimpleApplication;
38+
import com.jme3.light.AmbientLight;
39+
import com.jme3.math.ColorRGBA;
40+
import com.jme3.scene.Geometry;
41+
import com.jme3.scene.Mesh;
42+
import com.jme3.scene.Node;
43+
import com.jme3.scene.VertexBuffer;
44+
45+
/**
46+
* Test for JMonkeyEngine issue #2076: software skinning requires vertex
47+
* normals.
48+
*
49+
* <p>If the issue is resolved, 2 copies of the Jaime model will be rendered.
50+
*
51+
* <p>If the issue is present, then the application will immediately crash,
52+
* typically with a {@code NullPointerException}.
53+
*
54+
* @author Stephen Gold
55+
*/
56+
public class TestIssue2076 extends SimpleApplication {
57+
/**
58+
* Main entry point for the TestIssue2076 application.
59+
*
60+
* @param args array of command-line arguments (not null)
61+
*/
62+
public static void main(String... argv) {
63+
TestIssue2076 app = new TestIssue2076();
64+
app.start();
65+
}
66+
67+
/**
68+
* Initialize this application.
69+
*/
70+
@Override
71+
public void simpleInitApp() {
72+
AmbientLight ambientLight = new AmbientLight();
73+
ambientLight.setColor(new ColorRGBA(1f, 1f, 1f, 1f));
74+
rootNode.addLight(ambientLight);
75+
/*
76+
* The original Jaime model was chosen for testing because it includes
77+
* tangent buffers (needed to trigger issue #2076) and uses the old
78+
* animation system (so it can be easily used to test both systems).
79+
*/
80+
String assetPath = "Models/Jaime/Jaime.j3o";
81+
82+
testOldAnimationSystem(assetPath);
83+
testNewAnimationSystem(assetPath);
84+
}
85+
86+
/**
87+
* Attach the specified model (which uses the old animation system) to the
88+
* scene graph, enable software skinning, and remove its vertex normals.
89+
*
90+
* @param assetPath the asset path of the test model (not null)
91+
*/
92+
private void testOldAnimationSystem(String assetPath) {
93+
Node oldJaime = (Node) assetManager.loadModel(assetPath);
94+
rootNode.attachChild(oldJaime);
95+
oldJaime.setLocalTranslation(-1f, 0f, 0f);
96+
97+
// enable software skinning:
98+
SkeletonControl skeletonControl
99+
= oldJaime.getControl(SkeletonControl.class);
100+
skeletonControl.setHardwareSkinningPreferred(false);
101+
102+
// remove its vertex normals:
103+
Geometry oldGeometry = (Geometry) oldJaime.getChild(0);
104+
Mesh oldMesh = oldGeometry.getMesh();
105+
oldMesh.clearBuffer(VertexBuffer.Type.Normal);
106+
oldMesh.clearBuffer(VertexBuffer.Type.BindPoseNormal);
107+
}
108+
109+
/**
110+
* Attach the specified model, converted to the new animation system, to the
111+
* scene graph, enable software skinning, and remove its vertex normals.
112+
*
113+
* @param assetPath the asset path of the test model (not null)
114+
*/
115+
private void testNewAnimationSystem(String assetPath) {
116+
Node newJaime = (Node) assetManager.loadModel(assetPath);
117+
AnimMigrationUtils.migrate(newJaime);
118+
rootNode.attachChild(newJaime);
119+
newJaime.setLocalTranslation(1f, 0f, 0f);
120+
121+
// enable software skinning:
122+
SkinningControl skinningControl
123+
= newJaime.getControl(SkinningControl.class);
124+
skinningControl.setHardwareSkinningPreferred(false);
125+
126+
// remove its vertex normals:
127+
Geometry newGeometry = (Geometry) newJaime.getChild(0);
128+
Mesh newMesh = newGeometry.getMesh();
129+
newMesh.clearBuffer(VertexBuffer.Type.Normal);
130+
newMesh.clearBuffer(VertexBuffer.Type.BindPoseNormal);
131+
}
132+
}

0 commit comments

Comments
 (0)