Infobar warning if symbol loading was cancelled.

Also makes sure the progress dialog is closed when we're done reading
symbols (it used to stay up for much of the symbol editor initialization).

Also makes sure that any cancel in the preLoad step is honoured in the
sync step.  (The preload is done because it is multi-threaded and therefore
faster than the single-threaded sync.)

Also makes sure that individual threads pay attention to the cancellation,
not just the GUI thread.

Fixes https://gitlab.com/kicad/code/kicad/issues/8372

common/widgets/progress_reporter_base.cpp

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2017 CERN
- * Copyright (C) 2021 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2021-2022 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * Author: Tomasz Wlostowski <tomasz.wlostowski@cern.ch>
  *
@@ -119,7 +119,7 @@ bool PROGRESS_REPORTER_BASE::KeepRefreshing( bool aWait )
                 return false;
             }
 
-            wxMilliSleep( 20 );
+            wxMilliSleep( 33 /* 30 FPS refresh rate */ );
         }
 
         return true;

eeschema/CMakeLists.txt

@@ -181,7 +181,7 @@ set( EESCHEMA_SRCS
     fields_grid_table.cpp
     files-io.cpp
     generate_alias_info.cpp
-    getpart.cpp
+    picksymbol.cpp
     hierarch.cpp
     lib_field.cpp
     lib_item.cpp

eeschema/getpart.cpp → eeschema/picksymbol.cpp

@@ -106,7 +106,10 @@ PICKED_SYMBOL SCH_BASE_FRAME::PickSymbolFromLibTree( const SYMBOL_LIBRARY_FILTER
     Pgm().GetSettingsManager().GetAppSettings<EESCHEMA_SETTINGS>();
     Pgm().GetSettingsManager().GetAppSettings<SYMBOL_EDITOR_SETTINGS>();
 
-    wxObjectDataPtr<LIB_TREE_MODEL_ADAPTER> adapter = SYMBOL_TREE_MODEL_ADAPTER::Create( this, libs );
+    wxObjectDataPtr<LIB_TREE_MODEL_ADAPTER> dataPtr
+                                    = SYMBOL_TREE_MODEL_ADAPTER::Create( this, libs );
+    SYMBOL_TREE_MODEL_ADAPTER* modelAdapter
+                                    = static_cast<SYMBOL_TREE_MODEL_ADAPTER*>( dataPtr.get() );
     bool loaded = false;
 
     if( aFilter )
@@ -118,14 +121,14 @@ PICKED_SYMBOL SCH_BASE_FRAME::PickSymbolFromLibTree( const SYMBOL_LIBRARY_FILTER
             if( libs->HasLibrary( liblist[ii], true ) )
             {
                 loaded = true;
-                static_cast<SYMBOL_TREE_MODEL_ADAPTER*>( adapter.get() )->AddLibrary( liblist[ii] );
+                modelAdapter->AddLibrary( liblist[ii] );
             }
         }
 
-        adapter->AssignIntrinsicRanks();
+        modelAdapter->AssignIntrinsicRanks();
 
         if( aFilter->GetFilterPowerSymbols() )
-            adapter->SetFilter( SYMBOL_TREE_MODEL_ADAPTER::SYM_FILTER_POWER );
+            modelAdapter->SetFilter( SYMBOL_TREE_MODEL_ADAPTER::SYM_FILTER_POWER );
     }
 
     std::vector< LIB_TREE_ITEM* > history_list;
@@ -139,29 +142,34 @@ PICKED_SYMBOL SCH_BASE_FRAME::PickSymbolFromLibTree( const SYMBOL_LIBRARY_FILTER
             history_list.push_back( symbol );
     }
 
-    adapter->DoAddLibrary( "-- " + _( "Recently Used" ) + " --", wxEmptyString, history_list,
-                           true );
+    modelAdapter->DoAddLibrary( wxT( "-- " ) + _( "Recently Used" ) + wxT( " --" ), wxEmptyString,
+                                history_list, true );
 
     if( !aHistoryList.empty() )
-        adapter->SetPreselectNode( aHistoryList[0].LibId, aHistoryList[0].Unit );
+        modelAdapter->SetPreselectNode( aHistoryList[0].LibId, aHistoryList[0].Unit );
 
     const std::vector< wxString > libNicknames = libs->GetLogicalLibs();
 
     if( !loaded )
-         static_cast<SYMBOL_TREE_MODEL_ADAPTER*>( adapter.get() )->AddLibraries( libNicknames,
-                                                  this );
+    {
+        if( !modelAdapter->AddLibraries( libNicknames, this ) )
+        {
+            // loading cancelled by user
+            return PICKED_SYMBOL();
+        }
+    }
 
     if( aHighlight && aHighlight->IsValid() )
-        adapter->SetPreselectNode( *aHighlight, /* aUnit */ 0 );
+        modelAdapter->SetPreselectNode( *aHighlight, /* aUnit */ 0 );
 
     wxString dialogTitle;
 
-    if( adapter->GetFilter() == SYMBOL_TREE_MODEL_ADAPTER::SYM_FILTER_POWER )
-        dialogTitle.Printf( _( "Choose Power Symbol (%d items loaded)" ), adapter->GetItemCount() );
+    if( modelAdapter->GetFilter() == SYMBOL_TREE_MODEL_ADAPTER::SYM_FILTER_POWER )
+        dialogTitle.Printf( _( "Choose Power Symbol (%d items loaded)" ), dataPtr->GetItemCount() );
     else
-        dialogTitle.Printf( _( "Choose Symbol (%d items loaded)" ), adapter->GetItemCount() );
+        dialogTitle.Printf( _( "Choose Symbol (%d items loaded)" ), dataPtr->GetItemCount() );
 
-    DIALOG_CHOOSE_SYMBOL dlg( this, dialogTitle, adapter, aConvert, aAllowFields, aShowFootprints,
+    DIALOG_CHOOSE_SYMBOL dlg( this, dialogTitle, dataPtr, aConvert, aAllowFields, aShowFootprints,
                               aUseLibBrowser );
 
     if( dlg.ShowModal() == wxID_CANCEL )

eeschema/symbol_async_loader.cpp

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2021 Jon Evans <jon@craftyjon.com>
- * Copyright (C) 2021 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2021-2022 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -35,8 +35,7 @@ SYMBOL_ASYNC_LOADER::SYMBOL_ASYNC_LOADER( const std::vector<wxString>& aNickname
         m_onlyPowerSymbols( aOnlyPowerSymbols ),
         m_output( aOutput ),
         m_reporter( aReporter ),
-        m_nextLibrary( 0 ),
-        m_canceled( false )
+        m_nextLibrary( 0 )
 {
     wxASSERT( m_table );
     m_threadCount = std::max<size_t>( 1, std::thread::hardware_concurrency() );
@@ -48,7 +47,7 @@ SYMBOL_ASYNC_LOADER::SYMBOL_ASYNC_LOADER( const std::vector<wxString>& aNickname
 
 SYMBOL_ASYNC_LOADER::~SYMBOL_ASYNC_LOADER()
 {
-    Abort();
+    Join();
 }
 
 
@@ -88,13 +87,6 @@ bool SYMBOL_ASYNC_LOADER::Join()
 }
 
 
-void SYMBOL_ASYNC_LOADER::Abort()
-{
-    m_canceled.store( true );
-    Join();
-}
-
-
 bool SYMBOL_ASYNC_LOADER::Done()
 {
     return m_nextLibrary.load() >= m_nicknames.size();
@@ -110,11 +102,15 @@ std::vector<SYMBOL_ASYNC_LOADER::LOADED_PAIR> SYMBOL_ASYNC_LOADER::worker()
     for( size_t libraryIndex = m_nextLibrary++; libraryIndex < m_nicknames.size();
          libraryIndex = m_nextLibrary++ )
     {
-        if( m_canceled.load() )
+        const wxString& nickname = m_nicknames[libraryIndex];
+
+        if( m_reporter )
+            m_reporter->AdvancePhase( wxString::Format( _( "Loading library %s..." ), nickname ) );
+
+        if( m_reporter && m_reporter->IsCancelled() )
             break;
 
-        const wxString& nickname = m_nicknames[libraryIndex];
-        LOADED_PAIR     pair( nickname, {} );
+        LOADED_PAIR pair( nickname, {} );
 
         try
         {
@@ -129,9 +125,6 @@ std::vector<SYMBOL_ASYNC_LOADER::LOADED_PAIR> SYMBOL_ASYNC_LOADER::worker()
             std::lock_guard<std::mutex> lock( m_errorMutex );
             m_errors += msg;
         }
-
-        if( m_reporter )
-            m_reporter->AdvancePhase( wxString::Format( _( "Loading library %s..." ), nickname ) );
     }
 
     return ret;

eeschema/symbol_async_loader.h

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2021 Jon Evans <jon@craftyjon.com>
- * Copyright (C) 2021 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2021-2022 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -62,11 +62,6 @@ public:
      */
     bool Join();
 
-    /**
-     * Cancels a load in-progress
-     */
-    void Abort();
-
     ///< Return true if loading is done
     bool Done();
 
@@ -97,7 +92,6 @@ private:
 
     size_t              m_threadCount;
     std::atomic<size_t> m_nextLibrary;
-    std::atomic_bool    m_canceled;
     wxString            m_errors;
     std::mutex          m_errorMutex;
 

eeschema/symbol_editor/symbol_edit_frame.cpp

@@ -127,13 +127,20 @@ SYMBOL_EDIT_FRAME::SYMBOL_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
     LoadSettings( m_settings );
 
     m_libMgr = new SYMBOL_LIBRARY_MANAGER( *this );
+    bool loadingCancelled = false;
 
-    // Preload libraries before using SyncLibraries the first time, as the preload is threaded
-    WX_PROGRESS_REPORTER reporter( this, _( "Loading Symbol Libraries" ),
-                                   m_libMgr->GetLibraryCount(), true );
-    m_libMgr->Preload( reporter );
+    {
+        // Preload libraries before using SyncLibraries the first time, as the preload is
+        // multi-threaded
+        WX_PROGRESS_REPORTER reporter( this, _( "Loading Symbol Libraries" ),
+                                       m_libMgr->GetLibraryCount(), true );
+        m_libMgr->Preload( reporter );
+
+        loadingCancelled = reporter.IsCancelled();
+        wxSafeYield();
+    }
 
-    SyncLibraries( false );
+    SyncLibraries( false, loadingCancelled );
     m_treePane = new SYMBOL_TREE_PANE( this, m_libMgr );
 
     resolveCanvasType();
@@ -235,6 +242,9 @@ SYMBOL_EDIT_FRAME::SYMBOL_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
 
     // Ensure the window is on top
     Raise();
+
+    if( loadingCancelled )
+        ShowInfoBarWarning( _( "Symbol library loading was cancelled by user." ) );
 }
 
 
@@ -919,7 +929,8 @@ wxString SYMBOL_EDIT_FRAME::getTargetLib() const
 }
 
 
-void SYMBOL_EDIT_FRAME::SyncLibraries( bool aShowProgress, const wxString& aForceRefresh )
+void SYMBOL_EDIT_FRAME::SyncLibraries( bool aShowProgress, bool aPreloadCancelled,
+                                       const wxString& aForceRefresh )
 {
     LIB_ID selected;
 
@@ -938,7 +949,7 @@ void SYMBOL_EDIT_FRAME::SyncLibraries( bool aShowProgress, const wxString& aForc
                                                                     libName ) );
                 } );
     }
-    else
+    else if( !aPreloadCancelled )
     {
         m_libMgr->Sync( aForceRefresh,
                 [&]( int progress, int max, const wxString& libName )

eeschema/symbol_editor/symbol_edit_frame.h

@@ -296,7 +296,8 @@ public:
      * Synchronize the library manager to the symbol library table, and then the symbol tree
      * to the library manager.  Optionally displays a progress dialog.
      */
-    void SyncLibraries( bool aShowProgress, const wxString& aForceRefresh = wxEmptyString );
+    void SyncLibraries( bool aShowProgress, bool aPreloadCancelled = false,
+                        const wxString& aForceRefresh = wxEmptyString );
 
     /**
      * Filter, sort, and redisplay the library tree.

eeschema/symbol_editor/symbol_editor.cpp

@@ -1119,7 +1119,7 @@ bool SYMBOL_EDIT_FRAME::saveLibrary( const wxString& aLibrary, bool aNewFile )
         if( resyncLibTree )
         {
             FreezeLibraryTree();
-            SyncLibraries( true, forceRefresh );
+            SyncLibraries( true, false, forceRefresh );
             ThawLibraryTree();
         }
     }

eeschema/symbol_editor/symbol_library_manager.cpp

@@ -74,8 +74,6 @@ void SYMBOL_LIBRARY_MANAGER::Sync( const wxString& aForceRefresh,
 
 void SYMBOL_LIBRARY_MANAGER::Preload( PROGRESS_REPORTER& aReporter )
 {
-    const int progressIntervalMillis = 60;
-
     SYMBOL_ASYNC_LOADER loader( symTable()->GetLogicalLibs(), symTable(), false, nullptr,
                                 &aReporter );
 
@@ -85,19 +83,13 @@ void SYMBOL_LIBRARY_MANAGER::Preload( PROGRESS_REPORTER& aReporter )
 
     while( !loader.Done() )
     {
-        aReporter.KeepRefreshing();
+        if( !aReporter.KeepRefreshing() )
+            break;
 
-        wxMilliSleep( progressIntervalMillis );
+        wxMilliSleep( 33 /* 30 FPS refresh rate */ );
     }
 
-    if( aReporter.IsCancelled() )
-    {
-        loader.Abort();
-    }
-    else
-    {
-        loader.Join();
-    }
+    loader.Join();
 
     if( !loader.GetErrors().IsEmpty() )
     {
@@ -116,7 +108,7 @@ void SYMBOL_LIBRARY_MANAGER::Preload( PROGRESS_REPORTER& aReporter )
 
 bool SYMBOL_LIBRARY_MANAGER::HasModifications() const
 {
-    for( const auto& lib : m_libs )
+    for( const std::pair<const wxString, LIB_BUFFER>& lib : m_libs )
     {
         if( lib.second.IsModified() )
             return true;
@@ -130,7 +122,7 @@ int SYMBOL_LIBRARY_MANAGER::GetHash() const
 {
     int hash = symTable()->GetModifyHash();
 
-    for( const auto& lib : m_libs )
+    for( const std::pair<const wxString, LIB_BUFFER>& lib : m_libs )
         hash += lib.second.GetHash();
 
     return hash;
@@ -144,7 +136,7 @@ int SYMBOL_LIBRARY_MANAGER::GetLibraryHash( const wxString& aLibrary ) const
     if( libBufIt != m_libs.end() )
         return libBufIt->second.GetHash();
 
-    auto row = GetLibrary( aLibrary );
+    SYMBOL_LIB_TABLE_ROW* row = GetLibrary( aLibrary );
 
     // return -1 if library does not exist or 0 if not modified
     return row ? std::hash<std::string>{}( aLibrary.ToStdString() +
@@ -156,7 +148,7 @@ wxArrayString SYMBOL_LIBRARY_MANAGER::GetLibraryNames() const
 {
     wxArrayString res;
 
-    for( const auto& libName : symTable()->GetLogicalLibs() )
+    for( const wxString& libName : symTable()->GetLogicalLibs() )
         res.Add( libName );
 
     return res;

eeschema/symbol_tree_model_adapter.cpp

@@ -3,7 +3,7 @@
  *
  * Copyright (C) 2017 Chris Pavlina <pavlina.chris@gmail.com>
  * Copyright (C) 2014 Henner Zeller <h.zeller@acm.org>
- * Copyright (C) 2014-2021 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2014-2022 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -35,7 +35,7 @@
 
 bool SYMBOL_TREE_MODEL_ADAPTER::m_show_progress = true;
 
-#define PROGRESS_INTERVAL_MILLIS 66
+#define PROGRESS_INTERVAL_MILLIS 33      // 30 FPS refresh rate
 
 
 wxObjectDataPtr<LIB_TREE_MODEL_ADAPTER>
@@ -56,7 +56,7 @@ SYMBOL_TREE_MODEL_ADAPTER::~SYMBOL_TREE_MODEL_ADAPTER()
 {}
 
 
-void SYMBOL_TREE_MODEL_ADAPTER::AddLibraries( const std::vector<wxString>& aNicknames,
+bool SYMBOL_TREE_MODEL_ADAPTER::AddLibraries( const std::vector<wxString>& aNicknames,
                                               wxWindow* aParent )
 {
     std::unique_ptr<WX_PROGRESS_REPORTER> prg = nullptr;
@@ -82,20 +82,18 @@ void SYMBOL_TREE_MODEL_ADAPTER::AddLibraries( const std::vector<wxString>& aNick
 
     while( !loader.Done() )
     {
-        if( prg )
-            prg->KeepRefreshing();
+        if( prg && !prg->KeepRefreshing() )
+            break;
 
         wxMilliSleep( PROGRESS_INTERVAL_MILLIS );
     }
 
-    if( prg && prg->IsCancelled() )
-    {
-        loader.Abort();
-    }
-    else
-    {
-        loader.Join();
-    }
+    loader.Join();
+
+    bool cancelled = false;
+
+    if( prg )
+        cancelled = prg->IsCancelled();
 
     if( !loader.GetErrors().IsEmpty() )
     {
@@ -134,6 +132,8 @@ void SYMBOL_TREE_MODEL_ADAPTER::AddLibraries( const std::vector<wxString>& aNick
         prg.reset();
         m_show_progress = false;
     }
+
+    return !cancelled;
 }
 
 

eeschema/symbol_tree_model_adapter.h

@@ -3,7 +3,7 @@
  *
  * Copyright (C) 2017 Chris Pavlina <pavlina.chris@gmail.com>
  * Copyright (C) 2014 Henner Zeller <h.zeller@acm.org>
- * Copyright (C) 2014-2021 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2014-2022 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -49,8 +49,9 @@ public:
      *
      * @param aNicknames is the list of library nicknames
      * @param aParent is the parent window to display the progress dialog
+     * @return false if loading was cancelled by the user
      */
-    void AddLibraries( const std::vector<wxString>& aNicknames, wxWindow* aParent );
+    bool AddLibraries( const std::vector<wxString>& aNicknames, wxWindow* aParent );
 
     void AddLibrary( wxString const& aLibNickname );
 

eeschema/symbol_viewer_frame.cpp

@@ -837,11 +837,16 @@ void SYMBOL_VIEWER_FRAME::OnSelectSymbol( wxCommandEvent& aEvent )
 
     // Container doing search-as-you-type.
     SYMBOL_LIB_TABLE* libs = Prj().SchSymbolLibTable();
-    wxObjectDataPtr<LIB_TREE_MODEL_ADAPTER> adapter =
-            SYMBOL_TREE_MODEL_ADAPTER::Create( this, libs );
+    wxObjectDataPtr<LIB_TREE_MODEL_ADAPTER> dataPtr
+                                    = SYMBOL_TREE_MODEL_ADAPTER::Create( this, libs );
+    SYMBOL_TREE_MODEL_ADAPTER* modelAdapter
+                                    = static_cast<SYMBOL_TREE_MODEL_ADAPTER*>( dataPtr.get() );
 
-    const auto libNicknames = libs->GetLogicalLibs();
-    static_cast<SYMBOL_TREE_MODEL_ADAPTER*>( adapter.get() )->AddLibraries( libNicknames, this );
+    if( !modelAdapter->AddLibraries( libs->GetLogicalLibs(), this ) )
+    {
+        // loading cancelled by user
+        return;
+    }
 
     LIB_SYMBOL* current = GetSelectedSymbol();
     LIB_ID id;
@@ -850,13 +855,13 @@ void SYMBOL_VIEWER_FRAME::OnSelectSymbol( wxCommandEvent& aEvent )
     if( current )
     {
         id = current->GetLibId();
-        adapter->SetPreselectNode( id, unit );
+        modelAdapter->SetPreselectNode( id, unit );
     }
 
     wxString dialogTitle;
-    dialogTitle.Printf( _( "Choose Symbol (%d items loaded)" ), adapter->GetItemCount() );
+    dialogTitle.Printf( _( "Choose Symbol (%d items loaded)" ), modelAdapter->GetItemCount() );
 
-    DIALOG_CHOOSE_SYMBOL dlg( this, dialogTitle, adapter, m_convert, false, false, false );
+    DIALOG_CHOOSE_SYMBOL dlg( this, dialogTitle, dataPtr, m_convert, false, false, false );
 
     if( dlg.ShowQuasiModal() == wxID_CANCEL )
         return;

pcbnew/footprint_info_impl.cpp

@@ -143,7 +143,7 @@ bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxStri
         if( m_progress_reporter && !m_progress_reporter->KeepRefreshing() )
             m_cancelled = true;
 
-        wxMilliSleep( 20 );
+        wxMilliSleep( 33 /* 30 FPS refresh rate */);
     }
 
     if( m_cancelled )
@@ -326,7 +326,7 @@ bool FOOTPRINT_LIST_IMPL::joinWorkers()
         if( m_progress_reporter && !m_progress_reporter->KeepRefreshing() )
             m_cancelled = true;
 
-        wxMilliSleep( 30 );
+        wxMilliSleep( 33 /* 30 FPS refresh rate */ );
     }
 
     for( auto& thr : threads )

pcbnew/fp_tree_synchronizing_adapter.cpp

@@ -64,7 +64,7 @@ bool FP_TREE_SYNCHRONIZING_ADAPTER::IsContainer( const wxDataViewItem& aItem ) c
 }
 
 
-#define PROGRESS_INTERVAL_MILLIS 66
+#define PROGRESS_INTERVAL_MILLIS 33     // 30 FPS refresh rate
 
 void FP_TREE_SYNCHRONIZING_ADAPTER::Sync()
 {