Add copy_bone_skin_scale to RetargetModifier3D - #122729
Open
TokageItLab wants to merge 2 commits into
Open
Conversation
co-authored-by: lyuma <xn.lyuma@gmail.com>
AThousandShips
left a comment
AThousandShips
left a comment
Member
There was a problem hiding this comment.
(Some are missed cases from the original PR I believe, can apply them here or over there depending on which is most appropriate)
| <member name="copy_bone_skin_scale" type="bool" setter="set_copy_bone_skin_scale" getter="is_copying_bone_skin_scale" default="true"> | ||
| If [code]true[/code], copies [method Skeleton3D.get_bone_skin_scale] of the source skeleton's bones to the child skeletons' mapped bones. | ||
| If [code]false[/code], the skin scale of the child skeletons' mapped bones are reset to [code]Vector3(1, 1, 1)[/code] once and will not be overwritten after that. | ||
| [b]Note:[/b] If you want to use [method Skeleton3D.set_bone_skin_scale] on the child skeleton's mapped bone, set this to [code]false[/code]. Even if you are not using [method Skeleton3D.set_bone_skin_scale] with parent skeleton, [code]Vector3(1, 1, 1)[/code] will always be set for the child skeletons' mapped bones. |
Member
There was a problem hiding this comment.
Suggested change
| [b]Note:[/b] If you want to use [method Skeleton3D.set_bone_skin_scale] on the child skeleton's mapped bone, set this to [code]false[/code]. Even if you are not using [method Skeleton3D.set_bone_skin_scale] with parent skeleton, [code]Vector3(1, 1, 1)[/code] will always be set for the child skeletons' mapped bones. | |
| [b]Note:[/b] If you want to use [method Skeleton3D.set_bone_skin_scale] on a child skeleton's mapped bone, set this to [code]false[/code]. Even if you are not using [method Skeleton3D.set_bone_skin_scale] with parent skeleton, [code]Vector3(1, 1, 1)[/code] will always be set for the child skeletons' mapped bones. |
I'd say
| <param index="1" name="skin_scale" type="Vector3" /> | ||
| <description> | ||
| Sets the skin scale of the bone at [param bone_idx] to [param skin_scale]. This scale does not affect bone children. This is useful for modifying the visual thickness of a skin. | ||
| Even if the skin is visually deformed, neither scaling nor repositioning of the bones occurs. It means that you may need to adjust the origin of the child bone to synchronize its joint position with the scaling by using [BoneSpreader3D]. |
Member
There was a problem hiding this comment.
Suggested change
| Even if the skin is visually deformed, neither scaling nor repositioning of the bones occurs. It means that you may need to adjust the origin of the child bone to synchronize its joint position with the scaling by using [BoneSpreader3D]. | |
| Even if the skin is visually deformed, neither scaling nor repositioning of the bones occurs. This means that you may need to adjust the origin of the child bone to synchronize its joint position with the scaling by using [BoneSpreader3D]. |
Comment on lines
+7
to
+10
| Inputs a scale and converts it to the position of a child bone. Intended for use in combination with [method Skeleton3D.set_bone_skin_scale]. | ||
| The skin scale modification is sufficiently working for scaling along axes other than the one connecting the parent to the child bone. | ||
| However, if the child bone is positioned outside the axis, visual discrepancies occur. | ||
| This modifier converts the scale value into the child's bone position and corrects these discrepancies. |
Member
There was a problem hiding this comment.
I don't think this needs to be four paragraphs
Also what does "The skin scale modification is sufficiently working" mean?
| PopupMenu *p = skeleton_options->get_popup(); | ||
| p->add_shortcut(ED_SHORTCUT("skeleton_3d_editor/reset_all_poses", TTRC("Reset All Bone Poses")), SKELETON_OPTION_RESET_ALL_POSES); | ||
| p->add_shortcut(ED_SHORTCUT("skeleton_3d_editor/reset_selected_poses", TTRC("Reset Selected Poses")), SKELETON_OPTION_RESET_SELECTED_POSES); | ||
| p->add_shortcut(ED_SHORTCUT("skeleton_3d_editor/reset_all_pose_and_skin_scales", TTRC("Reset All Bone Pose & Skin Scales")), SKELETON_OPTION_RESET_ALL_POSE_AND_SKIN_SCALES); |
Member
There was a problem hiding this comment.
Suggested change
| p->add_shortcut(ED_SHORTCUT("skeleton_3d_editor/reset_all_pose_and_skin_scales", TTRC("Reset All Bone Pose & Skin Scales")), SKELETON_OPTION_RESET_ALL_POSE_AND_SKIN_SCALES); | |
| p->add_shortcut(ED_SHORTCUT("skeleton_3d_editor/reset_all_poses_and_skin_scales", TTRC("Reset All Bone Poses & Skin Scales")), SKELETON_OPTION_RESET_ALL_POSE_AND_SKIN_SCALES); |
| enum SkeletonOption { | ||
| SKELETON_OPTION_RESET_ALL_POSES, | ||
| SKELETON_OPTION_RESET_SELECTED_POSES, | ||
| SKELETON_OPTION_RESET_ALL_POSE_AND_SKIN_SCALES, |
Member
There was a problem hiding this comment.
Suggested change
| SKELETON_OPTION_RESET_ALL_POSE_AND_SKIN_SCALES, | |
| SKELETON_OPTION_RESET_ALL_POSES_AND_SKIN_SCALES, |
| <return type="bool" /> | ||
| <param index="0" name="index" type="int" /> | ||
| <description> | ||
| Returns whether to use the `skin_scale` property of a [Skeleton3D]'s bone of the setting at [param index]. |
Member
There was a problem hiding this comment.
Suggested change
| Returns whether to use the [code]skin_scale[/code] property of a [Skeleton3D]'s bone of the setting at [param index]. |
| <param index="0" name="index" type="int" /> | ||
| <param index="1" name="enabled" type="bool" /> | ||
| <description> | ||
| Sets whether to use the `skin_scale` property of a [Skeleton3D]'s bone of the setting at [param index]. |
Member
There was a problem hiding this comment.
Suggested change
| Sets whether to use the [code]skin_scale[/code] property of a [Skeleton3D]'s bone of the setting at [param index]. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BoneSpreader3D&skin_scaleproperty to Bone in Skeleton3D #120609What problem(s) does this PR solve?
When using RetargetModifier3D, if you want to scale a model’s skin using
skin_scale, you’ll need to scale the skin of theOriginalSkeletonbecause theGeneralSkeletondoes not have a skin. However, doing so causes a significant misalignment between theGeneralSkeletonand theOriginalSkeleton, making it difficult to view.If you apply
skin_scaleandBoneSpreaderonly toGeneralSkeleton, only the position animation will be retargeted, and the skin scaling will be ignored.We will add an option to RetargetModifier to copy the
skin_scalefrom the GeneralSkeleton to the OriginalSkeleton. This automates the coordinate transformation, allowingskin_scaleto be applied to both. The process is as follows: