Avoid the obsolete GetNextPin() call

This iterated over all pins to find the pin after a given item.  Because
out pattern is consistently to iterate in the outer loop, this means
that we were an O(n^2) loop for the pins just to find their names.  This
affected very large parts (e.g. FPGAs) when switching sheets to display

eeschema/dialogs/dialog_lib_edit_pin_table.cpp

@@ -861,7 +861,9 @@ DIALOG_LIB_EDIT_PIN_TABLE::~DIALOG_LIB_EDIT_PIN_TABLE()
 bool DIALOG_LIB_EDIT_PIN_TABLE::TransferDataToWindow()
 {
     // Make a copy of the pins for editing
-    for( LIB_PIN* pin = m_symbol->GetNextPin( nullptr ); pin; pin = m_symbol->GetNextPin( pin ) )
+    std::vector<LIB_PIN*> pins = m_symbol->GetAllLibPins();
+
+    for( LIB_PIN* pin : pins )
         m_pins.push_back( new LIB_PIN( *pin ) );
 
     m_dataModel->RebuildRows( m_pins, m_cbGroup->GetValue(), false );
@@ -888,7 +890,9 @@ bool DIALOG_LIB_EDIT_PIN_TABLE::TransferDataFromWindow()
         return false;
 
     // Delete the part's pins
-    while( LIB_PIN* pin = m_symbol->GetNextPin( nullptr ) )
+    std::vector<LIB_PIN*> pins = m_symbol->GetAllLibPins();
+
+    for( LIB_PIN* pin : pins )
         m_symbol->RemoveDrawItem( pin );
 
     // Transfer our pins to the part

eeschema/lib_symbol.cpp

@@ -831,35 +831,6 @@ void LIB_SYMBOL::AddDrawItem( LIB_ITEM* aItem, bool aSort )
 }
 
 
-LIB_ITEM* LIB_SYMBOL::GetNextDrawItem( const LIB_ITEM* aItem, KICAD_T aType )
-{
-    if( aItem == nullptr )
-    {
-        LIB_ITEMS_CONTAINER::ITERATOR it1 = m_drawings.begin( aType );
-
-        return (it1 != m_drawings.end( aType ) ) ? &( *( m_drawings.begin( aType ) ) ) : nullptr;
-    }
-
-    // Search for the last item, assume aItem is of type aType
-    wxASSERT( ( aType == TYPE_NOT_INIT ) || ( aType == aItem->Type() ) );
-    LIB_ITEMS_CONTAINER::ITERATOR it = m_drawings.begin( aType );
-
-    while( ( it != m_drawings.end( aType ) ) && ( aItem != &( *it ) ) )
-        ++it;
-
-    // Search the next item
-    if( it != m_drawings.end( aType ) )
-    {
-        ++it;
-
-        if( it != m_drawings.end( aType ) )
-            return &( *it );
-    }
-
-    return nullptr;
-}
-
-
 void LIB_SYMBOL::GetPins( LIB_PINS& aList, int aUnit, int aConvert ) const
 {
     /* Notes:

eeschema/lib_symbol.h

@@ -392,35 +392,10 @@ public:
 
     void RemoveField( LIB_FIELD* aField ) { RemoveDrawItem( aField ); }
 
-    /**
-     * Return the next draw object pointer.
-     *
-     * @param aItem - Pointer to the current draw item.  Setting item NULL
-     *                with return the first item of type in the list.
-     * @param aType - type of searched item (filter).
-     *                if TYPE_NOT_INIT search for all items types
-     * @return - The next drawing object in the list if found, otherwise NULL.
-     */
-    LIB_ITEM* GetNextDrawItem( const LIB_ITEM* aItem = nullptr, KICAD_T aType = TYPE_NOT_INIT );
-
     size_t GetPinCount() const { return m_drawings.size( LIB_PIN_T ); }
 
     size_t GetFieldCount() const { return m_drawings.size( LIB_FIELD_T ); }
 
-    /**
-     * Return the next pin object from the draw list.
-     *
-     * This is just a pin object specific version of GetNextDrawItem().
-     *
-     * @param aItem - Pointer to the previous pin item, or NULL to get the first pin in the draw
-     *                object list.
-     * @return - The next pin object in the list if found, otherwise NULL.
-     */
-    LIB_PIN* GetNextPin( LIB_PIN* aItem = nullptr )
-    {
-        return (LIB_PIN*) GetNextDrawItem( (LIB_ITEM*) aItem, LIB_PIN_T );
-    }
-
     /**
      * Return a list of pin object pointers from the draw item list.
      *

eeschema/sch_screen.cpp

@@ -1192,20 +1192,16 @@ LIB_PIN* SCH_SCREEN::GetPin( const VECTOR2I& aPosition, SCH_SYMBOL** aSymbol,
             if( !candidate->GetLibSymbolRef() )
                 continue;
 
-            for( pin = candidate->GetLibSymbolRef()->GetNextPin(); pin;
+            std::vector<LIB_PIN*> pins;
-                 pin = candidate->GetLibSymbolRef()->GetNextPin( pin ) )
+            candidate->GetLibPins( pins );
+
+            for( LIB_PIN* test_pin : pins )
             {
-                // Skip items not used for this part.
+                if( candidate->GetPinPhysicalPosition( test_pin ) == aPosition )
-                if( candidate->GetUnit() && pin->GetUnit() &&
+                {
-                    ( pin->GetUnit() != candidate->GetUnit() ) )
+                    pin = test_pin;
-                    continue;
-
-                if( candidate->GetConvert() && pin->GetConvert() &&
-                    ( pin->GetConvert() != candidate->GetConvert() ) )
-                    continue;
-
-                if( candidate->GetPinPhysicalPosition( pin ) == aPosition )
                     break;
+                }
             }
 
             if( pin )

eeschema/sch_symbol.cpp

@@ -353,10 +353,10 @@ void SCH_SYMBOL::UpdatePins()
 
     unsigned i = 0;
 
-    for( LIB_PIN* libPin = m_part->GetNextPin(); libPin; libPin = m_part->GetNextPin( libPin ) )
+    std::vector<LIB_PIN*> pins = m_part->GetAllLibPins();
-    {
-        wxASSERT( libPin->Type() == LIB_PIN_T );
 
+    for( LIB_PIN* libPin : pins )
+    {
         // NW: Don't filter by unit: this data-structure is used for all instances,
         // some if which might have different units.
         if( libPin->GetConvert() && m_convert && m_convert != libPin->GetConvert() )

eeschema/tools/symbol_editor_drawing_tools.cpp

@@ -618,10 +618,15 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::RepeatDrawItem( const TOOL_EVENT& aEvent )
         return 0;
 
     // See if we have a pin matching our weak ptr
-    for( LIB_PIN* test = symbol->GetNextPin();  test;  test = symbol->GetNextPin( test ) )
+    std::vector<LIB_PIN*> pins = symbol->GetAllLibPins();
+
+    for( LIB_PIN* test : pins )
     {
         if( (void*) test == g_lastPinWeakPtr )
+        {
             sourcePin = test;
+            break;
+        }
     }
 
     if( sourcePin )

eeschema/tools/symbol_editor_edit_tool.cpp

@@ -285,13 +285,10 @@ int SYMBOL_EDITOR_EDIT_TOOL::DoDelete( const TOOL_EVENT& aEvent )
                 int curr_convert = pin->GetConvert();
                 ELECTRICAL_PINTYPE etype = pin->GetType();
                 wxString name = pin->GetName();
-                LIB_PIN* next_pin = symbol->GetNextPin();
+                std::vector<LIB_PIN*> pins = symbol->GetAllLibPins();
 
-                while( next_pin != nullptr )
+                for( LIB_PIN* pin : pins )
                 {
-                    pin = next_pin;
-                    next_pin = symbol->GetNextPin( pin );
-
                     if( got_unit[pin->GetUnit()] )
                         continue;
 

eeschema/tools/symbol_editor_move_tool.cpp

@@ -164,8 +164,9 @@ int SYMBOL_EDITOR_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
 
                             got_unit[cur_pin->GetUnit()] = true;
 
-                            for( LIB_PIN* pin = symbol->GetNextPin(); pin;
+                            std::vector<LIB_PIN*> pins = symbol->GetAllLibPins();
-                                 pin = symbol->GetNextPin( pin ) )
+
+                            for( LIB_PIN* pin : pins )
                             {
                                 if( !got_unit[pin->GetUnit()]
                                  && pin->GetPosition() == cur_pin->GetPosition()

eeschema/tools/symbol_editor_pin_tool.cpp

@@ -221,7 +221,9 @@ bool SYMBOL_EDITOR_PIN_TOOL::PlacePin( LIB_PIN* aPin )
     LIB_SYMBOL* symbol = m_frame->GetCurSymbol();
     bool        ask_for_pin = true;   // Test for another pin in same position in other units
 
-    for( LIB_PIN* test = symbol->GetNextPin(); test; test = symbol->GetNextPin( test ) )
+    std::vector<LIB_PIN*> pins = symbol->GetAllLibPins();
+
+    for( LIB_PIN* test : pins )
     {
         if( test == aPin || aPin->GetPosition() != test->GetPosition() || test->GetEditFlags() )
             continue;
@@ -270,7 +272,7 @@ bool SYMBOL_EDITOR_PIN_TOOL::PlacePin( LIB_PIN* aPin )
     }
 
     // Put linked pins in new position, and clear flags
-    for( LIB_PIN* pin = symbol->GetNextPin();  pin;  pin = symbol->GetNextPin( pin ) )
+    for( LIB_PIN* pin : pins )
     {
         if( ( pin->GetEditFlags() & IS_LINKED ) == 0 )
             continue;
@@ -383,8 +385,9 @@ int SYMBOL_EDITOR_PIN_TOOL::PushPinProperties( const TOOL_EVENT& aEvent )
         return 0;
 
     saveCopyInUndoList( symbol, UNDO_REDO::LIBEDIT );
+    std::vector<LIB_PIN*> pins = symbol->GetAllLibPins();
 
-    for( LIB_PIN* pin = symbol->GetNextPin();  pin;  pin = symbol->GetNextPin( pin ) )
+    for( LIB_PIN* pin : pins )
     {
         if( pin == sourcePin )
             continue;

qa/unittests/eeschema/test_lib_part.cpp

@@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE( DefaultDrawings )
 {
     // default drawings exist
     BOOST_CHECK_EQUAL( m_part_no_data.GetDrawItems().size(), 4 );
-    BOOST_CHECK_EQUAL( m_part_no_data.GetNextDrawItem( nullptr, LIB_PIN_T ), nullptr );
+    BOOST_CHECK_EQUAL( m_part_no_data.GetAllLibPins().size(), 0 );
 }
 
 
@@ -413,22 +413,22 @@ BOOST_AUTO_TEST_CASE( Compare )
     testPart.AddDrawItem( new LIB_SHAPE( &testPart, SHAPE_T::RECT ) );
     m_part_no_data.AddDrawItem( new LIB_SHAPE( &m_part_no_data, SHAPE_T::RECT ) );
     BOOST_CHECK_EQUAL( m_part_no_data.Compare( testPart ), 0 );
-    m_part_no_data.RemoveDrawItem( m_part_no_data.GetNextDrawItem( nullptr, LIB_SHAPE_T ) );
+    m_part_no_data.RemoveDrawItem( &m_part_no_data.GetDrawItems()[LIB_SHAPE_T].front() );
     BOOST_CHECK( m_part_no_data.Compare( testPart ) < 0 );
-    testPart.RemoveDrawItem( testPart.GetNextDrawItem( nullptr, LIB_SHAPE_T ) );
+    testPart.RemoveDrawItem( &testPart.GetDrawItems()[LIB_SHAPE_T].front() );
     m_part_no_data.AddDrawItem( new LIB_SHAPE( &m_part_no_data, SHAPE_T::RECT ) );
     BOOST_CHECK( m_part_no_data.Compare( testPart ) > 0 );
-    m_part_no_data.RemoveDrawItem( m_part_no_data.GetNextDrawItem( nullptr, LIB_SHAPE_T ) );
+    m_part_no_data.RemoveDrawItem( &m_part_no_data.GetDrawItems()[LIB_SHAPE_T].front() );
 
     // Draw item list contents comparison tests.
     testPart.AddDrawItem( new LIB_SHAPE( &testPart, SHAPE_T::RECT ) );
     m_part_no_data.AddDrawItem( new LIB_SHAPE( &m_part_no_data, SHAPE_T::ARC ) );
     BOOST_CHECK( m_part_no_data.Compare( testPart ) > 0 );
-    m_part_no_data.RemoveDrawItem( m_part_no_data.GetNextDrawItem( nullptr, LIB_SHAPE_T ) );
+    m_part_no_data.RemoveDrawItem( &m_part_no_data.GetDrawItems()[LIB_SHAPE_T].front() );
     m_part_no_data.AddDrawItem( new LIB_PIN( &m_part_no_data ) );
     BOOST_CHECK( m_part_no_data.Compare( testPart ) > 0 );
-    m_part_no_data.RemoveDrawItem( m_part_no_data.GetNextDrawItem( nullptr, LIB_PIN_T ) );
+    m_part_no_data.RemoveDrawItem( &m_part_no_data.GetDrawItems()[LIB_PIN_T].front() );
-    testPart.RemoveDrawItem( testPart.GetNextDrawItem( nullptr, LIB_SHAPE_T ) );
+    testPart.RemoveDrawItem( &testPart.GetDrawItems()[LIB_SHAPE_T].front() );
 
     // Footprint filter array comparison tests.
     wxArrayString footPrintFilters;
@@ -559,7 +559,7 @@ BOOST_AUTO_TEST_CASE( GetUnitItems )
 
     m_part_no_data.RemoveDrawItem( pin2 );
     m_part_no_data.RemoveDrawItem( pin1 );
-    m_part_no_data.RemoveDrawItem( m_part_no_data.GetNextDrawItem() );
+    m_part_no_data.RemoveDrawItem( &*m_part_no_data.GetDrawItems().begin() );
 }