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

Added extents for GPU and CPU Particles2D emission points #30665

Closed

Conversation

eligt
Copy link
Contributor

@eligt eligt commented Jul 17, 2019

The emission mask now uses relative values which are multiplied by the emission extents. This also fixes the issue where emission points are dependent on original texture size.

This commits also change emission points texture to be centered on the particle system instead of placing the top left corner at the center.

I think this also fixes #16411

@clayjohn
Copy link
Member

Is this change relevant for CPUParticles2D as well? If so, that needs to be implemented as well.

The emission mask now uses relative values which are multiplied by the emission extents. This also fixes the issue where emission points are dependent on original texture size.

This commits also change emission points texture to be centered on the particle system instead of placing the top left corner at the center.

I think this also fixes godotengine#16411
Added CPU particles
@eligt eligt force-pushed the particles2d-emission-points-extents branch from 3edd49e to 954e24e Compare July 17, 2019 23:48
@eligt
Copy link
Contributor Author

eligt commented Jul 17, 2019

@clayjohn Done.

@akien-mga akien-mga added this to the 3.2 milestone Jul 18, 2019
@akien-mga akien-mga requested a review from reduz July 18, 2019 06:35
@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 7, 2019
@aaronfranke
Copy link
Member

@eligt Is this still desired? If so, it needs to be rebased on the latest master branch.

@Lambdanaut
Copy link

@aaronfranke @eligt I'd still really like to see this change.

@akien-mga akien-mga changed the title Added extents for GPU particles2d emission points Added extents for GPU and CPU Particles2D emission points Jun 18, 2021
@QbieShay
Copy link
Contributor

I'm not convinced that this is the right solution for the problem at hand.
There are two issues being addressed here:

  • the points are bound to the texture size, and they shouldn't be
  • the point center is on 0,0 instead of half the extents
    I think that offsetting by half the size is fine, but what I wouldn't do is the rescaling in the way that is done here: the implementation in this PR makes it necessary to specify the extents of the box, so there is an additional setup stage, where things won't work out of the box. I'd substitute the usage of box extents with a single float scale property that defaults to one and remove the normalization from the creation of the points texture, so that it can be scaled relatively to the size of the initial texture and not with an absolute value.

@YuriSizov YuriSizov requested a review from a team August 24, 2021 22:18
@akien-mga akien-mga requested a review from clayjohn July 26, 2022 12:38
@YuriSizov
Copy link
Contributor

Seems like there is no consensus on the solution, nor has there been any activity with this PR for a while. Since we're at the end of the beta stage, I have to close this. An agreed upon solution to the presented problems that doesn't break compatibility is likely still welcome in future.

@YuriSizov YuriSizov closed this Jan 25, 2023
@YuriSizov YuriSizov removed this from the 4.0 milestone Jan 25, 2023
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.

2D Particle Emission Mask is offset
8 participants