btHeightfieldTerrainShape bugfix, and updated Character Demo

[GoogleCodeExporter]

Hello all-

This is really two patches in one.  Let me know if you'd like me to split them.

HEIGHTFIELD BUGFIX ----------------------------

This is a bugfix for btHeightfieldTerrainShape.  I made several changes
earlier (released in 2.73), and they contain a bug with how heights are
calculated.

Specifically, when you supply a heightfield array to
btHeightfieldTerrainShape, it does this:
 - calculates AABB for heightfield
 - centers heightfield at the center of the AABB
 - makes height values relative to that center

For instance, if you provide a heightfield with a min/max height of -100
and +500, bullet will re-center it to be -300/+300.  The same extent, but
centered on the center of the AABB.

I didn't think about this, so when processing triangles the height values
weren't re-centered properly.  This meant some height values might fall
outside of bullet's AABB for the heightfield, meaning collisions wouldn't
be detected.  I think one user reported that the heightfield values seemed
shifted, which would be the effect.

I considered changing the bullet behavior, but I think it is the right
behavior for a physics engine.  It is counterintuitive based on rendering
engines, but I think for a physics engine the idea of centering is
important and it might be inconsistent for the heightfield not to do so.

So I've got a patch for the heightfield behavior, and updated Doxygen
comments for the class so users would be aware of what to expect.

CHARACTER DEMO UPDATES -----------------------------------

I rewrote most of the CharacterDemo.  I removed the BSP dependency, and
added additional features to test multiple character controllers (AI and
user-controlled), and had it generate a random world with staircases and
ramps for testing.

I also added a #include to btKinematicCharacterController.h.  I think it
had a hidden dependency, so it wouldn't compile on its own.

The changes are fairly invasive to CharacterDemo.  If you'd rather add a
new Demo instead (or just drop this one!) that's fine.

I also have a working Makefile.am for the old CharacterDemo, if you'd like
(not included in this patch).  The old CharacterDemo did not have a
workable build for Linux users.

As always, any comments/feedback welcome.

    -Thomas

Original issue reported on code.google.com by astroboa...@gmail.com on 28 Jan 2009 at 4:35

[GoogleCodeExporter]

I've attached a patch with the deltas from a recent sync of the svn source.

Original comment by astroboa...@gmail.com on 28 Jan 2009 at 4:47

Attachments:

• patch.txt

[GoogleCodeExporter]

Original comment by john.mcc...@gmail.com on 28 Jan 2009 at 6:06

[GoogleCodeExporter]

Tom,

I took a look at the patch. First off, the height field shape should be within 
the
shapes AABB so your fixes will be merged soon.

As for the character demo, I was able to get it to compile after I made some 
changes
(M_PI -> SIMD_PI, no random() function being defined, and some macro problems - 
how
did you even compile this demo?) Anyways, the demo is interesting and worth 
adding. 

But, why did the patch delete the dynamic character controller and the JAM and 
CMake
build system files? Please don't submit patches that remove working code from 
Bullet,
it just makes it harder for us to get things merged.

I'm going to continue cleaning up your patch and commit it soon.

Thanks,
John

Original comment by john.mcc...@gmail.com on 28 Jan 2009 at 6:10

[GoogleCodeExporter]

More feedback,

Bullet doesn't use STL, so the list and vector template classes are 
unacceptable.
Please keep this in mind for future contributions.

Thanks,
John

Original comment by john.mcc...@gmail.com on 28 Jan 2009 at 6:25

[GoogleCodeExporter]

Tom,

Could you remove the usage of the list and vector template classes (Use
btAlignedObjectArray) and use SIMD_PI and the GEN_rand() function and resubmit 
your
patch? Please make sure that it doesn't include the removal of the dynamic 
character
controller, the jam or cmake build system.

Thanks,
John

Original comment by john.mcc...@gmail.com on 28 Jan 2009 at 6:32

[GoogleCodeExporter]

Thanks for the comments!  Updated patch provided

Good to know about STL.  I used std::list<> without thinking.  I've changed 
that and
removed the dependency.

I renamed the ASSERT() macro so there shouldn't be any macro problems.

