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

Particle internal refactor and additions for more artistic control #79527

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

QbieShay
Copy link
Contributor

@QbieShay QbieShay commented Jul 16, 2023

DO NOT TEST ON IMPORTANT PROJECTS! MAKE A COPY!

How to test:

  1. Login into github
  2. Click the "checks" tab of this PR
  3. On the right there's an "artifacts" button.
  4. Download the appropriate build for your plaform

image

What to test

  1. Compatibility with existing projects! This should not break compatibility and if it does, if the particles look different, it's a bug.
  2. Features: are the new features functioning properly?

NOTE: this is GPUParticles only


Description:

This is a gigaenormous rework to particles because adding code to them had become impossibly difficult.
Now parameters are stored in lovely neat structs and subdivided in display, dynamics, accelerations.
This closes probably 90% of the proposals and issues about particles i opened.

IDEALLY: this is 100% compatible with the existing one. Practically there may be some bugs and regressions introduced during the rewrite.

This is an attempt at implementing https://godotshaders.com/shader/particle-process-v2/ and keeping compat.

How to conduct a test

Ideally, people will try to make something a little more complex than a single particle system.
I'm hoping something of the complexity of a crit hit effect (so I'm imagining 4-5 particle systems..?)
Sword slash, fireball, go for it!

Feel free to use my render material for testing it out (MPL license, means if you change a file you have to open source the changes to that specific file, but it doesn't spread like GPL)
vfx_kit.zip

Overview of the changes:

GPUParticles:

  • add an interp_to_end parameter to force particles to run to the end of their lifetime fast. This can be animated in the animation player, or tweened to make a complex VFX scene disappeared in a desired time interval. cool isn't it?
  • fixed billboard mode not taking into account angle stored in CUSTOM.x

ParticleProcess:

  • Shader entirely rearranged and reorganized. Data is split in different structs and there's function to generate the initial state that can be called from process(), so that there's no discrepancies (hopefully ....?)
  • Parameters entirely rearranged in submenus. I know this will be contentious but i arranged them based on my daily usage of said particles. The editor can remember unfolded submenus, since tweaking is like 80% of particle work, this shouldnt really be too much of an issue, i hope. It will be more prominent, of course, if the testing usage is "i make a lot of different particle systems"
    **New features **
    Under spawn section, inherit emitter velocity. Currently nonfucntional, will figure out how to make it functional later.

A completely new category of velocity: animated velocity. This is velocity entirely driven by curves. it will not be subject to damping
image

Available velocities:

  • angular (rotates the particle if billboard, same as before)
  • radial: velocity towards, or away, from a point
  • orbit: orbit velocity around XYZ axes. To use in compatibility mode, use flag enable z. To use in new mode, use CurveXYZ. Curve should be in degrees.
  • Velocity limit: a curve that defines the maximum velocity magnitude (combined with accelerations) that a particle can have along its lifetime. functions only when assigned.

Accelerations are the same as before. gravity is under accelerations, together with attractor interaction.

Display parameters:
This is where color, scale and animation properties live.
image

Color got two new optional curves, alpha curve and emission curve (emission as in brightness. perhaps brightness would be a better term). emission just multiplies the color so it needs to be read in the render shader as such. Feel free to use the material provided above. a single color ramp will work as before.

image

Scale over velocity!! This is really cool to add some sensation of smear for motion by scaling on the Y axis for example with a XYZ curve. The two min/max parameters indicate the range in which velocity is remapped onto the curve (so when do i make my scale biggest? when max velocity is 10, or 200? that's what the parameters are for)

Addresses

godotengine/godot-proposals#7082 - interp to end will make trails much easier: it offers a lifetime control.
godotengine/godot-proposals#6779 - improves portability of particle systems and makes technology for VFX color palettes actually possible
godotengine/godot-proposals#6657 - slightly different, but achievest almost all of the same results
godotengine/godot-proposals#6573 - offers it as a property instead of a function, functions can be implemented on top later.
godotengine/godot-proposals#5044 - emission transform built-ins
#79063 - uses custom.x rotation correctly

Bugsquad edit: Fixes: #82837

Who whants this

Edit: look at the reaction. Aww. Thanks y'all <3

I know that discussions on proposals didn't really get a huge amount of traction on Github, but i brought up this work in multiple VFX spaces, incorporated feedback from various professionals, and researched how alternatives (Unity, Unreal) are built.
This has been months of research that ultimately condensed into this work.
I believe that what testers will be able to achieve with this PR will speak for itself.

@QbieShay QbieShay added this to the 4.2 milestone Jul 16, 2023
@QbieShay QbieShay requested review from a team as code owners July 16, 2023 00:25
@QbieShay QbieShay changed the title [WIP] particle rework [WIP] Particle internal refactor and additions for more artistic control Jul 16, 2023
@QbieShay QbieShay marked this pull request as draft July 16, 2023 09:58
@YeldhamDev
Copy link
Member

I wonder if this is a good opportunity to ask for trail_size/color_modifier to make a comeback, those two were quite useful for making simple 2D particles that gave the illusion of depth.

@QbieShay
Copy link
Contributor Author

QbieShay commented Jul 16, 2023

Huh, I don't know exactly how that worked, but I can tentatively take a look, no promises tho, I'll have to investigate how they were before and why they were removed.

@YeldhamDev
Copy link
Member

@QbieShay Thanks! They were removed in this PR: #41810

I've yet to successfully replicate the old behavior with the new particle sub-emitters.

@QbieShay
Copy link
Contributor Author

QbieShay commented Jul 16, 2023

What's missing exactly? So i know what to add back (or how to change sub emitters)

@YeldhamDev
Copy link
Member

As I said before, a simple way to give the illusion of depth in 2D. Here's a section of my game for example:

3.* 4.*
Screenshot_20230716_144059 Screenshot_20230716_143826

There may be a way to do this currently that I missed, but for the life of me I couldn't find it.

@QbieShay
Copy link
Contributor Author

The 4.x screenshot is done with a subemitter or with trails?

@Calinou
Copy link
Member

Calinou commented Jul 16, 2023

I've tried to use a standard scale curve with this PR, but I get Vulkan pipeline error spam after creating a CurveXYZTexture (this also occurs with CurveTexture), even after saving and reloading the scene.

ERROR: Condition "!pipeline" is true.
   at: compute_list_bind_compute_pipeline (drivers/vulkan/rendering_device_vulkan.cpp:7789)
ERROR: This compute pipeline requires (0) bytes of push constant data, supplied: (32)
   at: compute_list_set_push_constant (drivers/vulkan/rendering_device_vulkan.cpp:8011)
ERROR: No compute pipeline was set before attempting to draw.
   at: compute_list_dispatch_threads (drivers/vulkan/rendering_device_vulkan.cpp:8094)

Testing project: test_pr_79527.zip

Under spawn section, inherit emitter velocity. Currently nonfucntional, will figure out how to make it functional later.

This is really interesting, as this is a feature I see a lot in racing games for dust particles under tires.

@QbieShay
Copy link
Contributor Author

Thank you @Calinou ! I should have addressed the issues you mentioned.

@RPicster
Copy link
Contributor

Maybe I couldn't find it, but one thing that I would 110% would love to see and is extremely helpful is an amount parameter that is dynamic.

I know that there is amount, but changing that value restarts the particle.

One can easily work around this limitation by using a custom particle shader, but it would be very, very useful to have this as a default parameter. In the workaround, the amount acts as a "maximum amount of particles" and when spawning.

@Calinou
Copy link
Member

Calinou commented Jul 17, 2023

Maybe I couldn't find it, but one thing that I would 110% would love to see and is extremely helpful is an amount parameter that is dynamic.

I guess this can be done by adding a Amount Ratio property just below Amount that controls how many particles will be emitted as a ratio of the Amount. 1.0 (default) means all particles are emitted at the usual rate, 0.5 means half of the particles are emitted, etc.

This ratio-based approach has the benefit of scaling nicely with changes to the Amount property (for instance, if you vary particle amount based on a graphics setting).

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 really good! There are a lot of nice changes in this PR

I've taken a much deeper look now and have left suggestions to fix a few things (mostly little things):

  1. Consistency with types (a lot of places in the API were mixing floats/doubles/real_t, I made suggestions to clean that up)
  2. OpenGL, some stuff is missing in OpenGL to make it functional
  3. Use of custom.x in the GPU Copy shader. We can't assume that custom.x always contains rotation. It will break particles when users write custom shaders. I suggest adding a new property for this. For this PR, I would remove the changes in gpu_copy and then make a follow up PR add a new ROTATION property, then in the process shader assign the rotation value to ROTATION and to custom.x (so user shaders continue to work).

doc/classes/GPUParticles2D.xml Outdated Show resolved Hide resolved
doc/classes/GPUParticles3D.xml Outdated Show resolved Hide resolved
drivers/gles3/storage/particles_storage.h Outdated Show resolved Hide resolved
drivers/gles3/storage/particles_storage.h Outdated Show resolved Hide resolved
drivers/gles3/shaders/particles.glsl Show resolved Hide resolved
scene/3d/gpu_particles_3d.h Outdated Show resolved Hide resolved
scene/3d/gpu_particles_3d.h Outdated Show resolved Hide resolved
scene/3d/gpu_particles_3d.h Outdated Show resolved Hide resolved
scene/3d/gpu_particles_3d.h Outdated Show resolved Hide resolved
scene/3d/gpu_particles_3d.h Outdated Show resolved Hide resolved
@QbieShay QbieShay force-pushed the qbe/particles-rework branch 2 times, most recently from 044c9a3 to 35cd0fd Compare October 9, 2023 12:02
@QbieShay QbieShay force-pushed the qbe/particles-rework branch 2 times, most recently from 57078d8 to 19574c7 Compare October 9, 2023 13:40
Co-authored-by: Hugo Locurcio <[email protected]>
Co-authored-by: A Thousand Ships <[email protected]>
Co-authored-by: Raul Santos <[email protected]>
Co-authored-by: Mew Pur Pur <[email protected]>
Co-authored-by: Clay John <[email protected]>
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.

Everything looks good to me now. I have tested locally on a few projects and it appears to work fine.

I have not tested extensively as I don't have complex particle projects available, but I trust the other reviews that were done and confirm that this works and doesn't break compatibility.

For future reference we discussed the additions extensively on Rocketchat. Ultimately I am a bit hesitant about adding properties to the Particles node (amount_ratio and interp_to_end) which basically are used to ferry data to the shader on a per-node basis. To me it feels messy and over-complicated. However, we discussed alternatives and there are no good alternatives to implement these features. So ultimately, I am fine with the implementation.

Everything else looks fantastic. I am certain that VFX users will be very happy with these changes.

Great work!

@QbieShay
Copy link
Contributor Author

Thank you!!!!!

@akien-mga akien-mga merged commit 55282dd into godotengine:master Oct 10, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks! Amazing work and dedication 💜

Let's get this tested widely in 4.2 beta 1 :)

I'm not sure which of the proposals and issues linked as "Addressed" in the OP can be closed, so I'll let you assess it.

@arkology
Copy link
Contributor

Looked through some of closed proposal and not sure that they could be closed as implemented only for GPUParticles. Please correct me if I'm wrong.
Also what about conversion between GPU and CPU particles, how does it work with new features added to GPUParticles?

@clayjohn
Copy link
Member

Looked through some of closed proposal and not sure that they could be closed as implemented only for GPUParticles. Please correct me if I'm wrong. Also what about conversion between GPU and CPU particles, how does it work with new features added to GPUParticles?

We aren't porting all GPUParticles features over the CPUParticles anymore. GPUParticles are supported by all backends now, so instead of being the fallback, CPUParticles are now the ultra-light alternative to GPUParticles. As such, we don't want to bloat CPUParticles or add to the development overhead for improving particles in general.

@arkology
Copy link
Contributor

@clayjohn thanks for clarifying. If so, I think this should be properly documented to prevent confusion.

@QbieShay
Copy link
Contributor Author

Yup sorry about that. I have a blogpost to publish with info about the changes and how particles will be maintained moving forward. I just haven't published it yet

@QbieShay
Copy link
Contributor Author

QbieShay commented Oct 16, 2023

@NathanLovato @Calinou mind if i use the videos you posted in the blogpost? With proper credit of course. Please leave a comment with the credit link if you consent. I plan to publish the blogpost end of week so if there's no reply i'll assume no consent is given. Thanks again!

@NathanLovato
Copy link
Contributor

@QbieShay you don't even have to ask when it comes to us! And if you want to credit someone, you can credit Thibaud, the artist. Whichever link you prefer:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: Done
Development

Successfully merging this pull request may close these issues.

Deterministic chain of random values in particle shader can be broken when turbulence is used