[Bf-codereview] Bullet Integration (issue 7092051)

[ideasman42 at gmail.com]

LGTM to go into trunk, however a few issues I noticed.

Summery

- If 'Physics World' id disabled, Id like this to work exactly the same
as if physics wasn't ever added. Currently there are a few places where
disabling physics wont stop pointcache, syncing functions and depsgraph
from using changed behavior. I've noted the ones that I found in the
review.

- Mesh conversion could be sped up by not adding triangles one at a time
with double-vertex checks, but this can be done after merge.

- I noticed scale is always removed from objects involved with physics,
IIRC the game engine manages to have scaled meshes with physics so it
could be supported.

- Id like to allow building with WITH_BULLET disabled, any hints as to
which branch I should look to getting the this change?



https://codereview.appspot.com/7092051/diff/1/source/blender/blenkernel/intern/depsgraph.c
File source/blender/blenkernel/intern/depsgraph.c (right):

https://codereview.appspot.com/7092051/diff/1/source/blender/blenkernel/intern/depsgraph.c#newcode2353
source/blender/blenkernel/intern/depsgraph.c:2353: if
(ob->rigidbody_object) ob->recalc |= OB_RECALC_OB;
Firstly, shouldn't this check if physics world is enabled or muted?

When the physics world is enabled:
Should this check if the physics object is sleeping before assuming it
needs to be recalculated.

https://codereview.appspot.com/7092051/diff/1/source/blender/blenkernel/intern/rigidbody.c
File source/blender/blenkernel/intern/rigidbody.c (right):

https://codereview.appspot.com/7092051/diff/1/source/blender/blenkernel/intern/rigidbody.c#newcode910
source/blender/blenkernel/intern/rigidbody.c:910:
MEM_freeN(rbw->objects);
suggest to re-alloc here and only when needed. likely in most cases the
array wont resize and existing array can be used.

https://codereview.appspot.com/7092051/diff/1/source/blender/blenkernel/intern/rigidbody.c#newcode1099
source/blender/blenkernel/intern/rigidbody.c:1099: void
BKE_rigidbody_sync_transforms(Scene *scene, Object *ob, float ctime)
I've noticed this function runs even when (rbw->flag & RBW_FLAG_MUTED),
Id prefer if un-checking the physics option panel disables physics
functions like this.

https://codereview.appspot.com/7092051/diff/1/source/blender/blenkernel/intern/rigidbody.c#newcode1147
source/blender/blenkernel/intern/rigidbody.c:1147: if
(rbw->physics_world == NULL || rbw->numbodies !=
BLI_countlist(&rbw->group->gobject)) {
counting the number of objects in the group might not be totally
reliable (objects could be swapped out).

Perhaps existing methods such as depsgraph flags or notifiers can be
used to set the cache as invalid.

https://codereview.appspot.com/7092051/diff/1/source/blender/blenkernel/intern/scene.c
File source/blender/blenkernel/intern/scene.c (right):

https://codereview.appspot.com/7092051/diff/1/source/blender/blenkernel/intern/scene.c#newcode1151
source/blender/blenkernel/intern/scene.c:1151: /* quick point cache
updates */
This runs even when physics is muted.

also - would be nice to add note that this is for rigidbody sim.

https://codereview.appspot.com/7092051/diff/1/source/blender/editors/object/object_add.c
File source/blender/editors/object/object_add.c (right):

https://codereview.appspot.com/7092051/diff/1/source/blender/editors/object/object_add.c#newcode947
source/blender/editors/object/object_add.c:947: /* remove rigid body
constraint from world before removing object */
There are other places objects are removed from a scene:
make_proxy_exec(). rna_Scene_object_unlink(), I think
'BLI_remlink(&scene->base, base)' should be replaced with a
BKE_scene_base_unlink() function that handles low level stuff like
freeing rigid body data.

Then all can call this.

https://codereview.appspot.com/7092051/diff/1/source/blender/rigidbody/rb_bullet_api.cpp
File source/blender/rigidbody/rb_bullet_api.cpp (right):

https://codereview.appspot.com/7092051/diff/1/source/blender/rigidbody/rb_bullet_api.cpp#newcode419
source/blender/rigidbody/rb_bullet_api.cpp:419: v_out[0] =
(float)vec[0];
*picky* suggest to add some local static functions to handle
conversions.

https://codereview.appspot.com/7092051/diff/1/source/blender/rigidbody/rb_bullet_api.cpp#newcode657
source/blender/rigidbody/rb_bullet_api.cpp:657: void
RB_trimesh_add_triangle(rbMeshData *mesh, const float v1[3], const float
v2[3], const float v3[3])
calling add triangle on each mesh face isnt that optimal, this isnt a
big deal since its not so hard to change this after the merge.

https://codereview.appspot.com/7092051/