Soft body updateConstraints() changes link rest length, and area not being recalculated

[GoogleCodeExporter]

btSoftBody::updateConstants() changes link resting lengths to their current 
length. This is not desired, as updateConstants() is called for things like 
transforming and scaling the soft body. 

Details can be found in the following thread: 
http://bulletphysics.org/Bullet/phpBB3/viewtopic.php?f=9&t=6616

Problem was originally identified using the 2.77 official release demos. 
Reproduced (see code in link above) in the current (svn r2379) demo code using 
the Mac OS X Demo app (running on an iMac).

There turned out to be several related problems. First, resting lengths should 
not be changed in updateConstants(). Second, the function isn't even needed by 
some of the other functions that call it (transform, scale), but it contains 
portions of code that are still needed by other functions. Third, the function 
recalculates node and face surface areas... however, this is the only place 
this is done, and the function is not called regularly, meaning forces that 
depend on current/accurate areas are not being calculated correctly.

I solved the above issues by modifying softbody.h and softbody.cpp (attached) 
like so:
1) Removing calls to updateConstants() from functions that have no need for it.
2) Removing code that changes resting lengths from updateConstants(), and 
putting it into its own function (resetLinkRestLengths()).
3) Placing the area recalculating code into its own function (updateArea()), 
and calling this from the appropriate places in applyForces() so that the 
correct area is always used for force calculations.

Miscellaneous changes:
- Material::kAST and Material::kVST aren't being used anywhere. Commented them 
in soft body header to indicate this.
- made small optimizations in applyForces() by adding more bool flags

Using updated area calculations resulted in slightly different dynamics for 
objects using pressure, volume conservation or aerodynamics. As such, the rigid 
plate in the "Pressure" demo no longer properly fell on the blob. I made a 
slight adjustment to the demo to fix this (softdemo.cpp, attached). Other demos 
seem fine.

Any impact on backwards compatibility should be minimal/rare, and only if 
anyone was using updateConstants() to explicitly update the area and change 
link resting lengths (which would be a bit of a hack to begin with). These 
operations can now be done (in my patched code) via updateArea() (though there 
shouldn't be a need to call this manually) and resetLinkRestLengths().

-David

Original issue reported on code.google.com by dave.bru...@gmail.com on 9 Apr 2011 at 8:01

Attachments:

• patch_updateConstants.zip

[GoogleCodeExporter]

By the way, the files attached above were modifications of the r2379 repository 
code, so they should be basically up-to-date (as of today).

Original comment by dave.bru...@gmail.com on 9 Apr 2011 at 8:04

[GoogleCodeExporter]

Just noticed this issue
http://code.google.com/p/bullet/issues/detail?id=470

So, majestik also implemented something to allow for manual rest length 
updates. However, judging from his code, there is only minimal overlap (though 
probably still some), since he implemented a scaling of the rest length, while 
I just created (from code extracted from updateConstants()) a function that 
sets the rest length as the current link lengths.

Original comment by dave.bru...@gmail.com on 9 Apr 2011 at 8:57

[GoogleCodeExporter]

Thanks a lot for your work.

Do you mind integrating majestik's rest length update in your patch?

Original comment by erwin.coumans on 9 Apr 2011 at 9:02

[GoogleCodeExporter]

Ok, I've updated my patch with majestik's code (along with a slight fix in my 
own) in this attached file.

Note: I modified his implementation slightly so that scaling resting lengths is 
a one-time event to all existing links explicitly when the user calls 
setRestLengthScale, and not also implicitly to every new link added after a 
scaling event. I believe this results in more "expected" behaviour for the 
user. I also modified his member names slightly for clarity (for example, 
m_restLength to m_restLengthScale).

Original comment by dave.bru...@gmail.com on 10 Apr 2011 at 9:29

Attachments:

• soft_body_patch.zip

[GoogleCodeExporter]

I modified dave.bru's patch and attached it here. This patch splits 
updateConstants method into three methods: resetLinkRestLengths, 
updateLinkConstants and updateArea. So calling updateConstants will work as it 
did and give us the backward compatibility. When people want to update a 
specific thing like area or link constants, individual method can be called. 

Regarding updating areas for nodes, updateArea method has a parameter and can 
switch between averaging areas of adjacent faces or one third of sum of 
adjacent faces. The first approach is what Bullet has been using. To ensure the 
backward compatibility, the default is the first approach. The second one is 
more accurate but breaks Init_Volume demo in AppSoftBodyDemo project.  

Original comment by saggitas...@gmail.com on 28 Aug 2012 at 6:58

Attachments:

• btSoftbody.patch

[GoogleCodeExporter]

This issue was closed by revision r2557.

Original comment by erwin.coumans on 9 Sep 2012 at 5:22

• Changed state: Fixed