diff --git a/pcbnew/router/pns_kicad_iface.cpp b/pcbnew/router/pns_kicad_iface.cpp index 8f193229a3..b19a8c4ecb 100644 --- a/pcbnew/router/pns_kicad_iface.cpp +++ b/pcbnew/router/pns_kicad_iface.cpp @@ -124,7 +124,7 @@ public: int ClearanceEpsilon() const override { return m_clearanceEpsilon; } - void ClearCacheForItem( const PNS::ITEM* aItem ) override; + void ClearCacheForItems( std::vector& aItems ) override; void ClearCaches() override; private: @@ -410,13 +410,30 @@ bool PNS_PCBNEW_RULE_RESOLVER::QueryConstraint( PNS::CONSTRAINT_TYPE aType, } -void PNS_PCBNEW_RULE_RESOLVER::ClearCacheForItem( const PNS::ITEM* aItem ) +void PNS_PCBNEW_RULE_RESOLVER::ClearCacheForItems( std::vector& aItems ) { - CLEARANCE_CACHE_KEY key = { aItem, nullptr, false }; - m_clearanceCache.erase( key ); + int n_pruned = 0; + std::set remainingItems( aItems.begin(), aItems.end() ); - key.Flag = true; - m_clearanceCache.erase( key ); +/* We need to carefully check both A and B item pointers in the cache against dirty/invalidated + items in the set, as the clearance relation is commutative ( CL[a,b] == CL[b,a] ). The code + below is a bit ugly, but works in O(n*log(m)) and is run once or twice during ROUTER::Move() call + - so I hope it still gets better performance than no cache at all */ + for( auto it = m_clearanceCache.begin(); it != m_clearanceCache.end(); ) + { + bool dirty = remainingItems.find( it->first.A ) != remainingItems.end(); + dirty |= remainingItems.find( it->first.B) != remainingItems.end(); + + if( dirty ) + { + it = m_clearanceCache.erase( it ); + n_pruned++; + } else + it++; + } +#if 0 + printf("ClearCache : n_pruned %d\n", n_pruned ); +#endif } @@ -492,7 +509,15 @@ int PNS_PCBNEW_RULE_RESOLVER::Clearance( const PNS::ITEM* aA, const PNS::ITEM* a if( aUseClearanceEpsilon && rv > 0 ) rv = std::max( 0, rv - m_clearanceEpsilon ); - m_clearanceCache[ key ] = rv; + +/* It makes no sense to put items that have no owning NODE in the cache - they can be allocated on stack + and we can't really invalidate them in the cache when they are destroyed. Probably a better idea would be + to use a static unique counter in PNS::ITEM constructor to generate the cache keys. */ + if( aA && aB && aA->Owner() && aB->Owner() ) + { + m_clearanceCache[ key ] = rv; + } + return rv; } diff --git a/pcbnew/router/pns_node.cpp b/pcbnew/router/pns_node.cpp index 69d285bc52..69963eb997 100644 --- a/pcbnew/router/pns_node.cpp +++ b/pcbnew/router/pns_node.cpp @@ -87,7 +87,7 @@ NODE::~NODE() m_joints.clear(); - std::vector toDelete; + std::vector toDelete; toDelete.reserve( m_index->Size() ); @@ -97,12 +97,17 @@ NODE::~NODE() toDelete.push_back( item ); } - for( ITEM* item : toDelete ) + for( const ITEM* item : toDelete ) { wxLogTrace( wxT( "PNS" ), wxT( "del item %p type %s" ), item, item->KindStr().c_str() ); delete item; } + if( m_ruleResolver ) + { + m_ruleResolver->ClearCacheForItems( toDelete ); + } + releaseGarbage(); unlinkParent(); @@ -1389,6 +1394,9 @@ void NODE::releaseGarbage() if( !isRoot() ) return; + std::vector cacheCheckItems; + cacheCheckItems.reserve( m_garbageItems.size() ); + for( ITEM* item : m_garbageItems ) { if( !item->BelongsTo( this ) ) @@ -1396,6 +1404,11 @@ void NODE::releaseGarbage() } m_garbageItems.clear(); + + if( m_ruleResolver ) + { + m_ruleResolver->ClearCacheForItems( cacheCheckItems ); + } } diff --git a/pcbnew/router/pns_node.h b/pcbnew/router/pns_node.h index 1f0d91341a..c6cbb56a4e 100644 --- a/pcbnew/router/pns_node.h +++ b/pcbnew/router/pns_node.h @@ -155,7 +155,7 @@ public: virtual wxString NetName( int aNet ) = 0; - virtual void ClearCacheForItem( const ITEM* aItem ) {} + virtual void ClearCacheForItems( std::vector& aItems ) {} virtual void ClearCaches() {} virtual int ClearanceEpsilon() const { return 0; } diff --git a/pcbnew/router/pns_router.cpp b/pcbnew/router/pns_router.cpp index 6969546e8c..d0d99f0a3d 100644 --- a/pcbnew/router/pns_router.cpp +++ b/pcbnew/router/pns_router.cpp @@ -103,6 +103,7 @@ void ROUTER::ClearWorld() { if( m_world ) { + m_world->SetRuleResolver( nullptr ); m_world->KillChildren(); m_world.reset(); } @@ -716,9 +717,11 @@ void ROUTER::updateView( NODE* aNode, ITEM_SET& aCurrent, bool aDragging ) aNode->GetUpdatedItems( removed, added ); + std::vector cacheCheckItems( added.begin(), added.end() ); + GetRuleResolver()->ClearCacheForItems( cacheCheckItems ); + for( ITEM* item : added ) { - GetRuleResolver()->ClearCacheForItem( item ); int clearance = GetRuleResolver()->Clearance( item, nullptr ); m_iface->DisplayItem( item, clearance, aDragging ); }