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

GLTF: Import step interpolation for loc/rot/scale as nearest #86016

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

demolke
Copy link
Contributor

@demolke demolke commented Dec 11, 2023

Currently all transform animation tracks get imported as linear.

image

To support things like camera cuts honor the interpolation mode set in the gltf.

@demolke demolke requested a review from a team as a code owner December 11, 2023 01:13
@fire
Copy link
Member

fire commented Dec 11, 2023

Note that GLTF cubic is not the same as godot cubic, so it will need to be baked or you can devise a curve fitting schema.

	} else if (gltf_interp == GLTFAnimation::INTERP_CUBIC_SPLINE) {
		return Animation::InterpolationType::INTERPOLATION_CUBIC;
	} else {
		return Animation::InterpolationType::INTERPOLATION_CUBIC;

@demolke demolke force-pushed the master branch 2 times, most recently from 821ac38 to 7d19976 Compare December 11, 2023 09:33
@demolke demolke changed the title Import interpolation for loc/rot/scale from GLTF Import step interpolation for loc/rot/scale from GLTF as nearest Dec 11, 2023
@demolke
Copy link
Contributor Author

demolke commented Dec 11, 2023

Thanks a lot for taking a look - I should have been clearer, this is only about the STEP/NEAREST interpolation.

The method already does handle baking.

For STEP interpolation we do not want any in-betweens generated, so we have to mark the resulting animation track with NEAREST interpolation, the other tracks will get baked correctly.

I've made the change handle only the STEP case, wdyt?

@demolke demolke force-pushed the master branch 2 times, most recently from 19cba4c to 1252a2d Compare December 11, 2023 09:43
@AThousandShips AThousandShips added this to the 4.x milestone Dec 11, 2023
@fire fire requested a review from a team December 12, 2023 21:26
@demolke demolke force-pushed the master branch 2 times, most recently from 5f8afba to c2e1077 Compare December 13, 2023 00:11
@demolke demolke requested a review from fire December 15, 2023 21:03
@fire fire requested a review from a team December 16, 2023 19:28
@fire
Copy link
Member

fire commented Dec 16, 2023

@demolke
Copy link
Contributor Author

demolke commented Dec 17, 2023

I think the change is working as it should. The existing code bakes everything to linear, so the only time we need to propagate the original interpolation intent is when it's STEP/NEAREST.

Currently all object transform animation tracks get imported and baked
as linear. For step interpolation mark the resulting animation track
with Nearest interpolation to make sure there are no in-betweens
generated. This is useful for camera cuts or similar.
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Makes sense to me. We also have the same logic for weights a bit over 50 lines below here, so it makes sense to do this for pos/rot/scale tracks as well.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 8, 2024
@akien-mga akien-mga merged commit 60f557c into godotengine:master Jan 8, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@demolke
Copy link
Contributor Author

demolke commented Jan 8, 2024

Thank you

@akien-mga akien-mga changed the title Import step interpolation for loc/rot/scale from GLTF as nearest GLTF: Import step interpolation for loc/rot/scale as nearest Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants