facebookincubator / FBX2glTF

A command-line tool for the conversion of 3D model assets on the FBX file format to the glTF file format.
Other
2.08k stars 331 forks source link

Animation data is lost with current parsing strategy #67

Open johnshaughnessy opened 6 years ago

johnshaughnessy commented 6 years ago

The way animations are handle in Fbx2Raw goes something like this:

This algorithm is not working for me. I have an FBX model (exported from Maya using its GameExporter) of a humanoid with left and right hand animations on separate FbxAnimStacks. Each animation (open_L, open_R, fist_L, fist_R, point_L, point_R, etc.) was created to only affect the rotation of certain bones in my skeleton. The "default" transform for each of the FbxNodes in the algorithm above is whatever the transform of that bone happens to be when I export from Maya. (All of the animations are on a single timeline in Maya, so if I slide along the timeline I see all of the animations play one after another.) All of my animations have only a single frame, because I intend to transition between them as "poses". However, I have two problems: 1) If my "default" pose has open hands, then the "open_L" and "open_R" animations don't differ from the default pose and thus don't end up in my glTF model. Likewise, if one of my hand poses leaves some fingers in the default pose, the glTF model will end up with only the changed fingers in the resulting animation. 2) In the Maya timeline, the left hand is animated through a series of poses, then left it at its last pose. Then the right hand is animated through its poses. None of the right hand's poses explicitly include information or keyframes about the left hand, but because of the parsing algorithm, all of the right hand poses end up with information about the left hand.

I compared FBX2glTF to blender's FBX import step and the biggest difference I can tell is that blender reads from the FbxAnimCurveNodes while FBX2glTF doesn't. Is this on purpose? I think the animation data we actually want does not require a comparison from an evaluated value to a "default" value, but can be obtained accurately and without the problems I mentioned by reading the curve data in the FBX more "directly". I tried to come up with a fix but I am stuck and looking for help finishing / understanding. I am new to FBX, glTF, and animation and any corrections / advice is welcome.

Here is my fork of Fbx2Raw.cpp where I tried to come up with a fix. https://github.com/facebookincubator/FBX2glTF/compare/master...johnshaughnessy:bug/animations I am stuck at the point where I am reading FbxCurves but do not know how to evaluate them at a given time and then associate their values with the correct nodes. What I am printing out is all the information I figured out how to extract from a given FbxAnimStack. I noticed that pNode->GetUniqueID() is different than the unique ID's given by the FbxObjects associated with the FbxProperties animated by the curves. I am lost as to how one connects to the other.

Below is example output from my fork with an FBX model as I described:

...

FbxAnimCurveNode T has dstProperty Lcl Translation
FbxAnimCurveNode T has 3 channels.
  Channel X has 0 curves.
  Channel Y has 0 curves.
  Channel Z has 0 curves.

FbxAnimCurveNode S has dstProperty Lcl Scaling
FbxAnimCurveNode S has 3 channels.
  Channel X has 0 curves.
  Channel Y has 0 curves.
  Channel Z has 0 curves.

FbxAnimCurveNode R has dstProperty Lcl Rotation
FbxAnimCurveNode R has 3 channels.
  Channel X has 1 curves.
    Curve 0 has 1 dstProperties
      R has ID 2271
  Channel Y has 0 curves.
  Channel Z has 0 curves.

...
Node RightHandThumb1 has ID 95
Node RightHandThumb2 has ID 96
Node RightHandThumb3 has ID 97
Node RightHandThumb4 has ID 98
Node RightHandIndex1 has ID 99
...

(The full output can be found here, but it isn't more informative than this: https://github.com/johnshaughnessy/FBX2glTF/blob/111b75d764befc34ece1b6caeb9e89c9f679979f/sample_output.txt) How can I associate the FbxAnimCurveNode (whose X channel curve points to a property whose FbxObject has ID 2271) with one of the nodes of the hand (with ID ~90)?

zellski commented 6 years ago

Hi there @johnshaughnessy! Thanks for your report, and I appreciate you giving it a solid go yourself. The issue you're filing is related to urges of my own, see most obviously https://github.com/facebookincubator/FBX2glTF/issues/36 for an example. In my case, it's the inefficiency of fully baked animation that distressed me, but I can well imagine that single-frame animations also suffer from the current implementation.

I had to suspend FBX2glTF work for a few months, and while I hope to have more time for it shortly, and this issue is on my radar, it's also not top priority... I encourage you to pursue it yourself, with the caveat that a full replacement of the current approach with a new, curve-centric one, is probably a substantial bit of work.

The way FBX SDK classes interconnect is... not always the most obvious. I find the documentation is not always helpful. In the past, while trying to sort this stuff out myself, I've generally gotten more useful clues from studying in the example projects. The 'ImportScene' one is very good at illustrating the basic assumptions. When I run it against one of my test models, and scroll to the animation section, I see things like:

> ImportScene ~/models/fennec.fbx | less

(scroll forward about 10,000 lines)

Animation
---------

Animation Stack Name: standing_idle
   contains 1 Animation Layer

AnimLayer 0
     Node Name: RootNode

     Node Name: root

        Property lockInfluenceWeights (Label: lockInfluenceWeights)

     Node Name: spine_1

        TX
            Key Time: 1.... Key Value: -0.000000 [ cubic | Auto | None | None ]
            Key Time: 2.... Key Value: -0.000000 [ cubic | Auto | None | None ]
            Key Time: 3.... Key Value: -0.000000 [ cubic | Auto | None | None ]
            Key Time: 4.... Key Value: -0.000000 [ cubic | Auto | None | None ]
            Key Time: 5.... Key Value: -0.000000 [ cubic | Auto | None | None ]
            Key Time: 6.... Key Value: -0.000000 [ cubic | Auto | None | None ]
            Key Time: 7.... Key Value: -0.000000 [ cubic | Auto | None | None ]
            Key Time: 8.... Key Value: -0.000000 [ cubic | Auto | None | None ]
            Key Time: 9.... Key Value: -0.000000 [ cubic | Auto | None | None ]
... and so on.

This does look like it correctly ties AnimCurves to FbxNodes, aye? The related source is here and contains many repetitive sections along the lines of:

        lAnimCurve = pNode->LclTranslation.GetCurve(pAnimLayer, FBXSDK_CURVENODE_COMPONENT_X);
        if (lAnimCurve)
        {
            FBXSDK_printf("        TX\n");
            DisplayCurve(lAnimCurve);
        }

In other words, it looks like you don't enumerate which properties your AnimCurves control; you explicitly ask a certain property (e..g. pNode0->LclTranslation) if it is affected by a certain curve in a certain way. Or, at least, this is one of the ways you can do it, and the fact that it's the one they chose in the SDK example implementation suggests it's at least not an insane approach.

Since glTF also has only very specific properties available for animation (translation, scaling, rotation, and morph target weights), in practice the above should work well.

Please let me know if you chose to continue work on your PR. Maybe I can help a bit along the way. If you find it hard going, I will be able to get to this sooner or later.

It's also possible that the current approach could be tweaked to handle your use case correctly? That would certainly be less work.

johnshaughnessy commented 6 years ago

Thank you @zellski, this helps a lot!

willeastcott commented 5 years ago

Just checking in on whether this was worked on at all. I have just noticed that animation import bakes keys for all nodes at 24fps:

https://github.com/facebookincubator/FBX2glTF/blob/master/src/fbx/Fbx2Raw.cpp#L677

So there's plenty of room for optimization here (I just had a confused artist ask me why his anims were the same size regardless of the keyframes he's set). At the very least, we could expose a '--keyinterval \<t>' command line option that'd override the default 24fps.

But ideally, yes, since glTF supports curve-based interpolation of keyframes, just exporting the keys set by the artist should be the ultimate goal.

zellski commented 5 years ago

@willeastcott No, it's a significant chunk of work that I won't realistically get to anytime soon. If anyone were interested in contributing a PR, though...

simonwashere commented 5 years ago

@zellski hello, although obviously replacing the baked animations with a new system is a lot of work the suggestion @willeastcott made of being able to change the baking framerate would be very useful for me (prefer 30fps to 24fps), so have just opened a PR with those changes, hope that is ok, thanks!