Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support all types of animation interpolation from gltf #10755

Merged
merged 5 commits into from
Dec 31, 2023

Conversation

mockersf
Copy link
Member

Objective

  • Support step and cubic spline interpolation from gltf

Solution

  • Support step and cubic spline interpolation from gltf

Tested with https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/InterpolationTest
expected:

result:
output


Migration Guide

When manually specifying an animation VariableCurve, the interpolation type must be specified:

  • Bevy 0.12
        VariableCurve {
            keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0],
            keyframes: Keyframes::Rotation(vec![
                Quat::IDENTITY,
                Quat::from_axis_angle(Vec3::Y, PI / 2.),
                Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.),
                Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.),
                Quat::IDENTITY,
            ]),
        },
  • Bevy 0.13
        VariableCurve {
            keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0],
            keyframes: Keyframes::Rotation(vec![
                Quat::IDENTITY,
                Quat::from_axis_angle(Vec3::Y, PI / 2.),
                Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.),
                Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.),
                Quat::IDENTITY,
            ]),
            interpolation: Interpolation::Linear,
        },

@mockersf mockersf added A-Animation Make things move and change over time M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Nov 26, 2023
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a cursory initial scan through. I'll do a full review at a later point in time.

crates/bevy_animation/src/lib.rs Show resolved Hide resolved
crates/bevy_animation/src/lib.rs Outdated Show resolved Hide resolved
@nicopap nicopap self-requested a review November 27, 2023 08:00
@alice-i-cecile alice-i-cecile added the C-Feature A new feature, making something new possible label Nov 27, 2023
@alice-i-cecile
Copy link
Member

Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay on this review! Everything looks good to me now.

crates/bevy_animation/src/lib.rs Show resolved Hide resolved
@torsteingrindvik
Copy link
Contributor

@alice-i-cecile or @james7132 should this get the ready for final review label?

I'm hoping this PR fixes a GLTF scene I have which uses step interpolation.

Copy link
Contributor

@torsteingrindvik torsteingrindvik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this PR on my model which is broken on main due to it using step interpolation and this PR fixes that.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Dec 29, 2023
Copy link
Contributor

@nicopap nicopap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mockersf
Copy link
Member Author

added the requested comment, can be merged now 👍

@mockersf mockersf added this pull request to the merge queue Dec 31, 2023
Merged via the queue into bevyengine:main with commit 71adb77 Dec 31, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time C-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants