fix cache in threaded pipeline

panda/src/gobj/geom.I

@@ -137,7 +137,7 @@ modify_primitive(int i) {
   nassertr(i >= 0 && i < (int)cdata->_primitives.size(), NULL);
   cdata->_got_usage_hint = false;
   cdata->_modified = Geom::get_next_modified();
-  do_clear_cache(cdata);
+  clear_cache_stage();
   if (cdata->_primitives[i]->get_ref_count() > 1) {
     cdata->_primitives[i] = cdata->_primitives[i]->make_copy();
   }
@@ -320,17 +320,73 @@ mark_internal_bounds_stale(CData *cdata) {
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: Geom::CacheEntry::Constructor
+//     Function: Geom::CDataCache::Constructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE Geom::CDataCache::
+CDataCache() :
+  _source(NULL),
+  _geom_result(NULL),
+  _data_result(NULL)
+{
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: Geom::CDataCache::Copy Constructor
 //       Access: Public
 //  Description: 
 ////////////////////////////////////////////////////////////////////
+INLINE Geom::CDataCache::
+CDataCache(const Geom::CDataCache &copy) :
+  _source(copy._source),
+  _geom_result(copy._geom_result),
+  _data_result(copy._data_result)
+{
+  if (_geom_result != _source && _geom_result != (Geom *)NULL) {
+    _geom_result->ref();
+  }
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: Geom::CDataCache::set_result
+//       Access: Public
+//  Description: Stores the geom_result and data_result on the cache,
+//               upping and/or dropping the reference count
+//               appropriately.
+////////////////////////////////////////////////////////////////////
+INLINE void Geom::CDataCache::
+set_result(const Geom *geom_result, const GeomVertexData *data_result) {
+  if (geom_result != _geom_result) {
+    if (_geom_result != _source && _geom_result != (Geom *)NULL) {
+      unref_delete(_geom_result);
+    }
+    _geom_result = geom_result;
+    if (_geom_result != _source && _geom_result != (Geom *)NULL) {
+      _geom_result->ref();
+    }
+  }
+  _data_result = data_result;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: Geom::CacheEntry::Constructor
+//       Access: Public
+//  Description: This constructor makes an invalid CacheEntry that
+//               holds no data.  CacheEntries like this are normally
+//               not stored in the cache; this is usually used as a
+//               key to find an existing (valid) CacheEntry already in
+//               the cache.
+//
+//               However, it is possible for an empty CacheEntry to
+//               end up in the cache if its data gets cleared by
+//               clear_cache_stage().
+////////////////////////////////////////////////////////////////////
 INLINE Geom::CacheEntry::
 CacheEntry(const GeomVertexData *source_data, const GeomMunger *modifier) :
   _source(NULL),
   _source_data(source_data),
-  _modifier(modifier),
-  _geom_result(NULL),
-  _data_result(NULL)
+  _modifier(modifier)
 {
 }
 
@@ -341,17 +397,11 @@ CacheEntry(const GeomVertexData *source_data, const GeomMunger *modifier) :
 ////////////////////////////////////////////////////////////////////
 INLINE Geom::CacheEntry::
 CacheEntry(Geom *source, const GeomVertexData *source_data,
-           const GeomMunger *modifier, const Geom *geom_result, 
-           const GeomVertexData *data_result) :
+           const GeomMunger *modifier) :
   _source(source),
   _source_data(source_data),
-  _modifier(modifier),
-  _geom_result(geom_result),
-  _data_result(data_result)
+  _modifier(modifier)
 {
-  if (_geom_result != _source) {
-    _geom_result->ref();
-  }
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geom.cxx

@@ -64,11 +64,7 @@ void Geom::
 operator = (const Geom &copy) {
   TypedWritableReferenceCount::operator = (copy);
 
-  OPEN_ITERATE_ALL_STAGES(_cycler) {
-    CDStageWriter cdata(_cycler, pipeline_stage);
-    do_clear_cache(cdata);
-  } 
-  CLOSE_ITERATE_ALL_STAGES(_cycler);
+  clear_cache();
 
   _cycler = copy._cycler;
 
@@ -86,15 +82,7 @@ operator = (const Geom &copy) {
 ////////////////////////////////////////////////////////////////////
 Geom::
 ~Geom() {
-  // When we destruct, we should ensure that all of our cached
-  // entries, across all pipeline stages, are properly removed from
-  // the cache manager.
-  OPEN_ITERATE_ALL_STAGES(_cycler) {
-    CDStageWriter cdata(_cycler, pipeline_stage);
-    do_clear_cache(cdata);
-  }
-  CLOSE_ITERATE_ALL_STAGES(_cycler);
-
+  clear_cache();
   release_all();
 }
 
@@ -135,7 +123,7 @@ set_usage_hint(Geom::UsageHint usage_hint) {
     (*pi)->set_usage_hint(usage_hint);
   }
 
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
 }
 
@@ -159,7 +147,7 @@ modify_vertex_data() {
   if (cdata->_data->get_ref_count() > 1) {
     cdata->_data = new GeomVertexData(*cdata->_data);
   }
-  do_clear_cache(cdata);
+  clear_cache_stage();
   mark_internal_bounds_stale(cdata);
   return cdata->_data;
 }
@@ -179,7 +167,7 @@ set_vertex_data(const GeomVertexData *data) {
   nassertv(check_will_be_valid(data));
   CDWriter cdata(_cycler, true);
   cdata->_data = (GeomVertexData *)data;
-  do_clear_cache(cdata);
+  clear_cache_stage();
   mark_internal_bounds_stale(cdata);
   reset_geom_rendering(cdata);
 }
@@ -222,7 +210,7 @@ offset_vertices(const GeomVertexData *data, int offset) {
   }
 
   cdata->_modified = Geom::get_next_modified();
-  do_clear_cache(cdata);
+  clear_cache_stage();
   nassertv(all_is_valid);
 }
 
@@ -290,7 +278,7 @@ make_nonindexed(bool composite_only) {
     cdata->_data = new_data;
     cdata->_primitives.swap(new_prims);
     cdata->_modified = Geom::get_next_modified();
-    do_clear_cache(cdata);
+    clear_cache_stage();
   }
 
   return num_changed;
@@ -335,7 +323,7 @@ set_primitive(int i, const GeomPrimitive *primitive) {
   reset_geom_rendering(cdata);
   cdata->_got_usage_hint = false;
   cdata->_modified = Geom::get_next_modified();
-  do_clear_cache(cdata);
+  clear_cache_stage();
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -379,7 +367,7 @@ add_primitive(const GeomPrimitive *primitive) {
   reset_geom_rendering(cdata);
   cdata->_got_usage_hint = false;
   cdata->_modified = Geom::get_next_modified();
-  do_clear_cache(cdata);
+  clear_cache_stage();
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -403,7 +391,7 @@ remove_primitive(int i) {
   reset_geom_rendering(cdata);
   cdata->_got_usage_hint = false;
   cdata->_modified = Geom::get_next_modified();
-  do_clear_cache(cdata);
+  clear_cache_stage();
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -425,7 +413,7 @@ clear_primitives() {
   cdata->_primitive_type = PT_none;
   cdata->_shade_model = SM_uniform;
   reset_geom_rendering(cdata);
-  do_clear_cache(cdata);
+  clear_cache_stage();
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -460,7 +448,7 @@ decompose_in_place() {
 
   cdata->_modified = Geom::get_next_modified();
   reset_geom_rendering(cdata);
-  do_clear_cache(cdata);
+  clear_cache_stage();
 
   nassertv(all_is_valid);
 }
@@ -496,7 +484,7 @@ rotate_in_place() {
   }
 
   cdata->_modified = Geom::get_next_modified();
-  do_clear_cache(cdata);
+  clear_cache_stage();
 
   nassertv(all_is_valid);
 }
@@ -568,7 +556,7 @@ unify_in_place() {
   cdata->_primitives.push_back(new_prim);
 
   cdata->_modified = Geom::get_next_modified();
-  do_clear_cache(cdata);
+  clear_cache_stage();
   reset_geom_rendering(cdata);
 }
 
@@ -802,14 +790,42 @@ write(ostream &out, int indent_level) const {
 //  Description: Removes all of the previously-cached results of
 //               munge_geom().
 //
+//               This blows away the entire cache, upstream and
+//               downstream the pipeline.  Use clear_cache_stage()
+//               instead if you only want to blow away the cache at
+//               the current stage and upstream.
+////////////////////////////////////////////////////////////////////
+void Geom::
+clear_cache() {
+  for (Cache::iterator ci = _cache.begin();
+       ci != _cache.end();
+       ++ci) {
+    CacheEntry *entry = (*ci);
+    entry->erase();
+  }
+  _cache.clear();
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: Geom::clear_cache_stage
+//       Access: Published
+//  Description: Removes all of the previously-cached results of
+//               munge_geom(), at the current pipeline stage and
+//               upstream.  Does not affect the downstream cache.
+//
 //               Don't call this in a downstream thread unless you
 //               don't mind it blowing away other changes you might
 //               have recently made in an upstream thread.
 ////////////////////////////////////////////////////////////////////
 void Geom::
-clear_cache() {
-  CDWriter cdata(_cycler, true);
-  do_clear_cache(cdata);
+clear_cache_stage() {
+  for (Cache::iterator ci = _cache.begin();
+       ci != _cache.end();
+       ++ci) {
+    CacheEntry *entry = (*ci);
+    CDCacheWriter cdata(entry->_cycler);
+    cdata->set_result(NULL, NULL);
+  }
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -1060,22 +1076,6 @@ check_will_be_valid(const GeomVertexData *vertex_data) const {
 
   return true;
 }
-
-////////////////////////////////////////////////////////////////////
-//     Function: Geom::do_clear_cache
-//       Access: Private
-//  Description: The private implementation of clear_cache().
-////////////////////////////////////////////////////////////////////
-void Geom::
-do_clear_cache(Geom::CData *cdata) {
-  for (Cache::iterator ci = cdata->_cache.begin();
-       ci != cdata->_cache.end();
-       ++ci) {
-    CacheEntry *entry = (*ci);
-    entry->erase();
-  }
-  cdata->_cache.clear();
-}
   
 ////////////////////////////////////////////////////////////////////
 //     Function: Geom::do_draw
@@ -1238,15 +1238,23 @@ fillin(DatagramIterator &scan, BamReader *manager) {
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: Geom::CacheEntry::Destructor
+//     Function: Geom::CDataCache::Destructor
 //       Access: Public, Virtual
 //  Description: 
 ////////////////////////////////////////////////////////////////////
-Geom::CacheEntry::
-~CacheEntry() {
-  if (_geom_result != _source) {
-    unref_delete(_geom_result);
-  }
+Geom::CDataCache::
+~CDataCache() {
+  set_result(NULL, NULL);
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: Geom::CDataCache::make_copy
+//       Access: Public, Virtual
+//  Description:
+////////////////////////////////////////////////////////////////////
+CycleData *Geom::CDataCache::
+make_copy() const {
+  return new CDataCache(*this);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -1257,13 +1265,10 @@ Geom::CacheEntry::
 ////////////////////////////////////////////////////////////////////
 void Geom::CacheEntry::
 evict_callback() {
-  OPEN_ITERATE_ALL_STAGES(_source->_cycler) {
-    CDStageWriter cdata(_source->_cycler, pipeline_stage);
-    // Because of the multistage pipeline, we might not actually have
-    // a cache entry at every stage.  No big deal if we don't.
-    cdata->_cache.erase(this);
-  }
-  CLOSE_ITERATE_ALL_STAGES(_source->_cycler);
+  Cache::iterator ci = _source->_cache.find(this);
+  nassertv(ci != _source->_cache.end());
+  nassertv((*ci) == this);
+  _source->_cache.erase(ci);
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geom.h

@@ -111,6 +111,7 @@ PUBLISHED:
   virtual void write(ostream &out, int indent_level = 0) const;
 
   void clear_cache();
+  void clear_cache_stage();
 
   void prepare(PreparedGraphicsObjects *prepared_objects);
   bool release(PreparedGraphicsObjects *prepared_objects);
@@ -152,7 +153,6 @@ private:
   void clear_prepared(PreparedGraphicsObjects *prepared_objects);
   bool check_will_be_valid(const GeomVertexData *vertex_data) const;
 
-  void do_clear_cache(CData *cdata);
   void do_draw(GraphicsStateGuardianBase *gsg, 
 	       const GeomMunger *munger,
 	       const GeomVertexData *vertex_data,
@@ -169,26 +169,49 @@ private:
   // cache needs to be stored in the CycleData, which makes accurate
   // cleanup more difficult.  We use the GeomCacheManager class to
   // avoid cache bloat.
+
+  // Note: the above comment is no longer true.  The cache is not
+  // stored in the CycleData, which just causes problems; instead, we
+  // cycle each individual CacheEntry as needed.  Need to investigate
+  // if we could simplify the cache system now.
+
+  // The pipelined data with each CacheEntry.
+  class CDataCache : public CycleData {
+  public:
+    INLINE CDataCache();
+    INLINE CDataCache(const CDataCache &copy);
+    virtual ~CDataCache();
+    virtual CycleData *make_copy() const;
+    virtual TypeHandle get_parent_type() const {
+      return Geom::get_class_type();
+    }
+
+    INLINE void set_result(const Geom *geom_result, const GeomVertexData *data_result);
+
+    Geom *_source;  // A back pointer to the containing Geom
+    const Geom *_geom_result;  // ref-counted if not NULL and not same as _source
+    CPT(GeomVertexData) _data_result;
+  };
+  typedef CycleDataReader<CDataCache> CDCacheReader;
+  typedef CycleDataWriter<CDataCache> CDCacheWriter;
+  
   class CacheEntry : public GeomCacheEntry {
   public:
     INLINE CacheEntry(const GeomVertexData *source_data,
                       const GeomMunger *modifier);
     INLINE CacheEntry(Geom *source, 
                       const GeomVertexData *source_data,
-                      const GeomMunger *modifier, 
-                      const Geom *geom_result, 
-                      const GeomVertexData *data_result);
-    virtual ~CacheEntry();
+                      const GeomMunger *modifier);
     INLINE bool operator < (const CacheEntry &other) const;
 
     virtual void evict_callback();
     virtual void output(ostream &out) const;
 
-    Geom *_source;
+    Geom *_source;  // A back pointer to the containing Geom
     CPT(GeomVertexData) _source_data;
     CPT(GeomMunger) _modifier;
-    const Geom *_geom_result;  // ref-counted if not same as _source
-    CPT(GeomVertexData) _data_result;
+
+    PipelineCycler<CDataCache> _cycler;
   };
   typedef pset<PT(CacheEntry), IndirectLess<CacheEntry> > Cache;
 
@@ -213,19 +236,20 @@ private:
     UsageHint _usage_hint;
     bool _got_usage_hint;
     UpdateSeq _modified;
-    Cache _cache;
   
     CPT(BoundingVolume) _internal_bounds;
     bool _internal_bounds_stale;
     CPT(BoundingVolume) _user_bounds;
   };
-
+ 
   PipelineCycler<CData> _cycler;
   typedef CycleDataReader<CData> CDReader;
   typedef CycleDataWriter<CData> CDWriter;
   typedef CycleDataStageReader<CData> CDStageReader;
   typedef CycleDataStageWriter<CData> CDStageWriter;
 
+  Cache _cache;
+
   // This works just like the Texture contexts: each Geom keeps a
   // record of all the PGO objects that hold the Geom, and vice-versa.
   typedef pmap<PreparedGraphicsObjects *, GeomContext *> Contexts;

panda/src/gobj/geomMunger.cxx

@@ -109,35 +109,37 @@ munge_geom(CPT(Geom) &geom, CPT(GeomVertexData) &data) {
 
   // Look up the munger in the geom's cache--maybe we've recently
   // applied it.
-  {
-    Geom::CDReader cdata(geom->_cycler);
-    Geom::CacheEntry temp_entry(source_data, this);
-    temp_entry.local_object();
-    Geom::Cache::const_iterator ci = cdata->_cache.find(&temp_entry);
-    if (ci != cdata->_cache.end()) {
-      Geom::CacheEntry *entry = (*ci);
-
-      if (geom->get_modified() <= entry->_geom_result->get_modified() &&
-          data->get_modified() <= entry->_data_result->get_modified()) {
-        // The cache entry is still good; use it.
-
-        // Record a cache hit, so this element will stay in the cache a
-        // while longer.
-        entry->refresh();
-        geom = entry->_geom_result;
-        data = entry->_data_result;
-        return;
-      }
-
-      // The cache entry is stale, remove it.
-      if (gobj_cat.is_debug()) {
-        gobj_cat.debug()
-          << "Cache entry " << *entry << " is stale, removing.\n";
-      }
-      entry->erase();
-      Geom::CDWriter cdataw(((Geom *)geom.p())->_cycler, cdata);
-      cdataw->_cache.erase(entry);
+  PT(Geom::CacheEntry) entry;
+
+  Geom::CacheEntry temp_entry(source_data, this);
+  temp_entry.local_object();
+  Geom::Cache::const_iterator ci = geom->_cache.find(&temp_entry);
+  if (ci != geom->_cache.end()) {
+    entry = (*ci);
+    nassertv(entry->_source == geom);
+
+    // Here's an element in the cache for this computation.  Record a
+    // cache hit, so this element will stay in the cache a while
+    // longer.
+    entry->refresh();
+
+    // Now check that it's fresh.
+    Geom::CDCacheReader cdata(entry->_cycler);
+    nassertv(cdata->_source == geom);
+    if (cdata->_geom_result != (Geom *)NULL &&
+	geom->get_modified() <= cdata->_geom_result->get_modified() &&
+	data->get_modified() <= cdata->_data_result->get_modified()) {
+      // The cache entry is still good; use it.
+      
+      geom = cdata->_geom_result;
+      data = cdata->_data_result;
+      return;
     }
+    
+    // The cache entry is stale, but we'll recompute it below.  Note
+    // that there's a small race condition here; another thread might
+    // recompute the cache at the same time.  No big deal, since it'll
+    // compute the same result.
   }
 
   // Ok, invoke the munger.
@@ -147,22 +149,23 @@ munge_geom(CPT(Geom) &geom, CPT(GeomVertexData) &data) {
   data = munge_data(data);
   munge_geom_impl(geom, data);
 
-  {
-    // Record the new result in the cache.
-    Geom::CacheEntry *entry;
-    {
-      Geom::CDWriter cdata(((Geom *)orig_geom.p())->_cycler);
-      entry = new Geom::CacheEntry((Geom *)orig_geom.p(), source_data, this,
-                                     geom, data);
-      bool inserted = cdata->_cache.insert(entry).second;
-      nassertv(inserted);
-    }
-    
+  // Record the new result in the cache.
+  if (entry == (Geom::CacheEntry *)NULL) {
+    // Create a new entry for the result.
+    entry = new Geom::CacheEntry((Geom *)orig_geom.p(), source_data, this);
+    bool inserted = ((Geom *)orig_geom.p())->_cache.insert(entry).second;
+    nassertv(inserted);
+  
     // And tell the cache manager about the new entry.  (It might
     // immediately request a delete from the cache of the thing we
     // just added.)
     entry->record();
   }
+
+  // Finally, store the cached result on the entry.
+  Geom::CDCacheWriter cdata(entry->_cycler, true);
+  cdata->_source = (Geom *)orig_geom.p();
+  cdata->set_result(geom, data);
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geomVertexData.I

@@ -238,22 +238,6 @@ get_modified() const {
   return cdata->_modified;
 }
 
-////////////////////////////////////////////////////////////////////
-//     Function: GeomVertexData::clear_cache
-//       Access: Published
-//  Description: Removes all of the previously-cached results of
-//               convert_to().
-//
-//               Don't call this in a downstream thread unless you
-//               don't mind it blowing away other changes you might
-//               have recently made in an upstream thread.
-////////////////////////////////////////////////////////////////////
-INLINE void GeomVertexData::
-clear_cache() {
-  CDWriter cdata(_cycler, true);
-  do_clear_cache(cdata);
-}
-
 ////////////////////////////////////////////////////////////////////
 //     Function: GeomVertexData::has_vertex
 //       Access: Public
@@ -383,15 +367,42 @@ add_transform(TransformTable *table, const VertexTransform *transform,
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: GeomVertexData::CacheEntry::Constructor
+//     Function: GeomVertexData::CDataCache::Constructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE GeomVertexData::CDataCache::
+CDataCache() {
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexData::CDataCache::Copy Constructor
 //       Access: Public
 //  Description: 
 ////////////////////////////////////////////////////////////////////
+INLINE GeomVertexData::CDataCache::
+CDataCache(const GeomVertexData::CDataCache &copy) :
+  _result(copy._result)
+{
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexData::CacheEntry::Constructor
+//       Access: Public
+//  Description: This constructor makes an invalid CacheEntry that
+//               holds no data.  CacheEntries like this are normally
+//               not stored in the cache; this is usually used as a
+//               key to find an existing (valid) CacheEntry already in
+//               the cache.
+//
+//               However, it is possible for an empty CacheEntry to
+//               end up in the cache if its data gets cleared by
+//               clear_cache_stage().
+////////////////////////////////////////////////////////////////////
 INLINE GeomVertexData::CacheEntry::
 CacheEntry(const GeomVertexFormat *modifier) :
   _source(NULL),
-  _modifier(modifier),
-  _result(NULL)
+  _modifier(modifier)
 {
 }
 
@@ -402,11 +413,9 @@ CacheEntry(const GeomVertexFormat *modifier) :
 ////////////////////////////////////////////////////////////////////
 INLINE GeomVertexData::CacheEntry::
 CacheEntry(GeomVertexData *source,
-           const GeomVertexFormat *modifier,
-           const GeomVertexData *result) :
+           const GeomVertexFormat *modifier) :
   _source(source),
-  _modifier(modifier),
-  _result(result)
+  _modifier(modifier)
 {
 }
 

panda/src/gobj/geomVertexData.cxx

@@ -154,11 +154,7 @@ void GeomVertexData::
 operator = (const GeomVertexData &copy) {
   TypedWritableReferenceCount::operator = (copy);
 
-  OPEN_ITERATE_ALL_STAGES(_cycler) {
-    CDStageWriter cdata(_cycler, pipeline_stage);
-    do_clear_cache(cdata);
-  } 
-  CLOSE_ITERATE_ALL_STAGES(_cycler);
+  clear_cache();
 
   _name = copy._name;
   _format = copy._format;
@@ -183,14 +179,7 @@ operator = (const GeomVertexData &copy) {
 ////////////////////////////////////////////////////////////////////
 GeomVertexData::
 ~GeomVertexData() {
-  // When we destruct, we should ensure that all of our cached
-  // entries, across all pipeline stages, are properly removed from
-  // the cache manager.
-  OPEN_ITERATE_ALL_STAGES(_cycler) {
-    CDStageWriter cdata(_cycler, pipeline_stage);
-    do_clear_cache(cdata);
-  }
-  CLOSE_ITERATE_ALL_STAGES(_cycler);
+  clear_cache();
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -232,7 +221,7 @@ set_usage_hint(GeomVertexData::UsageHint usage_hint) {
     }
     (*ai)->set_usage_hint(usage_hint);
   }
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
   cdata->_animated_vertices_modified = UpdateSeq();
 }
@@ -283,7 +272,7 @@ clear_rows() {
     }
     (*ai)->clear_rows();
   }
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
   cdata->_animated_vertices.clear();
 }
@@ -313,7 +302,7 @@ modify_array(int i) {
   if (cdata->_arrays[i]->get_ref_count() > 1) {
     cdata->_arrays[i] = new GeomVertexArrayData(*cdata->_arrays[i]);
   }
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
   cdata->_animated_vertices_modified = UpdateSeq();
 
@@ -337,7 +326,7 @@ set_array(int i, const GeomVertexArrayData *array) {
   CDWriter cdata(_cycler, true);
   nassertv(i >= 0 && i < (int)cdata->_arrays.size());
   cdata->_arrays[i] = (GeomVertexArrayData *)array;
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
   cdata->_animated_vertices_modified = UpdateSeq();
 }
@@ -361,7 +350,7 @@ set_transform_table(const TransformTable *table) {
 
   CDWriter cdata(_cycler, true);
   cdata->_transform_table = (TransformTable *)table;
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
   cdata->_animated_vertices_modified = UpdateSeq();
 }
@@ -388,7 +377,7 @@ modify_transform_blend_table() {
   if (cdata->_transform_blend_table->get_ref_count() > 1) {
     cdata->_transform_blend_table = new TransformBlendTable(*cdata->_transform_blend_table);
   }
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
   cdata->_animated_vertices_modified = UpdateSeq();
 
@@ -412,7 +401,7 @@ void GeomVertexData::
 set_transform_blend_table(const TransformBlendTable *table) {
   CDWriter cdata(_cycler, true);
   cdata->_transform_blend_table = (TransformBlendTable *)table;
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
   cdata->_animated_vertices_modified = UpdateSeq();
 }
@@ -438,7 +427,7 @@ set_slider_table(const SliderTable *table) {
 
   CDWriter cdata(_cycler, true);
   cdata->_slider_table = (SliderTable *)table;
-  do_clear_cache(cdata);
+  clear_cache_stage();
   cdata->_modified = Geom::get_next_modified();
   cdata->_animated_vertices_modified = UpdateSeq();
 }
@@ -723,18 +712,29 @@ convert_to(const GeomVertexFormat *new_format) const {
 
   // Look up the new format in our cache--maybe we've recently applied
   // it.
-  {
-    CDReader cdata(_cycler);
-    CacheEntry temp_entry(new_format);
-    temp_entry.local_object();
-    Cache::const_iterator ci = cdata->_cache.find(&temp_entry);
-    if (ci != cdata->_cache.end()) {
-      CacheEntry *entry = (*ci);
-      // Record a cache hit, so this element will stay in the cache a
-      // while longer.
-      entry->refresh();
-      return entry->_result;
+  PT(CacheEntry) entry;
+
+  CacheEntry temp_entry(new_format);
+  temp_entry.local_object();
+  Cache::const_iterator ci = _cache.find(&temp_entry);
+  if (ci != _cache.end()) {
+    entry = (*ci);
+    nassertr(entry->_source == this, NULL);
+
+    // Here's an element in the cache for this computation.  Record a
+    // cache hit, so this element will stay in the cache a while
+    // longer.
+    entry->refresh();
+
+    CDCacheReader cdata(entry->_cycler);
+    if (cdata->_result != (GeomVertexData *)NULL) {
+      return cdata->_result;
     }
+
+    // The cache entry is stale, but we'll recompute it below.  Note
+    // that there's a small race condition here; another thread might
+    // recompute the cache at the same time.  No big deal, since it'll
+    // compute the same result.
   }
 
   // Okay, convert the data to the new format.
@@ -752,15 +752,12 @@ convert_to(const GeomVertexFormat *new_format) const {
 
   new_data->copy_from(this, false);
 
-  {
-    // Record the new result in the cache.
-    CacheEntry *entry;
-    {
-      CDWriter cdata(((GeomVertexData *)this)->_cycler, false);
-      entry = new CacheEntry((GeomVertexData *)this, new_format, new_data);
-      bool inserted = cdata->_cache.insert(entry).second;
-      nassertr(inserted, new_data);
-    }
+  // Record the new result in the cache.
+  if (entry == (CacheEntry *)NULL) {
+    // Create a new entry for the result.
+    entry = new CacheEntry((GeomVertexData *)this, new_format);
+    bool inserted = ((GeomVertexData *)this)->_cache.insert(entry).second;
+    nassertr(inserted, new_data);
     
     // And tell the cache manager about the new entry.  (It might
     // immediately request a delete from the cache of the thing we
@@ -768,6 +765,10 @@ convert_to(const GeomVertexFormat *new_format) const {
     entry->record();
   }
 
+  // Finally, store the cached result on the entry.
+  CDCacheWriter cdata(entry->_cycler, true);
+  cdata->_result = new_data;
+
   return new_data;
 }
 
@@ -1119,6 +1120,50 @@ write(ostream &out, int indent_level) const {
   }
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexData::clear_cache
+//       Access: Published
+//  Description: Removes all of the previously-cached results of
+//               convert_to().
+//
+//               This blows away the entire cache, upstream and
+//               downstream the pipeline.  Use clear_cache_stage()
+//               instead if you only want to blow away the cache at
+//               the current stage and upstream.
+////////////////////////////////////////////////////////////////////
+void GeomVertexData::
+clear_cache() {
+  for (Cache::iterator ci = _cache.begin();
+       ci != _cache.end();
+       ++ci) {
+    CacheEntry *entry = (*ci);
+    entry->erase();
+  }
+  _cache.clear();
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexData::clear_cache_stage
+//       Access: Published
+//  Description: Removes all of the previously-cached results of
+//               convert_to(), at the current pipeline stage and
+//               upstream.  Does not affect the downstream cache.
+//
+//               Don't call this in a downstream thread unless you
+//               don't mind it blowing away other changes you might
+//               have recently made in an upstream thread.
+////////////////////////////////////////////////////////////////////
+void GeomVertexData::
+clear_cache_stage() {
+  for (Cache::iterator ci = _cache.begin();
+       ci != _cache.end();
+       ++ci) {
+    CacheEntry *entry = (*ci);
+    CDCacheWriter cdata(entry->_cycler);
+    cdata->_result = NULL;
+  }
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: GeomVertexData::get_array_info
 //       Access: Public
@@ -1354,7 +1399,7 @@ do_set_num_rows(int n, GeomVertexData::CData *cdata) {
   }
 
   if (any_changed) {
-    do_clear_cache(cdata);
+    clear_cache_stage();
     cdata->_modified = Geom::get_next_modified();
     cdata->_animated_vertices.clear();
   }
@@ -1504,22 +1549,6 @@ update_animated_vertices(GeomVertexData::CData *cdata) {
   }
 }
 
-////////////////////////////////////////////////////////////////////
-//     Function: GeomVertexData::do_clear_cache
-//       Access: Private
-//  Description: The private implementation of clear_cache().
-////////////////////////////////////////////////////////////////////
-void GeomVertexData::
-do_clear_cache(GeomVertexData::CData *cdata) {
-  for (Cache::iterator ci = cdata->_cache.begin();
-       ci != cdata->_cache.end();
-       ++ci) {
-    CacheEntry *entry = (*ci);
-    entry->erase();
-  }
-  cdata->_cache.clear();
-}
-
 ////////////////////////////////////////////////////////////////////
 //     Function: GeomVertexData::register_with_read_factory
 //       Access: Public, Static
@@ -1655,6 +1684,16 @@ fillin(DatagramIterator &scan, BamReader *manager) {
   manager->read_cdata(scan, _cycler);
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexData::CDataCache::make_copy
+//       Access: Public, Virtual
+//  Description:
+////////////////////////////////////////////////////////////////////
+CycleData *GeomVertexData::CDataCache::
+make_copy() const {
+  return new CDataCache(*this);
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: GeomVertexData::CacheEntry::evict_callback
 //       Access: Public, Virtual
@@ -1663,13 +1702,10 @@ fillin(DatagramIterator &scan, BamReader *manager) {
 ////////////////////////////////////////////////////////////////////
 void GeomVertexData::CacheEntry::
 evict_callback() {
-  OPEN_ITERATE_ALL_STAGES(_source->_cycler) {
-    CDStageWriter cdata(_source->_cycler, pipeline_stage);
-    // Because of the multistage pipeline, we might not actually have
-    // a cache entry at every stage.  No big deal if we don't.
-    cdata->_cache.erase(this);
-  }
-  CLOSE_ITERATE_ALL_STAGES(_source->_cycler);
+  Cache::iterator ci = _source->_cache.find(this);
+  nassertv(ci != _source->_cache.end());
+  nassertv((*ci) == this);
+  _source->_cache.erase(ci);
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geomVertexData.h

@@ -144,7 +144,8 @@ PUBLISHED:
   void output(ostream &out) const;
   void write(ostream &out, int indent_level = 0) const;
 
-  INLINE void clear_cache();
+  void clear_cache();
+  void clear_cache_stage();
 
 public:
   bool get_array_info(const InternalName *name, 
@@ -200,20 +201,35 @@ private:
 
   typedef pvector< PT(GeomVertexArrayData) > Arrays;
 
+  // The pipelined data with each CacheEntry.
+  class CDataCache : public CycleData {
+  public:
+    INLINE CDataCache();
+    INLINE CDataCache(const CDataCache &copy);
+    virtual CycleData *make_copy() const;
+    virtual TypeHandle get_parent_type() const {
+      return GeomVertexData::get_class_type();
+    }
+
+    CPT(GeomVertexData) _result;
+  };
+  typedef CycleDataReader<CDataCache> CDCacheReader;
+  typedef CycleDataWriter<CDataCache> CDCacheWriter;
+
   class CacheEntry : public GeomCacheEntry {
   public:
     INLINE CacheEntry(const GeomVertexFormat *modifier);
     INLINE CacheEntry(GeomVertexData *source,
-                      const GeomVertexFormat *modifier,
-                      const GeomVertexData *result);
+                      const GeomVertexFormat *modifier);
     INLINE bool operator < (const CacheEntry &other) const;
 
     virtual void evict_callback();
     virtual void output(ostream &out) const;
 
-    GeomVertexData *_source;
+    GeomVertexData *_source;  // A back pointer to the containing GeomVertexData
     CPT(GeomVertexFormat) _modifier;
-    CPT(GeomVertexData) _result;
+
+    PipelineCycler<CDataCache> _cycler;
   };
   typedef pset<PT(CacheEntry), IndirectLess<CacheEntry> > Cache;
 
@@ -238,7 +254,6 @@ private:
     PT(GeomVertexData) _animated_vertices;
     UpdateSeq _animated_vertices_modified;
     UpdateSeq _modified;
-    Cache _cache;
   };
 
   PipelineCycler<CData> _cycler;
@@ -247,10 +262,11 @@ private:
   typedef CycleDataStageReader<CData> CDStageReader;
   typedef CycleDataStageWriter<CData> CDStageWriter;
 
+  Cache _cache;
+
 private:
   bool do_set_num_rows(int n, CData *cdata);
   void update_animated_vertices(CData *cdata);
-  void do_clear_cache(CData *cdata);
 
   static PStatCollector _convert_pcollector;
   static PStatCollector _scale_color_pcollector;