Problems with key-framed (MD2) animations

[gouessej]

Hi

When I try to use a MD2 model on a very slow machine (then I have a low frame rate), the very last interpolation seems wrong, the model looks weird, especially when dying. This bug is reproducible with JMonkeyEngine 2 and Ardor3D:

Have you ever had the same problem?

The source code of the key frame controller is here:
https://github.com/Renanse/Ardor3D/blob/master/ardor3d-extras/src/main/java/com/ardor3d/extension/model/util/KeyframeController.java

The delta is always in [0;1]. I wonder if the variable containing the index of the next frame can take a wrong value, for example the first frame of the next part would be picked instead of the last frame of the current part.

[Riven]

I'm sure there already was a thread about this, right?

[Riven]

if you look at line 436 you'll see that the author already tried to fix this case. my assumption is the bug is in findFrame. why not do some system-out's (or breakpoints) when _curFrame and _nextFrame are assigned. the business logic there seems like it can miss several edge cases.

Games published by our own members! Check 'em out!

[Stranger]

I wonder if the variable containing the index of the next frame can take a wrong value, for example the first frame of the next part would be picked instead of the last frame of the current part.

Looks like that. Suspicious lines:

630                 _nextFrame = 1;
...
639                 for (_nextFrame = 0; _nextFrame < _keyframes.size() - 1; _nextFrame++) {

[gouessej]

Thank you for your help. I'm going to give it another try tonight. I think that it would be better if I kept a laptop with me to make some tests during my pause at work but my battery is broken.

Edit.: I don't call setSmoothTranslation in my case, _isSmooth is then always set to false.

[Stranger]

I think _nextFrame < _keyframes.size() should be in line 639 (and 671 too):

1   2   3   4   5   6

//line 639 for (_nextFrame = 0; _nextFrame < _keyframes.size() - 1; _nextFrame++) {     if (getMinTime() <= _keyframes.get(_nextFrame)._time) {         break;     } }

[gouessej]

I get this: http://pastebin.java-gaming.org/f809147595e

[gouessej]

Stranger's suggestion doesn't work.

I get this with some more logs:
delta:3.3000550600000054 _curTime:53.300055060000005 _curFrame:50 _nextFrame:51 _before._time:50.0 _after._time51.0 minTime:46.0 maxTime:53.0

delta is a lot too big, it can only produce something completely absurd.

[gouessej]

The problem seems to come from delta and _curTime. When "time" is too big (low frame rate -> big period), the interpolations are wrong. Forcing the delta to keep between -1 and 1 almost fixed the bug but animations are stopped before their end :s

[Riven]

Whatever the delta is, the logic should never allow the 'cursor' to go beyond its allowed range.

IMHO the implementation is very messy, and it could be replaced with a much simpler version that simply steps through the frames one at a time (like a doubly linked list, to support going backwards) , taking into account RepeatType when choosing the next frame.

Games published by our own members! Check 'em out!

[gouessej]

This class was not designed to support this case, in which the passed time is so big that it is longer than the time required to play all animations of a frame set. My animations would look a bit unregular without morphing, wouldn't it? I ws too lazy to rewrite this class, maybe I will have to do so.

[gouessej]

Riven, you're probably right, the bug comes from findFrame. Look at this case:
delta:3.3000550600000054 _curTime:53.300055060000005 _curFrame:50 _nextFrame:51 _before._time:50.0 _after._time51.0 minTime:46.0 maxTime:53.0

This happens at the end of an attack. _curTime becomes bigger than _maxTime and findFrame just does that:
line 631

1	if (_curTime > getMaxTime()) {

line 652

1   2

} else if (getRepeatType() == ComplexSpatialController.RepeatType.CLAMP) {                 return;

It should clamp _curTime to _maxTime if _movingForward is set to true instead of allowing the morphing code to use _curTime as is.

[Riven]

That whole class desperately needs a complete overhaul, but I guess you can simple tell Renanse to patch it

[gouessej]

Then I have to provide the patch 

[gouessej]

This seems to work:

1   2   3   4

double localMinTime = Math.min(_before._time, _after._time);           double localMaxTime = Math.max(_before._time, _after._time);           double clampedCurTime = Math.max(localMinTime, Math.min(_curTime, localMaxTime));           double delta = (clampedCurTime - _before._time) / (_after._time - _before._time);

but the weapon stays above the dead guy 

[gouessej]

I forgot that weapons have only 173 frames (less than enemies).

[gouessej]

I have fixed this bug with your help, thank you very much. I will make a pull request as soon as possible. I'm very happy as this class is very important for my game, I don't want to use skeletal animations yet.

[gouessej]

The pull request is here:
https://github.com/Renanse/Ardor3D/pull/44