Page MenuHome

Sculpt: Cloth Simulation Dynamic area mode
ClosedPublic

Authored by Pablo Dobarro (pablodp606) on Aug 26 2020, 10:08 PM.
Tags
None
Tokens
"Mountain of Wealth" token, awarded by marcog."Burninate" token, awarded by shader."Love" token, awarded by Raimund58."Burninate" token, awarded by Dir-Surya."Love" token, awarded by lopoIsaac."Love" token, awarded by andruxa696.

Details

Summary

This simulation area mode moves the active area with the brush. When
enabled, the cloth brush has no restrictions on stroke length, area or
mesh vertex count.

In order to work, this enables PBVH nodes dynamically for simulation as
the stroke location moves and builds the constraints for new nodes
during the stroke. When a node is not inside the simulated area, all the
constraints that were created for it and vertex collisions are not
computed. The simulation limits falloff areas and constraints tweaking
control how the simulated and no simulated nodes blend.


Diff Detail

Repository
rB Blender

Event Timeline

Pablo Dobarro (pablodp606) requested review of this revision.Aug 26 2020, 10:08 PM
Sergey Sharybin (sergey) added inline comments.
source/blender/blenkernel/intern/pbvh_intern.h
100–101 ↗(On Diff #28217)

I am having a mixed feeling about having simulation related logic in PBVH.

@Brecht Van Lommel (brecht), second opinion please :)

source/blender/editors/sculpt_paint/sculpt_cloth.c
994–1001

Split independent logic to own utility functions.

source/blender/blenkernel/intern/pbvh_intern.h
100–101 ↗(On Diff #28217)

Probably it should not be in the PBVH?

What is the lifetime of this state? If it's just for the duration of one brush execution, it can be stored in an array of size number of nodes.

If it lives longer that than, it raises the question about how this stays in sync with other data structures, and it's probably still not right to store it in the PBVH.

Pablo Dobarro (pablodp606) marked 3 inline comments as done.
  • Review Update: Store the nodes sim state outside the PBVH

@Brecht Van Lommel (brecht) I removed all the cloth sim flags and functionality from the PBVH. If this is ok, I would like to commit this before other small changes like D8764 because this is probably the one that can cause more problems.

I'll leave the review to @Sergey Sharybin (sergey), if state is no longer stored in that PBVH that aspect seems fine from my side.

Can't spot anything obvious.

@Pablo Dobarro (pablodp606) please make sure @Sebastian Parborg (zeddb) is fine and process from there. From me it's fine.

source/blender/editors/sculpt_paint/sculpt_cloth.c
946

i is more common for iterator, n is more common for the top boundary of the loop.

This revision is now accepted and ready to land.Wed, Sep 30, 2:59 PM

I don't spot anything bad/wrong either. LGTM

This revision was automatically updated to reflect the committed changes.
Pablo Dobarro (pablodp606) marked an inline comment as done.