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

Fixed blending value track without RESET and TRS3D track with RESET #60235

Merged
merged 1 commit into from
Apr 15, 2022

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Apr 14, 2022

Follow up #60093. I mistook value track blend process (in line 1328).

It may also be necessary to add an additional method to look for RESET track in multiple AnimationLiblaries. This will be added later. After discussions with reduz, it was concluded that the RESET track should be created by Godot and reside in Global.

Skeleton3D already has Rest so it should not use the RESET track. The 3D model is basically imported with the T-pose as rest, and the RESET track should be used to determine the pose at load time for its intended pose. And if a track is missing from the blend animation, it should be use rest value, which is reasonable considering the Optimizer and Retargeter (in the future) behavior.

@TokageItLab TokageItLab added this to the 4.0 milestone Apr 14, 2022
@TokageItLab TokageItLab requested a review from a team as a code owner April 14, 2022 09:24
@TokageItLab TokageItLab force-pushed the fix-value-track-blend branch 2 times, most recently from 8aec3b8 to 37002d5 Compare April 14, 2022 09:49
@TokageItLab TokageItLab changed the title Fixed value track blend animation without RESET Fixed value track blend animation without RESET and Bone rotation with RESET Apr 14, 2022
@TokageItLab TokageItLab force-pushed the fix-value-track-blend branch from 37002d5 to 57d6a0d Compare April 14, 2022 09:54
@TokageItLab TokageItLab force-pushed the fix-value-track-blend branch from 57d6a0d to 0fe723a Compare April 14, 2022 10:01
@TokageItLab TokageItLab changed the title Fixed value track blend animation without RESET and Bone rotation with RESET Fixed blending value track without RESET and TRS3D track with RESET Apr 14, 2022
@fire
Copy link
Member

fire commented Apr 14, 2022

Can we discuss having the reset track override skeletal rest if it exists?

@TokageItLab
Copy link
Member Author

TokageItLab commented Apr 14, 2022

@fire So what you mean is that if a RESET track is exist, the missing track will use the value of the RESET track instead Rest?

At least, the 3D rotation should respect the REST and calculate the shortest path. It can be done if the exception handling by has_rest does not handle only ref_rot, but I am not sure if that makes sense. See 57d6a0d.

@TokageItLab
Copy link
Member Author

TokageItLab commented Apr 14, 2022

@fire Let's consider a use case. For example, a model imported as T-pose and posed to sit-down in a RESET track. Then, applying RESET to the missing track would change the result of the combined retargeting and blending, depending on the pose of the RESET track. Perhaps it will cause confusion.

Ideally, we would like to have a RESET track in the Node instead of in the Animation, but that is a bit of an overstatement.

Since non-3DSkeleton animations are generally not used across scenes and rarely apply animations to different objects, the RESET track can be used as a Rest. However, for 3DSkeletons which not so, I think that Rest should be respected.

@fire
Copy link
Member

fire commented Apr 14, 2022

I mention this because GLTF files in the production do not provide t-pose rests most of the time.

@TokageItLab
Copy link
Member Author

TokageItLab commented Apr 14, 2022

I mention this because GLTF files in the production do not provide t-pose rests most of the time.

It is the T-poser's job in the Retarget/Import process to T-pose models that do not have a T-pose Rest, and it makes no sense to T-pose in RESET. It means that the 3D model will be loaded with the initial pose in T-pose when the scene is loaded. For example, some NPCs should have a T-pose rest, but it is not ideal for them to be loaded with T-pose.
Also, even for non-humanoid models that do not need to be loaded in T-pose, Rest should be set correctly in glTFExporter or SkeletonEditor.

@fire
Copy link
Member

fire commented Apr 14, 2022

Makes sense.

@akien-mga akien-mga merged commit 3639c27 into godotengine:master Apr 15, 2022
@akien-mga
Copy link
Member

Thanks!

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.

4 participants