fix caching of display lists

panda/src/glstuff/glGeomContext_src.I

@@ -26,8 +26,19 @@ INLINE CLP(GeomContext)::
 CLP(GeomContext)(Geom *geom) :
   GeomContext(geom)
 {
-  _index = 0;
+  _deprecated_index = 0;
 #ifdef DO_PSTATS
   _num_verts = 0;
 #endif
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: CLP(GeomContext)::DisplayList::Constructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE CLP(GeomContext)::DisplayList::
+DisplayList() :
+  _index(0)
+{
+}

panda/src/glstuff/glGeomContext_src.cxx

@@ -18,3 +18,98 @@
 
 TypeHandle CLP(GeomContext)::_type_handle;
 
+////////////////////////////////////////////////////////////////////
+//     Function: CLP(GeomContext)::Destructor
+//       Access: Public, Virtual
+//  Description: 
+////////////////////////////////////////////////////////////////////
+CLP(GeomContext)::
+~CLP(GeomContext)() {
+  nassertv(_display_lists.empty());
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: CLP(GeomContext)::get_display_list
+//       Access: Public
+//  Description: Looks up the display list index associated with the
+//               indicated munger, or creates a new one if the munger
+//               has not yet been used to render this context.  Fills
+//               index with the display list index, and returns true
+//               if the display list is current (that is, it has the
+//               same modified stamp).
+////////////////////////////////////////////////////////////////////
+bool CLP(GeomContext)::
+get_display_list(GLuint &index, const CLP(GeomMunger) *munger,
+                 UpdateSeq modified) {
+  DisplayList &dl = _display_lists[(CLP(GeomMunger) *)munger];
+  bool list_current = (dl._modified == modified);
+  if (dl._index == 0) {
+    dl._index = GLP(GenLists)(1);    
+    list_current = false;
+    ((CLP(GeomMunger) *)munger)->_geom_contexts.insert(this);
+  }
+
+  index = dl._index;
+  dl._modified = modified;
+  return list_current;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: CLP(GeomContext)::release_display_lists
+//       Access: Public
+//  Description: Called only from the draw thread by
+//               GLGraphicsStateGuardian::release_geom(), this should
+//               delete all of the queued display lists immediately.
+////////////////////////////////////////////////////////////////////
+void CLP(GeomContext)::
+release_display_lists() {
+  if (_deprecated_index != 0) {
+    if (GLCAT.is_debug()) {
+      GLCAT.debug()
+        << "releasing index " << _deprecated_index << "\n";
+    }
+    GLP(DeleteLists)(_deprecated_index, 1);
+    _deprecated_index = 0;
+  }
+
+  DisplayLists::iterator dli;
+  for (dli = _display_lists.begin();
+       dli != _display_lists.end();
+       ++dli) {
+    CLP(GeomMunger) *munger = (*dli).first;
+    const DisplayList &dl = (*dli).second;
+    munger->_geom_contexts.erase(this);
+
+    if (GLCAT.is_debug()) {
+      GLCAT.debug()
+        << "releasing index " << dl._index << "\n";
+    }
+    GLP(DeleteLists)(dl._index, 1);
+  }
+
+  _display_lists.clear();
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: CLP(GeomContext)::remove_munger
+//       Access: Public
+//  Description: Called when a glGeomMunger that we are pointing to
+//               destructs, this should remove the record of that
+//               munger (and mark the display list for deletion).
+////////////////////////////////////////////////////////////////////
+void CLP(GeomContext)::
+remove_munger(CLP(GeomMunger) *munger) {
+  DisplayLists::iterator dli = _display_lists.find(munger);
+  nassertv(dli != _display_lists.end());
+
+  GLuint index = (*dli).second._index;
+  _display_lists.erase(dli);
+
+  CLP(GraphicsStateGuardian) *glgsg;
+  DCAST_INTO_V(glgsg, munger->get_gsg());
+
+  // We can't delete the display list immediately, because we might be
+  // running in any thread.  Instead, enqueue the display list index
+  // and let it get deleted at the end of the current or next frame.
+  glgsg->record_deleted_display_list(index);
+}

panda/src/glstuff/glGeomContext_src.h

@@ -18,6 +18,12 @@
 
 #include "pandabase.h"
 #include "geomContext.h"
+#include "qpgeomMunger.h"
+#include "qpgeomVertexData.h"
+#include "pointerTo.h"
+#include "pmap.h"
+
+class CLP(GeomMunger);
 
 ////////////////////////////////////////////////////////////////////
 //       Class : GLGeomContext
@@ -26,11 +32,28 @@
 class EXPCL_GL CLP(GeomContext) : public GeomContext {
 public:
   INLINE CLP(GeomContext)(Geom *geom);
+  virtual ~CLP(GeomContext)();
+
+  bool get_display_list(GLuint &index, const CLP(GeomMunger) *munger, 
+                        UpdateSeq modified);
+  void release_display_lists();
+
+  void remove_munger(CLP(GeomMunger) *munger);
 
-  // This is the GL display list index.
-  GLuint _index;
+  // This is used only for the old Geom interface.
+  GLuint _deprecated_index;
 
-  UpdateSeq _modified;
+  // The different variants of the display list, for storing the
+  // different states the geom might have been rendered in (each using
+  // a different munger).
+  class DisplayList {
+  public:
+    INLINE DisplayList();
+    GLuint _index;
+    UpdateSeq _modified;
+  };
+  typedef pmap<CLP(GeomMunger) *, DisplayList> DisplayLists;
+  DisplayLists _display_lists;
 
   // The number of vertices encoded in the display list, for stats
   // reporting.

panda/src/glstuff/glGeomMunger_src.I

@@ -25,7 +25,19 @@
 INLINE CLP(GeomMunger)::
 CLP(GeomMunger)(GraphicsStateGuardian *gsg, const RenderState *state) :
   ColorMunger(gsg, state, 4, qpGeomVertexDataType::NT_uint8),
+  _gsg(gsg),
   _texture(state->get_texture()),
   _tex_gen(state->get_tex_gen())
 {
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: CLP(GeomMunger)::get_gsg
+//       Access: Public
+//  Description: Returns a pointer to the GSG that created this
+//               munger.
+////////////////////////////////////////////////////////////////////
+INLINE GraphicsStateGuardian *CLP(GeomMunger)::
+get_gsg() const {
+  return _gsg;
+}

panda/src/glstuff/glGeomMunger_src.cxx

@@ -20,6 +20,22 @@
 
 TypeHandle CLP(GeomMunger)::_type_handle;
 
+////////////////////////////////////////////////////////////////////
+//     Function: CLP(GeomMunger)::Destructor
+//       Access: Public, Virtual
+//  Description: 
+////////////////////////////////////////////////////////////////////
+CLP(GeomMunger)::
+~CLP(GeomMunger)() {
+  // We need to remove this pointer from all of the geom contexts that
+  // reference this object.
+  GeomContexts::iterator gci;
+  for (gci = _geom_contexts.begin(); gci != _geom_contexts.end(); ++gci) {
+    (*gci)->remove_munger(this);
+  }
+  _geom_contexts.clear();
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: CLP(GeomMunger)::munge_format_impl
 //       Access: Protected, Virtual

panda/src/glstuff/glGeomMunger_src.h

@@ -23,6 +23,8 @@
 #include "texGenAttrib.h"
 #include "renderState.h"
 
+class CLP(GeomContext);
+
 ////////////////////////////////////////////////////////////////////
 //       Class : GLGeomMunger
 // Description : This specialization on GeomMunger finesses vertices
@@ -32,6 +34,9 @@
 class EXPCL_GL CLP(GeomMunger) : public ColorMunger {
 public:
   INLINE CLP(GeomMunger)(GraphicsStateGuardian *gsg, const RenderState *state);
+  virtual ~CLP(GeomMunger)();
+
+  INLINE GraphicsStateGuardian *get_gsg() const;
 
 protected:
   virtual CPT(qpGeomVertexFormat) munge_format_impl(const qpGeomVertexFormat *orig);
@@ -39,9 +44,13 @@ protected:
   virtual int geom_compare_to_impl(const qpGeomMunger *other) const;
 
 private:
+  PT(GraphicsStateGuardian) _gsg;
   CPT(TextureAttrib) _texture;
   CPT(TexGenAttrib) _tex_gen;
 
+  typedef pset<CLP(GeomContext) *> GeomContexts;
+  GeomContexts _geom_contexts;
+
 public:
   static TypeHandle get_class_type() {
     return _type_handle;
@@ -58,6 +67,8 @@ public:
 
 private:
   static TypeHandle _type_handle;
+
+  friend class CLP(GeomContext);
 };
 
 #include "glGeomMunger_src.I"

panda/src/glstuff/glGraphicsStateGuardian_src.I

@@ -31,7 +31,7 @@ draw_display_list(GeomContext *gc) {
   if (gc != (GeomContext *)NULL && _vertex_colors_enabled) {
     _draw_primitive_pcollector.start();
     CLP(GeomContext) *ggc = DCAST(CLP(GeomContext), gc);
-    GLP(CallList)(ggc->_index);
+    GLP(CallList)(ggc->_deprecated_index);
 #ifdef DO_PSTATS
     _vertices_display_list_pcollector.add_level(ggc->_num_verts);
 #endif

panda/src/glstuff/glGraphicsStateGuardian_src.cxx

@@ -58,6 +58,7 @@
 #include "string_utils.h"
 #include "pnmImage.h"
 #include "config_gobj.h"
+#include "mutexHolder.h"
 #ifdef DO_PSTATS
 #include "pStatTimer.h"
 #endif
@@ -953,6 +954,24 @@ void CLP(GraphicsStateGuardian)::
 end_frame() {
   GraphicsStateGuardian::end_frame();
 
+  // Now is a good time to delete any pending display lists.
+  {
+    MutexHolder holder(_lock);
+    if (!_deleted_display_lists.empty()) {
+      DeletedDisplayLists::iterator ddli;
+      for (ddli = _deleted_display_lists.begin();
+           ddli != _deleted_display_lists.end();
+           ++ddli) {
+        if (GLCAT.is_debug()) {
+          GLCAT.debug()
+            << "releasing index " << (*ddli) << "\n";
+        }
+        GLP(DeleteLists)((*ddli), 1);
+      }
+      _deleted_display_lists.clear();
+    }
+  }
+
   {
 #ifdef DO_PSTATS
     PStatTimer timer(_flush_pcollector);
@@ -2044,7 +2063,7 @@ begin_draw_primitives(const qpGeom *geom, const qpGeomMunger *munger,
   }
   nassertr(_vertex_data != (qpGeomVertexData *)NULL, false);
 
-  _geom_display_list = NULL;
+  _geom_display_list = 0;
 
   if (geom->get_usage_hint() == qpGeomUsageHint::UH_static && 
       _vertex_data->get_usage_hint() == qpGeomUsageHint::UH_static &&
@@ -2054,15 +2073,16 @@ begin_draw_primitives(const qpGeom *geom, const qpGeomMunger *munger,
     GeomContext *gc = ((qpGeom *)geom)->prepare_now(get_prepared_objects(), this);
     nassertr(gc != (GeomContext *)NULL, false);
     CLP(GeomContext) *ggc = DCAST(CLP(GeomContext), gc);
+    const CLP(GeomMunger) *gmunger = DCAST(CLP(GeomMunger), _munger);
     UpdateSeq modified = max(geom->get_modified(), _vertex_data->get_modified());
-    if (ggc->_modified == modified) {
+    if (ggc->get_display_list(_geom_display_list, gmunger, modified)) {
       // If it hasn't been modified, just play the display list again.
       if (GLCAT.is_spam()) {
         GLCAT.spam()
-          << "calling display list " << ggc->_index << "\n";
+          << "calling display list " << _geom_display_list << "\n";
       }
 
-      GLP(CallList)(ggc->_index);
+      GLP(CallList)(_geom_display_list);
 #ifdef DO_PSTATS
       _vertices_display_list_pcollector.add_level(ggc->_num_verts);
 #endif
@@ -2073,21 +2093,16 @@ begin_draw_primitives(const qpGeom *geom, const qpGeomMunger *munger,
     
     if (GLCAT.is_debug()) {
       GLCAT.debug()
-        << "compiling display list " << ggc->_index << "\n";
-      cerr << "ggc = " << ggc << "," << ggc->_modified
-           << " geom = " << geom << "," << geom->get_modified()
-           << " vertex_data = " << _vertex_data << "," 
-           << _vertex_data->get_modified() << "\n";
+        << "compiling display list " << _geom_display_list << "\n";
     }
 
     // If it has been modified, or this is the first time, then we
     // need to build the display list up.
     if (CLP(compile_and_execute)) {
-      GLP(NewList)(ggc->_index, GL_COMPILE_AND_EXECUTE);
+      GLP(NewList)(_geom_display_list, GL_COMPILE_AND_EXECUTE);
     } else {
-      GLP(NewList)(ggc->_index, GL_COMPILE);
+      GLP(NewList)(_geom_display_list, GL_COMPILE);
     }      
-    ggc->_modified = modified;
 
 #ifdef DO_PSTATS
     // Count up the number of vertices used by primitives in the Geom,
@@ -2097,8 +2112,6 @@ begin_draw_primitives(const qpGeom *geom, const qpGeomMunger *munger,
       ggc->_num_verts += geom->get_primitive(i)->get_num_vertices();
     }
 #endif
-
-    _geom_display_list = ggc;
   }
 
   const qpGeomVertexArrayData *array_data;
@@ -2163,18 +2176,15 @@ begin_draw_primitives(const qpGeom *geom, const qpGeomMunger *munger,
         GLP(TexCoordPointer)(num_components, get_numeric_type(numeric_type), 
                              stride, client_pointer + start);
         GLP(EnableClientState)(GL_TEXTURE_COORD_ARRAY);
-        cerr << "sending " << *name << "\n";
 
       } else {
         // The vertex data doesn't have texcoords for this stage (even
         // though they're needed).
         GLP(DisableClientState)(GL_TEXTURE_COORD_ARRAY);
-        cerr << "not defined " << *name << "\n";
       }
     } else {
       // No texcoords are needed for this stage.
       GLP(DisableClientState)(GL_TEXTURE_COORD_ARRAY);
-      cerr << "not sending " << *stage->get_texcoord_name() << "\n";
     }
 
     ++stage_index;
@@ -2249,14 +2259,14 @@ draw_tristrips(const qpGeomTristrips *primitive) {
 ////////////////////////////////////////////////////////////////////
 void CLP(GraphicsStateGuardian)::
 end_draw_primitives() {
-  if (_geom_display_list != NULL) {
+  if (_geom_display_list != 0) {
     // If we were building a display list, close it now.
     GLP(EndList)();
     if (!CLP(compile_and_execute)) {
-      GLP(CallList)(_geom_display_list->_index);
+      GLP(CallList)(_geom_display_list);
     }      
   }
-  _geom_display_list = NULL;
+  _geom_display_list = 0;
 
   GraphicsStateGuardian::end_draw_primitives();
 }
@@ -2354,19 +2364,6 @@ prepare_geom(Geom *geom) {
   // actual Geom implementation.
   if (geom->is_exact_type(qpGeom::get_class_type())) {
     CLP(GeomContext) *ggc = new CLP(GeomContext)(geom);
-    ggc->_index = GLP(GenLists)(1);
-    if (GLCAT.is_debug()) {
-      GLCAT.debug()
-        << "preparing " << *geom << ", index " << ggc->_index << "\n";
-    }
-    if (ggc->_index == 0) {
-      GLCAT.error()
-        << "Ran out of display list indices.\n";
-      delete ggc;
-      return NULL;
-    }
-
-    report_my_gl_errors();
     return ggc;
 
   } else {
@@ -2389,12 +2386,12 @@ prepare_geom(Geom *geom) {
     }
 
     CLP(GeomContext) *ggc = new CLP(GeomContext)(geom);
-    ggc->_index = GLP(GenLists)(1);
+    ggc->_deprecated_index = GLP(GenLists)(1);
     if (GLCAT.is_debug()) {
       GLCAT.debug()
-        << "preparing " << *geom << ", index " << ggc->_index << "\n";
+        << "preparing " << *geom << ", index " << ggc->_deprecated_index << "\n";
     }
-    if (ggc->_index == 0) {
+    if (ggc->_deprecated_index == 0) {
       GLCAT.error()
         << "Ran out of display list indices.\n";
       delete ggc;
@@ -2418,7 +2415,7 @@ prepare_geom(Geom *geom) {
 #endif
 
     // Now define the display list.
-    GLP(NewList)(ggc->_index, GL_COMPILE);
+    GLP(NewList)(ggc->_deprecated_index, GL_COMPILE);
     geom->draw_immediate(this, NULL);
     GLP(EndList)();
 
@@ -2451,18 +2448,26 @@ prepare_geom(Geom *geom) {
 void CLP(GraphicsStateGuardian)::
 release_geom(GeomContext *gc) {
   CLP(GeomContext) *ggc = DCAST(CLP(GeomContext), gc);
-  if (GLCAT.is_debug()) {
-    GLCAT.debug()
-      << "releasing index " << ggc->_index << "\n";
-  }
-
-  GLP(DeleteLists)(ggc->_index, 1);
+  ggc->release_display_lists();
   report_my_gl_errors();
 
-  ggc->_index = 0;
   delete ggc;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: CLP(GraphicsStateGuardian)::record_deleted_display_list
+//       Access: Public
+//  Description: This is intended to be called only from the
+//               GLGeomContext destructor.  It saves the indicated
+//               display list index in the list to be deleted at the
+//               end of the frame.
+////////////////////////////////////////////////////////////////////
+void CLP(GraphicsStateGuardian)::
+record_deleted_display_list(GLuint index) {
+  MutexHolder holder(_lock);
+  _deleted_display_lists.push_back(index);
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: CLP(GraphicsStateGuardian)::prepare_vertex_buffer
 //       Access: Public, Virtual
@@ -2577,7 +2582,7 @@ setup_array_data(const qpGeomVertexArrayData *data) {
     // No support for buffer objects; always render from client.
     return data->get_data();
   }
-  if (!vertex_buffers || _geom_display_list != NULL ||
+  if (!vertex_buffers || _geom_display_list != 0 ||
       data->get_usage_hint() == qpGeomUsageHint::UH_client) {
     // The array specifies client rendering only, or buffer objects
     // are configured off.
@@ -2708,7 +2713,7 @@ setup_primitive(const qpGeomPrimitive *data) {
     // No support for buffer objects; always render from client.
     return data->get_flat_last_vertices();
   }
-  if (!vertex_buffers || _geom_display_list != NULL ||
+  if (!vertex_buffers || _geom_display_list != 0 ||
       data->get_usage_hint() == qpGeomUsageHint::UH_client) {
     // The array specifies client rendering only, or buffer objects
     // are configured off.

panda/src/glstuff/glGraphicsStateGuardian_src.h

@@ -38,6 +38,7 @@
 #include "pset.h"
 #include "pmap.h"
 #include "qpgeomVertexArrayData.h"
+#include "pmutex.h"
 #ifdef HAVE_CGGL
 #include "cgShader.h"
 #endif
@@ -107,6 +108,7 @@ public:
 
   virtual GeomContext *prepare_geom(Geom *geom);
   virtual void release_geom(GeomContext *gc);
+  void record_deleted_display_list(GLuint index);
 
   virtual VertexBufferContext *prepare_vertex_buffer(qpGeomVertexArrayData *data);
   void apply_vertex_buffer(VertexBufferContext *vbc);
@@ -319,7 +321,7 @@ protected:
 
   int _pass_number;
   bool _auto_rescale_normal;
-  CLP(GeomContext) *_geom_display_list;
+  GLuint _geom_display_list;
   int _last_max_stage_index;
   
   int _error_count;
@@ -362,6 +364,10 @@ public:
   GLenum _mirror_edge_clamp;
   GLenum _mirror_border_clamp;
 
+  Mutex _lock;
+  typedef pvector<GLuint> DeletedDisplayLists;
+  DeletedDisplayLists _deleted_display_lists;
+
 public:
   static GraphicsStateGuardian *
   make_GlGraphicsStateGuardian(const FactoryParams &params);

panda/src/gobj/qpgeom.I

@@ -155,7 +155,10 @@ CacheEntry(const qpGeomMunger *modifier) :
 ////////////////////////////////////////////////////////////////////
 INLINE bool qpGeom::CacheEntry::
 operator < (const CacheEntry &other) const {
-  return (_modifier < other._modifier);
+  if (_modifier != other._modifier) {
+    return (_modifier->geom_compare_to(*other._modifier) < 0);
+  }
+  return 0;
 }
 
 

panda/src/gobj/qpgeom.cxx

@@ -234,7 +234,7 @@ munge_geom(const qpGeomMunger *munger,
     // object, and require a write.
     const CData *cdata = _cycler.read();
     CacheEntry temp_entry(munger);
-    temp_entry.ref();  // big ugly hack to allow a static ReferenceCount object.
+    temp_entry.ref();  // big ugly hack to allow a stack-allocated ReferenceCount object.
     Cache::const_iterator ci = cdata->_cache.find(&temp_entry);
     if (ci != cdata->_cache.end()) {
       _cycler.release_read(cdata);
@@ -302,7 +302,7 @@ output(ostream &out) const {
   for (ti = types.begin(); ti != types.end(); ++ti) {
     out << " " << (*ti);
   }
-  out << " ], " << cdata->_data->get_num_vertices() << " vertices.";
+  out << " ], " << cdata->_data->get_num_vertices() << " vertices";
 }
 
 ////////////////////////////////////////////////////////////////////