Browse Source

Cosmetic: An extra assertion check added to prevent modifying trimesh data from within collision callbacks

Oleh Derevenko 9 years ago
parent
commit
e22117e704

+ 16 - 0
ode/src/collision_kernel.h

@@ -44,6 +44,11 @@ internal data structures and functions for collision detection.
 #define IS_SPACE(geom) \
     ((geom)->type >= dFirstSpaceClass && (geom)->type <= dLastSpaceClass)
 
+#define CHECK_NOT_LOCKED(space) \
+    dUASSERT ((space) == NULL || (space)->lock_count == 0, \
+        "Invalid operation for locked space")
+
+
 //****************************************************************************
 // geometry object base class
 
@@ -159,6 +164,8 @@ struct dxGeom : public dBase {
         }
     }
 
+    inline void markAABBBad();
+
     // add and remove this geom from a linked list maintained by a space.
 
     void spaceAdd (dxGeom **first_ptr) {
@@ -246,6 +253,15 @@ struct dxSpace : public dxGeom {
 };
 
 
+//////////////////////////////////////////////////////////////////////////
+
+/*inline */
+void dxGeom::markAABBBad() {
+    gflags |= (GEOM_DIRTY | GEOM_AABB_BAD);
+    CHECK_NOT_LOCKED(parent_space);
+}
+
+
 //****************************************************************************
 // Initialization and finalization functions
 

+ 4 - 1
ode/src/collision_quadtreespace.cpp

@@ -539,8 +539,11 @@ void dxQuadTreeSpace::cleanGeoms(){
         if (IS_SPACE(g)){
             ((dxSpace*)g)->cleanGeoms();
         }
+        
         g->recomputeAABB();
-        g->gflags &= (~(GEOM_DIRTY|GEOM_AABB_BAD));
+        dIASSERT((g->gflags & GEOM_AABB_BAD) == 0);
+
+        g->gflags &= ~GEOM_DIRTY;
 
         ((Block*)g->tome_ex)->Traverse(g);
     }

+ 5 - 1
ode/src/collision_sapspace.cpp

@@ -405,8 +405,12 @@ void dxSAPSpace::cleanGeoms()
         if( IS_SPACE(g) ) {
             ((dxSpace*)g)->cleanGeoms();
         }
+        
         g->recomputeAABB();
-        g->gflags &= (~(GEOM_DIRTY|GEOM_AABB_BAD));
+        dIASSERT((g->gflags & GEOM_AABB_BAD) == 0);
+        
+        g->gflags &= ~GEOM_DIRTY;
+        
         // remove from dirty list, add to geom list
         GEOM_SET_DIRTY_IDX( g, GEOM_INVALID_IDX );
         GEOM_SET_GEOM_IDX( g, geomSize + i );

+ 10 - 10
ode/src/collision_space.cpp

@@ -60,8 +60,7 @@ void dGeomMoved (dxGeom *geom)
     dxSpace *parent = geom->parent_space;
 
     while (parent && (geom->gflags & GEOM_DIRTY)==0) {
-        CHECK_NOT_LOCKED (parent);
-        geom->gflags |= GEOM_DIRTY | GEOM_AABB_BAD;
+        geom->markAABBBad();
         parent->dirty (geom);
         geom = parent;
         parent = parent->parent_space;
@@ -70,8 +69,7 @@ void dGeomMoved (dxGeom *geom)
     // all the remaining dirty geoms must have their AABB_BAD flags set, to
     // ensure that their AABBs get recomputed
     while (geom) {
-        geom->gflags |= GEOM_DIRTY | GEOM_AABB_BAD;
-        CHECK_NOT_LOCKED (geom->parent_space);
+        geom->markAABBBad();
         geom = geom->parent_space;
     }
 }
@@ -176,10 +174,6 @@ void dxSpace::add (dxGeom *geom)
     // enumerator has been invalidated
     current_geom = 0;
 
-    // new geoms are added to the front of the list and are always
-    // considered to be dirty. as a consequence, this space and all its
-    // parents are dirty too.
-    geom->gflags |= GEOM_DIRTY | GEOM_AABB_BAD;
     dGeomMoved (this);
 }
 
@@ -239,8 +233,11 @@ void dxSimpleSpace::cleanGeoms()
         if (IS_SPACE(g)) {
             ((dxSpace*)g)->cleanGeoms();
         }
+
         g->recomputeAABB();
-        g->gflags &= (~(GEOM_DIRTY|GEOM_AABB_BAD));
+        dIASSERT((g->gflags & GEOM_AABB_BAD) == 0);
+
+        g->gflags &= ~GEOM_DIRTY;
     }
     lock_count--;
 }
@@ -411,8 +408,11 @@ void dxHashSpace::cleanGeoms()
         if (IS_SPACE(g)) {
             ((dxSpace*)g)->cleanGeoms();
         }
+
         g->recomputeAABB();
-        g->gflags &= (~(GEOM_DIRTY|GEOM_AABB_BAD));
+        dIASSERT((g->gflags & GEOM_AABB_BAD) == 0);
+        
+        g->gflags &= ~GEOM_DIRTY;
     }
     lock_count--;
 }

+ 0 - 4
ode/src/collision_space_internal.h

@@ -31,10 +31,6 @@ stuff common to all spaces
 
 #define ALLOCA(x) dALLOCA16(x)
 
-#define CHECK_NOT_LOCKED(space) \
-    dUASSERT ((space)==0 || (space)->lock_count==0, \
-    "invalid operation for locked space");
-
 
 // collide two geoms together. for the hash table space, this is
 // called if the two AABBs inhabit the same hash table cells.

+ 1 - 1
ode/src/collision_trimesh_gimpact.cpp

@@ -288,7 +288,7 @@ void dGeomTriMeshSetData(dGeomID g, dTriMeshDataID Data)
     dxTriMesh* mesh = (dxTriMesh*) g;
     mesh->Data = Data;
     // I changed my data -- I know nothing about my own AABB anymore.
-    ((dxTriMesh*)g)->gflags |= (GEOM_DIRTY|GEOM_AABB_BAD);
+    ((dxTriMesh*)g)->markAABBBad();
 
     // GIMPACT only supports stride 12, so we need to catch the error early.
     dUASSERT

+ 1 - 1
ode/src/collision_trimesh_opcode.cpp

@@ -766,7 +766,7 @@ void dGeomTriMeshSetData(dGeomID g, dTriMeshDataID Data)
     dUASSERT(g && g->type == dTriMeshClass, "argument not a trimesh");
     ((dxTriMesh*)g)->Data = Data;
     // I changed my data -- I know nothing about my own AABB anymore.
-    ((dxTriMesh*)g)->gflags |= (GEOM_DIRTY|GEOM_AABB_BAD);
+    ((dxTriMesh*)g)->markAABBBad();
 }
 
 dTriMeshDataID dGeomTriMeshGetData(dGeomID g)