From ffbaf87dc961f2f4211d84c989ac44d6cec6bfbf Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Tue, 24 Sep 2013 14:45:57 -0400 Subject: [PATCH] Pcbnew footprint library plug in fixes and minor code cleaning. * When loading footprint do not retest every footprint in cache. Only test the footprint being loaded. Fixes long load times on libraries with a lot of parts. * Fix footprint name bug where file extension was added to the end of the footprint name. * Fix bug in path equivalence test due to Posix path separators in footprint library table. Convert paths to native separator before comparison. * Fix a bug in FOOTPRINT_VIEWER_FRAME::OnActivate() which cause the footprint list to always get reloaded when using footprint library tables. * Remove some unnecessary debugging messages. * Add a few Doxygen comments to FP_CACHE in kicad_plugin.cpp. --- common/footprint_info.cpp | 3 - pcbnew/kicad_plugin.cpp | 125 +++++++++++++++++++++++++++++--------- pcbnew/kicad_plugin.h | 2 +- pcbnew/modview_frame.cpp | 41 ++++++++++--- pcbnew/pcb_parser.cpp | 4 +- 5 files changed, 131 insertions(+), 44 deletions(-) diff --git a/common/footprint_info.cpp b/common/footprint_info.cpp index 9bb6ba5b27..c789119a9c 100644 --- a/common/footprint_info.cpp +++ b/common/footprint_info.cpp @@ -158,9 +158,6 @@ bool FOOTPRINT_LIST::ReadFootprintFiles( FP_LIB_TABLE& aTable ) wxString path = FP_LIB_TABLE::ExpandSubstitutions( row->GetFullURI() ); wxArrayString fpnames = pi->FootprintEnumerate( path ); - wxLogDebug( wxT( "Load footprint library type %s from path <%s>" ), - GetChars( row->GetType() ), GetChars( path ) ); - for( unsigned i=0; i m( pi->FootprintLoad( path, fpnames[i] ) ); diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp index 04d28d692c..91ebe6e1e8 100644 --- a/pcbnew/kicad_plugin.cpp +++ b/pcbnew/kicad_plugin.cpp @@ -59,7 +59,7 @@ using namespace std; /** * Definition for enabling and disabling footprint library trace output. See the - * wxWidgets documentation on useing the WXTRACE environment variable. + * wxWidgets documentation on using the WXTRACE environment variable. */ static const wxString traceFootprintLibrary( wxT( "KicadFootprintLib" ) ); @@ -95,12 +95,25 @@ FP_CACHE_ITEM::FP_CACHE_ITEM( MODULE* aModule, const wxFileName& aFileName ) : m_module( aModule ) { m_file_name = aFileName; - m_mod_time.Now(); + + if( m_file_name.FileExists() ) + m_mod_time = m_file_name.GetModificationTime(); + else + m_mod_time.Now(); } bool FP_CACHE_ITEM::IsModified() const { + if( !m_file_name.FileExists() ) + return false; + + wxLogTrace( traceFootprintLibrary, wxT( "File <%s>, m_mod_time %s-%s, file mod time: %s-%s." ), + GetChars( m_file_name.GetFullPath() ), + GetChars( m_mod_time.FormatDate() ), GetChars( m_mod_time.FormatTime() ), + GetChars( m_file_name.GetModificationTime().FormatDate() ), + GetChars( m_file_name.GetModificationTime().FormatTime() ) ); + return m_file_name.GetModificationTime() != m_mod_time; } @@ -136,9 +149,35 @@ public: void Remove( const wxString& aFootprintName ); - wxDateTime GetLibModificationTime(); + wxDateTime GetLibModificationTime() const; - bool IsModified(); + /** + * Function IsModified + * check if the footprint cache has been modified relative to \a aLibPath + * and \a aFootprintName. + * + * @param aLibPath is a path to test the current cache library path against. + * @param aFootprintName is the footprint name in the cache to test. If the footprint + * name is empty, the all the footprint files in the library are + * checked to see if they have been modified. + * @return true if the cache has been modified. + */ + bool IsModified( const wxString& aLibPath, + const wxString& aFootprintName = wxEmptyString ) const; + + /** + * Function IsPath + * checks if \a aPath is the same as the current cache path. + * + * This tests paths by converting \a aPath using the native separators. Internally + * #FP_CACHE stores the current path using native separators. This prevents path + * miscompares on Windows due to the fact that paths can be stored with / instead of \\ + * in the footprint library table. + * + * @param aPath is the library path to test against. + * @return true if \a aPath is the same as the cache path. + */ + bool IsPath( const wxString& aPath ) const; }; @@ -149,7 +188,7 @@ FP_CACHE::FP_CACHE( PCB_IO* aOwner, const wxString& aLibraryPath ) } -wxDateTime FP_CACHE::GetLibModificationTime() +wxDateTime FP_CACHE::GetLibModificationTime() const { return m_lib_path.GetModificationTime(); } @@ -181,7 +220,8 @@ void FP_CACHE::Save() // Allow file output stream to go out of scope to close the file stream before // renaming the file. { - // wxLogTrace( traceFootprintLibrary, wxT( "Creating temporary library file %s" ), GetChars( tempFileName ) ); + wxLogTrace( traceFootprintLibrary, wxT( "Creating temporary library file %s" ), + GetChars( tempFileName ) ); FILE_OUTPUTFORMATTER formatter( tempFileName ); @@ -228,9 +268,12 @@ void FP_CACHE::Load() m_owner->m_parser->SetLineReader( &reader ); - std::string name = TO_UTF8( fpFileName ); + std::string name = TO_UTF8( fullPath.GetName() ); + MODULE* footprint = (MODULE*) m_owner->m_parser->Parse(); - m_modules.insert( name, new FP_CACHE_ITEM( (MODULE*) m_owner->m_parser->Parse(), fpFileName ) ); + // The footprint name is the file name without the extension. + footprint->SetFPID( fullPath.GetName() ); + m_modules.insert( name, new FP_CACHE_ITEM( footprint, fullPath ) ); } while( dir.GetNext( &fpFileName ) ); @@ -263,30 +306,56 @@ void FP_CACHE::Remove( const wxString& aFootprintName ) } -bool FP_CACHE::IsModified() +bool FP_CACHE::IsPath( const wxString& aPath ) const { - if( !m_lib_path.DirExists() ) + // Converts path separators to native path separators + wxFileName newPath; + newPath.AssignDir( aPath ); + + return m_lib_path == newPath; +} + + +bool FP_CACHE::IsModified( const wxString& aLibPath, const wxString& aFootprintName ) const +{ + // The library is modified if the library path got deleted or changed. + if( !m_lib_path.DirExists() || !IsPath( aLibPath ) ) return true; - for( MODULE_ITER it = m_modules.begin(); it != m_modules.end(); ++it ) + // If no footprint was specified, check every file modification time against the time + // it was loaded. + if( aFootprintName.IsEmpty() ) { - wxFileName fn = it->second->GetFileName(); - - if( !fn.FileExists() ) + for( MODULE_CITER it = m_modules.begin(); it != m_modules.end(); ++it ) { - wxLogTrace( traceFootprintLibrary, wxT( "Footprint cache file '%s' does not exist." ), - fn.GetFullPath().GetData() ); - return true; - } + wxFileName fn = m_lib_path; + fn.SetName( it->second->GetFileName().GetName() ); + fn.SetExt( KiCadFootprintFileExtension ); - if( it->second->IsModified() ) - { - wxLogTrace( traceFootprintLibrary, - wxT( "Footprint cache file '%s' has been modified." ), - fn.GetFullPath().GetData() ); - return true; + if( !fn.FileExists() ) + { + wxLogTrace( traceFootprintLibrary, + wxT( "Footprint cache file '%s' does not exist." ), + fn.GetFullPath().GetData() ); + return true; + } + + if( it->second->IsModified() ) + { + wxLogTrace( traceFootprintLibrary, + wxT( "Footprint cache file '%s' has been modified." ), + fn.GetFullPath().GetData() ); + return true; + } } } + else + { + MODULE_CITER it = m_modules.find( TO_UTF8( aFootprintName ) ); + + if( it == m_modules.end() || it->second->IsModified() ) + return true; + } return false; } @@ -1590,9 +1659,9 @@ void PCB_IO::init( PROPERTIES* aProperties ) } -void PCB_IO::cacheLib( const wxString& aLibraryPath ) +void PCB_IO::cacheLib( const wxString& aLibraryPath, const wxString& aFootprintName ) { - if( !m_cache || m_cache->GetPath() != aLibraryPath || m_cache->IsModified() ) + if( !m_cache || m_cache->IsModified( aLibraryPath, aFootprintName ) ) { // a spectacular episode in memory management: delete m_cache; @@ -1630,7 +1699,7 @@ MODULE* PCB_IO::FootprintLoad( const wxString& aLibraryPath, const wxString& aFo init( aProperties ); - cacheLib( aLibraryPath ); + cacheLib( aLibraryPath, aFootprintName ); const MODULE_MAP& mods = m_cache->GetModules(); @@ -1817,7 +1886,7 @@ bool PCB_IO::FootprintLibDelete( const wxString& aLibraryPath, PROPERTIES* aProp wxMilliSleep( 250L ); #endif - if( m_cache && m_cache->GetPath() == aLibraryPath ) + if( m_cache && !m_cache->IsPath( aLibraryPath ) ) { delete m_cache; m_cache = NULL; diff --git a/pcbnew/kicad_plugin.h b/pcbnew/kicad_plugin.h index 8de731b4e8..008ab6e12c 100644 --- a/pcbnew/kicad_plugin.h +++ b/pcbnew/kicad_plugin.h @@ -210,7 +210,7 @@ private: throw( IO_ERROR ); /// we only cache one footprint library for now, this determines which one. - void cacheLib( const wxString& aLibraryPath ); + void cacheLib( const wxString& aLibraryPath, const wxString& aFootprintName = wxEmptyString ); void init( PROPERTIES* aProperties ); }; diff --git a/pcbnew/modview_frame.cpp b/pcbnew/modview_frame.cpp index 24e21aae95..8ca94a5210 100644 --- a/pcbnew/modview_frame.cpp +++ b/pcbnew/modview_frame.cpp @@ -149,7 +149,7 @@ FOOTPRINT_VIEWER_FRAME::FOOTPRINT_VIEWER_FRAME( PCB_BASE_FRAME* aParent, SetBoard( new BOARD() ); // Ensure all layers and items are visible: GetBoard()->SetVisibleAlls(); - SetScreen( new PCB_SCREEN(GetPageSizeIU()) ); + SetScreen( new PCB_SCREEN( GetPageSizeIU() ) ); GetScreen()->m_Center = true; // Center coordinate origins on screen. LoadSettings(); @@ -198,7 +198,7 @@ FOOTPRINT_VIEWER_FRAME::FOOTPRINT_VIEWER_FRAME( PCB_BASE_FRAME* aParent, DisplayLibInfos(); - // If a footprint was previsiously loaded, reload it + // If a footprint was previously loaded, reload it if( !m_libraryName.IsEmpty() && !m_footprintName.IsEmpty() ) { #if !defined( USE_FP_LIB_TABLE ) @@ -235,9 +235,9 @@ FOOTPRINT_VIEWER_FRAME::FOOTPRINT_VIEWER_FRAME( PCB_BASE_FRAME* aParent, mesg.MessageToolbarPane(); - // Manage main toolbal + // Manage main toolbar m_auimgr.AddPane( m_mainToolBar, - wxAuiPaneInfo( horiz ).Name( wxT ("m_mainToolBar" ) ).Top().Row( 0 ) ); + wxAuiPaneInfo( horiz ).Name( wxT( "m_mainToolBar" ) ).Top().Row( 0 ) ); wxSize minsize( 60, -1 ); @@ -290,6 +290,7 @@ FOOTPRINT_VIEWER_FRAME::~FOOTPRINT_VIEWER_FRAME() m_Draw3DFrame->Destroy(); } + /* return the frame name used when creating the frame * used to get a reference to this frame, if exists */ @@ -298,13 +299,14 @@ const wxChar* FOOTPRINT_VIEWER_FRAME::GetFootprintViewerFrameName() return FOOTPRINT_VIEWER_FRAME_NAME; } + /* return a reference to the current opened Footprint viewer * or NULL if no Footprint viewer currently opened */ FOOTPRINT_VIEWER_FRAME* FOOTPRINT_VIEWER_FRAME::GetActiveFootprintViewer() { return (FOOTPRINT_VIEWER_FRAME*) - wxWindow::FindWindowByName(GetFootprintViewerFrameName()); + wxWindow::FindWindowByName( GetFootprintViewerFrameName() ); } @@ -400,8 +402,8 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateLibraryList() } else { - /* If not found, clear current library selection because it can be - * deleted after a config change. */ + // If not found, clear current library selection because it can be deleted after + // a configuration change. m_libraryName = wxEmptyString; m_footprintName = wxEmptyString; } @@ -452,6 +454,7 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateFootprintList() if( !libLoaded ) { + m_footprintName = wxEmptyString; m_libraryName = wxEmptyString; wxString msg; @@ -462,7 +465,7 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateFootprintList() msg += _( "Files not found:\n\n" ) + fp_info_list.m_filesNotFound; if( !fp_info_list.m_filesInvalid.IsEmpty() ) - msg += _("\n\nFile load errors:\n\n" ) + fp_info_list.m_filesInvalid; + msg += _( "\n\nFile load errors:\n\n" ) + fp_info_list.m_filesInvalid; DisplayError( this, msg ); return; @@ -566,7 +569,7 @@ void FOOTPRINT_VIEWER_FRAME::DClickOnFootprintList( wxCommandEvent& event ) // event in the parent window which would cause the part to be parked // rather than staying in mode mode. // Remember the mouse button will be released in the parent window - // thus creating a mouse button release event which should be ingnored. + // thus creating a mouse button release event which should be ignored ((PCB_BASE_FRAME*)GetParent())->SkipNextLeftButtonReleaseEvent(); } } @@ -640,19 +643,37 @@ void FOOTPRINT_VIEWER_FRAME::OnActivate( wxActivateEvent& event ) m_selectedFootprintName.Empty(); // Ensure we have the right library list: +#if !defined( USE_FP_LIB_TABLE ) if( g_LibraryNames.GetCount() == m_LibList->GetCount() ) { unsigned ii; for( ii = 0; ii < g_LibraryNames.GetCount(); ii++ ) { - if( m_LibList->GetString(ii) != g_LibraryNames[ii] ) + if( m_LibList->GetString( ii ) != g_LibraryNames[ii] ) break; } if( ii == g_LibraryNames.GetCount() ) return; } +#else + std::vector< wxString > libNicknames = m_footprintLibTable->GetLogicalLibs(); + + if( libNicknames.size() == m_LibList->GetCount() ) + { + unsigned ii; + + for( ii = 0; ii < libNicknames.size(); ii++ ) + { + if( libNicknames[ii] != m_LibList->GetString( ii ) ) + break; + } + + if( ii == libNicknames.size() ) + return; + } +#endif // If we are here, the library list has changed, rebuild it ReCreateLibraryList(); diff --git a/pcbnew/pcb_parser.cpp b/pcbnew/pcb_parser.cpp index 4ef1695f2d..055dd02cee 100644 --- a/pcbnew/pcb_parser.cpp +++ b/pcbnew/pcb_parser.cpp @@ -754,8 +754,8 @@ T PCB_PARSER::lookUpLayer( const M& aMap ) throw( PARSE_ERROR, IO_ERROR ) // dump the whole darn table, there's something wrong with it. for( it = aMap.begin(); it != aMap.end(); ++it ) { - printf( &aMap == (void*)&m_layerIndices ? "lm[%s] = %d\n" : "lm[%s] = %08X\n", - it->first.c_str(), it->second ); + wxLogDebug( &aMap == (void*)&m_layerIndices ? wxT( "lm[%s] = %d" ) : + wxT( "lm[%s] = %08X" ), it->first.c_str(), it->second ); } #endif