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

(cherry picked from commit 6fa2eedb4b)
This commit is contained in:
Seth Hillbrand 2023-01-20 14:12:15 -08:00
parent b2a6f12645
commit 99cfa1fad7
10 changed files with 66 additions and 97 deletions

View File

@ -556,7 +556,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_part->GetNextPin( nullptr ); pin; pin = m_part->GetNextPin( pin ) )
std::vector<LIB_PIN*> pins = m_part->GetAllLibPins();
for( LIB_PIN* pin : pins )
m_pins.push_back( new LIB_PIN( *pin ) );
m_dataModel->RebuildRows( m_pins, m_cbGroup->GetValue() );
@ -573,7 +575,9 @@ bool DIALOG_LIB_EDIT_PIN_TABLE::TransferDataFromWindow()
return false;
// Delete the part's pins
while( LIB_PIN* pin = m_part->GetNextPin( nullptr ) )
std::vector<LIB_PIN*> pins = m_part->GetAllLibPins();
for( LIB_PIN* pin : pins )
m_part->RemoveDrawItem( pin );
// Transfer our pins to the part

View File

@ -732,35 +732,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:
@ -788,6 +759,15 @@ void LIB_SYMBOL::GetPins( LIB_PINS& aList, int aUnit, int aConvert ) const
}
std::vector<LIB_PIN*> LIB_SYMBOL::GetAllLibPins() const
{
std::vector<LIB_PIN*> pinList;
GetPins( pinList, 0, 0 );
return pinList;
}
LIB_PIN* LIB_SYMBOL::GetPin( const wxString& aNumber, int aUnit, int aConvert ) const
{
LIB_PINS pinList;

View File

@ -367,35 +367,10 @@ public:
*/
void RemoveDrawItem( LIB_ITEM* aItem );
/**
* 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.
*
@ -411,6 +386,12 @@ public:
*/
void GetPins( LIB_PINS& aList, int aUnit = 0, int aConvert = 0 ) const;
/**
* Return a list of pin pointers for all units / converts. Used primarily for SPICE where
* we want to treat all unit as a single part.
*/
std::vector<LIB_PIN*> GetAllLibPins() const;
/**
* Return pin object with the requested pin \a aNumber.
*

View File

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

View File

@ -335,10 +335,12 @@ void SCH_SYMBOL::UpdatePins()
unsigned i = 0;
for( LIB_PIN* libPin = m_part->GetNextPin(); libPin; libPin = m_part->GetNextPin( libPin ) )
{
wxASSERT( libPin->Type() == LIB_PIN_T );
std::vector<LIB_PIN*> pins = m_part->GetAllLibPins();
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() ) )
continue;

View File

@ -536,10 +536,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 )

View File

@ -282,30 +282,27 @@ 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* test_pin : pins )
{
pin = next_pin;
next_pin = symbol->GetNextPin( pin );
if( got_unit[pin->GetUnit()] )
if( got_unit[test_pin->GetUnit()] )
continue;
if( pin->GetPosition() != pos )
if( test_pin->GetPosition() != pos )
continue;
if( pin->GetConvert() != curr_convert )
if( test_pin->GetConvert() != curr_convert )
continue;
if( pin->GetType() != etype )
if( test_pin->GetType() != etype )
continue;
if( pin->GetName() != name )
if( test_pin->GetName() != name )
continue;
toDelete.insert( pin );
got_unit[pin->GetUnit()] = true;
toDelete.insert( test_pin );
got_unit[test_pin->GetUnit()] = true;
}
}
}

View File

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

View File

@ -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;

View File

@ -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 );
}
@ -268,22 +268,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 ) );
testPart.RemoveDrawItem( testPart.GetNextDrawItem( nullptr, LIB_SHAPE_T ) );
m_part_no_data.RemoveDrawItem( &m_part_no_data.GetDrawItems()[LIB_PIN_T].front() );
testPart.RemoveDrawItem( &testPart.GetDrawItems()[LIB_SHAPE_T].front() );
// Footprint filter array comparison tests.
wxArrayString footPrintFilters;
@ -414,7 +414,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() );
}