X Tutup
Skip to content

Fix NaN populating ParticleProcessMaterial Transform#97871

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
AtlaStar:fix-issue-97680
Oct 28, 2025
Merged

Fix NaN populating ParticleProcessMaterial Transform#97871
Repiteo merged 1 commit intogodotengine:masterfrom
AtlaStar:fix-issue-97680

Conversation

@AtlaStar
Copy link
Contributor

@AtlaStar AtlaStar commented Oct 6, 2024

Use of normalize will cause transform matrix slices to be populated with NaN when the scale value was zero on the previous compute step as is shown in this screenshot from RenderDoc. This bug doesn't occur when particle_flag_disable_z is set regardless of other flags being set, and will not happen if particle_flag_rotate_y is set. So any use of normalize there shouldn't be fragile enough to break.

image

The above was before the compute step was ran, while after the buffer had those zero values replaced with NaN
image

This shows that the only way to resolve this bug, is to not use normalize in specific areas. This PR resolves this bug by adding a normalize_or_else function. Parameters are _in, and _else, with the former being the argument to test to see if it is a zero vec3, and the latter the vec3 to use in the event of _in being the zero vec. Concept behind requiring a vec3 to be passed in is that some behaviors or flags may require a default other than what would be the corresponding slice of an identity matrix, although currently I am unaware if there is a necessity as the testing done suggested parity was retained with CPUParticles3D.

Just some visuals of one of the setups for the particles I was using
image
image
image

closes #97680
closes #97621

@AtlaStar AtlaStar requested a review from a team as a code owner October 6, 2024 08:04
@AtlaStar
Copy link
Contributor Author

AtlaStar commented Oct 6, 2024

@FireCatMagic figured that you'd be interested in an update, because if nothing else this presents a fix you can potentially start using now (all the stuff changed are things auto-gen'd into the particle process material shader)

@Chaosus Chaosus added this to the 4.4 milestone Oct 6, 2024
@TheYellowArchitect
Copy link
Contributor

TheYellowArchitect commented Oct 6, 2024

I had a GPUParticles3D which was one-shot, yet the "emitting" flag was like a dirty flag. So when the scene stopped, it was no longer emitting inside the godot editor, and I had to re-open the scene and enable the "emitting" flag, in order to launch the game with an emitting GPUParticles3D. I built this patch via https://docs.godotengine.org/en/stable/contributing/development/compiling/compiling_for_linuxbsd.html and it fixes this issue for me, despite not being completely the same problem as the OP's.

@AtlaStar
Copy link
Contributor Author

AtlaStar commented Oct 6, 2024

Oh, nice @TheYellowArchitect! Did you have an issue open for that bug already? If so I can link that issue so it will get closed when this PR is merged.

@TheYellowArchitect
Copy link
Contributor

TheYellowArchitect commented Oct 6, 2024

Nah, it was very similar to the linked issue #97680 (that is how I found this PR), so I didn't open an issue as it would be a duplicate. Here are the similarities of my GPUParticle3D compared to #97680 :
Quad Mesh + one-shot + alpha color curve + scale curve

Nothing else related to that issue (no curve with initial value 0, or billboards)

Edit: If I wait for the one-shot to finish, so the flag "emitting" becomes false, it also becomes false in the editor, despite emitting in the editor view. So I will make a new issue for that and link it here 👍
Regardless, this issue fixed the bug where I stop the main scene with emitting GPUParticle3Ds and it stops emitting in the GPUParticles3D scene (dirty flag)
With this PR, if I stop the main scene while emitting, GPUParticle3Ds continue emitting in their own scene properly.

@AtlaStar AtlaStar requested review from a team as code owners October 10, 2024 19:31
@AtlaStar AtlaStar requested review from a team as code owners October 10, 2024 22:46
@AtlaStar
Copy link
Contributor Author

sigh, screwed up the rebase on this...gonna fix it...

Use of normalize will cause transform matrix slices to be populated with NaN when the scale value was zero on the previous compute step.

Resolves this bug by adding a `normalize_or_else` function. Parameters are `_in`, and `_else`, with the former being the argument to test to see if it is a zero vec3, and the latter the vec3 to use in the event of `_in` being the zero vec.

closes godotengine#97680
closes godotengine#97621
@FireCatMagic
Copy link
Contributor

@FireCatMagic figured that you'd be interested in an update, because if nothing else this presents a fix you can potentially start using now (all the stuff changed are things auto-gen'd into the particle process material shader)

I didn't see this till now, this is great work!!
Messing around with GPU particles this fixes a lot of what buggy behavior there was, thank you for this

@fire
Copy link
Member

fire commented Oct 13, 2024

The physics interpolation has a way to avoid normalize you might want to investigate

@AtlaStar
Copy link
Contributor Author

