behdad / box2d

Automatically exported from code.google.com/p/box2d
2 stars 12 forks source link

b2Query doesn't always return a valid fixture #233

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
--------------------------------------

I have been unable to generate a test that consistently fails. However, for 
this reason I tend to think the bug is somewhere in the new broadphase balanced 
tree. I can usually get b2Query to fail if I have 10+ overlapping bodies, and I 
query a small bounding box at an overlapping location. This does not always 
fail, and requires multiple test runs, but it will almost definitely fail 1/5 
times.

What is the expected output? What do you see instead?
-----------------------------------------------------

I expect Box2D to always report a valid pointer to 
b2QueryCallback->ReportFixture(proxy->fixture). Instead, occasionally I get an 
invalid pointer such as 0xdfdfdfdf.

What version of the product are you using? On what operating system?
I am using the very latest google code version, however, this seems to have 
been an issue since I updated the new balanced tree code.

Please provide any additional information below.
------------------------------------------------

The attached patch to b2World.cpp fixes this bug, but I consider it a hack 
workaround. Perhaps it can give more insight. Platforms I have tested this 
error and solution on include Win32 with the microsoft compiler and G++, and 
ARM cross-compile with G++. I feel the root cause is deeper than this function, 
and I am only patching a problem produced by broadphase. I'm coding Box2D every 
day if you have any questions, I'm available within 12 hours.

Original issue reported on code.google.com by rocif...@gmail.com on 29 Aug 2011 at 2:57

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Here's the attached patch file (I accidentally used diff before)

Original comment by rocif...@gmail.com on 29 Aug 2011 at 3:13

Attachments:

GoogleCodeExporter commented 9 years ago
I'm not seeing this problem. I have a query in PolyShapes.h than can handle 
more than 10 overlapping shapes. I have never seen this crash.

Original comment by erinca...@gmail.com on 29 Aug 2011 at 8:06

GoogleCodeExporter commented 9 years ago
How can I view good debug information on the broadphase tree etc? I might be 
able to come up with a proper test that fails, since I can produce the bug on 
demand using some trial-and-error in my game setup.

Original comment by rocif...@gmail.com on 30 Aug 2011 at 8:37

GoogleCodeExporter commented 9 years ago
You could try uncommenting Validate in the insert/remove leaf functions in 
b2DynamicTree.cpp.

Does your code modify the world inside a callback?

Original comment by erinca...@gmail.com on 1 Sep 2011 at 7:59

GoogleCodeExporter commented 9 years ago
My code isn't modifying the world in callbacks, I specifically avoided this 
after getting assertion failures telling me I can't do that. I uncommented 
those two Validate() calls but they don't cause any assertions when my code 
fails. I'm going to do some more digging and try to generate a solid test case 
that fails every time.

Original comment by rocif...@gmail.com on 1 Sep 2011 at 12:17

GoogleCodeExporter commented 9 years ago
Here's my watch display for a failed node from within 
b2DynamicTree::GetUserData(int32 proxyId) eventually called from Query in 
b2DynamicTree.h
It seems like the variables have data that's intact but the userData is not a 
valid pointer. Is there anything that might explain this inconsistency?

-       m_nodes[proxyId]    {aabb={...} userData=0x10d80d18 parent=384} b2TreeNode
-       aabb    {lowerBound={...} upperBound={...} }    b2AABB
+       lowerBound  {x=5.3394465 y=15.581806 }  b2Vec2
+       upperBound  {x=7.5034966 y=18.216160 }  b2Vec2
        userData    0x10d80d18  void *
        parent  384 int
        next    384 int
        child1  -1  int
        child2  -1  int
        height  -1  int

Original comment by rocif...@gmail.com on 1 Sep 2011 at 12:26

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hmm, this may be a multi-threaded issue with my app, but I don't think so. I 
added a whole bunch of asserts in to test whether the world is locked when I am 
making relevant changes to it, and none of them fire. I'll have to try again 
tomorrow..

Original comment by rocif...@gmail.com on 1 Sep 2011 at 1:27

GoogleCodeExporter commented 9 years ago
Box2D is not thread safe. You need to use a critical section when you do 
anything that might modify the broad-phase.

Original comment by erinca...@gmail.com on 2 Sep 2011 at 5:42

GoogleCodeExporter commented 9 years ago
Ah, sorry Erin, I've finally discovered my code is actually modifying the world 
during a world step by deleting a body. Gee, this took a long time to find! I 
don't know why the assertion wasn't being triggered in b2World::DestroyBody()...

Original comment by rocif...@gmail.com on 12 Sep 2011 at 1:50

GoogleCodeExporter commented 9 years ago
Were you running a debug build of Box2D?

Original comment by erinca...@gmail.com on 12 Sep 2011 at 5:51

GoogleCodeExporter commented 9 years ago
Yes

Original comment by rocif...@gmail.com on 13 Sep 2011 at 12:45

GoogleCodeExporter commented 9 years ago
Well b2Assert just maps to the standard assert. Shrug. I'm closing this.

Original comment by erinca...@gmail.com on 17 Sep 2011 at 6:15