The particle system needs some tweaks so that it can be used for particles created in Mantaflow (i.e. to read both FLIP and secondary particles from Mantaflow).
Details
Details
- Reviewers
Brecht Van Lommel (brecht) Sergey Sharybin (sergey) Nils Thuerey (n_t) Georg Kohl (GeorgKohl) - Maniphest Tasks
- T59995: Mantaflow Review
- Commits
- rBceba74dcd7bb: Mantaflow [Part 8]: Customization for particle system
rB4331add63b4c: Mantaflow: Updated particle system to use newer flags and functions
rB30a29660dfd3: Merge branch 'bakeConfig' into particleSystem
rB27b26f6b1c4f: Merge branch 'bakeConfig' into particleSystem
rB350df53d2940: Mantaflow: Updated particle system
rB2933b9c895d4: Mantaflow: Customization for particle system
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
I guess this is mostly for @Sergey Sharybin (sergey) to review? Saw nothing obviously wrong here though…
Comment Actions
@Sergey Sharybin (sergey) this is the part of the Manta review that you might want to look at. It's basically the connection that loads the particles from Mantaflow into a Blender particle system.
I.e. it's very similar to the setup that the old fluid system uses to load secondary particles.
| source/blender/blenkernel/intern/particle_system.c | ||
|---|---|---|
| 593–594 | I don't understand it. Particle system can not be emitter and hair at the same time and type can not be a bitfield. | |
| 4227 | Shouldn't this be using seed from settings? But it also seems you've got way bigger design violation here. srand()modifies global state, so you can not use it from a function which runs in threads. You should be using reentrant variant of rand() instead. | |
| 4291–4293 | Merge pos{X,Y,Z} into a float[3] and use vector math. Sane for velocity. | |
| 4293 | Dead code is discouraged. | |
Comment Actions
See 63b0377e9619 and c54fd66c0a17 for changes.
| source/blender/blenkernel/intern/particle_system.c | ||
|---|---|---|
| 593–594 | I agree. This doesn't make any sense. Changed it. | |
| 4227 | I see. I figured that this is how other particle steps set the seed: c54fd66c0a17 Would this work? | |
Comment Actions
Note on the debug prints. Usually those are done as #if DEBUG_FOO (un)defined at the top of the file. Not sure if you usually want all of the debugs or whether you want really granular control over them. Also seems indentation is broken?
| source/blender/blenkernel/intern/particle_system.c | ||
|---|---|---|
| 593–594 | Why not to keep it as a switch statement? | |
| 4227 |
That would only mean that the other step functions are wrong. As for c54fd66c0a17: sim->rng the idea is correct. Just watch out for where rng is being allocated. Code is a bit hairy, so not sure if particles_manta_step ever called from dynamics_step. Also keep in mind that to access random numbers you are to use BLI_rng_get() (and not rand()). So depending on where the random values are accessed from you might need to go deeper. | |
Comment Actions
Changes are here 3d4e5a89eb57.
Regarding the debugs prints: Yes, I would rather like to have them more "granular". The indentation of the #if 0 was just because of clang-format.
| source/blender/blenkernel/intern/particle_system.c | ||
|---|---|---|
| 593–594 | Yes, let's do that. I don't really have an opinion on this. The whole change probably just came from me not liking case statement so much. | |
| 4227 | Okay, good to know. I am using now BLI_rng_get_double() to get the random value. | |