From 91cfecaa1201b4df7d8df6f5fe5c5148b54e3659 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sat, 28 Apr 2018 10:43:41 +0100 Subject: [PATCH] Don't allow wxDataViewCtrl updates during model update. Also checks to make sure libraries are activated before adding them to the component tree. Fixes: lp:1765286 * https://bugs.launchpad.net/kicad/+bug/1765286 --- common/lib_table_base.cpp | 10 ++++++++-- eeschema/cmp_tree_model_adapter_base.cpp | 10 ++++++++++ eeschema/cmp_tree_model_adapter_base.h | 8 ++++++++ eeschema/getpart.cpp | 2 +- eeschema/lib_edit_frame.cpp | 4 ++++ eeschema/lib_manager.cpp | 4 ++-- eeschema/lib_manager.h | 5 +++-- eeschema/lib_manager_adapter.cpp | 11 ++++++++++- include/lib_table_base.h | 3 ++- include/lib_table_grid.h | 2 -- 10 files changed, 48 insertions(+), 11 deletions(-) diff --git a/common/lib_table_base.cpp b/common/lib_table_base.cpp index 128bec2da8..f8ff1fef75 100644 --- a/common/lib_table_base.cpp +++ b/common/lib_table_base.cpp @@ -261,11 +261,17 @@ const wxString LIB_TABLE::GetDescription( const wxString& aNickname ) } -bool LIB_TABLE::HasLibrary( const wxString& aNickname ) const +bool LIB_TABLE::HasLibrary( const wxString& aNickname, bool aCheckEnabled ) const { const LIB_TABLE_ROW* row = findRow( aNickname ); - return row != nullptr; + if( row == nullptr ) + return false; + + if( aCheckEnabled && !row->GetIsEnabled() ) + return false; + + return true; } diff --git a/eeschema/cmp_tree_model_adapter_base.cpp b/eeschema/cmp_tree_model_adapter_base.cpp index 81d61e954c..0bc68bd262 100644 --- a/eeschema/cmp_tree_model_adapter_base.cpp +++ b/eeschema/cmp_tree_model_adapter_base.cpp @@ -80,6 +80,7 @@ CMP_TREE_MODEL_ADAPTER_BASE::CMP_TREE_MODEL_ADAPTER_BASE() :m_filter( CMP_FILTER_NONE ), m_show_units( true ), m_preselect_unit( 0 ), + m_freeze( 0 ), m_col_part( nullptr ), m_col_desc( nullptr ), m_widget( nullptr ) @@ -331,6 +332,12 @@ void CMP_TREE_MODEL_ADAPTER_BASE::GetValue( wxDataViewItem const& aItem, unsigned int aCol ) const { + if( IsFrozen() ) + { + aVariant = wxEmptyString; + return; + } + auto node = ToNode( aItem ); wxASSERT( node ); @@ -352,6 +359,9 @@ bool CMP_TREE_MODEL_ADAPTER_BASE::GetAttr( unsigned int aCol, wxDataViewItemAttr& aAttr ) const { + if( IsFrozen() ) + return false; + auto node = ToNode( aItem ); wxASSERT( node ); diff --git a/eeschema/cmp_tree_model_adapter_base.h b/eeschema/cmp_tree_model_adapter_base.h index 7f16e3565e..3efd3945e8 100644 --- a/eeschema/cmp_tree_model_adapter_base.h +++ b/eeschema/cmp_tree_model_adapter_base.h @@ -260,6 +260,13 @@ public: wxDataViewItem const& aItem, wxDataViewItemArray& aChildren ) const override; + // Freezing/Thawing. Used when updating the table model so that we don't try and fetch + // values during updating. Primarily a problem on OSX which doesn't pay attention to the + // wxDataViewCtrl's freeze count when updating the keyWindow. + void Freeze() { m_freeze++; } + void Thaw() { m_freeze--; } + bool IsFrozen() const { return m_freeze; } + protected: static wxDataViewItem ToItem( CMP_TREE_NODE const* aNode ); static CMP_TREE_NODE const* ToNode( wxDataViewItem aItem ); @@ -341,6 +348,7 @@ private: bool m_show_units; LIB_ID m_preselect_lib_id; int m_preselect_unit; + int m_freeze; wxDataViewColumn* m_col_part; wxDataViewColumn* m_col_desc; diff --git a/eeschema/getpart.cpp b/eeschema/getpart.cpp index 6b88f8bc40..49e803dd8a 100644 --- a/eeschema/getpart.cpp +++ b/eeschema/getpart.cpp @@ -125,7 +125,7 @@ SCH_BASE_FRAME::COMPONENT_SELECTION SCH_BASE_FRAME::SelectComponentFromLibrary( for( unsigned ii = 0; ii < liblist.GetCount(); ii++ ) { - if( libs->HasLibrary( liblist[ii] ) ) + if( libs->HasLibrary( liblist[ii], true ) ) { loaded = true; adapter->AddLibrary( liblist[ii] ); diff --git a/eeschema/lib_edit_frame.cpp b/eeschema/lib_edit_frame.cpp index 202f46a488..45754fc33d 100644 --- a/eeschema/lib_edit_frame.cpp +++ b/eeschema/lib_edit_frame.cpp @@ -453,8 +453,12 @@ void LIB_EDIT_FRAME::OnToggleSearchTree( wxCommandEvent& event ) void LIB_EDIT_FRAME::OnEditSymbolLibTable( wxCommandEvent& aEvent ) { + m_libMgr->GetAdapter()->Freeze(); + SCH_BASE_FRAME::OnEditSymbolLibTable( aEvent ); SyncLibraries( true ); + + m_libMgr->GetAdapter()->Thaw(); } diff --git a/eeschema/lib_manager.cpp b/eeschema/lib_manager.cpp index e3f9a8a8f3..672c6535ed 100644 --- a/eeschema/lib_manager.cpp +++ b/eeschema/lib_manager.cpp @@ -560,7 +560,7 @@ bool LIB_MANAGER::PartExists( const wxString& aAlias, const wxString& aLibrary ) } -bool LIB_MANAGER::LibraryExists( const wxString& aLibrary ) const +bool LIB_MANAGER::LibraryExists( const wxString& aLibrary, bool aCheckEnabled ) const { if( aLibrary.IsEmpty() ) return false; @@ -568,7 +568,7 @@ bool LIB_MANAGER::LibraryExists( const wxString& aLibrary ) const if( m_libs.count( aLibrary ) > 0 ) return true; - return symTable()->HasLibrary( aLibrary ); + return symTable()->HasLibrary( aLibrary, aCheckEnabled ); } diff --git a/eeschema/lib_manager.h b/eeschema/lib_manager.h index 7426e6667a..ce524eec2e 100644 --- a/eeschema/lib_manager.h +++ b/eeschema/lib_manager.h @@ -144,9 +144,10 @@ public: bool PartExists( const wxString& aAlias, const wxString& aLibrary ) const; /** - * Returns true if library exists. + * Returns true if library exists. If \a aCheckEnabled is set, then the library must + * also be enabled in the library table. */ - bool LibraryExists( const wxString& aLibrary ) const; + bool LibraryExists( const wxString& aLibrary, bool aCheckEnabled = false ) const; /** * Returns true if library has unsaved modifications. diff --git a/eeschema/lib_manager_adapter.cpp b/eeschema/lib_manager_adapter.cpp index a788a533b7..f0953f41bf 100644 --- a/eeschema/lib_manager_adapter.cpp +++ b/eeschema/lib_manager_adapter.cpp @@ -80,7 +80,7 @@ void LIB_MANAGER_ADAPTER::Sync( bool aForce, std::functionLibraryExists( name ) ) + if( !m_libMgr->LibraryExists( name, true ) ) { it = deleteLibrary( it ); continue; @@ -202,6 +202,12 @@ CMP_TREE_NODE* LIB_MANAGER_ADAPTER::findLibrary( const wxString& aLibNickName ) void LIB_MANAGER_ADAPTER::GetValue( wxVariant& aVariant, wxDataViewItem const& aItem, unsigned int aCol ) const { + if( IsFrozen() ) + { + aVariant = wxEmptyString; + return; + } + auto node = ToNode( aItem ); wxASSERT( node ); @@ -235,6 +241,9 @@ void LIB_MANAGER_ADAPTER::GetValue( wxVariant& aVariant, wxDataViewItem const& a bool LIB_MANAGER_ADAPTER::GetAttr( wxDataViewItem const& aItem, unsigned int aCol, wxDataViewItemAttr& aAttr ) const { + if( IsFrozen() ) + return false; + // change attributes only for the name field if( aCol != 0 ) return false; diff --git a/include/lib_table_base.h b/include/lib_table_base.h index 20433ada3b..576f6295f4 100644 --- a/include/lib_table_base.h +++ b/include/lib_table_base.h @@ -356,9 +356,10 @@ public: /** * Test for the existence of \a aNickname in the library table. * + * @param aCheckEnabled if true will only return true for enabled libraries * @return true if a library \a aNickname exists in the table. */ - bool HasLibrary( const wxString& aNickname ) const; + bool HasLibrary( const wxString& aNickname, bool aCheckEnabled = false ) const; /** * Return the logical library names, all of them that are pertinent to diff --git a/include/lib_table_grid.h b/include/lib_table_grid.h index 8a98e526f4..2168e27418 100644 --- a/include/lib_table_grid.h +++ b/include/lib_table_grid.h @@ -168,7 +168,6 @@ public: { LIB_TABLE_ROWS_ITER start = begin() + aPos; erase( start, start + aNumRows ); - if( GetView() ) { wxGridTableMessage msg( this, @@ -178,7 +177,6 @@ public: GetView()->ProcessTableMessage( msg ); } - return true; }