kicad-developers team mailing list archive

[jp charras <jp.charras@xxxxxxxxxx>]

Le 02/06/2018 à 11:02, jp charras a écrit :
> I have made a few tests (not a lot!) and did not see any problem.
> 

Hi Seth,

I found an issue due to changes in connectivity algo:
CONNECTIVITY_DATA::GetPadCount( int aNet ) is significantly slower.

And creates an issue in copper zones edition, when the net sorting is made by pad count.
The calculation time is so long it can freeze Pcbnew for large boards.

This is due to a absolutely non optimized sort function (not optimized due to a lot of changes since
it was written) and the increased calculation time in CONNECTIVITY_DATA::GetPadCount due to a
dynamic cast instead of a static cast.

Could you have a look into this patch that fixes this issue?
Thanks

> 
>>
>> On Sat, Jun 2, 2018 at 12:17 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx
>> <mailto:stambaughw@xxxxxxxxx>> wrote:
>>
>>     On 6/1/2018 3:58 PM, Tomasz Wlostowski wrote:
>>     > On 01/06/18 20:23, Seth Hillbrand wrote:
>>     >> I think maybe we are seeing the severity of these bugs differently. 
>>     >> There are three substantial issues addressed by this patch.  While they
>>     >> don't affect many users, they do prevent KiCad from being used for
>>     >> larger, more complex boards that we nominally support.  This is a also
>>     >> regression from v4.
>>     > 
>>     > Hi all,
>>     > 
>>     > I vote for merging Seth's patch.
>>     > 
>>     > For those concerned about its correctness: the connectivity algo is
>>     > designed in such way that in case of bugs in collision search (other
>>     > than in BOARD_ITEM::HitTest()/POLY_GRID_PARTITION which haven't been
>>     > touched for a year), it may find false unconnected items, but not false
>>     > connected ones. In the worst case, we'll have false 'unconnected items'
>>     > warnings in the DRC, but I doubt there will be any. The patch is simple
>>     > and elegant.
>>     > 
>>     > Tom
>>
>>     Tom,
>>
>>     Have you actually tested the patch?  Voting for it doesn't help me make
>>     an informed decision.  What I really need is test feedback.  I'm not
>>     opposed to merging it but I would like as large of a test group as
>>     possible so I have some confidence that it's not going to cause a bunch
>>     of issues this close to the stable release.  I plan on testing it
>>     tomorrow but I am an infinite sample of one.  A few other testers would
>>     be appreciated before I'm willing to sign off on committing it.
>>
>>     Cheers,
>>
>>     Wayne
>>
> 


-- 
Jean-Pierre CHARRAS

 pcbnew/class_board.cpp       | 20 ++++++++++++++++++--
 pcbnew/connectivity_data.cpp |  9 ++++++---
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/pcbnew/class_board.cpp b/pcbnew/class_board.cpp
index 720da1f..4f953de 100644
--- a/pcbnew/class_board.cpp
+++ b/pcbnew/class_board.cpp
@@ -1389,11 +1389,24 @@ MODULE* BOARD::FindModule( const wxString& aRefOrTimeStamp, bool aSearchByTimeSt
 
 
 // Sort nets by decreasing pad count. For same pad count, sort by alphabetic names
+std::map<int, int> padCountList;  // the pad count for each netcode
+
 static bool sortNetsByNodes( const NETINFO_ITEM* a, const NETINFO_ITEM* b )
 {
     auto connectivity = a->GetParent()->GetConnectivity();
-    int countA = connectivity->GetPadCount( a->GetNet() );
-    int countB = connectivity->GetPadCount( b->GetNet() );
+
+    int net = a->GetNet();
+
+    if( padCountList.find( net ) == padCountList.end() )
+        padCountList[net] = connectivity->GetPadCount( net );
+
+    net = b->GetNet();
+
+    if( padCountList.find( net ) == padCountList.end() )
+        padCountList[net] = connectivity->GetPadCount( net );
+
+    int countA = padCountList[a->GetNet()];
+    int countB = padCountList[b->GetNet()];
 
     if( countA == countB )
         return a->GetNetname() < b->GetNetname();
@@ -1426,7 +1439,10 @@ int BOARD::SortedNetnamesList( wxArrayString& aNames, bool aSortbyPadsCount )
 
     // sort the list
     if( aSortbyPadsCount )
+    {
+        padCountList.clear();
         sort( netBuffer.begin(), netBuffer.end(), sortNetsByNodes );
+    }
     else
         sort( netBuffer.begin(), netBuffer.end(), sortNetsByNames );
 
diff --git a/pcbnew/connectivity_data.cpp b/pcbnew/connectivity_data.cpp
index e3ed719..1b37901 100644
--- a/pcbnew/connectivity_data.cpp
+++ b/pcbnew/connectivity_data.cpp
@@ -475,12 +475,15 @@ unsigned int CONNECTIVITY_DATA::GetPadCount( int aNet ) const
 {
     int n = 0;
 
-    for( auto pad : m_connAlgo->ItemList() )
+    for( auto item : m_connAlgo->ItemList() )
     {
-        if( !pad->Valid() )
+        if( !item->Valid() )
             continue;
 
-        auto dpad = dynamic_cast<D_PAD*>( pad->Parent() );
+        auto dpad = static_cast<D_PAD*>( item->Parent() );
+
+        if( dpad->Type() != PCB_PAD_T )
+            continue;
 
         if( dpad && ( aNet < 0 || aNet == dpad->GetNetCode() ) )
         {