From 9029b7cdb153816fd00dc8877b258010d9534b2d Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Sun, 10 Dec 2017 19:28:34 -0800 Subject: [PATCH] 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. --- eeschema/sch_component.cpp | 144 ++++++++++++++++++--------------------------- eeschema/sch_component.h | 14 +---- 2 files changed, 59 insertions(+), 99 deletions(-) diff --git a/eeschema/sch_component.cpp b/eeschema/sch_component.cpp index d951a8c86..f4c3f4223 100644 --- a/eeschema/sch_component.cpp +++ b/eeschema/sch_component.cpp @@ -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 ); @@ -1469,6 +1470,8 @@ bool SCH_COMPONENT::Matches( wxFindReplaceData& aSearchData, void* aAuxData, void SCH_COMPONENT::GetEndPoints( std::vector & aItemList ) { + m_Pins.clear(); + if( PART_SPTR part = m_part.lock() ) { for( LIB_PIN* pin = part->GetNextPin(); pin; pin = part->GetNextPin( pin ) ) @@ -1483,72 +1486,71 @@ void SCH_COMPONENT::GetEndPoints( std::vector & aItemList ) DANGLING_END_ITEM item( PIN_END, pin, GetPinPhysicalPosition( pin ), this ); aItemList.push_back( item ); + + m_Pins.push_back( pin->GetPosition() ); } } } -bool SCH_COMPONENT::IsPinDanglingStateChanged( std::vector &aItemList, - LIB_PINS& aLibPins, unsigned aPin ) +bool SCH_COMPONENT::IsDanglingStateChanged( std::vector& aItemList ) { - 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] ); + bool changed = false; - for( DANGLING_END_ITEM& each_item : aItemList ) + for( size_t i = 0; i < m_Pins.size(); ++i ) { - // 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; + bool previousState; + wxPoint pos = m_transform.TransformCoordinate( m_Pins[ i ] ) + m_Pos; - switch( each_item.GetType() ) + if( i < m_isDangling.size() ) { - 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; + previousState = m_isDangling[ i ]; + m_isDangling[ i ] = true; + } + else + { + previousState = true; + m_isDangling.push_back( true ); } - if( !m_isDangling[aPin] ) - break; - } - return previousState != m_isDangling[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: -bool SCH_COMPONENT::IsDanglingStateChanged( std::vector& 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 ) - { - if( IsPinDanglingStateChanged( aItemList, libPins, i ) ) - changed = true; + 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 +1592,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 ); } @@ -1816,6 +1796,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 +1843,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(); } diff --git a/eeschema/sch_component.h b/eeschema/sch_component.h index 788223c86..2e8bacc4e 100644 --- a/eeschema/sch_component.h +++ b/eeschema/sch_component.h @@ -93,6 +93,7 @@ private: PART_REF m_part; ///< points into the PROJECT's libraries to the LIB_PART for this component std::vector m_isDangling; ///< One isDangling per pin + std::vector m_Pins; AUTOPLACED m_fieldsAutoplaced; ///< indicates status of field autoplacement @@ -530,19 +531,6 @@ public: void GetEndPoints( std::vector& 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& aItemList, - LIB_PINS& aLibPins, unsigned aPin ); - - /** * Test if the component's dangling state has changed for all pins. * * As a side effect, actually update the dangling status for all pins. -- 2.11.0