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

Fix collided particles getting stuck with zero velocity. #87320

Merged
merged 1 commit into from
May 15, 2024

Conversation

Rudolph-B
Copy link
Contributor

@Rudolph-B Rudolph-B commented Jan 18, 2024

Resolves #87221

Changes the behavior of slow collided particles from being set to completely stationary to only removing the part of the velocity that will move the particle into the obstruction.

Project showcasing the change:
CollisionTest.zip

It is basically GPUParticles3D falling on a GPUParticlesCollisionBox3D with a GPUParticlesAttractorBox3D trying to push the particles of.

The issue can be seen here. The particles get stuck even though this is supposed to be a friction less environment.

How the changes fix the issue can be seen here. The particles glide on top of the obstruction. And with the the GPUParticlesAttractorBox3D disabled particles still fall normally and become stationary as can be seen here

An other possible fix was to adjust the if statement found in particle_process_material.cpp:991

		code += "		if (length(VELOCITY) > 3.0) {\n";
		code += "			TRANSFORM[3].xyz += COLLISION_NORMAL * COLLISION_DEPTH;\n";
		code += "			VELOCITY -= COLLISION_NORMAL * dot(COLLISION_NORMAL, VELOCITY) * (1.0 + collision_bounce);\n";
		code += "			VELOCITY = mix(VELOCITY,vec3(0.0),clamp(collision_friction, 0.0, 1.0));\n";
		code += "		} else {\n";
		code += "			VELOCITY = vec3(0.0);\n";
		code += "		}\n";

To be less strict with length(VELOCITY) > 1.0 or something similar. This produced excessively bouncy particles though.

bugsquad edit: Fixes #91346

@clayjohn
Copy link
Member

It seems that this if statement comes from #55387

@RPicster Mentions the reason for zeroing the velocity as:

I also took care that collisions look good and happen in a way that makes sense (no further turbulence gets applied after a collision).

This PR will reintroduce that issue. I have a feeling that we will need to do something more intrusive. Right now the logic is just suppressing any movement at all if there is a collision and the velocity is low. But that has two problems:

  1. Its essentially an infinite amount of friction for slow moving particles
  2. The cutoff is arbitrary

