From 2e0941a1dedc3240d0e052200b4bb2431ff0609f Mon Sep 17 00:00:00 2001 From: Dick Hollenbeck Date: Tue, 8 May 2012 19:26:15 -0500 Subject: [PATCH] Fix some comments. Enhance LEGACY_PLUGIN such that it can tolerate, then fix bad legacy footprint libraries containing duplicate footprint names. This may have been an undocumented bug from more than a year ago, which manifested itself in *.mod files containing duplicate names. LEGACY_PLUGIN loads those now quietly, but appends "_v2", "_v3", etc. to each succeeding duplicate sharing the same name. --- common/base_screen.cpp | 2 +- common/common_plotPDF_functions.cpp | 14 ++++---- include/class_base_screen.h | 40 +++++++++++---------- pcbnew/legacy_plugin.cpp | 54 +++++++++++++++++++++++++---- 4 files changed, 77 insertions(+), 33 deletions(-) diff --git a/common/base_screen.cpp b/common/base_screen.cpp index ef0eac3d3c..97399deb45 100644 --- a/common/base_screen.cpp +++ b/common/base_screen.cpp @@ -122,7 +122,7 @@ bool BASE_SCREEN::SetZoom( double iu_per_du ) if( iu_per_du == m_Zoom ) return false; - wxLogDebug( "Zoom:%16g 1/Zoom:%16g", iu_per_du, 1/iu_per_du ); + wxLogDebug( "Zoom:%.16g 1/Zoom:%.16g", iu_per_du, 1/iu_per_du ); if( iu_per_du < GetMinAllowedZoom() ) return false; diff --git a/common/common_plotPDF_functions.cpp b/common/common_plotPDF_functions.cpp index 9a35b59345..bffdf049e6 100644 --- a/common/common_plotPDF_functions.cpp +++ b/common/common_plotPDF_functions.cpp @@ -410,17 +410,17 @@ void PDF_PLOTTER::closePdfStream() { wxASSERT( workFile ); - // Ask for the stream length int stream_len = ftell( workFile ); - // Now we do a trick: rewind the file, read in the page stream and FLATE it + // Rewind the file, read in the page stream and DEFLATE it fseek( workFile, 0, SEEK_SET ); unsigned char *inbuf = new unsigned char[stream_len]; int rc = fread( inbuf, 1, stream_len, workFile ); wxASSERT( rc == stream_len ); + (void) rc; - // We have done with the temporary file, junk it + // We are done with the temporary file, junk it fclose( workFile ); workFile = 0; ::wxRemoveFile( workFilename ); @@ -430,7 +430,7 @@ void PDF_PLOTTER::closePdfStream() { /* Somewhat standard parameters to compress in DEFLATE. The PDF spec is - misleading, it says it wants a FLATE stream but it really want a ZLIB + misleading, it says it wants a DEFLATE stream but it really want a ZLIB stream! (a DEFLATE stream would be generated with -15 instead of 15) rc = deflateInit2( &zstrm, Z_BEST_COMPRESSION, Z_DEFLATED, 15, 8, Z_DEFAULT_STRATEGY ); @@ -440,7 +440,9 @@ void PDF_PLOTTER::closePdfStream() zos.Write( inbuf, stream_len ); - } // flush the zip stream using destructor + delete[] inbuf; + + } // flush the zip stream using zos destructor wxStreamBuffer* sb = memos.GetOutputStreamBuffer(); @@ -448,8 +450,6 @@ void PDF_PLOTTER::closePdfStream() fwrite( sb->GetBufferStart(), 1, out_count, outputFile ); - delete[] inbuf; - fputs( "endstream\n", outputFile ); closePdfObject(); diff --git a/include/class_base_screen.h b/include/class_base_screen.h index 914da401e3..88c96f27a3 100644 --- a/include/class_base_screen.h +++ b/include/class_base_screen.h @@ -123,12 +123,12 @@ public: bool m_FirstRedraw; // Undo/redo list of commands - UNDO_REDO_CONTAINER m_UndoList; ///< Objects list for the undo command (old data) - UNDO_REDO_CONTAINER m_RedoList; ///< Objects list for the redo command (old data) - unsigned m_UndoRedoCountMax; ///< undo/Redo command Max depth + UNDO_REDO_CONTAINER m_UndoList; ///< Objects list for the undo command (old data) + UNDO_REDO_CONTAINER m_RedoList; ///< Objects list for the redo command (old data) + unsigned m_UndoRedoCountMax; ///< undo/Redo command Max depth // block control - BLOCK_SELECTOR m_BlockLocate; ///< Block description for block commands + BLOCK_SELECTOR m_BlockLocate; ///< Block description for block commands int m_ScreenNumber; int m_NumberOfScreen; @@ -303,26 +303,30 @@ public: */ double GetMinAllowedZoom() const { return m_ZoomList.size() ? *m_ZoomList.begin() : 1.0; } - /** - * Function GetScalingFactor - * returns the the current scale used to draw items on screen - * draw coordinates are user coordinates * GetScalingFactor() - */ - double GetScalingFactor() const; - /** * Function SetScalingFactor - * sets the scaling factor of "device units per internal unit". + * sets the scaling factor of "internal unit per device unit". * If the output device is a screen, then "device units" are pixels. The * "logical unit" is wx terminology, and corresponds to KiCad's "Internal Unit (IU)". - * - * Another way of thinking of scaling factor, when applied to a screen, - * is "pixelsPerIU" + *

+ * This scaling factor is "internal units per device unit". This function is + * the same thing currently as SetZoom(), but clamps the argument within a + * legal range. - * @param aScale = the the current scale used to draw items onto the device context wxDC. - * device coordinates (pixels) = IU coordinates * GetScalingFactor() + * @param iu_per_du is the current scale used to draw items onto the device + * context wxDC. */ - void SetScalingFactor( double aScale ); + void SetScalingFactor( double iu_per_du ); + + /** + * Function GetScalingFactor + * returns the inverse of the current scale used to draw items on screen. + *

+ * This function somehow got designed to be the inverse of SetScalingFactor(). + *

+ * device coordinates = user coordinates * GetScalingFactor() + */ + double GetScalingFactor() const; //-------------------------------------------------------------- diff --git a/pcbnew/legacy_plugin.cpp b/pcbnew/legacy_plugin.cpp index c537594264..29841bf480 100644 --- a/pcbnew/legacy_plugin.cpp +++ b/pcbnew/legacy_plugin.cpp @@ -942,6 +942,8 @@ MODULE* LEGACY_PLUGIN::LoadMODULE() else if( TESTLINE( "Li" ) ) // Library name of footprint { + // There can be whitespace in the footprint name on some old libraries. + // Grab everything after "Li" up to end of line: module->SetLibRef( FROM_UTF8( StrPurge( line + SZ( "Li" ) ) ) ); } @@ -3903,16 +3905,54 @@ void FPL_CACHE::LoadModules( LINE_READER* aReader ) // wxString footprintName = m->GetReference(); wxString footprintName = m->GetLibRef(); - std::pair r = m_modules.insert( footprintName, m ); + /* - // m's module is gone here, both on success or failure of insertion. - // no memory leak, container deleted m on failure. + There was a bug in old legacy library management code + (pre-LEGACY_PLUGIN) which was introducing duplicate footprint names + in legacy libraries without notification. To best recover from such + bad libraries, and use them to their fullest, there are a few + strategies that could be used. (Note: footprints must have unique + names to be accepted into this cache.) The strategy used here is to + append a differentiating version counter to the end of the name as: + _v2, _v3, etc. - if( !r.second ) + */ + + MODULE_CITER it = m_modules.find( footprintName ); + + if( it == m_modules.end() ) // footprintName is not present in cache yet. { - THROW_IO_ERROR( wxString::Format( - _( "library '%s' has a duplicate footprint named '%s'" ), - m_lib_name.GetData(), footprintName.GetData() ) ); + std::pair r = m_modules.insert( footprintName, m ); + + wxASSERT_MSG( r.second, wxT( "error doing cache insert using guaranteed unique name" ) ); + (void) r; + } + + // Bad library has a duplicate of this footprintName, generate a + // unique footprint name and load it anyway. + else + { + bool nameOK = false; + int version = 2; + + while( !nameOK ) + { + wxString newName = footprintName; + newName << wxT( "_v" ) << version++; + + it = m_modules.find( newName ); + + if( it == m_modules.end() ) + { + nameOK = true; + + m->SetLibRef( newName ); + std::pair r = m_modules.insert( newName, m ); + + wxASSERT_MSG( r.second, wxT( "error doing cache insert using guaranteed unique name" ) ); + (void) r; + } + } } }