gobj: try to fix some deadlocks modifying geometry while rendering

The correct locking order should be: the Geom's GeomVertexArrayData first, *then* the GeomPrimitive.

panda/src/cull/cullBinBackToFront.cxx

@@ -96,9 +96,10 @@ draw(bool force, Thread *current_thread) {
       nassertd(object->_geom != nullptr) continue;
 
       _gsg->set_state_and_transform(object->_state, object->_internal_transform);
-      data_reader.set_object(object->_munged_data);
+
+      GeomPipelineReader geom_reader(object->_geom, current_thread);
+      GeomVertexDataPipelineReader data_reader(object->_munged_data, current_thread);
       data_reader.check_array_readers();
-      geom_reader.set_object(object->_geom);
       geom_reader.draw(_gsg, &data_reader, force);
     } else {
       // It has a callback associated.

panda/src/cull/cullBinFixed.cxx

@@ -82,9 +82,10 @@ draw(bool force, Thread *current_thread) {
       nassertd(object->_geom != nullptr) continue;
 
       _gsg->set_state_and_transform(object->_state, object->_internal_transform);
-      data_reader.set_object(object->_munged_data);
+
+      GeomPipelineReader geom_reader(object->_geom, current_thread);
+      GeomVertexDataPipelineReader data_reader(object->_munged_data, current_thread);
       data_reader.check_array_readers();
-      geom_reader.set_object(object->_geom);
       geom_reader.draw(_gsg, &data_reader, force);
     } else {
       // It has a callback associated.

panda/src/cull/cullBinFrontToBack.cxx

@@ -96,9 +96,10 @@ draw(bool force, Thread *current_thread) {
       nassertd(object->_geom != nullptr) continue;
 
       _gsg->set_state_and_transform(object->_state, object->_internal_transform);
-      data_reader.set_object(object->_munged_data);
+
+      GeomPipelineReader geom_reader(object->_geom, current_thread);
+      GeomVertexDataPipelineReader data_reader(object->_munged_data, current_thread);
       data_reader.check_array_readers();
-      geom_reader.set_object(object->_geom);
       geom_reader.draw(_gsg, &data_reader, force);
     } else {
       // It has a callback associated.

panda/src/cull/cullBinStateSorted.cxx

@@ -81,9 +81,10 @@ draw(bool force, Thread *current_thread) {
       nassertd(object->_geom != nullptr) continue;
 
       _gsg->set_state_and_transform(object->_state, object->_internal_transform);
-      data_reader.set_object(object->_munged_data);
+
+      GeomPipelineReader geom_reader(object->_geom, current_thread);
+      GeomVertexDataPipelineReader data_reader(object->_munged_data, current_thread);
       data_reader.check_array_readers();
-      geom_reader.set_object(object->_geom);
       geom_reader.draw(_gsg, &data_reader, force);
     } else {
       // It has a callback associated.

panda/src/cull/cullBinUnsorted.cxx

@@ -55,9 +55,6 @@ void CullBinUnsorted::
 draw(bool force, Thread *current_thread) {
   PStatTimer timer(_draw_this_pcollector, current_thread);
 
-  GeomPipelineReader geom_reader(current_thread);
-  GeomVertexDataPipelineReader data_reader(current_thread);
-
   Objects::iterator oi;
   for (oi = _objects.begin(); oi != _objects.end(); ++oi) {
     CullableObject *object = (*oi);
@@ -66,9 +63,10 @@ draw(bool force, Thread *current_thread) {
       nassertd(object->_geom != nullptr) continue;
 
       _gsg->set_state_and_transform(object->_state, object->_internal_transform);
-      data_reader.set_object(object->_munged_data);
+
+      GeomPipelineReader geom_reader(object->_geom, current_thread);
+      GeomVertexDataPipelineReader data_reader(object->_munged_data, current_thread);
       data_reader.check_array_readers();
-      geom_reader.set_object(object->_geom);
       geom_reader.draw(_gsg, &data_reader, force);
     } else {
       // It has a callback associated.

panda/src/gobj/geom.cxx

@@ -195,6 +195,9 @@ offset_vertices(const GeomVertexData *data, int offset) {
   cdata->_data = (GeomVertexData *)data;
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(data, current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -203,7 +206,7 @@ offset_vertices(const GeomVertexData *data, int offset) {
     prim->offset_vertices(offset);
 
 #ifndef NDEBUG
-    if (!prim->check_valid(data)) {
+    if (!prim->check_valid(&data_reader)) {
       gobj_cat.warning()
         << *prim << " is invalid for " << *data << ":\n";
       prim->write(gobj_cat.warning(false), 4);
@@ -423,6 +426,9 @@ decompose_in_place() {
   CDWriter cdata(_cycler, true, current_thread);
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -431,7 +437,7 @@ decompose_in_place() {
     (*pi) = (GeomPrimitive *)new_prim.p();
 
 #ifndef NDEBUG
-    if (!new_prim->check_valid(cdata->_data.get_read_pointer(current_thread))) {
+    if (!new_prim->check_valid(&data_reader)) {
       all_is_valid = false;
     }
 #endif
@@ -457,6 +463,9 @@ doubleside_in_place() {
   CDWriter cdata(_cycler, true, current_thread);
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -465,7 +474,7 @@ doubleside_in_place() {
     (*pi) = (GeomPrimitive *)new_prim.p();
 
 #ifndef NDEBUG
-    if (!new_prim->check_valid(cdata->_data.get_read_pointer(current_thread))) {
+    if (!new_prim->check_valid(&data_reader)) {
       all_is_valid = false;
     }
 #endif
@@ -491,6 +500,9 @@ reverse_in_place() {
   CDWriter cdata(_cycler, true, current_thread);
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -499,7 +511,7 @@ reverse_in_place() {
     (*pi) = (GeomPrimitive *)new_prim.p();
 
 #ifndef NDEBUG
-    if (!new_prim->check_valid(cdata->_data.get_read_pointer(current_thread))) {
+    if (!new_prim->check_valid(&data_reader)) {
       all_is_valid = false;
     }
 #endif
@@ -525,6 +537,9 @@ rotate_in_place() {
   CDWriter cdata(_cycler, true, current_thread);
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -533,7 +548,7 @@ rotate_in_place() {
     (*pi) = (GeomPrimitive *)new_prim.p();
 
 #ifndef NDEBUG
-    if (!new_prim->check_valid(cdata->_data.get_read_pointer(current_thread))) {
+    if (!new_prim->check_valid(&data_reader)) {
       all_is_valid = false;
     }
 #endif
@@ -640,6 +655,9 @@ unify_in_place(int max_indices, bool preserve_order) {
     // primitives.)
     nassertv(false);
   }
+
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
 #endif
 
   // Finally, iterate through the remaining primitives, and copy them to the
@@ -649,7 +667,7 @@ unify_in_place(int max_indices, bool preserve_order) {
   for (npi = new_prims.begin(); npi != new_prims.end(); ++npi) {
     GeomPrimitive *prim = (*npi).second;
 
-    nassertv(prim->check_valid(cdata->_data.get_read_pointer(current_thread)));
+    nassertv(prim->check_valid(&data_reader));
 
     // Each new primitive, naturally, inherits the Geom's overall shade model.
     prim->set_shade_model(cdata->_shade_model);
@@ -748,6 +766,9 @@ make_lines_in_place() {
   CDWriter cdata(_cycler, true, current_thread);
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -756,7 +777,7 @@ make_lines_in_place() {
     (*pi) = (GeomPrimitive *)new_prim.p();
 
 #ifndef NDEBUG
-    if (!new_prim->check_valid(cdata->_data.get_read_pointer(current_thread))) {
+    if (!new_prim->check_valid(&data_reader)) {
       all_is_valid = false;
     }
 #endif
@@ -782,6 +803,9 @@ make_points_in_place() {
   CDWriter cdata(_cycler, true, current_thread);
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -790,7 +814,7 @@ make_points_in_place() {
     (*pi) = (GeomPrimitive *)new_prim.p();
 
 #ifndef NDEBUG
-    if (!new_prim->check_valid(cdata->_data.get_read_pointer(current_thread))) {
+    if (!new_prim->check_valid(&data_reader)) {
       all_is_valid = false;
     }
 #endif
@@ -816,6 +840,9 @@ make_patches_in_place() {
   CDWriter cdata(_cycler, true, current_thread);
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -824,7 +851,7 @@ make_patches_in_place() {
     (*pi) = (GeomPrimitive *)new_prim.p();
 
 #ifndef NDEBUG
-    if (!new_prim->check_valid(cdata->_data.get_read_pointer(current_thread))) {
+    if (!new_prim->check_valid(&data_reader)) {
       all_is_valid = false;
     }
 #endif
@@ -850,6 +877,9 @@ make_adjacency_in_place() {
   CDWriter cdata(_cycler, true, current_thread);
 
 #ifndef NDEBUG
+  GeomVertexDataPipelineReader data_reader(cdata->_data.get_read_pointer(current_thread), current_thread);
+  data_reader.check_array_readers();
+
   bool all_is_valid = true;
 #endif
   Primitives::iterator pi;
@@ -859,7 +889,7 @@ make_adjacency_in_place() {
       (*pi) = (GeomPrimitive *)new_prim.p();
 
 #ifndef NDEBUG
-      if (!new_prim->check_valid(cdata->_data.get_read_pointer(current_thread))) {
+      if (!new_prim->check_valid(&data_reader)) {
         all_is_valid = false;
       }
 #endif
@@ -1465,13 +1495,20 @@ clear_prepared(PreparedGraphicsObjects *prepared_objects) {
  */
 bool Geom::
 check_will_be_valid(const GeomVertexData *vertex_data) const {
-  CDReader cdata(_cycler);
+  Thread *current_thread = Thread::get_current_thread();
+
+  CDReader cdata(_cycler, current_thread);
+
+  GeomVertexDataPipelineReader data_reader(vertex_data, current_thread);
+  data_reader.check_array_readers();
 
   Primitives::const_iterator pi;
   for (pi = cdata->_primitives.begin();
        pi != cdata->_primitives.end();
        ++pi) {
-    if (!(*pi).get_read_pointer()->check_valid(vertex_data)) {
+    GeomPrimitivePipelineReader reader((*pi).get_read_pointer(), current_thread);
+    reader.check_minmax();
+    if (!reader.check_valid(&data_reader)) {
       return false;
     }
   }

panda/src/gobj/geomPrimitive.I

@@ -223,11 +223,19 @@ get_modified() const {
 INLINE bool GeomPrimitive::
 check_valid(const GeomVertexData *vertex_data) const {
   Thread *current_thread = Thread::get_current_thread();
-  GeomPrimitivePipelineReader reader(this, current_thread);
-  reader.check_minmax();
   GeomVertexDataPipelineReader data_reader(vertex_data, current_thread);
   data_reader.check_array_readers();
-  return reader.check_valid(&data_reader);
+  return check_valid(&data_reader);
+}
+
+/**
+ *
+ */
+INLINE bool GeomPrimitive::
+check_valid(const GeomVertexDataPipelineReader *data_reader) const {
+  GeomPrimitivePipelineReader reader(this, data_reader->get_current_thread());
+  reader.check_minmax();
+  return reader.check_valid(data_reader);
 }
 
 /**

panda/src/gobj/geomPrimitive.h

@@ -146,6 +146,7 @@ PUBLISHED:
   bool request_resident(Thread *current_thread = Thread::get_current_thread()) const;
 
   INLINE bool check_valid(const GeomVertexData *vertex_data) const;
+  INLINE bool check_valid(const GeomVertexDataPipelineReader *data_reader) const;
 
   virtual void output(std::ostream &out) const;
   virtual void write(std::ostream &out, int indent_level) const;