@fire I will look into it again then, but if it was the already existing safe_normalize testing showed that it didn't work and that setting the scaling params of the transform all to 0 would result in the transform never being set to a value other than 0 because in later steps the transform is not overwritten other than by the result of the normalization, and then used in a multiplicative operation. So the m11 minor being all 0s will result in it being 0 for the life of the particle, and never rendering.

The only solution I was able to find is specifically setting the transform values to the appropriate slice of an identity matrix, not a [0.0, 0.0, 0.0] vector.

@AtlaStar
Copy link
Contributor Author

@fire yeah testing still shows safe_normalize will cause issues, at least as of Godot v4.4.dev (92e51fc)
image

This was also the result withbillboarding both on and off, so the keep scale setting doesn't even matter when using the safe_normalize function. Because the transform is being multiplied by the scale value in order to properly set the particle scale, setting things to zero when normalization would fail just results in the particles not drawing (unless you minimize and maximize, for some reason the particles in editor draws when restoring the screen? Maybe it is doing CPU drawing when minimized?)

@lyuma
Copy link
Contributor

lyuma commented Oct 14, 2024

I don't see how the normalize function is to blame here.

But I don't understand this if statement in the code

if (length(final_velocity) > 0.0) {

Usually comparing floating points to precise values is not recommended because error can easily accumulate and make the check false.

So is it possible that it breaks for very small velocities like 1e-15? Would it be easier to solve this issue with a larger deadzone, like 0.00001 ?

@AtlaStar
Copy link
Contributor Author

I don't see how the normalize function is to blame here.

But I don't understand this if statement in the code

if (length(final_velocity) > 0.0) {

Usually comparing floating points to precise values is not recommended because error can easily accumulate and make the check false.

So is it possible that it breaks for very small velocities like 1e-15? Would it be easier to solve this issue with a larger deadzone, like 0.00001 ?

Because the transform is filled with all 0's on the scale and rotation bits. Normalizing a vector is just dividing each term by it's length, so when the length is 0... you get NaN. If instead you use the safe version, you still have all 0's in there, which you then use as the basis to multiply the scaling values by, resulting in it forever remaining 0.

The issue this solves was only occurring if at some point in the particles lifetime scale was 0. Any other time, there is no issue.

@AtlaStar
Copy link
Contributor Author

@lyuma also, to make it extra apparent in case it wasn't, the issue this causes is entirely caused by NaN being populated in the transform, and the two most common ways to produce NaN are adding/subtracting to infinity, or dividing 0 by 0. Normalize was basically doing (0 + 0 + 0)/0, hence NaN being produced by normalize.

The safe version when the row is 0 just keeps it 0, which in this specific case is actually wrong because of the later multiplication done in the compute shader.

@QbieShay
Copy link
Contributor

I believe at a certain point I had a fix for this that was max(scale, 0.00001). I wonder why it isn't working anymore.

@AtlaStar
Copy link
Contributor Author

@QbieShay When the scaling factor is 0, the sign of it is also 0. Personally I think that having 0 sized particles should be possible, which is solved by replacing the transformation when it is all 0 to an identity prior to multiplying it by the scaling values at the end of the shader.

@QbieShay
Copy link
Contributor

I don't think there's a significant difference between 0.0 and 0.000001, and safe normalizing things costs performance

@AtlaStar
Copy link
Contributor Author

AtlaStar commented Oct 17, 2024

@QbieShay the issue with that is again that this scenario pops up when using a curve texture primarily. So you go from having to do some form of safe normalize to having to clamp individual color channels from values that are zero to some minimal amount, meaning that you'd have to do the logic you mentioned at the site of the texture read...and the problem of figuring out the sign still persists.

If negative scaling values weren't supported, this would be very simple...but because they are you can't just multiply things by the sign of the value, you have to branch on the sign being 0 to replace it with a small but non-zero value...and if a branch is required some form of safe normalization makes more sense imo.

@QbieShay
Copy link
Contributor

QbieShay commented Oct 17, 2024

Sorry, i should have taken the time to read your code properly before commeting. I had assumed you're calculating length but you aren't.

@AtlaStar
Copy link
Contributor Author

AtlaStar commented Oct 17, 2024

It is all good. In all reality this wouldn't be an issue if glsl had a signbit operator because you could just replace the use of sign with signbit

@AtlaStar
Copy link
Contributor Author

Ya know on saying the above out loud so to speak, I realize that writing a signbit operator probably would be more performant than this solution @QbieShay

@Repiteo Repiteo modified the milestones: 4.4, 4.5 Feb 24, 2025
@Repiteo Repiteo modified the milestones: 4.5, 4.6 Sep 8, 2025
@Repiteo Repiteo merged commit f1f3f06 into godotengine:master Oct 28, 2025
@Repiteo
Copy link
Contributor

Repiteo commented Oct 28, 2025

Thanks!

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.

GPUParticle3D stops rendering in editor under specific circumstances Converted CPUParticles3D to GPUParticles3D weird particle creation behavior

8 participants

X Tutup