Allow preserving the initial bone pose in rest fixer #88821
Conversation
lyuma
force-pushed
the
preserve_initial_pose
branch
from
92f8603
to
7828f28
Compare
|
Is there a reason why this cannot be defaulted on? |
|
Can you explain what the actual case this solves is? |
|
A model may be in some pose upon export from the DCC. ufbx has some code to guess the rest pose from skin binds, and the retarget silhouette fixer will also attempt to calculate a new rest pose. The retarget system operates entirely on bone rest. however, some code at the end of the retarget flow copies the bone rest to the bone pose, losing the initial pose (which may have been important). It is probably not important for some usecases, but I need the initial pose in Unidot because scene inheritance may depend on knowing the initial pose. |
|
I understand that in cases where the model includes both rests and poses, it is important that the importer be based on rests only. I am still unclear about the case in question. Is the problem case where the rest is a T-pose but the pose is not a T-pose? Or opposite? Is it only the pose that appears after import that is affected? Is the animation also affected? Can animations be shared correctly between multiple models using this workflow? It would be helpful to have a file for testing to help me understand |
|
From what I understand, your concern is that a model is imported which perhaps only has a subset of animation tracks (for example, Left and Right arms)
Correct, this is the only thing affected
It should not be affected by the initial pose. My understanding is the rest fixer guarantees that animation keys are added for all bones in the bone map (unless the user manually disables this) Basically, the animation keys would be relative to the initial pose. However, during retarget we may adjust the model's pose and it is critical that the animation continue to look correct. Therefore, Godot must already be adding the necessary keys, and animations should play regardless of pose. My assumption here is that the animation optimizer won't remove tracks that are equal to the bone pose: if so, this would cause t-pose animations to have no tracks in current version of godot.
I do not see how the initial bone pose would affect animation sharing. The initial bone pose should be lost when animations are played. This PR is really a matter of "correctness" or a "technicality". If a model is initially posed as doing a handshake without retarget, it should also be initially posed in a handshake with retarget. Retarget should be adjusting the rolls of the bones, but not affect the visual result of the model import. To be perfectly honest, this PR is not critical. But it feels like the right thing to do, to avoid losing information. |
|
If I understand correctly, we could change the option name to like
However, I believe this option should be outside of Retargeter. And then, it may be fine to disable it by default, since it is visually independent of the retarget workflow. |
|
Reset all Bone Poses after Import sounds good! As for why it's in retargeter, it is because that is the only place I could guarantee correctness. However, if we blindly reset bone poses outside of rest fixer, then we may need to add additional animation tracks for each bone we changed which means there will be possible side effects of using this option that the user may not expect. To contrast, in retargeting, it is expected that all animation tracks exist after retargeting is done, so there will be no side effects of adjusting the bone pose. To summarize:
|
|
After renaming it to |
Adds a "Reset All Bone Poses After Import" option to the Skeleton3D Rest Fixer settings. Default value of true resets the bone poses to rest (usually a t-pose), matching previous behavior. If disabled, keeps the bones posed as they would have been before silhouette adjustment.
lyuma
force-pushed
the
preserve_initial_pose
branch
from
7828f28
to
6a6bd23
Compare
| @@ -603,6 +609,30 @@ void PostImportPluginSkeletonRestFixer::internal_process(InternalImportCategory | |||
| } | |||
| } | |||
| } | |||
| if (p_options.has("retarget/rest_fixer/reset_all_bone_poses_after_import") && !bool(p_options["retarget/rest_fixer/reset_all_bone_poses_after_import"])) { | |||
There was a problem hiding this comment.
If store the bool results of this earlier, you can avoid unnecessary calculations for some pose changes in line 117-120 and 131 as well.
There was a problem hiding this comment.
I considered this, but the number of calculations are low, and doing them in that part of the code preserves correctness (counteracting scale changes).
|
Thanks! |
Adds a "Preserve Initial Pose" boolean option to the Skeleton3D Rest Fixer settings
retarget/rest_fixer/preserve_initial_poseThis option preserves the bone pose as it would have been before silhouette adjustment.
Basically, ufbx calculates the rest pose differently in some models which use skin binds. Rest pose should only used in the retargeting process, but prior to this change, the retargeting flow left the model in rest pose and discarded the actual bone_pose, which now makes scene inheritance dependent on the retargeter's rest pose.
Before:
Show Rest Only (same before and after):
After: