SkeletonUtils: Fix retargetClip() duration and last frame handling.

[AlaricBaraou]

Fixed #25288

Description

Description

This PR addresses two main issues in the animation retargeting process:

Issue 1: Incorrect Delta Calculation

Problem: The calculation of the relation between numFrames and delta was incorrect.

Expected Behavior: For an animation lasting 2 seconds with 3 frames spaced evenly, the expected delta should be 1s.

• Frame 0: 0s
• Frame 1: 1s
• Frame 2: 2s

For a given 1.5 FPS ( 3 frames / 2 seconds ) = 1.5
amount of frames = Duration ( 2s ) * 1.5 FPS = 3 frames
New Formula: The revised formula correctly calculates the delta:
duration (2s) / (amount of frames (3) - 1) => 1s
Old Formula: Previously, the formula incorrectly calculated the delta:
1 second / FPS (1.5) => 0.666666s
Impact: This incorrect delta calculation resulted in the animation retrieving the wrong part, leading to a shortened and trimmed appearance.

Issue 2: Incorrect Last Frame in Loop

Problem: In the final loop iteration, the last frame was identical to the first frame, instead of representing the state of the last frame of the original animation.

To illustrate, here is how a 3 frame animation loop back to its start position without that fix.
Before adding if ( i === numFrames - 2 )
https://github.com/mrdoob/three.js/assets/7174039/c2bb49ef-3810-4b53-b6d4-a0e413a8140f
After
https://github.com/mrdoob/three.js/assets/7174039/226ed358-bc8d-4d38-bdde-5354b25c5953

[Mugen87]

I've locally tested the change with the fiddle from #25288 (comment). These are the console outputs:

original duration: 4.924997806549072
webgl_loader_bvh.html:75 original track0 frames: 592
webgl_loader_bvh.html:79 retarget duration: 4.924997806549072
webgl_loader_bvh.html:80 retarget track0 frames: 591

The duration is now correct but it seems the new keyframes are missing one entry (591 vs. 592). I would expect matching keyframe length.

[AlaricBaraou]

How did you calculate the FPS option?
I assume you used the existing method from the fiddle that give this result.
If so, can you try after following what I explained in this comment ?
#25288 (comment)
I used the same example yesterday and it was working at the condition of using the right option.fps value.

[AlaricBaraou]

Also, like mentioned in this comment #25288 (comment) it could make sense to default to the original animation FPS to avoid this small alteration.
But maybe that would be considered a breaking change ?
Personally I think it's not really breaking, if that's fine with you, I can add this change too.

[Mugen87]

If so, can you try after following what I explained in this comment ?

Ah, the keyframe length is now correct!

Also, like mentioned in this comment #25288 (comment) it could make sense to default to the original animation FPS to avoid this small alteration.

The new suggested default makes sense to me.

[AlaricBaraou]

Sorry my previous commit would work in most cases but it's not guaranteed that the first track is using the most frames.
Some animation that don't update the scale have a length of 2 on those tracks while the others have more.

In the last commit we're certain to use the correct fps.
It's not very efficient / readable but that way it runs only if no fps was provided.

[Mugen87]

Let's give this a try!