PR for shapekey name support

Development
22

At this point in time, the extras.targetNames is not part of the official specification. (But I have heard that there is likely to be an official method of doing this in the future) But right now, the consensus between tools is to use the extras.targetNames.

Yes, I think there probably should be an exception thrown, I need to test an export in blender where I only set some of the shapekey names but not all to see what happens… (Can I even do that?)

1 Like
23

hmm. i would try add 3 shape keys in blender, and dont touch name for second and setup for 1 and 3.
Probably default names( like “Shape 1”) are send anyway? i dont know it, but might worth to check.

anyway if you say extras is not part of the official specification, then some condition would be worth to add just to know if there are extras like

if(extras) {

like you have “meshData.has(“extras”)” just for

if (meshData.has("extras") && meshData.getAsJsonObject("extras").has("targetNames")) {
    morphTarget.setName(targetNames.pop());
}

or put it in some variable to not execute getAsJsonObject a lot times.

boolean hasTargetNames = meshData.has(“extras”) && meshData.getAsJsonObject(“extras”).has(“targetNames”)

and use it later for both.

or am i wrong?

24

Ah, that is probably a good idea. Tonight I’ll make some optimization improvements to it.

1 Like
25

SO after looking at it. At the moment, the only thing stored in extras (that I can find) are the targetNames. The only thing I can optimize out by making the change is a single call to getJsonObject Considering that the this provides almost no benefit, I am going to leave the code as it is. In the future as the extras field is use more, it would be a good idea to build a section of the importer to handle the extras.

1 Like
26

ok then, seems like its done, thank you again :slight_smile:

27

OK, if everyone would like to review the PR, it is open for a bit for review before it gets merged in.

28

@tlf30 regarding morph state, my understanding is getMorphState() in Geometry class is actually returning current morph weight, Yes? If so then why it’s not named as getMorphWeight()?

29

@Ali_RS seeing as it is an existing function, I do not think it would be a good idea to rename it, as we do not want to break backwards compatibility for all the apps that are currently using the function.

@sgold what do you think?

30

Of course, I do not mean to rename it, as you said it may break stuff, I just wanted to get sure that I am understanding it right and morph state is actually the morph weight.

31

Ah sorry, I misunderstood you.

I believe that it is the raw values. It gets updated here from the mesh when the state is set.

public void setMorphState(float[] state) {
        if (mesh == null || mesh.getMorphTargets().length == 0){
            return;
        }

        int nbMorphTargets = mesh.getMorphTargets().length;

        if (morphState == null) {
            morphState = new float[nbMorphTargets];
        }
        System.arraycopy(state, 0, morphState, 0, morphState.length);
        this.dirtyMorph = true;
    }
32

… or perhaps it does not get updates there…
EDIT: Yes, that is where it gets updated

33

Right, I can also confirm that here:

1 Like