diff --git a/panda/src/gobj/geom.I b/panda/src/gobj/geom.I index a3f551460e..9137bc386a 100644 --- a/panda/src/gobj/geom.I +++ b/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 ©) : + _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(); - } } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/gobj/geom.cxx b/panda/src/gobj/geom.cxx index cf5d814a1c..fa5e6379a4 100644 --- a/panda/src/gobj/geom.cxx +++ b/panda/src/gobj/geom.cxx @@ -64,11 +64,7 @@ void Geom:: operator = (const Geom ©) { 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 ©) { //////////////////////////////////////////////////////////////////// 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); } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/gobj/geom.h b/panda/src/gobj/geom.h index 0f152111fa..0bd0e6c29b 100644 --- a/panda/src/gobj/geom.h +++ b/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 ©); + 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 CDCacheReader; + typedef CycleDataWriter 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 _cycler; }; typedef pset > 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 _cycler; typedef CycleDataReader CDReader; typedef CycleDataWriter CDWriter; typedef CycleDataStageReader CDStageReader; typedef CycleDataStageWriter 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 Contexts; diff --git a/panda/src/gobj/geomMunger.cxx b/panda/src/gobj/geomMunger.cxx index dc33e9236e..408c0771fb 100644 --- a/panda/src/gobj/geomMunger.cxx +++ b/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); + PT(Geom::CacheEntry) entry; - 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. + 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); - // 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; - } + // 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(); - // 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); + // 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); } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/gobj/geomVertexData.I b/panda/src/gobj/geomVertexData.I index 26f1c29787..aa1919dbea 100644 --- a/panda/src/gobj/geomVertexData.I +++ b/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 ©) : + _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) { } diff --git a/panda/src/gobj/geomVertexData.cxx b/panda/src/gobj/geomVertexData.cxx index ce0209a949..a5902d3b41 100644 --- a/panda/src/gobj/geomVertexData.cxx +++ b/panda/src/gobj/geomVertexData.cxx @@ -154,11 +154,7 @@ void GeomVertexData:: operator = (const GeomVertexData ©) { 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 ©) { //////////////////////////////////////////////////////////////////// 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); } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/gobj/geomVertexData.h b/panda/src/gobj/geomVertexData.h index b9b100e41d..6a046e0d82 100644 --- a/panda/src/gobj/geomVertexData.h +++ b/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 ©); + virtual CycleData *make_copy() const; + virtual TypeHandle get_parent_type() const { + return GeomVertexData::get_class_type(); + } + + CPT(GeomVertexData) _result; + }; + typedef CycleDataReader CDCacheReader; + typedef CycleDataWriter 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 _cycler; }; typedef pset > Cache; @@ -238,7 +254,6 @@ private: PT(GeomVertexData) _animated_vertices; UpdateSeq _animated_vertices_modified; UpdateSeq _modified; - Cache _cache; }; PipelineCycler _cycler; @@ -247,10 +262,11 @@ private: typedef CycleDataStageReader CDStageReader; typedef CycleDataStageWriter 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;