I think my use of random() was an accident, but somehow it compiled on Linux.  I
meant to use rand() throughout.  Is that okay?  Or should the rand() calls also 
be
updated to be GEN_rand()?  The updated patch uses rand().

DynamicCharacterController: these were #ifdef'd out of the code, as far as I 
could
tell.  The original and updated CharacterDemo doesn't use that class.  But I 
have
re-added it.

Removal of CMakeList.txt and Jamfile: unintentional!  Sorry.  I've readded 
them.  I
don't have MSVC, nor do I use Jam, so I haven't tested them.  But they should 
work (I
didn't add/remove any source files, and DynamicCharacterController still 
compiles).

Build problems: funny, CharacterDemo didn't compile for me out-of-the-box, I 
had to
create my own Makefile.am file so it would build.

Unfortunately, I forgot to include Makefile.am in my original patch!  It is 
there now.

But with that, everything compiled fine.  I'm guessing many of the failures 
were due
to different environments?  I'm building on Fedora 10.

Original comment by astroboa...@gmail.com on 29 Jan 2009 at 3:33

Attachments:

• patch.txt

[GoogleCodeExporter]

Tom,

Thanks for the update. I have one more request. Could you create a new demo 
instead
of overwriting the existing character controller demo? Thanks.

John

Original comment by john.mcc...@gmail.com on 9 Feb 2009 at 4:21

[GoogleCodeExporter]

Will do!  Any recommendation for a name?

CharacterDemo2 or KinematicControllerDemo are my two favorites right now.  I'm
leaning towards the latter.

Original comment by astroboa...@gmail.com on 9 Feb 2009 at 7:12

[GoogleCodeExporter]

Also, I finally did some reading on cmake.  Very cool!  I had naively assumed 
it was
VisualStudio only, obviously that is incorrect.

Has Bullet switched to use CMake exclusively?  If so, I'll just create the
CMakeList.txt file for the new demo, and not create the older Makefile.am files.

Original comment by astroboa...@gmail.com on 9 Feb 2009 at 7:13

[GoogleCodeExporter]

Tom,

The name of the demo doesn't matter too much. I think CharacterControllerDemo2 
would
be fine. No, Bullet isn't CMake only. Bullet supports CMake, automake, jam and
multiple visual studio versions. 

Oh, I merged your height field terrain shape fixes this morning so you can drop 
them
from your patches.

One last thing, I forgot to answer your question on rand() vs. GEN_rand(). You 
should
be using GEN_rand() and not rand().

Thanks,
John

Original comment by john.mcc...@gmail.com on 9 Feb 2009 at 9:13

[GoogleCodeExporter]

Hello John-

Okay, updated patch submitted!  I opted for "CharacterDemo2" just so it was 
clear
what Demo it was the v2 of.  btHeightfield changes removed, since you've 
already got
them.

Summary of changes:

 - updated btKinematicCharacterController.h with necessary #include

 - updated top-level configure.ac to include CharacterDemos

 - updated Demos/CMakeLists.txt to include CharacterDemo2, TerrainDemo

 - updated Demos/Makefile.am to include CharacterDemos

 - added CharacterDemo2 files

 - updated BspDemo and CharacterDemo to fix build warnings

 - added a Makefile.am to CharacterDemo so it would build under automake

For CharacterDemo2, I added an automake makefile (Makefile.am), a cmake file
(CMakeList.txt), and a Jamfile.  I tested automake and cmake, but didn't test 
Jam.

I also updated the CharacterDemo2 code to use GEN_rand().

One comment: the source tree apparently includes the top-level config.h.in, 
which is
an automake-generated file.  I recommend removing it.

I've become a big fan of cmake!  It builds cleanly and let me spot warnings 
that I
missed under automake.  I'm sure Visual Studio support will be required for a 
while,
but it is worth considering dropping automake and Jam in favor of cmake.

Thanks!

    -Thomas

Original comment by astroboa...@gmail.com on 11 Feb 2009 at 5:13

Attachments:

• patch.txt

[GoogleCodeExporter]

I'm closing old issues that are not going to be fixed. We focus on Bullet 3.x 
now, at http://github.com/erwincoumans/bullet3

thanks for the old contribution anyway.
Erwin

Original comment by erwin.coumans on 11 Sep 2013 at 12:04

• Changed state: WontFix