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

Turbulence/Noise for ParticleMaterial (2D / 3D) #55387

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

RPicster
Copy link
Contributor

@RPicster RPicster commented Nov 27, 2021

It's a feature I miss a lot and brings much more life and interesting patterns to almost any Particle System.

I uploaded a longer demo video showing the features and parameters in action:
https://www.youtube.com/watch?v=oTsEaKpzseE

Here are some small examples:

godot.windows.opt.tools.64_sNd3QdukI7.mp4
godot.windows.opt.tools.64_TUfD4UlVNY.mp4
godot.windows.opt.tools.64_ZBGLjShgWi.mp4
godot.windows.tools.64_OEH3Ll39JA.mp4
godot.windows.tools.64_JYbgrKcvHO.mp4

It comes with the following parameters

  • Turbulence Mode: Control if turbulence will be used and the turbulence style.
  • Turbulence Noise Contrast: Controls the contrast of the turbulence noise.
  • Turbulence Noise Scale: Controls the overall scale of the turbulence noise pattern
  • Turbulence Noise Speed: Controls the movement speed of the turbulence noise pattern (vec3)
  • Turbulence Noise Speed Random: Randomizes the Noise Speed - this helps to break up visual patterns if they are visible.
  • Turbulence Influence over life : This controls how much each particle will be influenced by the turbulence over it's lifetime
  • Position Influence : How much the turbulence influences each particles position (translation)
  • Velocity Influence : How much the turbulence influences each particles velocity
  • Color Ramp : Offers the option to multiply each particles color with this ramp depending on the turbulence direction

turbulence_options

(The original pull request also had rotation and scale influece, those were removed to reduce complexity)

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

The newly added code snippets for the noise in the shader have been checked with the original authors and both gave their (very friendly) approval to be included in the engine code.

TO DO:

  • Find out if licensed code parts are ok to use (Got feeback from Inigo Quilez that it's no problem)
  • Fix problems that Inigo Quilez mentioned: I'd however recommend against the use of 43758.5453123, it's a too large number and will fail in lower GPUs such as in Intel cards, VR headsets and other mobile GPUs. I'd also not use sin() within the hash function for the same reason. fract(fratct()) instead of fract(sin()) is often a better options.
  • Save the original EMISSION_TRANSFORM in a persistent way to make it looking good when moving the particle system around
  • Find the most important parameters, get rid of parameters that will probably never be used (candidates: rotation, color)
  • Prepare demo content for parameters
  • Find reason for biased, directional movement of particles

@RPicster RPicster force-pushed the particles-turbulence branch from a7baafa to 51cc4d6 Compare November 27, 2021 22:00
@RPicster RPicster requested a review from a team as a code owner November 27, 2021 22:00
@RPicster RPicster force-pushed the particles-turbulence branch 5 times, most recently from 0c47293 to ef79491 Compare November 28, 2021 11:40
@QbieShay
Copy link
Contributor

What is the license of the noise? I couldn't find it

@Calinou
Copy link
Member

Calinou commented Nov 28, 2021

What is the license of the noise? I couldn't find it

The PR description says:

As you can see in the implementation, the shader code for the 3D noise is by Fabrice Neyret & Inigo Quilez, the code is CC-BY, so it should be no problem

I would prefer if we were explicitly allowed to use the code under the MIT license, as CC BY is more restrictive than MIT overall (CC BY has stricter requirements for attribution). Feel free to contact Inigo Quilez about this 🙂

@RPicster
Copy link
Contributor Author

What is the license of the noise? I couldn't find it

The PR description says:

As you can see in the implementation, the shader code for the 3D noise is by Fabrice Neyret & Inigo Quilez, the code is CC-BY, so it should be no problem

I would prefer if we were explicitly allowed to use the code under the MIT license, as CC BY is more restrictive than MIT overall (CC BY has stricter requirements for attribution). Feel free to contact Inigo Quilez about this 🙂

I used this shader code because it is compact and works well.

As I tried different shaders in my research, I have a couple of alternatives if the license is not too great.

A shader with MIT license that I tried and worked perfectly with some super small modifications is this one:
https://github.com/ashima/webgl-noise/blob/master/src/noise3D.glsl

I could swap it without a big problem - it's just more code - but with some space saving actions it can be reduced to a moderate level.

I also just did a performance test and I couldn't make out any difference compared to the current code of this PR (tested with 12x1mio particle systems).

@QbieShay
Copy link
Contributor

Sorry for the oversight, I was looking for it into the code.

Thanks a lot @RPicster for the work!
I have pulled and tested the PR and I have a few observations about it.

My first observation is that I find this fairly complex to use. The knobs require a lot of tuning and there are a lot of configurations that won't work. I'd like to understand if we can reduce the knobs on turbulence to obtain something more "plug and play".
Compared with Niagara's curl noise for example, it's way more complex to use and tune.
https://www.youtube.com/watch?v=tAtny_0E2Is
Screenshot_2021-11-29_11-44-28

Another issue that I had is that power and size behave in very counter intuitive ways. With power set to 0.07, my particles are still accelerated very very far, and I also observed a some flickering and I'm not sure if it's something I am doing wrong, or if there's some bugs?
https://gfycat.com/enormousimmediateglobefish

I'm happy to chat about this on rocket chat if you wish. Thanks again for looking into this!!

@RPicster
Copy link
Contributor Author

RPicster commented Nov 29, 2021

Thanks for looking into the PR and for your feedback!

I'm a friend of giving the user a lot of control with good default settings.
As a user I love "plug and play" checkboxes - I click it and it looks great.

So that is something that could definetely be improved a lot on this PR - the default settings aren't great.

On the other hand, as a user, I love to experiment around with settings to get a big variety of effects.
With the option of always coming back to the default settings, this is a great way of finding new, cool effects.

The comparison to Niagara is a bit "apples and pears" because Niagara is built up entirely different.
A user can combine different systems like Vertex Fields and Noise - That's what I try to replicate in a small scale with color, scale, rotation, position and velocity.

If you like to compare this PR to what Unreal offers you need to compare it more to Vertex Fields instead of Curl Noise.

Anyway, to make the long story short: I love systems that offer great default settings so a beginner can activate the checkbox and have a great experience and an advanced user can play around and even achieve effects like lightning, fire, and all kind of effects.

Eventually it would make sense to only offer these two parameters and hide the rest behind a "advanced settings" checkbox.
turbulence-default

About your flickering issue: I guess it has to do with the scale settings.

@QbieShay
Copy link
Contributor

@RPicster right now with these new systems we want to move towards giving more control through the visual particle shaders, and having the default one be tunable but not too advanced.

I would perhaps remove a number of settings, specifically color, rotation influence (that could be achieved by using orient to velocity), scale influence. I didn't fully understand the difference between velocity influence and position influence (I didn't read the code yet).
I love that you have three different modes for noise (I don't think other engines do?) and I'd like to keep that.

So perhaps

  1. mode
  2. power
  3. noise panning (I'm edging more towards a float than a vec3 but I'm open to either)
  4. scale

For all the complex behaviours like tying the turbulence value to a color ramp, I'd expect the user to use a visual shader.

What do you think?

@RPicster
Copy link
Contributor Author

I don't like taking power away and say: if you want cool stuff you have to use a more complicated tool.

I rather hide the advanced settings behind a checkbox.

Dumbing down tools often makes them borderline useless for many cases ending up in people going back to customized shaders - at least that's what people in my network do (fellow good Godot users).

The difference between velocity and position is that position directly influences the particles position and velocity influences velocity (oh man that sounds dumb).

Eg. With position you can theoretically build a lightning bolt because the particle will be forced to a certain position offset.
Velocity influences the position inderectly / in the future.

My suggestions for simple settings would be:

  • Mode
  • Scale
  • Speed/Panning (as float)
  • Velocity Influence

When extending the settings, Speed is switched to a Vector3 (makes a lot of sense to e.g. make a fire that makes it look more like moving upwards).

And the rest of the settings are shown.

As said above, I want to empower the user and give a system that is flexible out of the box - saying "just use nodes" is really not a good solution imho as setting up a particle system with nodes is much more time consuming.

@QbieShay
Copy link
Contributor

Hey @RPicster, I understand where you're coming from but it is a current goal to expand more on what can be done with nodes rather than putting everything in the default particle system.

I fully agree on the statement of dumbing down the system, but this isn't what I'm suggesting. I believe advanced users should be able to harness the potential of it, and i believe that newcomers shouldn't have too much on their plate.
We align on the goal of not removing options from the users, but in this case the advanced option is the visual shader, where people have way more control and can fully make use of the output of the turbulence node.

I'm not sure you've seen it, but I'm referencing this work: #42248

@Chaosus
Copy link
Member

Chaosus commented Nov 29, 2021

As said above, I want to empower the user and give a system that is flexible out of the box - saying "just use nodes" is really not a good solution imho as setting up a particle system with nodes is much more time consuming.

In my opinion, the particle system especially and (some) other shader effects are much easier and support advanced effects if used through visual shaders. If you cannot do it yourself - I can take and implement turbulence to them myself later (after merging) but saying that is not a good solution is a bit hastily.

@RPicster
Copy link
Contributor Author

RPicster commented Nov 29, 2021

I think it is an absolute must have inside the visual shaders. I totally agree with you there. I love the idea, approach and power of the system.

I just also have the opinion that a user wanting to create a good looking standard effect should not be reliant on visual shader nodes.

It is much more complicated for a beginner and intermediate user compared to playing around with some sliders.

Giving the user only the 3-4 parameters will create another set of options that are just quite not powerful enough to create high quality assets with that System.

What is speaking against hiding advanced options behind a checkbox? It is not the same as having to use an entirely different setup.

@QbieShay
Copy link
Contributor

What is speaking against hiding advanced options behind a checkbox?

It's inconsistent with the rest: options that are deemed advanced are a target for the visual shader.

I just also have the opinion that a user wanting to create a good looking standard effect should not be reliant on visual shader nodes.

I'm confident that the standard can look good enough for most use cases ^^

I wanted to ask about something else: in my tests i noticed that there was a strong directional push, even with scrolling noise (most particles aligning on a tube along a diagonal on the zy plane). Is this intended?

@RPicster
Copy link
Contributor Author

RPicster commented Nov 29, 2021

Then I would rather not hide / dumb options down and keep them in the way they are.
I don't feel that users should be forced to use another tool for just some cool look.

Visual Shaders / Nodes have their use cases, but I think for such basic things (imho) it shouldn't be neccessary.

About the directional push: I noticed that too and still try to find ways to reduce it / get rid of it.

By the way - I totally disagree that Visual Shader Nodes should be considered the exclusive tool for advanced users and cool looking features should be limited to that.

@marcosa65
Copy link

I think it's a powerfull feature! and will be a shame to not have it , it can be more simple, true, but you can easily just have some default and advanced options, in order to keep it simple and allow more advanced use at the same time.

@Zireael07
Copy link
Contributor

and will be a shame to not have it

Who said it won't be merged?

@YuriSizov
Copy link
Contributor

Visual Shaders / Nodes have their use cases, but I think for such basic things (imho) it shouldn't be neccessary.

I don't think that this should be decided here, in this specific PR. If the design goal of the particle team is to move complexity and possible permutations to the visual shaders, then the new additions must adhere to the existing principles. Options can always be added later (new options with matching defaults don't break compat), but it's much harder to remove them (removing stuff always breaks compat).

