Page MenuHome

Mantaflow [Part 8]: Customization for particle system
ClosedPublic

Authored by Sebastián Barschkis (sebbas) on Oct 29 2018, 6:03 PM.

Diff Detail

Repository
rB Blender

Event Timeline

Bug fix for part->disp field and some refactoring.

Update manta files after merge with master.

Harbormaster completed remote builds in B3298: Diff 14649.

I guess this is mostly for @Sergey Sharybin (sergey) to review? Saw nothing obviously wrong here though…

Updated diff with latest changes from fluid-mantaflow branch

Updated diff with latest changes from fluid-mantaflow branch

@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.

Sergey Sharybin (sergey) requested changes to this revision.Dec 10 2019, 3:19 PM
Sergey Sharybin (sergey) added inline comments.
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.

This revision now requires changes to proceed.Dec 10 2019, 3:19 PM
Sebastián Barschkis (sebbas) marked 3 inline comments as done.Dec 11 2019, 10:31 AM

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?

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

I figured that this is how other particle steps set the seed

That would only mean that the other step functions are wrong.
We've already had issues with this during Spring (re-rendering same frame would distribute particles differently).

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.

Sebastián Barschkis (sebbas) marked 3 inline comments as done.Dec 11 2019, 5:28 PM

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.

Sebastián Barschkis (sebbas) marked 2 inline comments as done.Dec 11 2019, 5:31 PM

Clang-format again... Fixed in here 6b62eb17a028

From quick look into updates seems fine.

This revision is now accepted and ready to land.Dec 12 2019, 9:36 AM

Updated diff with latest changes from fluid-mantaflow branch

Updated diff with latest changes from fluid-mantaflow branch