Implement Particle Trails #48242

reduz · 2021-04-27T15:55:56Z

Enable the trails and set the length in seconds
Provide a mesh with a skeleton and a skin
Or, alternatively use one of the built-in TubeTrailMesh/RibbonTrailMesh
Works deterministically
Fixed particle collisions (were broken)

moar.mp4

partimeshes.mp4

Once this PR is merged, will implement 2D GPU particles (currently not working).

reduz · 2021-04-27T20:12:11Z

I have no idea why this is not passing sanitizer, it makes no sense and I can't reproduce this here, but it looks like the Particles object is not being freed..

reduz · 2021-04-27T20:48:40Z

@akien-mga may it be a problem with one of the CI projects run?

akien-mga · 2021-04-27T21:24:31Z

Any idea @qarmin? reduz suggests the leak might be a bug in the test project.

BastiaanOlij · 2021-04-28T00:20:28Z

scene/3d/gpu_particles_3d.cpp

+ update_configuration_warnings();
+}
+void GPUParticles3D::set_trail_length(float p_seconds) {
+ ERR_FAIL_COND(p_seconds < 0.001);


Dumb question but while the duration influences the length, is length the right name for this property?

I think it is, "length" can be safely applied to time as well as distance

BastiaanOlij · 2021-04-28T00:25:27Z

scene/resources/curve.cpp

 if (_baked_cache_dirty) {
 // Last-second bake if not done already
- bake();
+ const_cast<Curve *>(this)->bake();
 }


Is this safe behavior? Seeing bake changes the data within the object it's not constant. Is there a reason for making this function constant?

Its an internal cache, so its either this or making it mutable and I did not want to change a lot of code.

BastiaanOlij

Read through all the code changes and I think I get most of it. I'd have to do some proper playing around to fully understand the existing GPU particles implementation. Other then two little nitpicking things this all looks completely sensible to me. Will be interesting to see if I can get it going on the mobile renderer but I don't think it will need many changes.

I'll do a bit more experimenting with it but I think this is a cool addition to Godot

qarmin · 2021-04-28T06:35:41Z

Test project is really hard to broke, because use list of classes taken directly from engine and only some breaking changes for basic elements are able to do this like OS -> Platform or instance() to instantiate().

This time leak isn't false positive.
Scene Nodes/Nodes.tscn causes it(which removes and adds all available nodes to scene)
Smaller reproducible code is here

extends Node

var time_to_exit : float = 5.0

var TIME_TO_DELETE : float = 1.0
var time_to_delete : float = TIME_TO_DELETE
	
var classes : Array = ["ClippedCamera3D", "GPUParticles3D", "CPUParticles3D"]

func _populate() -> void:
	for _i in range(5):
		for j in classes:	
			add_child(ClassDB.instance(j))
				
func _ready() -> void: 
	_populate()

# Move nodes a little and delete and readd them later
func _process(delta: float) -> void:
	time_to_exit -= delta
	if time_to_exit < 0:
		get_tree().quit()
			
	time_to_delete -= delta
	if time_to_delete < 0:
		time_to_delete += TIME_TO_DELETE
		
		for i in get_children():
			i.queue_free()
			
		_populate()

qarmin · 2021-04-28T14:52:57Z

I looked at the code and this leak is caused by LocalVector

Direct leak of 6288 byte(s) in 1 object(s) allocated from:
    #0 0x7fc85d8bfbc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #1 0x12e92443 in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:76
    #2 0x12e92a7a in Memory::realloc_static(void*, unsigned long, bool) core/os/memory.cpp:100
    #3 0x11b45a77 in LocalVector<RendererStorageRD::ParticlesFrameParams, unsigned int, false>::resize(unsigned int) core/templates/local_vector.h:148
    #4 0x11a75255 in RendererStorageRD::update_particles() servers/rendering/renderer_rd/renderer_storage_rd.cpp:4855
    #5 0x112f23f1 in RenderingServerDefault::_draw(bool, double) servers/rendering/rendering_server_default.cpp:110
    #6 0x112fd301 in RenderingServerDefault::draw(bool, double) servers/rendering/rendering_server_default.cpp:385
    #7 0x1fc63d7 in Main::iteration() main/main.cpp:2520
    #8 0x1e69cd6 in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:261
    #9 0x1e5c114 in main platform/linuxbsd/godot_linuxbsd.cpp:58
    #10 0x7fc85c97d0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

after applying this patch, leak is gone

diff --git a/servers/rendering/renderer_rd/renderer_storage_rd.cpp b/servers/rendering/renderer_rd/renderer_storage_rd.cpp
index 873a5ac197..0bd3f7500c 100644
--- a/servers/rendering/renderer_rd/renderer_storage_rd.cpp
+++ b/servers/rendering/renderer_rd/renderer_storage_rd.cpp
@@ -4848,7 +4848,6 @@ void RendererStorageRD::update_particles() {
 
                        if (uint32_t(history_size) != particles->frame_history.size()) {
                                particles->frame_history.resize(history_size);
-                               zeromem(particles->frame_history.ptr(), sizeof(ParticlesFrameParams) * history_size);
                        }
 
                        if (uint32_t(trail_steps) != particles->trail_params.size() || particles->frame_params_buffer.is_null()) {
diff --git a/servers/rendering/renderer_rd/renderer_storage_rd.h b/servers/rendering/renderer_rd/renderer_storage_rd.h
index 8ba9fe5a49..b1daf341a8 100644
--- a/servers/rendering/renderer_rd/renderer_storage_rd.h
+++ b/servers/rendering/renderer_rd/renderer_storage_rd.h
@@ -30,7 +30,7 @@
 
 #ifndef RENDERING_SERVER_STORAGE_RD_H
 #define RENDERING_SERVER_STORAGE_RD_H
-
+#include<vector>
 #include "core/templates/list.h"
 #include "core/templates/local_vector.h"
 #include "core/templates/rid_owner.h"
@@ -772,7 +772,7 @@ private:
 
                float trail_length = 1.0;
                bool trails_enabled = false;
-               LocalVector<ParticlesFrameParams> frame_history;
+               std::vector<ParticlesFrameParams> frame_history;
                LocalVector<ParticlesFrameParams> trail_params;
 
                Particles() {

clayjohn · 2021-04-28T17:40:06Z

scene/3d/gpu_particles_3d.cpp

@@ -462,8 +601,9 @@ GPUParticles3D::GPUParticles3D() {
 set_one_shot(false);
 set_amount(8);
 set_lifetime(1);
- set_fixed_fps(0);
+ set_fixed_fps(30);


What is the reason for defaulting to 30 fixed FPS? Is it for performance?

Interpolation has been implemented for particles, so unlike CPU/GPU particles in 3.x, it should look smooth regardless of rendering FPS when fixed FPS is enabled now.

Fixing particle FPS to 30 should help improve performance when using collision and attractors, especially with many complex shapes.

See the discussion starting from here: https://chat.godotengine.org/channel/rendering?msg=G8cNTJHiDcnEwQbuy

reduz · 2021-04-28T19:17:22Z

@qarmin found the issue, it was on my side, but its weird that this did not fail before. I would assume it is related to changes on the CI.

reduz · 2021-04-28T20:49:19Z

Given feedback from @QbieShay and @JFonS will work a tiny bit more on this today, please dont merge.

reduz · 2021-04-30T20:23:00Z

Ok, should be ready for review or merging.

-Enable the trails and set the length in seconds -Provide a mesh with a skeleton and a skin -Or, alternatively use one of the built-in TubeTrailMesh/RibbonTrailMesh -Works deterministically -Fixed particle collisions (were broken) -Not working in 2D yet (that will happen next)

QbieShay · 2021-04-30T21:24:57Z

Can you rename the last option for transform to "RibbonBillboard" ?

reduz · 2021-04-30T21:38:47Z

@QbieShay I prefer to use the current naming so its a bit more obvious to the user that Z is used for billboard and Y for aligning. The ribbon is mostly the trail mesh, and you can use this option for aligning regular meshes or quads too, so its not necessarily specific to ribbons.

QbieShay · 2021-04-30T21:44:00Z

I think that practically this will be used only for ribbons, so I'm not sure it makes sense to be this specific in the dropdown. Calling it ribbon billboard will also make it way easier to understand for people that want to setup a ribbon billboard, rather than having to search what ZBillbordYToVelocity means ..

akien-mga · 2021-04-30T21:52:11Z

I agree that ZBillboardYToVelocity sounds a bit cryptic (and hard to read). But let's merge and refine further in coming days/weeks based on user feedback, changing names is the easy part (after a lot of bikeshedding :P).

akien-mga · 2021-04-30T21:52:18Z

Thanks!

reduz · 2021-05-01T00:11:47Z

@QbieShay I am not entirely sure you want to use this only for Ribbons. I may add speed velocity scale at some point (this was kind of requested also) and this option will be useful for it too.

reduz requested review from a team as code owners April 27, 2021 15:55

akien-mga added enhancement topic:rendering labels Apr 27, 2021

akien-mga added this to the 4.0 milestone Apr 27, 2021

BastiaanOlij reviewed Apr 28, 2021

View reviewed changes

BastiaanOlij approved these changes Apr 28, 2021

View reviewed changes

clayjohn reviewed Apr 28, 2021

View reviewed changes

reduz force-pushed the particle-trails branch from df5f5c0 to 77ede90 Compare April 28, 2021 19:08

reduz force-pushed the particle-trails branch 2 times, most recently from 66a092e to b632630 Compare April 28, 2021 20:25

reduz marked this pull request as draft April 28, 2021 20:48

reduz force-pushed the particle-trails branch from b632630 to 45bf35c Compare April 30, 2021 20:22

reduz marked this pull request as ready for review April 30, 2021 20:22

reduz force-pushed the particle-trails branch from 45bf35c to 9005646 Compare April 30, 2021 20:38

akien-mga approved these changes Apr 30, 2021

View reviewed changes

akien-mga merged commit 4a7679e into godotengine:master Apr 30, 2021

madmiraal mentioned this pull request May 1, 2021

Fix documentation following implementation of particle trails #48345

Merged

Calinou mentioned this pull request Sep 17, 2021

Consider adding support for particle chains, deformable clouds . . . godotengine/godot-proposals#3308

Closed

briansemrau mentioned this pull request Nov 8, 2021

Editor crash when closing scene with Joint3D #54747

Closed

trollodel mentioned this pull request May 23, 2022

Fix Joint3D and Joint2D warning causing a crash #61311

Merged

KoBeWi mentioned this pull request Sep 10, 2022

Don't defer SceneTreeEditor::_warning_changed signal connection #65630

Merged

Mooses2k mentioned this pull request Mar 1, 2023

Make candles flicker when movement near them Mooses2k/SecretHistories#374

Open

goatchurchprime mentioned this pull request Mar 13, 2023

GPUParticles3D can't plot a different mesh at the front end of a trail #74877

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Particle Trails #48242

Implement Particle Trails #48242

reduz commented Apr 27, 2021 •

edited

Loading

reduz commented Apr 27, 2021

reduz commented Apr 27, 2021

akien-mga commented Apr 27, 2021

BastiaanOlij Apr 28, 2021

reduz Apr 28, 2021

BastiaanOlij Apr 28, 2021

reduz Apr 28, 2021

BastiaanOlij left a comment

qarmin commented Apr 28, 2021

qarmin commented Apr 28, 2021

clayjohn Apr 28, 2021

Calinou Apr 28, 2021 •

edited

Loading

reduz commented Apr 28, 2021

reduz commented Apr 28, 2021

reduz commented Apr 30, 2021

QbieShay commented Apr 30, 2021

reduz commented Apr 30, 2021

QbieShay commented Apr 30, 2021

akien-mga commented Apr 30, 2021

akien-mga commented Apr 30, 2021

reduz commented May 1, 2021

Implement Particle Trails #48242

Implement Particle Trails #48242

Conversation

reduz commented Apr 27, 2021 • edited Loading

reduz commented Apr 27, 2021

reduz commented Apr 27, 2021

akien-mga commented Apr 27, 2021

BastiaanOlij Apr 28, 2021

Choose a reason for hiding this comment

reduz Apr 28, 2021

Choose a reason for hiding this comment

BastiaanOlij Apr 28, 2021

Choose a reason for hiding this comment

reduz Apr 28, 2021

Choose a reason for hiding this comment

BastiaanOlij left a comment

Choose a reason for hiding this comment

qarmin commented Apr 28, 2021

qarmin commented Apr 28, 2021

clayjohn Apr 28, 2021

Choose a reason for hiding this comment

Calinou Apr 28, 2021 • edited Loading

Choose a reason for hiding this comment

reduz commented Apr 28, 2021

reduz commented Apr 28, 2021

reduz commented Apr 30, 2021

QbieShay commented Apr 30, 2021

reduz commented Apr 30, 2021

QbieShay commented Apr 30, 2021

akien-mga commented Apr 30, 2021

akien-mga commented Apr 30, 2021

reduz commented May 1, 2021

reduz commented Apr 27, 2021 •

edited

Loading

Calinou Apr 28, 2021 •

edited

Loading