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

Negative scaling is broken when using Jolt Physics #103408

Open
JJ505 opened this issue Feb 28, 2025 · 3 comments · May be fixed by #103426 or #103440
Open

Negative scaling is broken when using Jolt Physics #103408

JJ505 opened this issue Feb 28, 2025 · 3 comments · May be fixed by #103426 or #103440

Comments

@JJ505
Copy link

JJ505 commented Feb 28, 2025

Tested versions

v4.4.rc2.mono.official [01545c9]

This error appeared after upgrading to 4.4rc2 from 4.3. I was previously using the jolt physics add on.

This error goes away if i switch to the godot physics engine instead. It reappears if using jolt.

System information

Godot v4.4.rc2.mono - Windows 10 (build 19045) - Multi-window, 2 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 (NVIDIA; 32.0.15.6603) - AMD Ryzen 7 3700X 8-Core Processor (16 threads)

Issue description

With Jolt Physics enabled, when adding a static body as a child of a VehicleWheel3D I get a flood of quaternion errors:

E 0:00:01:120   get_quaternion: Basis [X: (1.0, 0.0, 0.0), Y: (0.0, 1.0, 0.0), Z: (0.0, 0.0, -1.0)] must be 
normalized in order to be casted to a Quaternion. Use get_rotation_quaternion() or call orthonormalized() 
if the Basis contains linearly independent vectors.
  <C++ Error>   Condition "!is_rotation()" is true. Returning: Quaternion()
  <C++ Source>  core/math/basis.cpp:730 @ get_quaternion()

I can get rid of the errors if I move the StaticBody3D outside of the VehicleWheel3D. I do expect it to work as before. I prefer adding a staticBody3D as a child of VehicleWheel3D for organizations sake.

Steps to reproduce

  1. Enable Jolt Physics under Project -> Project Settings -> Physics -> 3D -> Physics Engine
  2. Add a VehicleBody3D node with a collision mesh
  3. Add a VehicleWheel3D node
  4. Add a StaticBody3D node as a child of the VehicleWheel3D
  5. Launch the scene and a flood of quaternion errors should appear

Including project as well. Open the project and run.

Thank you.

Minimal reproduction project (MRP)

mrpWheelStaticBodyChild.zip

@Alex2782 Alex2782 added this to the 4.x milestone Feb 28, 2025
@vmbaillie
Copy link

Suggested fix here

#103426

There may be other places to reduce calculation repetition, best to get an expert to review before any merge

@mihe mihe added the topic:3d label Mar 1, 2025
@mihe
Copy link
Contributor

mihe commented Mar 1, 2025

So, for whatever reason, a VehicleWheel3D apparently gets a transform set on it (by the VehicleBody3D) that's mirrored on the Z-axis (i.e. negatively scaled), which strikes me as a potential bug in itself, not to mention the inherent problems with mixed negative scaling (as mentioned here).

However, negative scaling is seemingly supported by Godot Physics (at least for convex StaticBody3D) and was supported in the Godot Jolt extension as well, but it seems I broke it in the process of porting the code, as I was trying to get rid of this decompose function in favor of using what was available through Basis and Transform3D (while knowingly sacrificing some performance through redundant calculations).

So the much more basic MRP here is to just create a StaticBody3D and scale it negatively on one or more axes, which will produce the same error message.

While Basis::get_rotation_quaternion (as seen in #103426) fixes this for the case where we're converting only a Basis, there are more places that do orthonormalization, some of which do it before we convert a Godot Transform3D to a Jolt JPH::Mat44, which goes through a different conversion function:

_FORCE_INLINE_ JPH::RMat44 to_jolt_r(const Transform3D &p_transform) {
const Basis &b = p_transform.basis;
const Vector3 &o = p_transform.origin;
return JPH::RMat44(
JPH::Vec4(b[0][0], b[1][0], b[2][0], 0.0f),
JPH::Vec4(b[0][1], b[1][1], b[2][1], 0.0f),
JPH::Vec4(b[0][2], b[1][2], b[2][2], 0.0f),
JPH::RVec3(o.x, o.y, o.z));
}

I suspect we'll run into trouble there as well.

(Also, just as an aside, you shouldn't be using a StaticBody3D when childing physics bodies like this, as they're meant to be non-moving and therefore have no velocities associated with them, which will produce odd collision results, along with other weird issues. The better option is to use something like a RigidBody3D with freeze enabled and freeze_mode set to "Kinematic".)

@mihe mihe changed the title StaticBody3D as child of VehicleWheel3D results in flood of quaternion errors with jolt physics. 4.3 -> 4.4RC2 upgrade issue. Negative scaling is broken when using Jolt Physics Mar 1, 2025
@mihe mihe modified the milestones: 4.x, 4.5 Mar 1, 2025
@mihe mihe self-assigned this Mar 1, 2025
@mihe mihe linked a pull request Mar 1, 2025 that will close this issue
@mihe
Copy link
Contributor

mihe commented Mar 1, 2025

I put up an alternative fix in #103440, which essentially just brings back the code that seems to have worked well enough in the extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment