Skip to content

Conversation

hmans
Copy link

@hmans hmans commented Feb 10, 2023

Prelude

This is a Draft PR that I'm putting up for discussion, both about finding out if and how this may be able to get merged, but also about what other tweaks and improvements I could make while I'm at it.

If this should be a full-on proposal, please let me know and I will post one.

Summary

Visual Shaders are great, but the current implementation for Particle Shaders as found in master, the recent 4.0 betas and RCs, is slightly lacking:

This PR aims at improving these with as little changes as possible:

  • It fixes the issue(s) linked to above
  • It upgrades the scale input from a float to a vec3
  • It adds a new position input that allows the user to overwrite the particle's position
  • It implements the conventions established by ParticlesProcessMaterial and writes per-particle time information into CUSTOM:
    • CUSTOM.y being the progress over the default lifetime configured for the particle effect
    • CUSTOM.w being the randomness ratio (currently hardcoded to 1.0, but we can now expose per-particle lifetime as an input on the Start graph)
  • It persists each particle's rotation axis and angle in USERDATA1 so they can be mutated in the Process/Collide graphs
  • It adds a couple of new outputs to the Input node in particle mode:
    • age: the particle's age in seconds
    • progress: the progress (0.0-1.0) across the particle's lifetime
    • position: the particle's current position (vec3)
    • scale: the particle's current scale (vec3)
    • rotation_axis: the particle's rotation axis (vec3)
    • rotation_angle: the particle's rotation angle around the above axis (float)

Example Visual Shader

These changes make visual particle shaders like this possible — please see this comment for details and an example project:

image

@QbieShay
Copy link
Contributor

Hey @hmans ! It's awesome you put your changes up, thank you for taking the time!

The only thing i'm not a fan of, is to write particle lifetime in CUSTOM.x.

The default particle shader writes lifetime in CUSTOM.y and the random lifetime in CUSTOM.w, so I think that as a built-in node it should respect this interface, otherwise any rendering shader that reads from INSTANCE_CUSTOM (the counterpart of CUSTOM that can be read in spatial shaders) to do things like emission over lifetime will have to be rewritten depending on which type of Visual Shader nodes are used

@hmans
Copy link
Author

hmans commented Feb 10, 2023

I've fixed an issue around updating the scale in the Process graph and added a scale output to my Particle node. You now have the same flexibility in updating a particle's scale that you get for updating its position.

image

@hmans
Copy link
Author

hmans commented Feb 10, 2023

The default particle shader writes lifetime in CUSTOM.y and the random lifetime in CUSTOM.w, so I think that as a built-in node it should respect this interface, otherwise any rendering shader that reads from INSTANCE_CUSTOM (the counterpart of CUSTOM that can be read in spatial shaders) to do things like emission over lifetime will have to be rewritten depending on which type of Visual Shader nodes are used

That is a good point.

I've updated the code to follow ParticleProcessMaterial's conventions, and updated the PR description to reflect this change.

@QbieShay QbieShay requested review from Chaosus and QbieShay February 10, 2023 13:57
@YuriSizov
Copy link
Contributor

YuriSizov commented Feb 14, 2023

@fracteed There is nothing to approve, CI has been run already (perhaps someone pressed the button earlier). Artifacts should be available.

@fracteed
Copy link

fracteed commented Feb 16, 2023

@hmans thanks for working on this, really promising already!

  • I have been playing with this build and getting some nice results. All seems to work as expected, that I have seen so far. Just having access to the particle progress attribute and the transform now working correctly in process opens up a whole world that wasn't possible before.

I have attached a project file with 3 demo particle systems, to test out the new functionality.

The first demo is quite simple, but shows the particle progress driving color, alpha, velocity, and scale. The curves and the 2d texture ramp all work as expected. Good to see that color over life works, though not sure if I set it up correctly but using life to drive uv's does seem to work.

rising_bubbles0000-0399.mp4

Second demo adds in rotation, with random rotation speed. When they hit a certain age, the color changes and particles rise by adding to position, and they spin faster.

cube_evolution0000-0399.mp4

Third demo is more complex and doesn't use any velocity with the position set directly in process. Combining age driven and distance based effects to get a motion graphics effect.

grid_ripples0000-0299.mp4

The first demo node graph:

particle_graph

Some general notes:

  • particles don't move when moving the emitter in the default world coords... works as expected in local coords
  • there seems no way to set lifetime or random lifetime with nodes. I did try setting the CUSTOM.w in start but it doesn't seem to work
  • would be good to be able to directly set LIFETIME attribute in the start method. Then you could do things like set lifetime based on distance or texture.
  • There is a need for a way to orient particles along the velocity vector like can be done with standard particles "Align Y" flag
  • would be good to have all the particle related attributes in one particle attribute node in a dropdown, in the same style as the Input node. Instead of having one gaint node with all outputs or individual nodes for each. I guess the lifetime attribute in the current Input node would be moved to this particle node?

visual_particles_pr_test1.zip

I will be interested to see how this patch progresses and will keep playing with it!

@hmans hmans force-pushed the visual-particle-shader-tweaks branch from 368b90e to de4f549 Compare February 16, 2023 16:13
@hmans hmans marked this pull request as ready for review February 16, 2023 16:15
@hmans hmans requested a review from a team as a code owner February 16, 2023 16:15
@hmans hmans marked this pull request as draft February 16, 2023 16:15
@hmans
Copy link
Author

hmans commented Feb 16, 2023

Sorry, I accidentally submitted this PR for review. This was a misclick. I've returned it to Draft status.

@QbieShay
Copy link
Contributor

rather than align y to velocity, id prefer having a look_at node of some sort that can be used for that

@fracteed
Copy link

rather than align y to velocity, id prefer having a look_at node of some sort that can be used for that

@QbieShay Oh yeah, definitely need something more flexible than just align y to velocity. I would love to see an "align rotation to vector" node like Blender geometry nodes has. That way you could align rotation to velocity or have it look at a position, etc on any axis.

https://docs.blender.org/manual/en/2.93/modeling/geometry_nodes/point/align_rotation_to_vector.html

@QbieShay
Copy link
Contributor

would be good to have all the particle related attributes in one particle attribute node in a dropdown, in the same style as the Input node. Instead of having one gaint node with all outputs or individual nodes for each. I guess the lifetime attribute in the current Input node would be moved to this particle node?

I second what @fracteed said, i think we should follow VisualShader's conventions for inputs and have them in a dropdown

Also, please don't add any more features to this PR. Smaller PRs are way easier to review (and have a way higher change to be merged) than larger PRs.

@hmans
Copy link
Author

hmans commented Feb 19, 2023

I'd be happy to move this forward into something that's actually mergeable, and I'd love to limit the scope to a minimum that makes sense to merge while still being useful. To me, that would currently be:

  • The additional/upgraded particle shader root node inputs introduced in this PR
  • The changes in the shader code as currently implemented in this PR
  • A single new Visual Shader node that exposes the various bits of per-particle state using the dropdown paradigm (as suggested above; TODO)

What do you think?

The one part of all this that I'm not happy with is rotation, because there's currently no clean way to mutate rotation across frames (but maybe there is a way to extract axis and angle from TRANSFORM that I'm too dumb to see. Help appreciated! 🙏)

@QbieShay
Copy link
Contributor

Sorry, I didn't fully understand.

This PR does the following things:

  • add input nodes ( to be merged in a single one with a dropdown rather than multiple)
  • add output plugs (position and scale)
  • fixes the overriding of transform

is that what you want to move forward with?

@hmans
Copy link
Author

hmans commented Feb 20, 2023

is that what you want to move forward with?

I was asking you, because you mentioned keeping the scope small :)

If the above makes sense w.r.t. scope, the only thing that's missing is the new node. The experimental nodes I've built for this only live in my own project, and they're implemented in GDScript. I'm assuming this node will need to be implemented in C++ if it is to be part of this PR. Scary, but I think I can do it.

Also, as mentioned before, it'd be great to find a way to improve the handling of rotation, but this can probably go into a separate PR.

@QbieShay
Copy link
Contributor

Oh. I completely misunderstood what this PR does. I thought it contained the lifetime input nodes as well?

@hmans hmans force-pushed the visual-particle-shader-tweaks branch from de4f549 to 5e0f99f Compare February 21, 2023 08:13
@hmans
Copy link
Author

hmans commented Feb 21, 2023

I would appreciate some design input please. Should the new particle inputs (age, progress, position, etc.) live in a new node (as discussed in this PR previously), or should they instead be housed within VisualShaderNodeInput?

I'm asking because while looking through the codebase, I found this:

// PARTICLES-FOR-ALL

add_options.push_back(AddOption("Active", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "active", "ACTIVE"), { "active" }, VisualShaderNode::PORT_TYPE_BOOLEAN, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("AttractorForce", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "attractor_force", "ATTRACTOR_FORCE"), { "attractor_force" }, VisualShaderNode::PORT_TYPE_VECTOR_3D, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Color", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "color", "COLOR"), { "color" }, VisualShaderNode::PORT_TYPE_VECTOR_4D, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Custom", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "custom", "CUSTOM"), { "custom" }, VisualShaderNode::PORT_TYPE_VECTOR_4D, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Delta", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "delta", "DELTA"), { "delta" }, VisualShaderNode::PORT_TYPE_SCALAR, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("EmissionTransform", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "emission_transform", "EMISSION_TRANSFORM"), { "emission_transform" }, VisualShaderNode::PORT_TYPE_TRANSFORM, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Index", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "index", "INDEX"), { "index" }, VisualShaderNode::PORT_TYPE_SCALAR_UINT, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("LifeTime", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "lifetime", "LIFETIME"), { "lifetime" }, VisualShaderNode::PORT_TYPE_SCALAR, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Number", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "number", "NUMBER"), { "number" }, VisualShaderNode::PORT_TYPE_SCALAR_UINT, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("RandomSeed", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "random_seed", "RANDOM_SEED"), { "random_seed" }, VisualShaderNode::PORT_TYPE_SCALAR_UINT, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Restart", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "restart", "RESTART"), { "restart" }, VisualShaderNode::PORT_TYPE_BOOLEAN, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Time", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "time", "TIME"), { "time" }, VisualShaderNode::PORT_TYPE_SCALAR, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Transform", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "transform", "TRANSFORM"), { "transform" }, VisualShaderNode::PORT_TYPE_TRANSFORM, -1, Shader::MODE_PARTICLES));
add_options.push_back(AddOption("Velocity", "Input/All", "VisualShaderNodeInput", vformat(input_param_shader_modes, "velocity", "VELOCITY"), { "velocity" }, VisualShaderNode::PORT_TYPE_VECTOR_3D, -1, Shader::MODE_PARTICLES));

...which gives me the feeling that it should be the latter. I'd be happy to just add these to the existing input node. Is this the way forward?

@hmans
Copy link
Author

hmans commented Feb 21, 2023

Thanks for the feedback, @fracteed! I'm happy to see that these changes are useful — it's motivating me to flesh these out now.

  • particles don't move when moving the emitter in the default world coords... works as expected in local coords

I may be misunderstanding you, but is this not the intended behavior? I've understood "world mode" to simulate particles in world space, meaning that moving the emitter will merely move the location of newly spawned particles, without affecting previously spawned ones.

  • there seems no way to set lifetime or random lifetime with nodes. I did try setting the CUSTOM.w in start but it doesn't seem to work
  • would be good to be able to directly set LIFETIME attribute in the start method. Then you could do things like set lifetime based on distance or texture.

I'll figure something out for the Start node.

  • There is a need for a way to orient particles along the velocity vector like can be done with standard particles "Align Y" flag

We should be able to do this with the (upgraded) nodes already. The Input node already provides Velocity (even without my changes), so you should be able to feed this into the rotation axis/angle inputs.

  • would be good to have all the particle related attributes in one particle attribute node in a dropdown, in the same style as the Input node. Instead of having one gaint node with all outputs or individual nodes for each. I guess the lifetime attribute in the current Input node would be moved to this particle node?

Please see my comment above — I'm unsure if the new inputs should also go into the Input node, considering that it's already housing some of the other values, or into a new node. I'll start by adding them to the existing node; if there is a consensus that they should live in a new node, I'm happy to change things around.

@hmans
Copy link
Author

hmans commented Feb 21, 2023

I've pushed a few changes and would appreciate feedback. If anyone wants to give this build a try, I've attached a simple project that uses this PR's features:

EXP-Godot-Visual-Shader-Nodes.zip

Here's a screenshot of the example Visual Shader from the project:

image

Stuff that is going on here:

  • The particle progress (a value that moves from 0 to 1 over the lifetime of the individual particle) is fed into Alpha.
  • Each particle's scale is updated every frame by sourcing the current scale using a new scale output on the Input node and adding a vector constant to it.
  • Each particle's rotation angle is set every frame with a value derived from age (also now provided by the Input node.)
  • Each particle's position is updated every frame by sourcing its current position from the new position output on the Input node and adding a vector to it.

The new node outputs are, as of now, all housed within the Input node (which also provides other per-particle data like velocity.) I've made sure to give the new outputs fitting descriptions. Examples:

image
image

Stuff that's missing and/or needs to be decided:

  • Putting these new outputs into the existing Input node felt a little iffy because so far, Input has only been reflecting built-ins (eg. its velocity output simply maps to the VELOCITY built-in, and so on.) But putting some per-particle values into a separate node while leaving others inside Input felt even iffier. Opinions and feedback welcome!
  • I don't have a way yet to source the current rotation of the particle, but I would like to add this if I can figure out how to extract this data from the transform. As mentioned previously, help is always appreciated.
  • I haven't yet upgraded the Collide graph's root node with the new and upgraded inputs (position, scale, etc.)

@hmans
Copy link
Author

hmans commented Feb 21, 2023

I've also added a per-particle lifetime factor input for the Start graph's root node:

image

Feedback about naming and implementation appreciated!

@QbieShay
Copy link
Contributor

What is "lifetime factor"? Is it what is normally assigned in CUSTOM.w?

@fracteed
Copy link

@hmans nice work, it is all looking very promising!

  • Are you able to specify an exact lifetime with that lifetime factor socket, or is that just for randomising the current value? I do think it would be ideal to give an exact value or specific range, entirely in the graph nodes. Not sure if that is possible?
  • Last time I checked your PR, when moving an emitter in world mode, all particles moved with it not just the birth position. so I would need to try again, as it may have been fixed
  • I didn't realise that the rotation axis would accept any normalised vector. Not sure if plugging in the normalised velocity would work correctly though?

Could one of the maintainers please approve the workflow, so I can try an artifact auto build :)

@QbieShay
Copy link
Contributor

Are you able to specify an exact lifetime with that lifetime factor socket, or is that just for randomising the current value? I do think it would be ideal to give an exact value or specific range, entirely in the graph nodes. Not sure if that is possible?

I think it's possible to make a lower bound random, but definitely not an upper bound. So yuo'd have a lifetime of (say, 4) in the properties, and a random min lifetime in the particle system shader. It's not ideal but I understand why the randomization is done like that and frankly i can't imagine better solutions that don't involve significant refactors.

@hmans
Copy link
Author

hmans commented Feb 21, 2023

  • Are you able to specify an exact lifetime with that lifetime factor socket, or is that just for randomising the current value? I do think it would be ideal to give an exact value or specific range, entirely in the graph nodes. Not sure if that is possible?

In addition to @QbieShay's comment above, I think you can still exact quite a lot of control over these values by setting the particle system's global lifetime to something high (essentially the upper bound of lifetimes you want to support; this can be a very high value if you want "potentially unlimited"), and figuring out the lifetime ratio by some multiplication math: if desired_lifetime_in_seconds = max_lifetime * ratio, then ratio = desired_lifetime_in_seconds / max_lifetime.

The value you're setting here is per particle (!).

  • Last time I checked your PR, when moving an emitter in world mode, all particles moved with it not just the birth position. so I would need to try again, as it may have been fixed

Ah, I read your original comment exactly the other way. :-) I'll check this, thanks!

  • I didn't realise that the rotation axis would accept any normalised vector. Not sure if plugging in the normalised velocity would work correctly though?

It probably won't, because there won't be any rotation as long as the angle input remains at 0. I need to think about this deeper (I'm currently in code mode and not math mode, and both for me are mutually exclusive; I'm getting old :b), but I would guess that "rotating into the direction of the velocity" is a matter of throwing some cross and/or dot products at each other? I'll try to figure this out in a quiet minute.

@hmans
Copy link
Author

hmans commented Feb 21, 2023

@fracteed,

  • Last time I checked your PR, when moving an emitter in world mode, all particles moved with it not just the birth position. so I would need to try again, as it may have been fixed

I just checked this and I think the behavior of the already spawned particles works as intended; if the particle system is set to "Local Coords", particles will move together with the particle system's scene node; if it's not set to "Local Coords", particles will be simulated in world space.

However, I noticed that the existing emitter nodes (like SphereEmitter) don't take the scene node's transform into account automatically. I think this is good (certainly better than assuming that all emitters are located at the origin of the particle system's node), but it does require some hackery to have the emitter node actually follow the node's transform:

image

I think semantically this is sound, but setting this up of course isn't ideal because a) you can't quickly grab the emission position from the inputs, just the complete transform, and b) you can only extract its position component to combine with SphereEmitter's position output; any scaling and rotation will have to be ignored unless you're ready to do some heavy-duty transform matrix composition and decomposition, both of which currently seem to be a little hard to do through the nodes available.

To me, for now, this is a "let's improve this in another PR" type deal.

Update: I've found the TransformVectorMult node, which lets you apply a transform matrix to a vector, simplifying this somewhat:

image

@QbieShay
Copy link
Contributor

I think this is good (certainly better than assuming that all emitters are located at the origin of the particle system's node), but it does require some hackery to have the emitter node actually follow the node's transform:

I think we can address that in a later PR by adding configuration to these nodes?

@fracteed
Copy link

@QbieShay thanks, that makes sense as this all has to fit within the confines of the existing particle system. As @hmans explains, you can get any value you want based on the ratio maximum life, so it is quite workable the way it is.

@hmans thanks for all the explanations, they certainly helped! I was able to use the transform mult to get moving emitters, no problem. I created some more tests and didn't really have any issues, so it all seems quite solid. The rotation issues may be better tackled in another PR, as that is a tricky one.

  • the only issue I came across was getting this all to work with 2d gpu particles. Not sure if that a known thing?

Here are 3 more tests...

  • the first one uses the distance from the origin to determine the particle lifetime as color. I used the transform mult idea, so that the emitter can be moved around. You get an interesting effect when the emitter is moved away from the origin, as the lifetime becomes zero.
rising_cubes20000-0299.mp4
  • the second was an attempt to get the rotation to align along the velocity. At this speed it does look like it, but it is not actually working correctly. Also tried to abuse the particle attractor.
attraction0000-0299.mp4
  • third one is a quick and dirty bounce collision to test the collision context. Particles bounce with color change and fade on collision.
bounce0000-0299.mp4

project file:

visual_particles_pr_test2.zip

@hmans
Copy link
Author

hmans commented Feb 22, 2023

  • the only issue I came across was getting this all to work with 2d gpu particles. Not sure if that a known thing?

Thanks for pointing this out! This seems to be a regression introduced with this PR; I've created a simple 2D scene with a GPUParticles2D node and a visual shader that's emitting particles using SphereEmitter in 2D mode, which works fine in RC2, but doesn't emit any particles with the version from this branch. I'll take a closer look!

Update: fixed!

@hmans hmans force-pushed the visual-particle-shader-tweaks branch from 196366e to ff3e9b1 Compare February 25, 2023 13:09
@hmans
Copy link
Author

hmans commented Feb 25, 2023

I think this PR is getting ready to submit for review soon — I've implemented the Collide graph (and used the opportunity to fix it so that within a frame where a particle is colliding, the normal Process graph is still run in addition to the Collide graph — this matches the behavior of ParticleProcessMaterial.)

There are some parts of visual_shader_particle_nodes.cpp that I would love to refactor a little to make them a little easier to reason about, but I'm not sure if I should do it within this PR, because it would obfuscate the changes I've made to some extent; particularly the very long and convoluted nested switch/case/ifs like this one, which I feel tempted to restructure for improved maintainability.

If you have an opinion on whether I should do this in this PR, in a separate PR, or maybe not at all, please let me know.

Other than that, it'd be great if we could put the changes in this PR through their paces a little. They've worked fine in what testing I've performed for them, but who knows.

@YuriSizov
Copy link
Contributor

Should we look into finishing this for 4.2? If so, it would be great to resolve all concerns and do a rebase in the next few weeks.

@QbieShay
Copy link
Contributor

Yep - I have contacted OP just today and notified them that someone else will pick up this PR. Can be me, or someone else that's motivated.

@QbieShay QbieShay changed the title Fix VisualShaderNodeParticleOutput issues, make it more useful Add missing features to VisualShaderNodeParticleOutput Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs implementer
Development

Successfully merging this pull request may close these issues.

5 participants