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

BoneLookController: Fix bone scaling being lost #14987

Merged
merged 1 commit into from Apr 17, 2024

Conversation

Popov72
Copy link
Contributor

@Popov72 Popov72 commented Apr 16, 2024

See https://forum.babylonjs.com/t/why-is-bone-scale-resetting-during-bonelookcontroller-update/49667

This PR fixes the problem, HOWEVER I don't understand where in the calculation in BoneLookController or in Bone the bone scaling is lost.... It would be better to understand, to be sure it's the right fix...

@bjsplat
Copy link
Collaborator

bjsplat commented Apr 16, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Apr 16, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Apr 16, 2024

Visualization tests for WebGPU (Experimental)
Important - these might fail sporadically. This is an optional test.

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/14987/merge/testResults/webgpuplaywright/index.html

@bjsplat
Copy link
Collaborator

bjsplat commented Apr 16, 2024

Copy link
Contributor

@CedricGuillemet CedricGuillemet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not surprised to see scaling lost somewhere because of matrix decompose/recompose.

@RaananW
Copy link
Member

RaananW commented Apr 16, 2024

I am merging this. @Popov72 , @CedricGuillemet - do we want to open an issue to track the question in the description? Or is this the accepted solution?

@CedricGuillemet
Copy link
Contributor

Let's see if @bghgary has some recommendation. For me, it's an acceptable fix.

@RaananW
Copy link
Member

RaananW commented Apr 16, 2024

Might be the culprit? - https://github.com/BabylonJS/Babylon.js/blob/master/packages/dev/core/src/Bones/bone.ts#L438

I'll wait for @bghgary to merge this.

@bjsplat
Copy link
Collaborator

bjsplat commented Apr 16, 2024

Visualization tests for WebGPU (Experimental)
Important - these might fail sporadically. This is an optional test.

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/14987/merge/testResults/webgpuplaywright/index.html

@Popov72
Copy link
Contributor Author

Popov72 commented Apr 16, 2024

Might be the culprit? - https://github.com/BabylonJS/Babylon.js/blob/master/packages/dev/core/src/Bones/bone.ts#L438

The decompose gets the scaling, so that should be ok.

@RaananW RaananW merged commit 5ec504d into BabylonJS:master Apr 17, 2024
11 checks passed
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.

None yet

4 participants