Fix CPUParticles3D using angle incorrectly when ROTATE_Y is set.
#89595
Conversation
AThousandShips
changed the title
CPUParticles3D using angle incorrectly when ROTATE_Y is set.
akien-mga
changed the title
CPUParticles3D using angle incorrectly when ROTATE_Y is set.CPUParticles3D using angle incorrectly when ROTATE_Y is set.
|
Seems to be working as intended 👍 |
| @@ -1124,7 +1124,7 @@ void CPUParticles3D::_particles_process(double p_delta) { | |||
| //turn particle by rotation in Y | |||
| if (particle_flags[PARTICLE_FLAG_ROTATE_Y]) { | |||
| Basis rot_y(Vector3(0, 1, 0), p.custom[0]); | |||
| p.transform.basis = p.transform.basis * rot_y; | |||
| p.transform.basis = rot_y; | |||
There was a problem hiding this comment.
The equivalent GPU particles code is this:
// turn particle by rotation in Y
if (particle_flags[PARTICLE_FLAG_ROTATE_Y]) {
code += " vec4 origin = TRANSFORM[3];\n";
code += " TRANSFORM = mat4(vec4(cos(CUSTOM.x), 0.0, -sin(CUSTOM.x), 0.0), vec4(0.0, 1.0, 0.0, 0.0), vec4(sin(CUSTOM.x), 0.0, cos(CUSTOM.x), 0.0), vec4(0.0, 0.0, 0.0, 1.0));\n";
code += " TRANSFORM[3] = origin;\n";
}I'm not too familiar with the math here, but aren't we losing at least the origin potentially?
There was a problem hiding this comment.
Nevermind, we're only changing basis, so origin is untouched.
There was a problem hiding this comment.
The equivalent GPU particles code is this:
These look the same indeed. 👍
There was a problem hiding this comment.
We are losing align with velocity though. But GpuParticles behavior seems to be similiar, it's one or another.
| @@ -1124,7 +1124,7 @@ void CPUParticles3D::_particles_process(double p_delta) { | |||
| //turn particle by rotation in Y | |||
| if (particle_flags[PARTICLE_FLAG_ROTATE_Y]) { | |||
| Basis rot_y(Vector3(0, 1, 0), p.custom[0]); | |||
| p.transform.basis = p.transform.basis * rot_y; | |||
| p.transform.basis = rot_y; | |||
There was a problem hiding this comment.
The equivalent GPU particles code is this:
These look the same indeed. 👍
|
Thanks! And congrats for your first (!?) merged Godot contribution 🎉 I guess this will help with getting CI running out of the box to finalize #80710 once the rendering team is ready for it :D |
Resolves #89559.