PR for shapekey name support

Hello,
I have opened a rough WIP PR for adding support for shape key (mesh morph) names and would like feedback on the core jme changes it makes.

Thank you,
Trevor Flynn

2 Likes

My initial reaction is that it feels weird to modify Geometry to support thisā€¦ but Iā€™m not sure what the end state is yet so I donā€™t know.

2 Likes

Yep, I also agree with Paul, Not sure but I think neither of those changes in Geometry class is needed.

For convenience maybe we just need to add a method in Mesh class?

public int getMorphTargetIndex(String morphName)  {
   // find index of morph target with given name if not found return -1
}

im not 100% sure if all gltf exporters and 3d programms support shape key names, but Blender support them and like you said newest Blender gltf plugin too.

nbvnbvnbvn

So i agree it would help, why? to make code more human readable and make key order not important, where each model can have different.

Nice commit btw.
But i hope your commit will not break loading morphs for older gltf plugins.

also i agree it might need to be in Mesh, there are a lot of mesh.getMorphTargets() and you still reference to mesh anyway.

1 Like

@tlf30, @Ali_RS

noticed that minus values are not supported in JME while supported in blender?

or do i something wrong?

i tried set minus values and nothing happend, even if in blender i had minimum lower than 0.

Also i noticed that one of my shape keys work opposite, suddenly it go below instead of up, but i think its gltf exporter new issue. its odd because its about only one of them.

@oxplay2 not sure about your issue.

Probably you should better to ask it in a new topic :slightly_smiling_face:

@tlf30 any news on this PR?
I am agree yet with adding support for shape key names.

I agree, the changes should go in Mesh, not in Geometry.

1 Like

Sorry for the late reply, I am currently doing some work in the artic circle in Alaska, and my internet is worse than dial up. I will be getting back home on Thursday next week. I will look back into it then. (It took me over 5 minutes to load the forum to make this reply)

4 Likes

hehe, i just re-made this shape key and it work fineā€¦ some gltf or blender issue.

but what about minus values, are you able to use them in JME? it might be future feature request if someone might be interested.

About names, im even happier now it will be added, because had a lot issue with random shape key each time i export - for some reason. Names will solve this.

Anyway i got one question about PR.

there is:

JsonArray targetNames = meshObject.getAsJsonArray("targetNames");

do it magically work via some reflection? or maybe something is missing?

because i dont seen any usage of targetNames.

so i just let know, because maybe some additional code is missing. Sometimes fresh view can catch errors like this.

Note, as mentioned in the first post this PR is still WIP means it is not completed yet and missing some parts.

1 Like

ah ok, sorry :slight_smile:

one more thing i just noticed. (i know its about PR, but its shape key related, i dont want create new topic now, its just a reminder for future shape key fixes)

for some reason i cant add more shape keys - i understand there were some limit for one verticle shapes. but i touch verticles that are not used by other shape keys, but anyway it throw error.

its about:

/**
 * Vertex attribs currently bound and enabled. If a slot is null, then
 * it is disabled.
 */
public VertexBuffer[] boundAttribs = new VertexBuffer[16];

each shape key seems to add 2 indexes (before remove i had 19)