The bounciness you see from decreasing the VELOCITY threshold comes from the fact that the particles are set to be a bit bouncy by default and this logic applies the bounce every frame (i.e. bounce amount isn't effectively scaled by velocity appropriately). I think the issue is that only the magnitude of VELOCITY is taken into account when calculating the bounce. So a large horizontal velocity translates into a large movement along the normal, which doesn't make a lot of sense.

As a follow up I would check a few things:

  1. Is it possible to suppress the noise without setting VELOCITY to 0? If so, then we can remove this if branch completely and just suppress the noise when the magnitude of VELOCITY is less than 0
  2. Can we modify the bounce term to take direction of VELOCITY into account (more than just as a scale factor) (perhaps multiplying by the absolute direction of VELOCITY would be enough?)

@RPicster
Copy link
Contributor

Please make sure to test your intended behavior with all kinds of settings and velocities/accelerations. Turbulence including.

Changing things in particle behavior may work under certain circumstances (eg for your use case), but may break others.

@AThousandShips AThousandShips changed the title Fixed collided particles getting stuck with zero velocity. Fix collided particles getting stuck with zero velocity. Apr 24, 2024
@Rudolph-B
Copy link
Contributor Author

I've made updates to the code: I replaced the if statement with a step function to handle the transition from bouncing to sliding in particles. The step function uses the particle's velocity along the collision normal to determine how long it bounces before it starts sliding. Additionally, particles defined with higher bounciness will also bounce longer before sliding.

I have adjusted my test project to include some more test cases:
CollisionTest.zip

Everything looks good to me but I am not really sure how the interaction with turbulence is supposed to work.

@clayjohn
Copy link
Member

Testing out the new MRP, I can confirm that, with this PR, turbulence is free to drag particles across the surface of the collider. I can also confirm that adding a bit of friction stops that. To me, that is what is intuitive.

@RPicster When you check this out, please pay special attention to distinguish between particles sliding across the surface of collisions due to turbulence, and particles jittering on the surface of collisions due to turbulence. I suspect that the bouncing/jittering is the behaviour that was most annoying before as the sliding can easily be addressed by adding friction.

@RPicster
Copy link
Contributor

RPicster commented May 2, 2024

Testing out the new MRP, I can confirm that, with this PR, turbulence is free to drag particles across the surface of the collider. I can also confirm that adding a bit of friction stops that. To me, that is what is intuitive.

@RPicster When you check this out, please pay special attention to distinguish between particles sliding across the surface of collisions due to turbulence, and particles jittering on the surface of collisions due to turbulence. I suspect that the bouncing/jittering is the behaviour that was most annoying before as the sliding can easily be addressed by adding friction.

I tested this PR in different scenarios and I find the behaviour very nice. It works like Clay described it. Increasing friction will give control over the behaviour.

Get's a 👍 from my side.

One thing I noticed and it was mentioned by Clay in the Rendering Meeting was the "distance" from the particle to the surface after collision - so after turning up the collision size of the particles I observed bouncing particles on the surface.

I also tested this in 4.3dev6 and this PR and it seems that this behaviour is not great in this PR.
Not saying it was "great" before, but the problem that was already there is now much more visible and visually impactful.

I think it is related to interpolation/fixed FPS.

This is 4.3dev6 - you can observe the particle is just stopped on collision, sometimes at the wrong distance:
https://github.com/godotengine/godot/assets/9423774/af4448bf-77e3-4e6d-ad44-a7ec49a188f2

This PR - the particle starts bouncing in and out of the surface. Increasing fixed FPS makes it much less visible.
https://github.com/godotengine/godot/assets/9423774/b8e3765f-96b2-47a2-a25d-659c9de44477

I'd say it would be great if this could be fixed 🥇

@Rudolph-B
Copy link
Contributor Author

I have made the adjustments requested by @Ansraer.

I am still looking into the problem @RPicster noted.

@Rudolph-B
Copy link
Contributor Author

@RPicster Can you maybe share a MRP. I am struggling to clearly see the issue.

@RPicster
Copy link
Contributor

RPicster commented May 3, 2024

@RPicster Can you maybe share a MRP. I am struggling to clearly see the issue.

bouncy.zip
Here is a scene where the issue is easy to spot 👍

@Rudolph-B
Copy link
Contributor Author

I believe I have fixed the issue. What happened was, after a particle collided, it was move to just touch the surface of the collider. When determining whether a particle is collided this particle then often didn't count as collided. In cases where the particle then instantly started moving again in the direction of the collider it would cause a jarring snap like affect. Especially at lower FPS.

The solution I implemented was to add an EPSILON term to all 4 collision calculations in servers/rendering/renderer_rd/shaders/particles.glsl. The goal being that particles stay collided until they move away from the collider.

The below project has a scene name "bouncy" showcasing the behavior for all 4 3D collision shapes.
CollisionTest.zip

@RPicster Can you please test this again.

I have not yet checked other collision system if they have similar issues.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

As discussed in the rendering meeting. Just separate the particles.glsl stuff to a new PR (add it to OpenGL at the same time) and add some comments about the new order of operations for friction. Then should be good to merge!

@Rudolph-B
Copy link
Contributor Author

@clayjohn Added the comments and removed the particles.glsl stuff. Will make the new PR for particles.glsl a bit later I would first like to do some tests and see if 2D particle collisions have a similar issue.

@Rudolph-B Rudolph-B requested a review from clayjohn May 14, 2024 19:02
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks great to me! Great work

@akien-mga akien-mga merged commit ca2ed80 into godotengine:master May 15, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@RPicster
Copy link
Contributor

It doesn't really add anything to this PR - But I just happen to come across the original problem this PR solves and I could apply the changed shader code in a "real-world" scenario.

It looks really perfect - so thank you again @Rudolph-B !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants