Improved synchronization algorithm for LIB_MANAGER<->COMPONENT_TREE

eeschema/cmp_tree_model.h

@@ -80,9 +80,11 @@ public:
         ROOT, LIB, LIBID, UNIT, INVALID
     };
 
-    CMP_TREE_NODE*                              Parent;     ///< Parent node or null
+    typedef std::vector<std::unique_ptr<CMP_TREE_NODE>> PTR_VECTOR;
-    std::vector<std::unique_ptr<CMP_TREE_NODE>> Children;   ///< List of child nodes
+
-    enum TYPE                                   Type;       ///< Node type
+    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

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!" );
     }
 }
 

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
+    int libTableHash = m_symbolTable->GetModifyHash();
-    // TODO using filepath as the key?
+
-    // TODO should not be compared with symboltable, but between the adapter and manager
+    if( aForce || m_syncHash != libTableHash )
-    // TODO move to treepane?
+    {
         getAdapter()->Sync();
-
+        m_syncHash = libTableHash;
-    //int libTableHash = m_symbolTable->GetModifyHash();
+    }
-
-    //if( m_syncHash != libTableHash )
-    //{
-        //getAdapter()->Sync();
-        //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<std::string>{}( 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;
 

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.
+     * Updates the LIB_MANAGER data to synchronize with Symbol Library Table.
-     * @see PROJECT::SchLibs()
      */
-    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<wxString, LIB_BUFFER> m_libs;
 
+    // TODO
+    int m_syncHash;
+
     LIB_MANAGER_ADAPTER::PTR m_adapter;
     LIB_MANAGER_ADAPTER* getAdapter() { return static_cast<LIB_MANAGER_ADAPTER*>( m_adapter.get() ); }
 };

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 = deleteLibrary( it );
-            it = m_tree.Children.erase( 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 )
-        {
+            AddLibrary( libName );
-            auto& libNode = m_tree.AddLib( libName );       // Use AddLibrary?
-            ItemAdded( root, ToItem( &libNode ) );
-            updateLibrary( libNode );
-        }
     }
 
+    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<LIB_ALIAS*> 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 )
 {
 }

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<wxString, int> m_libHashes;
 
-    int getSyncHash() const
+    ///> LIB_MANAGER hash value returned in the last synchronization
-    {
+    int m_lastSyncHash;
-        int hash = 0;
-
-        for( const auto& h : m_libHashes )
-            hash += h.second;
-
-        return hash;
-    }
 };
 
 #endif /* LIB_MANAGER_ADAPTER_H */

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 ) );
 }