SEVERE: Uncaught exception thrown in Thread[jME3 Main,5,main]
java.lang.ArrayIndexOutOfBoundsException: 17
at com.jme3.renderer.opengl.GLRenderer.setVertexAttrib(GLRenderer.java:2755)
at com.jme3.renderer.opengl.GLRenderer.setVertexAttrib(GLRenderer.java:2808)
at com.jme3.renderer.opengl.GLRenderer.renderMeshDefault(GLRenderer.java:3040)
at com.jme3.renderer.opengl.GLRenderer.renderMesh(GLRenderer.java:3077)
at com.jme3.material.logic.DefaultTechniqueDefLogic.renderMeshFromGeometry(DefaultTechniqueDefLogic.java:70)
at com.jme3.material.logic.SinglePassAndImageBasedLightingLogic.render(SinglePassAndImageBasedLightingLogic.java:260)
at com.jme3.material.Technique.render(Technique.java:166)
at com.jme3.material.Material.render(Material.java:1024)
at com.jme3.renderer.RenderManager.renderGeometry(RenderManager.java:614)
at com.jme3.renderer.queue.RenderQueue.renderGeometryList(RenderQueue.java:266)
at com.jme3.renderer.queue.RenderQueue.renderQueue(RenderQueue.java:305)
at com.jme3.renderer.RenderManager.renderViewPortQueues(RenderManager.java:877)
at com.jme3.renderer.RenderManager.flushQueue(RenderManager.java:779)
at com.jme3.renderer.RenderManager.renderViewPort(RenderManager.java:1108)
at com.jme3.water.WaterFilter.preFrame(WaterFilter.java:180)
at com.jme3.post.FilterPostProcessor.preFrame(FilterPostProcessor.java:368)
at com.jme3.renderer.RenderManager.renderViewPort(RenderManager.java:1078)
at com.jme3.renderer.RenderManager.render(RenderManager.java:1158)
at com.jme3.app.SimpleApplication.update(SimpleApplication.java:270)
at com.jme3.system.lwjgl.LwjglAbstractDisplay.runLoop(LwjglAbstractDisplay.java:151)
at com.jme3.system.lwjgl.LwjglDisplay.runLoop(LwjglDisplay.java:197)
at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:232)

its connected somehow with water filter?

anyway i currently have only 5 shape keys used, and its max for meā€¦

did you had similar issues?

OK, I have hopped back into this again.
SO, the state of the morphs is currently (jme-master) handled by the geometry class.
What I have added to geometry is manipulating the state of the morphs via name. There is no way as far as I can tell to do this via the mesh class. What I am moving into the mesh class is the function to get the morph names, and I am exposing it (as getMorphNames) in the geometry class just like getMophState is exposed.

With that, I have also added the morphName to be saved and loaded in the MorphTarget class.

Thoughts?

1 Like

OK, I think it is ready. Everyone can check it out on github.

1 Like

thank you, recently i had some issues with random shape key number on export so it sounds like important PR. code looks ok for me.

btw i hope there is no way that JSON ā€œextrasā€ have less or more records than ā€œtargetsā€ because targetNames.pop() might not work correctly. because i understand it cant be done in one loop.

1 Like

That is correct. My understanding is that a correctly formatted gltf file should always have the same number of targets as it does extra.targetNames. With that said, I have been debating about some error checking in there. I think the best course of action might be to throw some form of a runtime exception, but I am not sure yet.

EDIT:
With this PR, if someone with experience with FBX would like to, they can modify that importer to pull the shape key names as well. I have no experience working with FBX except for exporting from Daz Studio to import to Blender. But I know it keeps the shape key name information.

1 Like

Due to how I have this:

if (targetNames.size() > 0) {
    morphTarget.setName(targetNames.pop());
}

Even if there are more or less targetNames than targets, the code will at least not break anything. Which is a good behavior when dealing with a malformed file.

1 Like

sorry if i mislead you with my earlier comment.

without if condition seems to be more proper, i just said i hope in gltf there will never be situation when extra.targetNames is different length than targets.

it will not break code using this, but it would lead to odd issues without any exceptions.

like i would got shape keys in blender: 1:a, 2:b, 3:c, 4:d

and 1:a,2:c,3:d would be in targetNames,
then a = a, b = c, c = d that would be very unpredictible.

i understand probably it will never happen anyway, because extra.targetNames will probably always be same length as targets.

but anyway some exception or severe log should be thrown too.

i red glTF/README.md at master Ā· KhronosGroup/glTF Ā· GitHub

but i was unable to find anything about targetNames length or key relation.

Anyway Its just my personal opinion, but as i know its required to know something is wrong. (much easier to find issue later)