From 5c61b61c27bd8bbb2188b4dacb090e60921bbe84 Mon Sep 17 00:00:00 2001 From: Maciej Suminski Date: Sun, 12 Nov 2017 14:24:55 +0100 Subject: [PATCH] Improved synchronization algorithm for LIB_MANAGER<->COMPONENT_TREE --- eeschema/cmp_tree_model.h | 8 +- eeschema/cmp_tree_model_adapter_base.cpp | 4 +- eeschema/lib_manager.cpp | 34 ++++--- eeschema/lib_manager.h | 15 ++- eeschema/lib_manager_adapter.cpp | 121 ++++++++++++++++------- eeschema/lib_manager_adapter.h | 18 ++-- eeschema/libeditframe.cpp | 8 +- 7 files changed, 133 insertions(+), 75 deletions(-) diff --git a/eeschema/cmp_tree_model.h b/eeschema/cmp_tree_model.h index ac130a7e58..ab062ee4a0 100644 --- a/eeschema/cmp_tree_model.h +++ b/eeschema/cmp_tree_model.h @@ -80,9 +80,11 @@ public: ROOT, LIB, LIBID, UNIT, INVALID }; - CMP_TREE_NODE* Parent; ///< Parent node or null - std::vector> Children; ///< List of child nodes - enum TYPE Type; ///< Node type + typedef std::vector> PTR_VECTOR; + + CMP_TREE_NODE* Parent; ///< Parent node or null + PTR_VECTOR Children; ///< List of child nodes + enum TYPE Type; ///< Node type /** * The rank of the item before any search terms are applied. This is diff --git a/eeschema/cmp_tree_model_adapter_base.cpp b/eeschema/cmp_tree_model_adapter_base.cpp index 873f56e31f..2b05df1bce 100644 --- a/eeschema/cmp_tree_model_adapter_base.cpp +++ b/eeschema/cmp_tree_model_adapter_base.cpp @@ -179,6 +179,7 @@ void CMP_TREE_MODEL_ADAPTER_BASE::AttachTo( wxDataViewCtrl* aDataViewCtrl ) ColWidth( m_tree, 0, part_head ) ); m_col_desc = aDataViewCtrl->AppendTextColumn( desc_head, 1, wxDATAVIEW_CELL_INERT, ColWidth( m_tree, 1, desc_head ) ); + m_col_part->SetSortOrder( 0 ); aDataViewCtrl->Thaw(); } @@ -281,14 +282,13 @@ void CMP_TREE_MODEL_ADAPTER_BASE::GetValue( switch( aCol ) { + default: // column == -1 is used for default Compare function case 0: aVariant = node->Name; break; case 1: aVariant = node->Desc; break; - default: - wxFAIL_MSG( "Invalid column ID!" ); } } diff --git a/eeschema/lib_manager.cpp b/eeschema/lib_manager.cpp index 66dbb227a7..8beb642129 100644 --- a/eeschema/lib_manager.cpp +++ b/eeschema/lib_manager.cpp @@ -43,21 +43,15 @@ LIB_MANAGER::LIB_MANAGER( LIB_EDIT_FRAME& aFrame ) } -void LIB_MANAGER::Sync() +void LIB_MANAGER::Sync( bool aForce ) { - // TODO handle renaming libraries in the sym-lib-table dialog - // TODO using filepath as the key? - // TODO should not be compared with symboltable, but between the adapter and manager - // TODO move to treepane? + int libTableHash = m_symbolTable->GetModifyHash(); + + if( aForce || m_syncHash != libTableHash ) + { getAdapter()->Sync(); - - //int libTableHash = m_symbolTable->GetModifyHash(); - - //if( m_syncHash != libTableHash ) - //{ - //getAdapter()->Sync(); - //m_syncHash = libTableHash; - //} + m_syncHash = libTableHash; + } } @@ -75,7 +69,14 @@ int LIB_MANAGER::GetHash() const int LIB_MANAGER::GetLibraryHash( const wxString& aLibrary ) const { const auto libBufIt = m_libs.find( aLibrary ); - return libBufIt != m_libs.end() ? libBufIt->second.GetHash() : 0; + + if( libBufIt != m_libs.end() ) + return libBufIt->second.GetHash(); + + auto row = m_symbolTable->FindRow( aLibrary ); + + // return -1 if library does not exist or 0 if not modified + return row ? std::hash{}( row->GetFullURI( true ).ToStdString() ) : -1; } @@ -282,7 +283,7 @@ bool LIB_MANAGER::UpdatePart( LIB_PART* aPart, const wxString& aLibrary, wxStrin screen->SetModify(); } - Sync(); // TODO update only the changed part + getAdapter()->UpdateLibrary( aLibrary ); return true; } @@ -377,6 +378,9 @@ bool LIB_MANAGER::PartExists( const wxString& aAlias, const wxString& aLibrary ) bool LIB_MANAGER::LibraryExists( const wxString& aLibrary ) const { + if( aLibrary.IsEmpty() ) + return false; + if( m_libs.count( aLibrary ) > 0 ) return true; diff --git a/eeschema/lib_manager.h b/eeschema/lib_manager.h index 4ddbef5b12..4d9f96b356 100644 --- a/eeschema/lib_manager.h +++ b/eeschema/lib_manager.h @@ -50,13 +50,19 @@ public: LIB_MANAGER( LIB_EDIT_FRAME& aFrame ); /** - * Updates the LIB_MANAGER data to account for the changes introduced to the project libraries. - * @see PROJECT::SchLibs() + * Updates the LIB_MANAGER data to synchronize with Symbol Library Table. */ - void Sync(); + void Sync( bool aForce = false ); int GetHash() const; + /** + * Retruns a library hash value to determine if it has changed. + * + * For buffered libraries, it returns a number corresponding to the number + * of modifications. For original libraries, hash is computed basing on the + * library URI. Returns -1 when the requested library does not exist. + */ int GetLibraryHash( const wxString& aLibrary ) const; /** @@ -340,6 +346,9 @@ private: ///> The library buffers std::map m_libs; + // TODO + int m_syncHash; + LIB_MANAGER_ADAPTER::PTR m_adapter; LIB_MANAGER_ADAPTER* getAdapter() { return static_cast( m_adapter.get() ); } }; diff --git a/eeschema/lib_manager_adapter.cpp b/eeschema/lib_manager_adapter.cpp index 8b73dfb5b4..1df471d43d 100644 --- a/eeschema/lib_manager_adapter.cpp +++ b/eeschema/lib_manager_adapter.cpp @@ -40,8 +40,6 @@ void LIB_MANAGER_ADAPTER::AddLibrary( const wxString& aLibNickname ) auto& lib_node = m_tree.AddLib( aLibNickname ); ItemAdded( wxDataViewItem( nullptr ), ToItem( &lib_node ) ); updateLibrary( lib_node ); - lib_node.AssignIntrinsicRanks(); - m_tree.AssignIntrinsicRanks(); } @@ -65,7 +63,8 @@ void LIB_MANAGER_ADAPTER::UpdateLibrary( const wxString& aLibraryName ) if( !node ) return; - ItemChanged( ToItem( node ) ); + updateLibrary( *(CMP_TREE_NODE_LIB*) node ); + Resort(); } @@ -85,25 +84,26 @@ bool LIB_MANAGER_ADAPTER::IsContainer( const wxDataViewItem& aItem ) const void LIB_MANAGER_ADAPTER::Sync() { - if( getSyncHash() == m_libMgr->GetHash() ) + int libMgrHash = m_libMgr->GetHash(); + + if( m_lastSyncHash == libMgrHash ) return; - wxDataViewItem root( nullptr ); + m_lastSyncHash = libMgrHash; // Process already stored libraries for( auto it = m_tree.Children.begin(); it != m_tree.Children.end(); /* iteration inside */ ) { - int mgrHash = m_libMgr->GetLibraryHash( it->get()->Name ); + const wxString& name = it->get()->Name; - if( mgrHash < 0 ) + if( !m_libMgr->LibraryExists( name ) ) { - deleteLibrary( * (CMP_TREE_NODE_LIB*) it->get() ); - it = m_tree.Children.erase( it ); + it = deleteLibrary( it ); continue; } - else if( mgrHash != m_libHashes[it->get()->Name] ) + else if( m_libMgr->GetLibraryHash( name ) != m_libHashes[name] ) { - updateLibrary( * (CMP_TREE_NODE_LIB*) it->get() ); + updateLibrary( *(CMP_TREE_NODE_LIB*) it->get() ); } ++it; @@ -113,46 +113,93 @@ void LIB_MANAGER_ADAPTER::Sync() for( const auto& libName : m_libMgr->GetLibraryNames() ) { if( m_libHashes.count( libName ) == 0 ) - { - auto& libNode = m_tree.AddLib( libName ); // Use AddLibrary? - ItemAdded( root, ToItem( &libNode ) ); - updateLibrary( libNode ); - } + AddLibrary( libName ); } + m_tree.AssignIntrinsicRanks(); Resort(); } void LIB_MANAGER_ADAPTER::updateLibrary( CMP_TREE_NODE_LIB& aLibNode ) +{ + if( m_libHashes.count( aLibNode.Name ) == 0 ) + { + // add a new library + addAliases( aLibNode ); + } + else + { + // update an existing libary +#if 1 + std::list aliases = m_libMgr->GetAliases( aLibNode.Name ); + wxDataViewItem parent = ToItem( &aLibNode ); + + // remove the common part from the aliases list + //for( const auto& node : aLibNode.Children ) + for( auto nodeIt = aLibNode.Children.begin(); nodeIt != aLibNode.Children.end(); /**/ ) + { + auto aliasIt = std::find_if( aliases.begin(), aliases.end(), + [&] ( const LIB_ALIAS* a ) { + return a->GetName() == (*nodeIt)->Name; + } ); + + if( aliasIt != aliases.end() ) + { + // alias exists both in the component tree and the library manager, + // no need to update + aliases.erase( aliasIt ); + ++nodeIt; + } + else + { + // node does not exist in the library manager, remove the corresponding node + ItemDeleted( parent, ToItem( nodeIt->get() ) ); + nodeIt = aLibNode.Children.erase( nodeIt ); + } + } + + // now the aliases list contains only new aliases that need to be added to the tree + for( auto alias : aliases ) + { + auto& aliasNode = aLibNode.AddAlias( alias ); + ItemAdded( parent, ToItem( &aliasNode ) ); + } +#else + // Bruteforce approach - remove everything and rebuild the branch + wxDataViewItem parent = ToItem( &aLibNode ); + + for( const auto& node : aLibNode.Children ) + ItemDeleted( parent, ToItem( node.get() ) ); + + aLibNode.Children.clear(); + addAliases( aLibNode ); +#endif + } + + aLibNode.AssignIntrinsicRanks(); + m_libHashes[aLibNode.Name] = m_libMgr->GetLibraryHash( aLibNode.Name ); +} + + +CMP_TREE_NODE::PTR_VECTOR::iterator LIB_MANAGER_ADAPTER::deleteLibrary( + CMP_TREE_NODE::PTR_VECTOR::iterator& aLibNodeIt ) +{ + m_libHashes.erase( aLibNodeIt->get()->Name ); + auto it = m_tree.Children.erase( aLibNodeIt ); + return it; +} + + +void LIB_MANAGER_ADAPTER::addAliases( CMP_TREE_NODE_LIB& aLibNode ) { wxDataViewItem parent = ToItem( &aLibNode ); - aLibNode.Children.clear(); for( auto alias : m_libMgr->GetAliases( aLibNode.Name ) ) { auto& aliasNode = aLibNode.AddAlias( alias ); ItemAdded( parent, ToItem( &aliasNode ) ); } - - // TODO faster? - /* - wxDataViewItemArray aliasItems; - aliasItems.reserve( aLibNode.Children.size() ); - - for( const auto& child : aLibNode.Children ) - aliasItems.Add( ToItem( child.get() ) ); - - ItemsAdded( parent, aliasItems ); - */ - - m_libHashes[aLibNode.Name] = m_libMgr->GetLibraryHash( aLibNode.Name ); -} - - -void LIB_MANAGER_ADAPTER::deleteLibrary( CMP_TREE_NODE_LIB& aLibNode ) -{ - wxASSERT( false ); } @@ -202,6 +249,6 @@ bool LIB_MANAGER_ADAPTER::GetAttr( wxDataViewItem const& aItem, unsigned int aCo LIB_MANAGER_ADAPTER::LIB_MANAGER_ADAPTER( LIB_MANAGER* aLibMgr ) - : m_libMgr( aLibMgr ) + : m_libMgr( aLibMgr ), m_lastSyncHash( -1 ) { } diff --git a/eeschema/lib_manager_adapter.h b/eeschema/lib_manager_adapter.h index 504c843cbf..57636a4b3f 100644 --- a/eeschema/lib_manager_adapter.h +++ b/eeschema/lib_manager_adapter.h @@ -50,7 +50,10 @@ public: protected: void updateLibrary( CMP_TREE_NODE_LIB& aLibNode ); - void deleteLibrary( CMP_TREE_NODE_LIB& aLibNode ); + CMP_TREE_NODE::PTR_VECTOR::iterator deleteLibrary( + CMP_TREE_NODE::PTR_VECTOR::iterator& aLibNodeIt ); + + void addAliases( CMP_TREE_NODE_LIB& aLibNode ); CMP_TREE_NODE* findLibrary( const wxString& aLibNickName ); @@ -61,18 +64,11 @@ protected: LIB_MANAGER* m_libMgr; - // TODO? use hashes to determine whcih libraries to update? + ///> Hashes to decide whether a library needs an update std::map m_libHashes; - int getSyncHash() const - { - int hash = 0; - - for( const auto& h : m_libHashes ) - hash += h.second; - - return hash; - } + ///> LIB_MANAGER hash value returned in the last synchronization + int m_lastSyncHash; }; #endif /* LIB_MANAGER_ADAPTER_H */ diff --git a/eeschema/libeditframe.cpp b/eeschema/libeditframe.cpp index ef65d78795..f3cce2b705 100644 --- a/eeschema/libeditframe.cpp +++ b/eeschema/libeditframe.cpp @@ -488,6 +488,8 @@ void LIB_EDIT_FRAME::OnToggleSearchTree( wxCommandEvent& event ) void LIB_EDIT_FRAME::OnEditSymbolLibTable( wxCommandEvent& aEvent ) { SCH_BASE_FRAME::OnEditSymbolLibTable( aEvent ); + m_libMgr->Sync( true ); + m_treePane->Refresh(); } @@ -543,9 +545,8 @@ void LIB_EDIT_FRAME::OnUpdateRedo( wxUpdateUIEvent& event ) void LIB_EDIT_FRAME::OnUpdateSaveCurrentLib( wxUpdateUIEvent& event ) { wxString lib = getTargetLib(); - SYMBOL_LIB_TABLE* table = Prj().SchSymbolLibTable(); - event.Enable( !lib.empty() && table->HasLibrary( lib ) && table->IsSymbolLibWritable( lib ) && + event.Enable( m_libMgr->LibraryExists( lib ) && !m_libMgr->IsLibraryReadOnly( lib ) && m_libMgr->IsLibraryModified( lib ) ); } @@ -553,9 +554,8 @@ void LIB_EDIT_FRAME::OnUpdateSaveCurrentLib( wxUpdateUIEvent& event ) void LIB_EDIT_FRAME::OnUpdateSaveCurrentLibAs( wxUpdateUIEvent& event ) { wxString lib = getTargetLib(); - SYMBOL_LIB_TABLE* table = Prj().SchSymbolLibTable(); - event.Enable( !lib.empty() && table->HasLibrary( lib ) ); + event.Enable( m_libMgr->LibraryExists( lib ) ); }