godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

The sphere emission shapes for 2D and 3D particle systems are not uniformly distributed #10305

Open StamesJames opened 1 month ago

StamesJames commented 1 month ago

Describe the project you are working on

A 2D Shoot'em Up

Describe the problem or limitation you are having in your project

I hat no problem so far with this but I have seen it while I was looking if an ring emission shape could be easily implemented. I am not sure if it is intentional or not and therefor I write the Issue here and not in the Bug tracker.

At the moment in both cases there are more particles in the middle then on the outside.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Particles would be sampled uniformly across the sphere or in 2D a circle.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

For 2D: The only difference to the existing code would be to put the Math::randf() that is multiplied to the emission_shere_radius inside a square root

case EMISSION_SHAPE_SPHERE: {
real_t t = Math_TAU * Math::randf();
real_t radius = emission_sphere_radius * Math::sqrt(Math::randf());
p.transform[2] = Vector2(Math::cos(t), Math::sin(t)) * radius;
}

for 3D: I am not sure if in 3D there is a faster method to do this

case EMISSION_SHAPE_SPHERE: {
real_t r = emission_sphere_radius * Math::cbrt(Math:randf());
real_t phi = Math_TAU * Math::randf();
real_t t = 2 * Math::randf() - 1;
real_t theta = Math::arccos( t );

p.transform.origin = Vector2(
   r * Math::sin(theta) * Math::cos(phi), 
   r * Math::sin(theta) * Math::sin(phi),
   r * cos(theta)
);
}

I think I could write a PR for this

If this enhancement will not be used often, can it be worked around with a few lines of script?

No because it affects directly how these primitive shapes work

Is there a reason why this should be core and not an add-on in the asset library?

Because primitive shapes should be in the core so that users can use them to build more complex shapes with them. Maybe it even would be good to have the option to manipulate the distribution in some way so that the old distribution can be obtained as well.

Calinou commented 1 month ago

For Sphere in 2D, this is a design decision as mentioned in the documentation (phrasing could be more obvious though):

  • EMISSION_SHAPE_SPHERE = 1
    • Particles will be emitted in the volume of a sphere flattened to two dimensions.
  • EMISSION_SHAPE_SPHERE_SURFACE = 2
    • Particles will be emitted on the surface of a sphere flattened to two dimensions.

An Uniform Sphere emission mode should be added as a new emission mode to avoid breaking visuals in projects that rely on the existing emitter shape.

StamesJames commented 1 month ago

@Calinou Ah ok I understand. But also in 3D the particles are not uniformly distributed. When my PR for a ring emission shape gets merged one can have a uniform circle by just making the inner radius 0 and the same is possible in 3D with the ring but at the moment there is no way to make a uniformly distributed sphere in 3D and I think the solution with the ring feels a bit hacky and could be unintuitive for users. I could add a emission Shape Disc in 2D and 3D that has a uniform distribution and in 3D a additional height and an axis like ring has. For the Sphere I could maybe just add a bool parameter "uniform distribution" or would it be better to add an additional emission shape Uniform_Sphere?

StamesJames commented 1 month ago

image

Here is an example screenshot where you can see that the CPUParticles3D are much denser in the middle for emission shape sphere.

Calinou commented 1 month ago

I could add a emission Shape Disc in 2D and 3D that has a uniform distribution and in 3D a additional height and an axis like ring has. For the Sphere I could maybe just add a bool parameter "uniform distribution" or would it be better to add an additional emission shape Uniform_Sphere?

I think a boolean property makes more sense, just make sure it's hidden when using an emission mode that isn't compatible with this.