Optimizing speed of sch_component pin references

Pins are weak_ptrs to the library, so they require a lock() before
accessing.  This imposes a performance hit on fast loops that access
the pin list repeatedly.  This patch caches the pin position locally
for each component, updating only when needed.

Fixes: lp:1737363
* https://bugs.launchpad.net/kicad/+bug/1737363
This commit is contained in:
Seth Hillbrand 2017-12-10 19:28:34 -08:00 committed by Wayne Stambaugh
parent 0871719cc9
commit 8e764f85a4
3 changed files with 169 additions and 109 deletions

View File

@ -172,6 +172,7 @@ SCH_COMPONENT::SCH_COMPONENT( const SCH_COMPONENT& aComponent ) :
m_convert = aComponent.m_convert;
m_lib_id = aComponent.m_lib_id;
m_part = aComponent.m_part;
m_Pins = aComponent.m_Pins;
SetTimeStamp( aComponent.m_TimeStamp );
@ -376,6 +377,7 @@ void SCH_COMPONENT::ResolveAll( const SCH_COLLECTOR& aComponents, PART_LIBS* aLi
SCH_COMPONENT* cmp = cmp_list[ii];
curr_libid = cmp->m_lib_id;
cmp->Resolve( aLibs );
cmp->UpdatePinCache();
// Propagate the m_part pointer to other members using the same lib_id
for( unsigned jj = ii+1; jj < cmp_list.size (); ++jj )
@ -386,6 +388,9 @@ void SCH_COMPONENT::ResolveAll( const SCH_COLLECTOR& aComponents, PART_LIBS* aLi
break;
next_cmp->m_part = cmp->m_part;
// Propagate the pin cache vector as well
next_cmp->m_Pins = cmp->m_Pins;
ii = jj;
}
}
@ -416,6 +421,7 @@ void SCH_COMPONENT::ResolveAll( const SCH_COLLECTOR& aComponents, SYMBOL_LIB_TAB
SCH_COMPONENT* cmp = cmp_list[ii];
curr_libid = cmp->m_lib_id;
cmp->Resolve( aLibTable, aCacheLib );
cmp->UpdatePinCache();
// Propagate the m_part pointer to other members using the same lib_id
for( unsigned jj = ii+1; jj < cmp_list.size (); ++jj )
@ -426,6 +432,75 @@ void SCH_COMPONENT::ResolveAll( const SCH_COLLECTOR& aComponents, SYMBOL_LIB_TAB
break;
next_cmp->m_part = cmp->m_part;
// Propagate the pin cache vector as well
next_cmp->m_Pins = cmp->m_Pins;
ii = jj;
}
}
}
void SCH_COMPONENT::UpdatePinCache()
{
if( PART_SPTR part = m_part.lock() )
{
m_Pins.clear();
for( LIB_PIN* pin = part->GetNextPin(); pin; pin = part->GetNextPin( pin ) )
{
wxASSERT( pin->Type() == LIB_PIN_T );
if( pin->GetUnit() && m_unit && ( m_unit != pin->GetUnit() ) )
continue;
if( pin->GetConvert() && m_convert && ( m_convert != pin->GetConvert() ) )
continue;
m_Pins.push_back( pin->GetPosition() );
}
}
}
void SCH_COMPONENT::UpdateAllPinCaches( const SCH_COLLECTOR& aComponents )
{
// Usually, many components use the same part lib.
// to avoid too long calculation time the list of components is grouped
// and once the lib part is found for one member of a group, it is also
// set for all other members of this group
std::vector<SCH_COMPONENT*> cmp_list;
// build the cmp list.
for( int i = 0; i < aComponents.GetCount(); ++i )
{
SCH_COMPONENT* cmp = dynamic_cast<SCH_COMPONENT*>( aComponents[i] );
wxASSERT( cmp );
if( cmp ) // cmp == NULL should not occur.
cmp_list.push_back( cmp );
}
// sort it by lib part. Cmp will be grouped by same lib part.
std::sort( cmp_list.begin(), cmp_list.end(), sort_by_libid );
LIB_ID curr_libid;
for( unsigned ii = 0; ii < cmp_list.size (); ++ii )
{
SCH_COMPONENT* cmp = cmp_list[ii];
curr_libid = cmp->m_lib_id;
cmp->UpdatePinCache();
// Propagate the m_Pins vector to other members using the same lib_id
for( unsigned jj = ii+1; jj < cmp_list.size (); ++jj )
{
SCH_COMPONENT* next_cmp = cmp_list[jj];
if( curr_libid != next_cmp->m_lib_id )
break;
// Propagate the pin cache vector as well
next_cmp->m_Pins = cmp->m_Pins;
ii = jj;
}
}
@ -1488,67 +1563,64 @@ void SCH_COMPONENT::GetEndPoints( std::vector <DANGLING_END_ITEM>& aItemList )
}
bool SCH_COMPONENT::IsPinDanglingStateChanged( std::vector<DANGLING_END_ITEM> &aItemList,
LIB_PINS& aLibPins, unsigned aPin )
{
bool previousState;
if( aPin < m_isDangling.size() )
{
previousState = m_isDangling[aPin];
m_isDangling[aPin] = true;
}
else
{
previousState = true;
m_isDangling.push_back( true );
}
wxPoint pin_position = GetPinPhysicalPosition( aLibPins[aPin] );
for( DANGLING_END_ITEM& each_item : aItemList )
{
// Some people like to stack pins on top of each other in a symbol to indicate
// internal connection. While technically connected, it is not particularly useful
// to display them that way, so skip any pins that are in the same symbol as this
// one.
if( each_item.GetParent() == this )
continue;
switch( each_item.GetType() )
{
case PIN_END:
case LABEL_END:
case SHEET_LABEL_END:
case WIRE_START_END:
case WIRE_END_END:
case NO_CONNECT_END:
case JUNCTION_END:
if( pin_position == each_item.GetPosition() )
m_isDangling[aPin] = false;
break;
default:
break;
}
if( !m_isDangling[aPin] )
break;
}
return previousState != m_isDangling[aPin];
}
bool SCH_COMPONENT::IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>& aItemList )
{
bool changed = false;
LIB_PINS libPins;
if( PART_SPTR part = m_part.lock() )
part->GetPins( libPins, m_unit, m_convert );
for( size_t i = 0; i < libPins.size(); ++i )
for( size_t i = 0; i < m_Pins.size(); ++i )
{
if( IsPinDanglingStateChanged( aItemList, libPins, i ) )
changed = true;
bool previousState;
wxPoint pos = m_transform.TransformCoordinate( m_Pins[ i ] ) + m_Pos;
if( i < m_isDangling.size() )
{
previousState = m_isDangling[ i ];
m_isDangling[ i ] = true;
}
else
{
previousState = true;
m_isDangling.push_back( true );
}
for( DANGLING_END_ITEM& each_item : aItemList )
{
// Some people like to stack pins on top of each other in a symbol to indicate
// internal connection. While technically connected, it is not particularly useful
// to display them that way, so skip any pins that are in the same symbol as this
// one.
if( each_item.GetParent() == this )
continue;
switch( each_item.GetType() )
{
case PIN_END:
case LABEL_END:
case SHEET_LABEL_END:
case WIRE_START_END:
case WIRE_END_END:
case NO_CONNECT_END:
case JUNCTION_END:
if( pos == each_item.GetPosition() )
m_isDangling[ i ] = false;
break;
default:
break;
}
if( !m_isDangling[ i ] )
break;
}
changed = ( changed || ( previousState != m_isDangling[ i ] ) );
}
while( m_isDangling.size() > m_Pins.size() )
m_isDangling.pop_back();
return changed;
}
@ -1590,30 +1662,8 @@ bool SCH_COMPONENT::IsSelectStateChanged( const wxRect& aRect )
void SCH_COMPONENT::GetConnectionPoints( std::vector< wxPoint >& aPoints ) const
{
if( PART_SPTR part = m_part.lock() )
{
for( LIB_PIN* pin = part->GetNextPin(); pin; pin = part->GetNextPin( pin ) )
{
wxCHECK_RET( pin->Type() == LIB_PIN_T,
wxT( "GetNextPin() did not return a pin object. Bad programmer!" ) );
// Skip items not used for this part.
if( m_unit && pin->GetUnit() && ( pin->GetUnit() != m_unit ) )
continue;
if( m_convert && pin->GetConvert() && ( pin->GetConvert() != m_convert ) )
continue;
// Calculate the pin position relative to the component position and orientation.
aPoints.push_back( m_transform.TransformCoordinate( pin->GetPosition() ) + m_Pos );
}
}
else
{
wxCHECK_RET( 0,
wxT( "Cannot add connection points to list. Cannot find component <" ) +
GetLibId().GetLibItemName() + wxT( "> in any of the loaded libraries." ) );
}
for( auto pin : m_Pins )
aPoints.push_back( m_transform.TransformCoordinate( pin ) + m_Pos );
}
@ -1621,6 +1671,22 @@ LIB_ITEM* SCH_COMPONENT::GetDrawItem( const wxPoint& aPosition, KICAD_T aType )
{
if( PART_SPTR part = m_part.lock() )
{
m_Pins.clear();
for( LIB_PIN* pin = part->GetNextPin(); pin; pin = part->GetNextPin( pin ) )
{
wxASSERT( pin->Type() == LIB_PIN_T );
if( pin->GetUnit() && m_unit && ( m_unit != pin->GetUnit() ) )
continue;
if( pin->GetConvert() && m_convert && ( m_convert != pin->GetConvert() ) )
continue;
m_Pins.push_back( pin->GetPosition() );
}
// Calculate the position relative to the component.
wxPoint libPosition = aPosition - m_Pos;
@ -1816,6 +1882,7 @@ SCH_ITEM& SCH_COMPONENT::operator=( const SCH_ITEM& aItem )
m_unit = c->m_unit;
m_convert = c->m_convert;
m_transform = c->m_transform;
m_Pins = c->m_Pins;
m_PathsAndReferences = c->m_PathsAndReferences;
@ -1862,17 +1929,8 @@ bool SCH_COMPONENT::HitTest( const EDA_RECT& aRect, bool aContained, int aAccura
bool SCH_COMPONENT::doIsConnected( const wxPoint& aPosition ) const
{
std::vector< wxPoint > pts;
GetConnectionPoints( pts );
for( size_t i = 0; i < pts.size(); i++ )
{
if( pts[i] == aPosition )
return true;
}
return false;
wxPoint new_pos = m_transform.InverseTransform().TransformCoordinate( aPosition - m_Pos );
return std::find( m_Pins.begin(), m_Pins.end(), new_pos ) != m_Pins.end();
}

View File

@ -93,6 +93,7 @@ private:
PART_REF m_part; ///< points into the PROJECT's libraries to the LIB_PART for this component
std::vector<bool> m_isDangling; ///< One isDangling per pin
std::vector<wxPoint> m_Pins;
AUTOPLACED m_fieldsAutoplaced; ///< indicates status of field autoplacement
@ -198,6 +199,18 @@ public:
int GetUnit() const { return m_unit; }
/**
* Updates the local cache of pin positions
*/
void UpdatePinCache();
/**
* Update the pin cache for all components in \a aComponents
*
* @param aComponents collector of components in screen
*/
static void UpdateAllPinCaches( const SCH_COLLECTOR& aComponents );
/**
* Change the unit number to \a aUnit
*
@ -529,19 +542,6 @@ public:
void GetEndPoints( std::vector<DANGLING_END_ITEM>& aItemList ) override;
/**
* Test if the symbol's dangling state has changed for one given pin index.
*
* As a side effect, it actually updates the dangling status for that pin.
*
* @param aItemList is list of all #DANGLING_END_ITEM items to be tested.
* @param aLibPins is list of all the #LIB_PIN items in this symbol
* @param aPin is the index into \a aLibPins that identifies the pin to test
* @return true if the pin's state has changed.
*/
bool IsPinDanglingStateChanged( std::vector<DANGLING_END_ITEM>& aItemList,
LIB_PINS& aLibPins, unsigned aPin );
/**
* Test if the component's dangling state has changed for all pins.
*

View File

@ -484,23 +484,25 @@ void SCH_SCREEN::UpdateSymbolLinks( bool aForce )
// Initialize or reinitialize the pointer to the LIB_PART for each component
// found in m_drawList, but only if needed (change in lib or schematic)
// therefore the calculation time is usually very low.
if( m_drawList.GetCount() )
{
SYMBOL_LIB_TABLE* libs = Prj().SchSymbolLibTable();
int mod_hash = libs->GetModifyHash();
SCH_TYPE_COLLECTOR c;
c.Collect( GetDrawItems(), SCH_COLLECTOR::ComponentsOnly );
// Must we resolve?
if( (m_modification_sync != mod_hash) || aForce )
{
SCH_TYPE_COLLECTOR c;
c.Collect( GetDrawItems(), SCH_COLLECTOR::ComponentsOnly );
SCH_COMPONENT::ResolveAll( c, *libs, Prj().SchLibs()->GetCacheLibrary() );
m_modification_sync = mod_hash; // note the last mod_hash
}
// Resolving will update the pin caches but we must ensure that this happens
// even if the libraries don't change.
else
SCH_COMPONENT::UpdateAllPinCaches( c );
}
}