cpShapeInit does not call cpShapeCacheBB since r435

[GoogleCodeExporter]

So, cpShapeCacheBB is commented out in cpShapeInit since revision r435 back in 
May 29 [1].  I guess this may not work very well if shape->body == NULL, and 
that's why that line of code was disabled since then.

[1] http://code.google.com/p/chipmunk-physics/source/detail?spec=svn540&r=435

However, for some reason sometimes the shape->bb is needed *before* 
cpShapeCacheBB is called.  I can't say for sure when that happens.  I have a 
test program in Haskell that destroys the space and creates a new one with some 
bodies and shapes (both active and static) when DEL is pressed.  If I hit DEL 
enough times, I can consistently get cpSpaceHashInsert called on a shape with 
an uncalculated bb.  See this snippet of gdb session:

  (gdb) bt
  #0  0x000000000042a3cc in hash_func (x=2191207412, y=0, n=1543) at Chipmunk-5.3.1/src/cpSpaceHash.c:222
  #1  0x000000000042a49e in hashHandle (hash=0x7d9550, hand=0xc06ff0, bb=...) at Chipmunk-5.3.1/src/cpSpaceHash.c:247
  #2  0x000000000042a5b2 in cpSpaceHashInsert (hash=0x7d9550, obj=0x7ffff2f30720, hashid=1, bb=...) at Chipmunk-5.3.1/src/cpSpaceHash.c:267
  #3  0x00000000004278af in addShapeRaw (shape=0x7ffff2f30720, hash=0x7d9550) at Chipmunk-5.3.1/src/cpSpace.c:351
  #4  0x0000000000427960 in cpSpaceAddShape (space=0x7ffff2f30160, shape=0x7ffff2f30720) at Chipmunk-5.3.1/src/cpSpace.c:372
  #5  0x000000000041ea18 in soYy_info ()
  #6  0x0000000000000000 in ?? ()
  (gdb) up
  #2  0x000000000042a5b2 in cpSpaceHashInsert (hash=0x7d9550, obj=0x7ffff2f30720, hashid=1, bb=...) at Chipmunk-5.3.1/src/cpSpaceHash.c:267
  267       hashHandle(hash, hand, bb);
  (gdb) info args
  hash = 0x7d9550
  obj = 0x7ffff2f30720
  hashid = 1
  bb = {l = -nan(0xf040000001701), b = 2.6354093953198767e-317, r = 4.9406564584124654e-324, t = 7.7468101924323985e-304}
  (gdb) up
  #3  0x00000000004278af in addShapeRaw (shape=0x7ffff2f30720, hash=0x7d9550) at Chipmunk-5.3.1/src/cpSpace.c:351
  351       cpSpaceHashInsert(hash, shape, shape->hashid, shape->bb);
  (gdb) info args
  shape = 0x7ffff2f30720
  hash = 0x7d9550
  (gdb) print shape->bb
  $1 = {l = -nan(0xf040000001701), b = 2.6354093953198767e-317, r = 4.9406564584124654e-324, t = 7.7468101924323985e-304}
  (gdb) print shape->klass->type
  $3 = CP_POLY_SHAPE
  (gdb) print ((cpPolyShape*)shape)->numVerts
  $5 = 4
  (gdb) print ((cpPolyShape*)shape)->verts[0]
  $6 = {x = -100, y = 1}
  (gdb) print ((cpPolyShape*)shape)->verts[1]
  $7 = {x = 100, y = 1}
  (gdb) print ((cpPolyShape*)shape)->verts[2]
  $8 = {x = 100, y = -1}
  (gdb) print ((cpPolyShape*)shape)->verts[3]
  $9 = {x = -100, y = -1}
  (gdb) print shape->klass->cacheData(shape,shape->body->p,shape->body->rot)
  $18 = {l = -100, b = -191, r = 100, t = -189}
  (gdb) print shape->bb
  $19 = {l = -nan(0xf040000001701), b = 2.6354093953198767e-317, r = 4.9406564584124654e-324, t = 7.7468101924323985e-304}
  (gdb) print shape->klass->cacheData(shape,shape->body->p,shape->body->rot)
  $20 = {l = -100, b = -191, r = 100, t = -189}
  (gdb) print cpShapeCacheBB(shape)
  $21 = {l = -100, b = -191, r = 100, t = -189}
  (gdb) print shape->bb
  $22 = {l = -100, b = -191, r = 100, t = -189}

Adding back the call to cpShapeCacheBB solves the problem.

Comments?

Original issue reported on code.google.com by felipe.l...@gmail.com on 26 Sep 2010 at 2:35

[GoogleCodeExporter]

Sorry for the delay, I've been working on a lot of contract work lately.

I can add the call to cpShapeCacheBB() back, but it sounds like there is a 
bigger problem here. Somehow it's possible to insert a shape into the spatial 
hashes without the shape's bounding box (and other collision data) being 
updated. That should not be able to happen.

I'm not sure how the bounding box is full of garbage either, iirc it should be 
calloced. I'll have to look into this more.

Original comment by slemb...@gmail.com on 28 Sep 2010 at 7:59

[GoogleCodeExporter]

I think I found the bug that was causing this and fixed it.

In cpSpaceAddShape() I was not calling cpShapeCacheBB() when inserting active 
objects. My intention was to delay the call to cpShapeCacheBB() until it was 
actually added. The point was to make queries work before calling 
cpSpaceStep(). I guess I forgot to actually add it to the new spot. :-\

If the shape was not zeroed when it was allocated, it would have any old random 
bounding box for it. Adding a shape with a zeroed bounding box is fine unless 
you were actually trying to query. Junk bounding boxes of course can be very 
bad. I guess I should have actually tested what I was trying to fix. (/me 
facepalm)

Original comment by slemb...@gmail.com on 30 Sep 2010 at 9:42

[GoogleCodeExporter]

Original comment by slemb...@gmail.com on 30 Sep 2010 at 9:42

• Changed state: Fixed

[GoogleCodeExporter]

Thanks for tracking this one down. It was not calloced, it was alloced into a 
pinned object in GC area of the Haskell run time system.

Cheers!

Original comment by felipe.l...@gmail.com on 30 Sep 2010 at 9:48

[GoogleCodeExporter]

Yeah, I figured as much. Chipmunk's init functions aren't supposed to assume 
zeroed memory, but I rarely test it that way. I use calloc in the allocation 
functions to track down bugs faster, and most GCs that I've worked with return 
zeroed memory.

Hopefully that was the bug then. Cheers.

Original comment by slemb...@gmail.com on 30 Sep 2010 at 10:04