-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Add inherit_scale to bones #83903
base: master
Are you sure you want to change the base?
Add inherit_scale to bones #83903
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As described in godotengine/godot-proposals#4190 (comment), this should be Editor Only feature and enable only for BoneRest. "Pose Scale is propagated to the children" is never a problem, so this fix is not a proper solution.
replying to godotengine/godot-proposals#4190 (comment) and #83903 (review)
If this was editor only, it would make certain runtime character proportion editing and procedural animation impossible. By making it an option that can be applied per bone at runtime, we open the possibilities for non-authored procedural animations that weren't possible before. I'd also like to mention that even though blender does use rest transforms for its bone Edit Mode, even it has an option to control how transforms are propagated during animation per bone:
This can be a problem if you are trying to make character customization that relies on proportional editing, which is the primary use case for needing to change bone scale inheritance. #83903 addresses two problems. Firstly, it adds a feature that allows users to choose which bones inherit their parent's full transform, or only inherit rotation/translation. In the current version of Godot, you can manually change a child bone's transform to account for the scale propagation, but the problem is that even with manual scale correction, there are major sheering and rotation artifacts which get worse the further the child scale is from 1.0. The second problem this PR addresses is the aforementioned sheering and rotation artifacts, when using |
Read that comment godotengine/godot-proposals#4190 (comment) right. This is all I have to say.
Then relative animation/local transform (means same behavior with 3.x) should complement the case which you are described - character animation with scaled bone in runtime as long as you only allow the BoneRest to have a scale, not BonePose... but there is no use case there for BonePose i.e. to make the animation scale at runtime so that it does not propagate to the children. not accepted by Godot core if there is no need for it. If you are thinking of using Pose to change the shape of your character in the first place, it is completely wrong.
Also, Blender animation system is a bit unique as well as the bone structure. They will be discarded when export it as glTF, so it makes no sense to make them compatible there. |
However, if we make the Editor allows independent bone manipulation, I think it would be possible to implement a usability function in Skeleton3D for independent bone manipulation. This should make it possible to create addons that do not propagate specific elements by overriding |
Even in 3.x, the rest transform is propagated to children. This is a problem because as stated in my last comment, even if you correct scaling yourself, there are sheering artifacts which cannot be prevented from outside of the actual propagation function. One way to solve this would be to allow users to define their own transform propagation function similar to Array.sort_custom(), but that would be far too complicated for most users. The other way to solve this is by adding functionality for the most common use cases people would have for this feature, which is what this PR does. This very same optional transformation correction can be expanded to rest transforms when/if they get the old 3.x functionality back in 4.x.
There absolutely are use cases for changing transform propagation at runtime. For games, procedural animation and character creation with proportional editing like ARK. Some games actually scale bones locally on the x/y axes when the mesh is under armor to reduce clipping. For robotics, inverse kinematics with telescopic/linear actuators. Keep in mind, it's an optional feature so people who don't need it aren't affected by it. It can also be useful to toggle this on and off as the given animation requires, especially for procedural animation.
All components of a pose, namely the location, rotation, and scale, are intended to change the shape, as it represents a transformation. That's the point. If you mean explicitly the customizable proportions, that is absolutely something that belongs in pose, and would come down to the needs of the individual project and its animation team.
This isn't necessarily something that ever needs to be added to the importer, because it's probably not possible given the glTF spec. It's just there for the cases (usually for advanced IK armatures) that need analogous functionality. Users can toggle this feature as it is needed on a per-bone basis. |
Even so, it would need to be an option like "Pose ignores Rest's scale", not "whether to propagate the scale". However, regarding Rest deformations affecting it, I believe it is simplest to implement a function that "rebinds the skins and initializes the scales". This will lose the scale information, but if you need to use them again in the character creator, you can make them into a dictionary to store it.
As I have said many times, body shape adjustments should be done in Rest.
This is more of a bone structure issue. They should not have a parent-child relationship to begin with. If you want to make animation on Godot, Constraint/IK should be implemented or a feature such as BoneAttachment should be used. Additionally, it is not a valid theory because sharing animations of robotics is very rare. As shown above, godotengine/godot-proposals#4190 issue can be solved by adding some editor options and functions. So there is no point that really needs to implement this property in your described use cases. To begin with, Bone and Node3D have almost the same concept of parent-child relationship behavior for deformations, and implementing the "do not propagate parent scale" option only for Bone is not good from an architectural design point of view and would require adding it to Node3D as well. Moreover, such options can break animation sharing in retargeting, so basically, options for bones should be implemented in the SkeletonProfile. However, these would be too extensive and would not be allowed. In summary, we believe that the correct architectural design to solve those problems is to divide them into the following two implementations. 1. implement an Independent mode in BoneEditor (or Node3DEditor if needed) that automatically adds cancel deformations to children of deformed bones
2. implement functions like
|
Ignoring Rest's scale wouldn't be enough, because of sheering. Please note that this doesn't just prevent scale propagation, but performs other transformation correction during propagation. These corrections cannot be applied after the fact, and must be applied during propagation.
I agree that pose should be relative to rest, but that is just not how things are right now. Having
That is irrelevant because while Bone and Node3D are very similar, they in no way affect each other or should have feature parity because of this similarity. I would also like to point out that Node3D already has similar functionality to this, however this PR only discludes scale inheritance, and does all necessary rotation and translation correction, as users expect. See https://docs.godotengine.org/en/stable/classes/class_node3d.html#class-node3d-property-top-level
These solutions are needlessly complex, and retargeting/animation sharing would not be affected in any way, as this is intended to be used after a skeleton has already been retargeted, during live animation. I'd like to demonstrate what this PR does visually. The next gif is from 4.x. As we know, pose is not relative to rest in Godot 4, but the same transformation as the first gif is being applied to the pose transform. The animation does not contain scale keyframes, so it is valid to change scale in pose. This final gif is in my fork of 4.x master with this PR applied. The upper arm has a pose scale of (1.0, 4.0, 1.0), and the lower arm has a scale of (1.0, 1.0, 1.0) and its I say again: Adding optional scale propagation and sheering correction is the only way to achieve these results, even with pose relative to rest. |
I have understood what it is supposed to do. But as I said above, it is absolutely sufficient to statically remove scale by rebind. If we are concerned about processing at runtime, the best we can approve is to implement some of the functions (such as
The problem is that when you share an animation created on a model that uses that property with a model that does not have that property set, you have to rewrite the properties of the target model.
Not make sense, the main problem with your PR implementation is that you are confusing the animation with the adjustment of the body shape. When we split the implementation into two, we have more steps, but the workflow is simpler; no one thinks applying scale on Blender is complicated. |
Simply rebinding the skin won't affect BoneAttachment3D, IK, or anything else that relies on the direct bone transform. This isn't intended for preprocessing animations or skeletons like retargeting. This is intended as an additive effect after the skeleton is already imported, retargeted, etc.
This isn't true. This isn't a property on models. It is a property on Bone in Skeleton3D. It is skeleton/model agnostic and non-destructive. You only disable
One of the most important parts of this PR is that it does translation correction when a parent bone is scaled. Simply not inheriting parent scale is not enough for the intended effect. When you scale a parent bone, you expect the child bone to still inherit its position as if it was scaled from its parent, then it adjusts scale as needed.
I don't understand what you mean by "no one thinks applying scale on Blender is complicated". Some games have character creators that allow proportional editing, which by its very nature cannot be pre-authored, or applied in Blender. Such games are not currently possible in Godot without major animation artifacting. Yes, rebinding the skin can change the scale of bones, but not the correct proportional offsets if you change the length of a bone. It has to be done on the skeleton, and as demonstrated earlier, even pose relative to rest will result in artifacts when scaled non-uniformly. |
I already answered this above. If the current scaling method has problems with IK, it is a bone structure issue. It should not have a parent-child relationship. Besides, it would be more ideal if all the bone scales were removed for IK and PhysicalBone in the first place.
No, bones are tied to the model. For example, SkeletonProfile may change the Rest of the bone by importing. And, I am pointing out that if you have two models, there is no way to synchronize their properties since a model may have a large number of bones, the bone options involved in such deformations should be presettable in a SkeletonProfile or copied by the utility function. Don't distract from the point. And the rest, I can only say is that you are confusing animation with body shape adjustment. You may be overzealous with your first Pull Request, but you need to get calm down. Your implementation confuses multiple issues, and so problematic from an architectural design, that we cannot approve it. |
Perhaps for physical bones because scaling collision shapes/rigid bodies is problematic, but sometimes there are uses for scaling in BoneAttachment3D, and IK. Examples: parenting armor to a BoneAttachment3D when the character's proportions are edited, procedural/IK squash and stretch rigs, rendering linear actuators/pistons for robotics development.
This change does not affect import, glTF models, or SkeletonProfile. It is for use after skeletons are already imported in the engine. Its primary use is procedural animation, and character scaling. I believe the problem you are describing is if someone imports two separate models that have different skeleton proportions and wants to unify animations between them using SkeletonProfile. This PR is still compatible with that, as it only adds optional transform correction during bone transform propagation.
My implementation is almost a direct correlation to how Blender and Maya solve it. It doesn't complicate any existing transform logic. It is intended to expand on what is already possible with scale animation in the least intrusive way possible. |
Your repeating things I have negated many times above does not make them correct.
Blender's bone structure is quite special, but Maya also has a special bone system. Maya's storing and controlling local axes way is special. Furthermore, the glTF skins output from Maya are bound in a special way. I remember I had to implement some complex calculations for a glTF output from Maya when I worked on the importer. Godot is Blender friendly, but it is not Blender, and the existence of a feature in Blender is not a reason to implement that feature in Godot. The only case in which Godot will follow the addition of an external feature is when glTF is extended. |
Memo: If the purpose of the bone transformation is to adjust the body shape, then the method I commented on above should work just fine. However, in some cases where the purpose of the animation is not to adjust the body shape, it may be useful to prevent certain properties from being inherited. In such cases, it is common in other game engines to use Bone Constraint to dynamically constrain certain properties. It seems completely make sense since it does not change the properties of existing Skeleton in Godot. Bone Constraint is not yet implemented in Godot, but we do have plans to work on it at the same time we implement IK, etc. In conclusion, the correct way is to implement a method like Bone Constraint that controls the bone from the outside, not having the bone itself have the properties of the constraint as in this PR. |
Add inherit_scale to bones Adds inherit_scale property to bones in Skeleton3D. Setting this to false will allow the bone to be scaled independently of its parent.
Adds inherit_scale property to bones in Skeleton3D. Setting this to false will allow the bone to be scaled independently of its parent.
This is useful for character customization that requires proportional editing. This is also useful for some animation techniques that were not possible before.
Addresses godotengine/godot-proposals#4190