godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.3k stars 21.05k forks source link

Particles still emit at old location when emission is turned off, moved and turned back on #20160

Open Zylann opened 6 years ago

Zylann commented 6 years ago

Bugsquad note: This issue has been confirmed several times already. No need to confirm it further.


Godot 3.0.5 Windows 10 64 bits GeForce GTX 1060 6GB/PCIe/SSE2

Repro: 1) Place world space particles at position A and enable emission. Wait some time. 2) Disable emission 3) Move particles at position B 4) Enable emission: see a few particles are still wrongly emitted from A 5) Next particles are correctly emitted from B

Test project: ParticlesEmissionBug.zip Use left and right arrow keys to emit from left or right

I use this in my game to emit sparks when scrapping walls, and this bug makes them appear sometimes at the wrong place instead of the contact point.

malbach commented 6 years ago

Hi, I can confirm the bug with the test project, even on latest master. As I'm waiting for another PR to be (in)validated, I won't modify my fork or create a new branch for that but I can offer a solution : add a call to update_particles() just before setting emitting

diff --git a/drivers/gles3/rasterizer_storage_gles3.cpp b/drivers/gles3/rasterizer_storage_gles3.cpp
index e67b0bea2..948e6ba19 100644
--- a/drivers/gles3/rasterizer_storage_gles3.cpp
+++ b/drivers/gles3/rasterizer_storage_gles3.cpp
@@ -5564,6 +5564,8 @@ void RasterizerStorageGLES3::particles_set_emitting(RID p_particles, bool p_emit
                // Restart is overridden by set_emitting
                particles->restart_request = false;
        }
+       if (p_emitting)
+           update_particles();
        particles->emitting = p_emitting;
 }
malbach commented 6 years ago

@Zylann if you don't want to wait for a patch, you can try to modify your script to wait one (or more) frame before turning emitting on A quick fix from your test project :

extends Node

onready var _particles = get_node("ScrapeSparks")
var fTimer = -1
var fMin = 0
var fMax = 5

func _process(delta):

    if Input.is_key_pressed(KEY_LEFT):
        _particles.global_transform.origin = Vector3(-2, 0, 0)
        fTimer = fMax

    elif Input.is_key_pressed(KEY_RIGHT):
        _particles.global_transform.origin = Vector3(2, 0, 0)
        fTimer = fMax

    if fTimer >= fMin and fTimer < fMax:
        _particles.emitting = true

    if fTimer == -1:
        _particles.emitting = false

    if fTimer > -1:
        fTimer = fTimer - 1

The particles will be turned on when you release the arrow keys Modify fMin and/or fMax to change the particles apparition time/delay

Calinou commented 4 years ago

I can reproduce this on Godot 3.2.2beta4 (albeit only occasionally).

akien-mga commented 2 years ago

Fixed by #54733.

akien-mga commented 2 years ago

Reopening as it doesn't seem fixed in my tests, but actually worsened.

This is the behavior I get in 3.4-stable (before #54733):

https://user-images.githubusercontent.com/4701338/145006297-41418ba6-2c84-4eb4-90f6-643987e09db9.mp4

I'm not sure what the bug initially described by @Zylann should look like, but in the above video it seems to work fairly well, there's only an emission at an old location when switching very fast (probably when switching before the end of the lifetime of the last emission at the old location). When waiting more than lifetime before switching, there's no issue.

And now the behavior in 3.x (71d8ccbe9e5087d509fa61744982b7ba2339dcd3) (after #54733):

https://user-images.githubusercontent.com/4701338/145006758-d2d69473-5f02-4852-b2b0-eb21fff0509d.mp4

Now the issue happens systematically, even when waiting a very long time before switching. There's also a new issue where the first emission happens at the origin, as pointed out by @LunaticInAHat in https://github.com/godotengine/godot/pull/54733#issuecomment-987259686.

akien-mga commented 2 years ago

I'm not sure what the bug initially described by @Zylann should look like, but in the above video it seems to work fairly well, there's only an emission at an old location when switching very fast (probably when switching before the end of the lifetime of the last emission at the old location). When waiting more than lifetime before switching, there's no issue.

As I understand it, the "bug" reported here and seen in the MRP is that when you switch emission off and on while there are still alive particles, they get restarted at the same location.

So what we see in the first video above is that when I switch sides, a pre-existing particle on one side gets restarted on switch while the other new particles get emitted in the correct location. So the actual bug is the lifetime reset of already emitted particles when emitting is toggled off and on.

AlbuquerqueSenior commented 2 years ago

Maybe this implementation can solve this problem #3649

LunaticInAHat commented 2 years ago

Maybe this implementation can solve this problem #3649

Not sure I see the relationship to that issue.

AlbuquerqueSenior commented 2 years ago

Maybe this implementation can solve this problem #3649

Not sure I see the relationship to that issue.

Ops, Im sorry. https://github.com/godotengine/godot-proposals/issues/3649

jasonwinterpixel commented 1 year ago

Pretty sure doing this fixes it

particles.amount = particles.amount

particles.restart() does not work if the the particles are offscreen

Look at RasterizerStorageGLES3::particles_set_amount:

Setting amount actually instantly binds and uploads a full clear to the GPU.

https://github.com/godotengine/godot/blob/3.x/drivers/gles3/rasterizer_storage_gles3.cpp#L6247

void RasterizerStorageGLES3::particles_set_amount(RID p_particles, int p_amount) {
    Particles *particles = particles_owner.getornull(p_particles);
    ERR_FAIL_COND(!particles);

    particles->amount = p_amount;

    int floats = p_amount * 24;
    float *data = memnew_arr(float, floats);

    for (int i = 0; i < floats; i++) {
        data[i] = 0;
    }

    for (int i = 0; i < 2; i++) {
        glBindVertexArray(particles->particle_vaos[i]);

        glBindBuffer(GL_ARRAY_BUFFER, particles->particle_buffers[i]);
        glBufferData(GL_ARRAY_BUFFER, floats * sizeof(float), data, GL_STATIC_DRAW);

        for (int j = 0; j < 6; j++) {
            glEnableVertexAttribArray(j);
            glVertexAttribPointer(j, 4, GL_FLOAT, GL_FALSE, sizeof(float) * 4 * 6, CAST_INT_TO_UCHAR_PTR(j * 16));
        }
    }

    if (particles->histories_enabled) {
        for (int i = 0; i < 2; i++) {
            glBindVertexArray(particles->particle_vao_histories[i]);

            glBindBuffer(GL_ARRAY_BUFFER, particles->particle_buffer_histories[i]);
            glBufferData(GL_ARRAY_BUFFER, floats * sizeof(float), data, GL_DYNAMIC_COPY);

            for (int j = 0; j < 6; j++) {
                glEnableVertexAttribArray(j);
                glVertexAttribPointer(j, 4, GL_FLOAT, GL_FALSE, sizeof(float) * 4 * 6, CAST_INT_TO_UCHAR_PTR(j * 16));
            }
            particles->particle_valid_histories[i] = false;
        }
    }

    glBindVertexArray(0);

    particles->prev_ticks = 0;
    particles->phase = 0;
    particles->prev_phase = 0;
    particles->clear = true;

    memdelete_arr(data);
}

I'm thinking the fix is either to do these GL operations in set_restart or to investigate what this particles->clear is supposed to do. If it's supposed to clear the particle transforms, then it seems like it fails to do it.

jasonwinterpixel commented 1 year ago

Another solution is to massively increase the visibility rect for your particles. It's simply when they are offscreen, they 'freeze' the particles in place, and when the particle system comes back on screen, they render at the old location. And restart() doesnt work when offscreen, but amount=amount does what restart should do.

jasonwinterpixel commented 1 year ago

Ok, nevermind, sorry, my issue is specific to 'offscreen' particles being impossible to properly 'clear'. I dont think it's directly related to this issue, but I am starting to think perhaps theres an off by 1 frame error in the particle system rendering that is causing it to render 1 frame before the current particle position data. This would perhaps explain why restart() doesnt work when particles are offscreen. This is a pretty big problem when you have a moving camera and particle systems warping around. It makes particles suddenly appear randomly on screen pretty frequently. The concept of the 'visibility rect' is insufficient for particles that arent using local coords. Particles emitted by a player are frequently not using global coordinates. If you teleport that player around (think about in a networked game or respawning a player), the 'visibility rect' for the particle system now must enclose the entire playable area. Particle systems that go off screen should automatically elegantly fade out their particles, without creating new ones. I've written a work around script to force clear particles completely when the particle system is detected to be off screen. Below is my test case, video evidence of the problem, and my workaround solution.

Repro script which moves a particle system from the left side of the screen to the right:

extends Node2D

export var _particles:NodePath
onready var particles:Particles2D = get_node(_particles)

var y := 1000.0

func _ready() -> void:
    _warp_particles()

func _process(delta: float) -> void:
    particles.position = Vector2(particles.position.x + delta * 1500.0, y)  

    if particles.position.x > 1500.0:
        _warp_particles()

func _warp_particles():
    particles.position = Vector2(-200.0, y)

Good, particles fully clear when offscreen:

https://github.com/godotengine/godot/assets/78934401/b3b6dff6-7a63-43e5-b1d5-a6a019efdf70

Bad, particles reappear in old location when coming on screen:

https://github.com/godotengine/godot/assets/78934401/43f0d830-0afa-4a49-a8cd-50580fbe172c

Fixed with this crazy workaround script:

extends Node
class_name ParticleClear

# Workaround for particles being 'frozen' at their old location when they go offscreen
# https://github.com/godotengine/godot/issues/20160
# The only way to actually reset an offscreen particle system is to set particles.amount = particles.amount
onready var particles:Particles2D = get_parent()
var was_on_screen := false
onready var viewport := get_viewport()

func _process(delta: float) -> void:
    var particles_rect := particles.visibility_rect
    particles_rect.position += particles.global_position
    var visible_rect:Rect2 = viewport.canvas_transform.affine_inverse().xform(viewport.get_visible_rect())
    var is_on_screen := visible_rect.intersects(particles_rect)
    if not is_on_screen and was_on_screen:
        # particles.reset() doesnt work when off screen!
        # must use this crazy hack!
        particles.amount = particles.amount
    was_on_screen = is_on_screen
filipinyo commented 8 months ago

This is still an issue.

Proggle commented 6 months ago

I am seeing a similar issue with particle emitters I instantiate (with emission off). If I start emission on the same frame they're instantiated, the first frame shoots off a burst of particles at the origin of the map (regardless of whether or not their position is changed before emission starts).

EDIT: having trouble making a MRP. In my main project it seems to happen when the instantiation is happening in response to some signals, but not happen in the _process or _physics_process functions. Maybe there's something involved there?

wwwizzarrdry commented 5 months ago

So you're just not supposed to use GPUParticle3D in Godot? How is this still an issue in 4.2.1?

What's really bizarre is that removing and re-adding the particles node doesn't fix the issues either. What kind of Devilry is that?

# Get the child node you want to remove.
child_node = get_node("path/to/child/node")

# Remove the child node from the scene tree.
remove_child(child_node)

# Add the child node back to the scene tree.
add_child(child_node)

Edit: 😂 I just noticed that it's been over 6 years now that no one can use particles in Godot because they phantom spawn at their previous positions and it looks dumb as hell.

Calinou commented 5 months ago

@wwwizzarrdry I have to remind you that we have a Code of Conduct. Please stay constructive.

wwwizzarrdry commented 5 months ago

@wwwizzarrdry I have to remind you that we have a Code of Conduct. Please stay constructive. Ok, Hugo. I'll check back in another 6 years I guess.

timoschwarzer commented 1 month ago

MRP for 2D GPU particles in 4.4-dev.2: particles-test.zip

In 4.4-dev.2, calling restart() after setting the position of the particles does seem to fix the issue, but this isn't always a solution, e.g. in cases where the particles position is updated constantly.

wwwizzarrdry commented 1 month ago

@wwwizzarrdry I have to remind you that we have a Code of Conduct. Please stay constructive. Still not fixed in latest Godot... 🤣

akien-mga commented 1 month ago

@wwwizzarrdry Since you can't follow the code of conduct and be constructive, I'm restricting you from further interacting on Godot's GitHub organization. Contributors and users deserve a respectful and constructive work environment, snark isn't part of it.