fix threading on windows

panda/src/display/graphicsEngine.cxx

@@ -1567,13 +1567,25 @@ do_add_window(GraphicsOutput *window,
     cull->add_window(cull->_cdraw, window);
   }
   
-  // We should ask the pipe which thread it prefers to run its
-  // windowing commands in (the "window thread").  This is the
-  // thread that handles the commands to open, resize, etc. the
-  // window.  X requires this to be done in the app thread, but some
-  // pipes might prefer this to be done in draw, for instance.  For
-  // now, we assume this is the app thread.
+  // Ask the pipe which thread it prefers to run its windowing
+  // commands in (the "window thread").  This is the thread that
+  // handles the commands to open, resize, etc. the window.  X
+  // requires this to be done in the app thread (along with all the
+  // other windows, since X is strictly single-threaded), but Windows
+  // requires this to be done in draw (because once an OpenGL context
+  // has been bound in a given thread, it cannot subsequently be bound
+  // in any other thread, and we have to bind a context in
+  // open_window()).
+  
+  switch (window->get_pipe()->get_preferred_window_thread()) {
+  case GraphicsPipe::PWT_app:
     _app.add_window(_app._window, window);
+    break;
+
+  case GraphicsPipe::PWT_draw:
+    draw->add_window(draw->_window, window);
+    break;
+  }
 
   if (display_cat.is_debug()) {
     display_cat.debug()

panda/src/display/graphicsPipe.cxx

@@ -76,33 +76,16 @@ GraphicsPipe::
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: GraphicsPipe::get_num_hw_channels
+//     Function: GraphicsPipe::get_preferred_window_thread
 //       Access: Public, Virtual
-//  Description: Returns the number of hardware channels available for
-//               pipes of this type.  See get_hw_channel().
+//  Description: Returns an indication of the thread in which this
+//               GraphicsPipe requires its window processing to be
+//               performed: typically either the app thread (e.g. X)
+//               or the draw thread (Windows).
 ////////////////////////////////////////////////////////////////////
-int GraphicsPipe::
-get_num_hw_channels() {
-  return 0;
-}
-
-////////////////////////////////////////////////////////////////////
-//     Function: GraphicsPipe::get_hw_channel
-//       Access: Public, Virtual
-//  Description: Creates and returns an accessor to the
-//               HardwareChannel at the given index number, which must
-//               be in the range 0 <= index < get_num_hw_channels().
-//               This function will return NULL if the index number is
-//               out of range or the hardware channel at that index is
-//               unavailable.
-//
-//               Most kinds of GraphicsPipes do not have any special
-//               hardware channels available, and this function will
-//               always return NULL.
-////////////////////////////////////////////////////////////////////
-HardwareChannel *GraphicsPipe::
-get_hw_channel(GraphicsOutput *, int) {
-  return (HardwareChannel*)0L;
+GraphicsPipe::PreferredWindowThread 
+GraphicsPipe::get_preferred_window_thread() const {
+  return PWT_draw;
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/display/graphicsPipe.h

@@ -26,7 +26,6 @@
 #include "pointerTo.h"
 #include "pmutex.h"
 
-class HardwareChannel;
 class GraphicsOutput;
 class GraphicsWindow;
 class GraphicsBuffer;
@@ -100,8 +99,11 @@ PUBLISHED:
   virtual string get_interface_name() const=0;
 
 public:
-  virtual int get_num_hw_channels();
-  virtual HardwareChannel *get_hw_channel(GraphicsOutput *window, int index);
+  enum PreferredWindowThread {
+    PWT_app,
+    PWT_draw
+  };
+  virtual PreferredWindowThread get_preferred_window_thread() const;
 
   INLINE GraphicsDevice *get_device() const;
   virtual PT(GraphicsDevice) make_device(void *scrn = NULL);

panda/src/gobj/geom.I

@@ -467,10 +467,10 @@ GeomPipelineReader(const Geom *object, Thread *current_thread) :
 {
 #ifdef _DEBUG
   nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
 #endif // _DEBUG
+#ifdef DO_PIPELINING
+  _cdata->ref();
+#endif  // DO_PIPELINING
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -502,12 +502,13 @@ INLINE GeomPipelineReader::
 ~GeomPipelineReader() {
 #ifdef _DEBUG
   nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
 #endif // _DEBUG
   //  _object->_cycler.release_read(_cdata);
 
+#ifdef DO_PIPELINING
+  unref_delete((CycleData *)_cdata);
+#endif  // DO_PIPELINING
+
 #ifdef _DEBUG
   _object = NULL;
   _cdata = NULL;

panda/src/gobj/geom.cxx

@@ -1369,7 +1369,13 @@ check_usage_hint() const {
       }
 
       // Save the new pointer, and then let the lock release itself.
+#ifdef DO_PIPELINING
+      unref_delete((CycleData *)_cdata);
+#endif
       ((GeomPipelineReader *)this)->_cdata = fresh_cdata;
+#ifdef DO_PIPELINING
+      _cdata->ref();
+#endif
     }
   }
 

panda/src/gobj/geom.h

@@ -352,11 +352,7 @@ public:
 private:
   const Geom *_object;
   Thread *_current_thread;
-#ifdef DO_PIPELINING
-  CPT(Geom::CData) _cdata;
-#else
   const Geom::CData *_cdata;
-#endif  // DO_PIPELINING
 };
 
 INLINE ostream &operator << (ostream &out, const Geom &obj);

panda/src/gobj/geomPrimitive.I

@@ -483,7 +483,7 @@ GeomPrimitivePipelineReader(const GeomPrimitive *object,
 {
   nassertv(_object->test_ref_count_nonzero());
 #ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
+  _cdata->ref();
 #endif  // DO_PIPELINING
   if (_cdata->_vertices != (GeomVertexArrayData *)NULL) {
     _vertices_reader =
@@ -525,12 +525,13 @@ INLINE GeomPrimitivePipelineReader::
   }
 #ifdef _DEBUG
   nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
 #endif // _DEBUG
   //  _object->_cycler.release_read(_cdata);
 
+#ifdef DO_PIPELINING
+  unref_delete((CycleData *)_cdata);
+#endif  // DO_PIPELINING
+
 #ifdef _DEBUG
   _vertices_reader = NULL;
   _object = NULL;

panda/src/gobj/geomPrimitive.cxx

@@ -1514,7 +1514,13 @@ check_minmax() const {
       }
 
       // Save the new pointer, and then let the lock release itself.
+#ifdef DO_PIPELINING
+      unref_delete((CycleData *)_cdata);
+#endif
       ((GeomPrimitivePipelineReader *)this)->_cdata = fresh_cdata;
+#ifdef DO_PIPELINING
+      _cdata->ref();
+#endif
     }
   }
 

panda/src/gobj/geomPrimitive.h

@@ -334,11 +334,7 @@ public:
 private:
   const GeomPrimitive *_object;
   Thread *_current_thread;
-#ifdef DO_PIPELINING
-  CPT(GeomPrimitive::CData) _cdata;
-#else
   const GeomPrimitive::CData *_cdata;
-#endif  // DO_PIPELINING
 
   GeomVertexArrayDataPipelineReader *_vertices_reader;
 };

panda/src/gobj/geomVertexArrayData.I

@@ -213,6 +213,33 @@ GeomVertexArrayDataPipelineBase(GeomVertexArrayData *object,
   _current_thread(current_thread),
   _cdata(cdata)
 {
+#ifdef _DEBUG
+  nassertv(_object->test_ref_count_nonzero());
+#endif // _DEBUG
+#ifdef DO_PIPELINING
+  _cdata->ref();
+#endif  // DO_PIPELINING
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexArrayDataPipelineBase::Destructor
+//       Access: Public
+//  Description:
+////////////////////////////////////////////////////////////////////
+INLINE GeomVertexArrayDataPipelineBase::
+~GeomVertexArrayDataPipelineBase() {
+#ifdef _DEBUG
+  nassertv(_object->test_ref_count_nonzero());
+#endif // _DEBUG
+
+#ifdef DO_PIPELINING
+  unref_delete((CycleData *)_cdata);
+#endif  // DO_PIPELINING
+
+#ifdef _DEBUG
+  _object = NULL;
+  _cdata = NULL;
+#endif  // _DEBUG
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -297,12 +324,6 @@ GeomVertexArrayDataPipelineReader(const GeomVertexArrayData *object,
                                   current_thread,
                                   (GeomVertexArrayData::CData *)object->_cycler.read_unlocked(current_thread))
 {
-#ifdef _DEBUG
-  nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
-#endif // _DEBUG
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -334,18 +355,7 @@ operator = (const GeomVertexArrayDataPipelineReader &) {
 ////////////////////////////////////////////////////////////////////
 INLINE GeomVertexArrayDataPipelineReader::
 ~GeomVertexArrayDataPipelineReader() {
-#ifdef _DEBUG
-  nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
-#endif // _DEBUG
   //  _object->_cycler.release_read(_cdata);
-
-#ifdef _DEBUG
-  _object = NULL;
-  _cdata = NULL;
-#endif  // _DEBUG
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -369,12 +379,6 @@ GeomVertexArrayDataPipelineWriter(GeomVertexArrayData *object, bool force_to_0,
   GeomVertexArrayDataPipelineBase(object, current_thread,
                                   object->_cycler.write_upstream(force_to_0, current_thread))
 {
-#ifdef _DEBUG
-  nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
-#endif // _DEBUG
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -406,18 +410,7 @@ operator = (const GeomVertexArrayDataPipelineWriter &) {
 ////////////////////////////////////////////////////////////////////
 INLINE GeomVertexArrayDataPipelineWriter::
 ~GeomVertexArrayDataPipelineWriter() {
-#ifdef _DEBUG
-  nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
-#endif // _DEBUG
   _object->_cycler.release_write(_cdata);
-
-#ifdef _DEBUG
-  _object = NULL;
-  _cdata = NULL;
-#endif  // _DEBUG
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geomVertexArrayData.h

@@ -193,8 +193,9 @@ protected:
   INLINE GeomVertexArrayDataPipelineBase(GeomVertexArrayData *object, 
                                          Thread *current_thread,
                                          GeomVertexArrayData::CData *cdata);
-
 public:
+  INLINE ~GeomVertexArrayDataPipelineBase();
+
   INLINE Thread *get_current_thread() const;
 
   INLINE const GeomVertexArrayFormat *get_array_format() const;
@@ -208,11 +209,7 @@ public:
 protected:
   GeomVertexArrayData *_object;
   Thread *_current_thread;
-#ifdef DO_PIPELINING
-  PT(GeomVertexArrayData::CData) _cdata;
-#else
   GeomVertexArrayData::CData *_cdata;
-#endif  // DO_PIPELINING
 };
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geomVertexData.I

@@ -478,6 +478,33 @@ GeomVertexDataPipelineBase(GeomVertexData *object,
   _current_thread(current_thread),
   _cdata(cdata)
 {
+#ifdef _DEBUG
+  nassertv(_object->test_ref_count_nonzero());
+#endif // _DEBUG
+#ifdef DO_PIPELINING
+  _cdata->ref();
+#endif  // DO_PIPELINING
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexDataPipelineBase::Destructor
+//       Access: Public
+//  Description:
+////////////////////////////////////////////////////////////////////
+INLINE GeomVertexDataPipelineBase::
+~GeomVertexDataPipelineBase() {
+#ifdef _DEBUG
+  nassertv(_object->test_ref_count_nonzero());
+#endif // _DEBUG
+
+#ifdef DO_PIPELINING
+  unref_delete((CycleData *)_cdata);
+#endif  // DO_PIPELINING
+
+#ifdef _DEBUG
+  _object = NULL;
+  _cdata = NULL;
+#endif  // _DEBUG
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -593,10 +620,6 @@ GeomVertexDataPipelineReader(const GeomVertexData *object,
                              (GeomVertexData::CData *)object->_cycler.read_unlocked(current_thread)),
   _got_array_readers(false)
 {
-  nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -631,18 +654,7 @@ INLINE GeomVertexDataPipelineReader::
   if (_got_array_readers) {
     delete_array_readers();
   }
-#ifdef _DEBUG
-  nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
-#endif // _DEBUG
   //  _object->_cycler.release_read(_cdata);
-
-#ifdef _DEBUG
-  _object = NULL;
-  _cdata = NULL;
-#endif  // _DEBUG
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -777,18 +789,7 @@ INLINE GeomVertexDataPipelineWriter::
   if (_got_array_writers) {
     delete_array_writers();
   }
-#ifdef _DEBUG
-  nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
-#endif // _DEBUG
   _object->_cycler.release_write(_cdata);
-
-#ifdef _DEBUG
-  _object = NULL;
-  _cdata = NULL;
-#endif  // _DEBUG
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geomVertexData.h

@@ -325,6 +325,9 @@ protected:
                                     Thread *current_thread,
                                     GeomVertexData::CData *cdata);
 
+public:
+  INLINE ~GeomVertexDataPipelineBase();
+
 public:
   INLINE Thread *get_current_thread() const;
 
@@ -343,11 +346,7 @@ public:
 protected:
   GeomVertexData *_object;
   Thread *_current_thread;
-#ifdef DO_PIPELINING
-  PT(GeomVertexData::CData) _cdata;
-#else
   GeomVertexData::CData *_cdata;
-#endif  // DO_PIPELINING
 };
 
 ////////////////////////////////////////////////////////////////////

panda/src/pgraph/pandaNode.I

@@ -1176,10 +1176,14 @@ PandaNodePipelineReader(const PandaNode *object, Thread *current_thread) :
 {
 #ifdef _DEBUG
   nassertv(_object->test_ref_count_nonzero());
-#ifdef DO_PIPELINING
-  nassertv(_cdata->test_ref_count_nonzero());
-#endif  // DO_PIPELINING
 #endif // _DEBUG
+
+#ifdef DO_PIPELINING
+  // We node_ref the CData pointer, so that if anyone makes changes to
+  // the PandaNode while we hold this pointer, it will force a
+  // copy--so that this object will remain unchanged (if out-of-date).
+  _cdata->node_ref();
+#endif  // DO_PIPELINING
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -1193,6 +1197,10 @@ PandaNodePipelineReader(const PandaNodePipelineReader &copy) :
   _current_thread(copy._current_thread),
   _cdata(copy._cdata)
 {
+#ifdef DO_PIPELINING
+  _cdata->node_ref();
+#endif  // DO_PIPELINING
+
   /*
   if (_cdata != (PandaNode::CData *)NULL) {
     _object->_cycler.increment_read(_cdata);
@@ -1215,9 +1223,17 @@ operator = (const PandaNodePipelineReader &copy) {
   }
   */
 
+#ifdef DO_PIPELINING
+  node_unref_delete((CycleData *)_cdata);
+#endif  // DO_PIPELINING
+
   _object = copy._object;
   _cdata = copy._cdata;
 
+#ifdef DO_PIPELINING
+  _cdata->node_ref();
+#endif  // DO_PIPELINING
+
   /*
   if (_cdata != (PandaNode::CData *)NULL) {
     _object->_cycler.increment_read(_cdata);
@@ -1238,6 +1254,10 @@ INLINE PandaNodePipelineReader::
   }
   */
 
+#ifdef DO_PIPELINING
+  node_unref_delete((CycleData *)_cdata);
+#endif  // DO_PIPELINING
+
 #ifdef _DEBUG
   _object = NULL;
   _cdata = NULL;

panda/src/pgraph/pandaNode.cxx

@@ -3645,6 +3645,9 @@ check_bounds() const {
     // We'll need to get a fresh read pointer, since another thread
     // might already have modified the pointer on the object since we
     // queried it.
+#ifdef DO_PIPELINING
+    node_unref_delete((CycleData *)_cdata);
+#endif  // DO_PIPELINING
     ((PandaNodePipelineReader *)this)->_cdata = NULL;
     int pipeline_stage = _current_thread->get_pipeline_stage();
     PandaNode::CDLockedStageReader fresh_cdata(_object->_cycler, pipeline_stage, _current_thread);
@@ -3652,7 +3655,12 @@ check_bounds() const {
       // What luck, some other thread has already freshened the
       // cache for us.  Save the new pointer, and let the lock
       // release itself.
+      if (_cdata != (const PandaNode::CData *)fresh_cdata) {
         ((PandaNodePipelineReader *)this)->_cdata = fresh_cdata;
+#ifdef DO_PIPELINING
+        _cdata->node_ref();
+#endif  // DO_PIPELINING
+      }
       
     } else {
       // No, the cache is still stale.  We have to do the work of
@@ -3661,7 +3669,12 @@ check_bounds() const {
       nassertv(cdataw->_last_update == cdataw->_next_update);
       // As above, we save the new pointer, and then let the lock
       // release itself.
+      if (_cdata != (const PandaNode::CData *)cdataw) {
         ((PandaNodePipelineReader *)this)->_cdata = cdataw;
+#ifdef DO_PIPELINING
+        _cdata->node_ref();
+#endif  // DO_PIPELINING
+      }
     }
   }
 

panda/src/pgraph/pandaNode.h

@@ -675,14 +675,7 @@ private:
   const PandaNode *_object;
   Thread *_current_thread;
 
-#ifdef DO_PIPELINING
-  // We node_ref the CData pointer, so that if anyone makes changes to
-  // the PandaNode while we hold this pointer, it will force a
-  // copy--so that this object will remain unchanged (if out-of-date).
-  NCPT(PandaNode::CData) _cdata;
-#else
   const PandaNode::CData *_cdata;
-#endif  // DO_PIPELINING
 };
 
 INLINE ostream &operator << (ostream &out, const PandaNode &node) {