If you find the overall goals of the particle team to be misguided, it's worth a dedicated discussion, and likely a proposal suggesting an alternative. Do keep in mind, though, that node-based systems seem to be the most popular trend in the digital art these days, so visual shaders would probably come more natural to a lot of people.

@RPicster
Copy link
Contributor Author

I would gladly take part in that discussion (I'm on the rocket chat) and I agree that node based systems are popular in certain software packages.
But Godot is not a visual effect package and should not be compared to Houdini or Blender in that regard.
Other Game Engines I used do not require the user to setup a Node Based setup to get advanced results.

A node system certainly does not come more natural, they require a lot of knowledge which nodes can be plugged where, which nodes exists to even start with etc. Learning a new node based system is a complex task.
I have used Stardust very extensively which is a node based Particle System - and it takes a long time to learn even for vfx professionals.

These people are not the typical Godot users - in my experience it's game devs.

Anyway - my personal next step with this PR will be to

  • Improve the default parameters
  • Improve the max/min values of the parameters
  • Prepare examples why I think the more complex / unneccesary parameters have an important role

As said, we can gladly meet up and discuss Particle Systems in voice or in some channel!

@YuriSizov
Copy link
Contributor

@RPicster I think you've made your point, but you can of course provide more examples if you think that they would swing the opinions of the particle team. However, if they don't, are you prepared to comply with the reviews and requirements from maintainers of the area even if you don't agree on the rationale?

@djrain
Copy link

djrain commented Nov 29, 2021

options that are deemed advanced are a target for the visual shader.

But parameters for rotation or scale influence can hardly be called "advanced options". They are no more advanced or complex than one for velocity. I would wager that most users using the turbulence at all would like to tune these as well. Therefore, to limit the influence to velocity only would be quite arbitrary, IMO.

@RPicster
Copy link
Contributor Author

RPicster commented Nov 29, 2021

Oh well, that sounds to me like the pistol on the chest - I don't hope that's what you try to say 😉😘
I would rather have the dedicated discussion that you suggested with the particle team before making any changes that I personally don't stand behind to this PR.

@RPicster RPicster force-pushed the particles-turbulence branch from c4389f7 to d80de57 Compare July 28, 2022 08:49
@RPicster
Copy link
Contributor Author

RPicster commented Jul 28, 2022

Hello @RPicster ,

There seems to be a bug where activating turbulence dampens the speed of the particles

I looked into it and I also noticed the behaviour that you are describing as damping.

I was very confused at first and checked the shader code... but if you think about it, it actually makes sense.
If you have a initial velocity and this velocity is modified by the turbulence, then of course the initial velocity is modified and due to the nature of the turbulence it can even be completely "negated" or even point into another direction.

I added a bit of code to compensate for it a bit... but I won't be able to "negate" it... as it would also make no sense from my understanding.

At least this was my conclusion... if you add some linear acceleration it behaves very differently.
If you think my conclusion is wrong or that we need to do changes... I'm totally open for it 😁

Please also check it again as I said: I changed some shader code and it could also have been another bug I found with the noise scale.

@RPicster RPicster force-pushed the particles-turbulence branch from d80de57 to 9137ba7 Compare July 28, 2022 09:04
@QbieShay
Copy link
Contributor

Hey @RPicster , I don't know why it doesn't show in this page but I think that it would be enough to normalize the resulting normal vector of the velocity (I commented on the code)

@clayjohn
Copy link
Member

I just see no real, practical benefit from changing the original hash function.
If you point me to a working alternative and a real world example where the original hash function produces real problems, I am happy to switch it out. (maybe the license would play another role. The original function just has a "add me to the shader code and everything is fine" while a MIT license would require equal treatment to FreeType, mbed TLS, etc.)

The current noise breaks down if positions are greater than ~100 units. So a particle system located 100 units away from the origin would start to show issues and one that is located 1000s of units away from origin will really start to break down.

That being said, I can't figure out why Hash33 is causing problems in your case. It should be ideal for this situation and in my testing it is working fine. Unfortunately I won't have time anytime soon to really dig into this.

Since you have tested very thoroughly and haven't run into any issues, I am fine merging with the hash function you have now. If the issues I anticipate arise we can easily switch out the hash function later.

@QbieShay
Copy link
Contributor

After testing it turns out that noise and turbulence actually looks better if particles lose velocity when changing directions harshly.
Here's video comparison
https://gfycat.com/fittingfakeamericanrobin

rightmost is with "dampened" turbulence, left is my own "fix.

Copy link
Contributor

@QbieShay QbieShay left a comment

Choose a reason for hiding this comment

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

Great work and really great addition for Godot.

Thank you so much and thanks for bearing with my whining.

doc/classes/ParticlesMaterial.xml Outdated Show resolved Hide resolved
scene/resources/particles_material.cpp Outdated Show resolved Hide resolved
@RPicster
Copy link
Contributor Author

The current noise breaks down if positions are greater than ~100 units. So a particle system located 100 units away from the origin would start to show issues and one that is located 1000s of units away from origin will really start to break down.

That being said, I can't figure out why Hash33 is causing problems in your case. It should be ideal for this situation and in my testing it is working fine. Unfortunately I won't have time anytime soon to really dig into this.

Since you have tested very thoroughly and haven't run into any issues, I am fine merging with the hash function you have now. If the issues I anticipate arise we can easily switch out the hash function later.

I tested this again as it did keep me up at night... 🤯
I could not observe any break down or changes in the turbulence at positions or 100/1.000/10.000/100.000 (xyz) of the emitter (of course with local_coords=false, otherwise the transform used by the hash function would always be small).

So... I think in a practical application it still works with the slightly not-perfect noise.
My suggestion would be to merge and see if real problems come up in applications - and if they do, we can still change the hash function and invest more time into researching why the suggested hash33 is causing those artifacts.

@QbieShay
Copy link
Contributor

@clayjohn @RPicster what's your hardware specs? graphic cards?

@RPicster
Copy link
Contributor Author

RPicster commented Jul 29, 2022

@clayjohn @RPicster what's your hardware specs? graphic cards?

I'm on a NVidia RTX 2080.
If you ask for performance reasons, I've been using more or less the same code in Beat Invaders which runs pretty solid, even on the Steam Deck ✌

@QbieShay
Copy link
Contributor

@RPicster nono, i was asking because you said there are discrepancies between your results and clay's. Perhaps it's a driver issue

@RPicster
Copy link
Contributor Author

@RPicster nono, i was asking because you said there are discrepancies between your results and clay's. Perhaps it's a driver issue

To be sure that we test the same way, we would need the same test first.

@reduz
Copy link
Member

reduz commented Jul 31, 2022

Code looks good on my side, so if @QbieShay gives it the go, it should be fine.

@QbieShay
Copy link
Contributor

go gogogoogogogogoogo

@akien-mga akien-mga merged commit beb8fd5 into godotengine:master Aug 1, 2022
@akien-mga
Copy link
Member

Thanks! Amazing work :)

@RPicster
Copy link
Contributor Author

RPicster commented Aug 5, 2022

Now... The question of questions. Should I backport thisto 3.x? It's not too much work.

@kleonc
Copy link
Member

kleonc commented Aug 14, 2022

Now... The question of questions. Should I backport thisto 3.x? It's not too much work.

I think there's no "you should". But if you can/want I think there's nothing against it. This PR looks like a pure enhacement. I don't see any breaking changes or usage of some new features not available in 3.x. So I'd say just give it a go.

Also, don't you know the new motto? 😄

j97awYgaCV

@Calinou
Copy link
Member

Calinou commented Aug 14, 2022

I think this is worth backporting for 3.6 🙂

@KdotJPG
Copy link
Contributor

KdotJPG commented Aug 15, 2022

Just seeing this now. Great idea and great work! It does seem like it could be done with significantly fewer noise calls, though. Right now it's using 12 (or 18 if the compiler doesn't optimize away the unused ones) plus one for time.

Just two 4D simplex noise evaluations with gradient / partial derivative vector support (this with the output vector feature from this) should do the trick:

vec3 curl_3d(vec3 p, float time) {
    vec4 gradA, gradB;
    snoise(vec4(p, time), out gradA);
    snoise(vec4(p, time) + SOME_OFFSET, out gradB);
    return normalize(cross(gradA.xyz, gradB.xyz));
}

This would also reduce visible directional bias, because it's using more isotropic noise through a more isotropic formula.

I'm currently working on a proof-of-concept for this, and perhaps making a follow-up PR. If such an update does get merged, I think it would be good to do so before including a backport in a 3.X release, so that there isn't a change in patterns in the official versions.

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.