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

Scale doesn't reset when undoing a rotation with non uniform scale bones in Skeleton3D #84249

Closed
Tracked by #91519
marciocjr13 opened this issue Oct 31, 2023 · 11 comments

Comments

@marciocjr13
Copy link

Godot version

v4.1.1.stable.mono.official [bd6af8e]

System information

Godot v4.1.1.stable.mono - Windows 10.0.19044 - Vulkan (Forward+) - dedicated AMD Radeon RX 580 2048SP (Advanced Micro Devices, Inc.; 31.0.12027.9001) - Intel(R) Core(TM) i5-10400F CPU @ 2.90GHz (12 Threads)

Issue description

If the parent of a bone is of non uniform scale (anything other than 1), then the bone scale of the child will change when rotating it, this is actually normal behaviour to keep the proportions as far as I am aware, but the issue is when you undo the rotation (in case of a mistake), the editor will revert the rotation but will not revert the scale, which can lead to models eventually looking "distorted" if you don't notice it at first.

Example Screenshots:

  • Here is the skeleton with the torax bone having a non uniform scale, look at the scale of the arm and compare it in the following screenshots:
    Screenshot 2023-10-31 102536

  • Now with rotation applied to the arm, the scale has changed. The scale does not change in uniform scale bones but as far as I am aware this is normal behaviour to keep proportions consistent.
    Screenshot 2023-10-31 102557

  • Now here is the issue: if you undo, the rotation will revert back, but the scale will not. Eventually, it will add up and distort the model.
    Screenshot 2023-10-31 102612

In the example project I included both an uniform and non uniform variant of the same skeleton so you can compare the results.

Steps to reproduce

  1. Scale a bone with children to a non uniform (non 1) scale
  2. Rotate the children
  3. Undo

Minimal reproduction project

Skeleton Scale Issue.zip

@AThousandShips
Copy link
Member

Can you please try this with a supported version, like 4.1.2 (only the latest patch version is supported, and it might already be fixed)

@marciocjr13
Copy link
Author

Can you please try this with a supported version, like 4.1.2 (only the latest patch version is supported, and it might already be fixed)

I have tried it now on v4.1.2.stable.mono.official [399c9dc] and with v4.2.beta4.mono.official [93cdacb] and can confirm the issue persists.

@rsburke4
Copy link
Contributor

rsburke4 commented Nov 11, 2023

As shown in the video below, I think I found a solution to the "undo" issue, but there's something more serious going on here. (Note: I flattened the torso a lot to exaggerate the effect here. The more the scaling, the greater the effect of the propagated error.)

2023-11-10.20-38-04.mp4

If the behavior we're trying to achieve is localizing the non-uniform scale to bones higher in the hierarchy without affecting children than I think there's a little extra work to do. I don't mind looking into this further.

Edit: Is what we're trying to achieve something akin to "Flat Stanley"? Where we can scale a bone non-uniformly and have all of its children bones inherit this property, but then keep the bone lengths the same? For instance, a character flattened in one dimension could bend its flattened arms into other dimensions and maintain their flatness relative to their original flattening? (Sorry I'm having a hard time wording that better.)

@TokageItLab
Copy link
Member

TokageItLab commented Nov 11, 2023

Related godotengine/godot-proposals#4190. I believe one of the solution to this is #83903 (comment). I will put it in a proposal later but I am currently working on another task so please wait a little while.

Now with rotation applied to the arm, the scale has changed. The scale does not change in uniform scale bones but as far as I am aware this is normal behaviour to keep proportions consistent.

Probably there is a problem of normalization in bone, but I guess #83903 (comment) works as a workaround.

@TokageItLab TokageItLab modified the milestones: 3.x, 4.x Nov 11, 2023
@fire
Copy link
Member

fire commented Nov 11, 2023

@bbqbarbhg developed a way to emulate the two modes mentioned in ufbx for godot engine. Can you describe how they work?

V-Sekai/godot-ufbx#28

@bqqbarbhg
Copy link
Contributor

bqqbarbhg commented Nov 11, 2023

Not sure if it applies here, but the scale compensation by undoing parent scale as in #4190 will break for non-uniform scaling. What I did in ufbx is to add auxiliary in case animated or non-uniform scaling is used. The scale of the parent node is moved into a helper:

Parent (scale set to 1)
+ ScaleHelper (scale moved here)
| + (skin reference)
| + ScaledChildren
+ UnscaledChildren

To make this work you still need to modify the translation of UnscaledChildren by the scaling amount of ScaleHelper. This works for animations contained within the file but fails for runtime animations, as either you scale Parent which will scale UnscaledChildren or ScaleHelper which will cause UnscaledChildren to not move as intended. So ideally supporting non-scale-inherited bones natively in the implementation without helpers would be ideal.

If a native non-scale-inheriting bone mode would be implemented it would benefit the ufbx importer as well, as the workaround has the above caveat and some others caused by using external animation library assets. FBX also contains a mode that propagates the scale without shearing, but it is so rarely used that it's not worth implementing into a generic skeleton in my opinion.

@fire
Copy link
Member

fire commented Nov 12, 2023

The previous link was godotengine/godot-proposals#4190

@fire

This comment was marked as resolved.

@fire
Copy link
Member

fire commented May 17, 2024

#92012 was merged to fix this. Can you try?

@fire
Copy link
Member

fire commented May 24, 2024

I tested a build with #92012 and further changes and it seems to be fixed. Did not narrow down the list of changes though.

@fire
Copy link
Member

fire commented May 27, 2024

Thanks for your bug report, as far as I can tell it's fixed, but feel free to comment if it's still broken.

@fire fire closed this as completed May 27, 2024
@AThousandShips AThousandShips modified the milestones: 4.x, 4.3 May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants