Particle emitter - part2

[caguero]

This pull request focuses on the API of the particle emitter. Here are a few points for potential discussion:

1. How does the ParticleEmitter class API set its type?

Typically, there's a way to specify the geometry of the emitter, that dictates the origin from where the particles are created (point, box, cylinder, ellipsoid, hollow ellipsoid, ring, ...). Each shape can be specified with its own parameters. E.g.: {width, height, depth} for a box and {width, height, depth, internal_width, internal_height, internal_depth} for a hollow ellipsoid.

1.1: Ogre decides to have a particle emitter for each type subclassing from the ParticleEmitter base class. Additionally, they provide a few instances of emitters for the regular types mentioned before. This design allows to create custom shapes for the emitters.

1.2: Another idea could be to create a Geometry class that can be subclassed with all the different geometries to support. The SetType() function should accept a Geometry parameter. Probably, each derived Geometry should have its own initialization parameters and all of them should have a way to sample a particle from that geometry.

1.3: The simplest approach that I can imagine is to offer a SetType() function where we specify the geometry as an enum, and then, to offer a SetEmitterSize() that accepts a Vector3d. The interpretation of the vector varies depending on the type, but we can at least represent a point, box, cylinder and ellipsoid.

I decided to go for (1.3) for its simplicity and for avoiding extra inheritance. Let me know if you think we should go for a more general approach even at the cost of adding some extra inheritance. I do think that (1.2) is a more elegant and could be a good balanced approach between complexity and flexibility.

2. When does the emitter start emitting particles?

2.1 One idea could be to start as soon as it can. This probably means when the emitter is instantiated. It will have default parameters, so it should be possible. As soon as the emitter instance is modified via the API, the behavior should change on the fly.

2.2 The alternative approach is to use a function (e.g.: SetEmitting()) to start/stop emitting particles. Then, you could instantiate your object, set all the parameters, and then, call SetEmitting(true) when you're ready to emit particles. Even in this approach we could consider if modifying attributes while the emitter is enabled makes sense.

This pull request uses (2.2) because the behavior looks a bit more controlled to me. I'm guessing that the expected behavior should be to support changes in the attributes even if we're emitting particles but I can convinced otherwise.

3. What's the behavior of SetColorRange()?

As far as I know Ogre samples a color from the [start_color, end_color]. In our API discussion, we mentioned interpolation between the two colors. Not sure if this means that the particle should have start_color when is spawn, should finish with end_color at the end of its lifetime, and in between we should interpolate the color smoothly. In this PR, I went for the Ogre approach based on simplicity but again, totally open to discussion.

4. Getters?

This is almost trivial but we didn't include it in our initial API. I didn't add them to this patch but I'm almost convinced that we should do it.

[codecov]

Codecov Report

Merging #113 into master will decrease coverage by 0.34%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master    #113      +/-   ##
=========================================
- Coverage    7.56%   7.22%   -0.35%     
=========================================
  Files          26      27       +1     
  Lines        1744    2229     +485     
=========================================
+ Hits          132     161      +29     
- Misses       1612    2068     +456     

Impacted Files	Coverage Δ
include/ignition/rendering/base/BaseScene.hh	0.00% <0.00%> (ø)
src/base/BaseScene.cc	0.00% <0.00%> (ø)
src/LidarVisual.cc	0.00% <0.00%> (ø)
src/RenderEngineManager.cc	26.50% <0.00%> (+1.50%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdf0886...1843624. Read the comment docs.

[iche033]

looks good to me! I made some minor style changes in 8a7b2ee, mainly removing const from primitive types as we've been doing that recently.

1.3: sounds good. I agree we should start with something simple. Using enum's is also consistent with how we are handling different types of markers.

2.2: yep also agree with this. I think ogre equivalent is the setEnabled function.

3: I made an inline comment about doxygen comment of the color range function as it currently says the colors are randomly chosen. I think doing interpolation is better.

4: +1 yeah we should have getters. The accessor functions will also help with unit testing.

[caguero]

looks good to me! I made some minor style changes in 8a7b2ee, mainly removing const from primitive types as we've been doing that recently.

Thanks!

1.3: sounds good. I agree we should start with something simple. Using enum's is also consistent with how we are handling different types of markers.

2.2: yep also agree with this. I think ogre equivalent is the setEnabled function.

3: I made an inline comment about doxygen comment of the color range function as it currently says the colors are randomly chosen. I think doing interpolation is better.

Changed in 0323230.

4: +1 yeah we should have getters. The accessor functions will also help with unit testing.

Accessors added in 0323230.