pcbnew: Standardize zone fills threading

This is a boyscouting commit to standardize the threading of zone fills.
We do not need to join threads after their completion, instead we simply
allow them to clean up their memory without blocking the user.  This
also sets the maximum number of threads that may be created to the
number of zones being filled.  More than this will only leave un-used
threads being created and immediately killed.

We also include the connectivity search as a phase in the fill progress
reporter.  This was the case before but did not utilize the correct
maxsize, leading to stalled progress bar.

pcbnew/connectivity_algo.cpp

@@ -324,17 +324,19 @@ void CN_CONNECTIVITY_ALGO::searchConnections()
     PROF_COUNTER search_basic( "search-basic" );
 #endif
 
+    size_t numDirty = std::count_if( m_itemList.begin(), m_itemList.end(), [] ( CN_ITEM* aItem )
+            { return aItem->Dirty(); } );
+
     if( m_progressReporter )
     {
-        m_progressReporter->SetMaxProgress( m_itemList.IsDirty() ? m_itemList.Size() : 0 );
+        m_progressReporter->SetMaxProgress( numDirty );
+        m_progressReporter->KeepRefreshing();
     }
 
     if( m_itemList.IsDirty() )
     {
         std::atomic<int> nextItem( 0 );
         std::atomic<size_t> threadsFinished( 0 );
-        size_t numDirty = std::count_if( m_itemList.begin(), m_itemList.end(), [] ( CN_ITEM* aItem )
-                { return aItem->Dirty(); } );
 
         size_t parallelThreadCount = std::min<size_t>(
                 std::max<size_t>( std::thread::hardware_concurrency(), 2 ),
@@ -353,11 +355,11 @@ void CN_CONNECTIVITY_ALGO::searchConnections()
                     {
                         CN_VISITOR visitor( item, &m_listLock );
                         m_itemList.FindNearby( item, visitor );
-                    }
 
                         if( m_progressReporter )
                             m_progressReporter->AdvanceProgress();
                     }
+                }
 
                 threadsFinished++;
             } );

pcbnew/tools/zone_filler_tool.cpp

@@ -90,7 +90,7 @@ int ZONE_FILLER_TOOL::ZoneFill( const TOOL_EVENT& aEvent )
     }
 
     std::unique_ptr<WX_PROGRESS_REPORTER> progressReporter(
-            new WX_PROGRESS_REPORTER( frame(), _( "Fill Zone" ), 3 )
+            new WX_PROGRESS_REPORTER( frame(), _( "Fill Zone" ), 4 )
             );
 
     ZONE_FILLER filler( board(), &commit );
@@ -113,7 +113,7 @@ int ZONE_FILLER_TOOL::ZoneFillAll( const TOOL_EVENT& aEvent )
     }
 
     std::unique_ptr<WX_PROGRESS_REPORTER> progressReporter(
-            new WX_PROGRESS_REPORTER( frame(), _( "Fill All Zones" ), 3 )
+            new WX_PROGRESS_REPORTER( frame(), _( "Fill All Zones" ), 4 )
             );
 
     ZONE_FILLER filler( board(), &commit );

pcbnew/zone_filler.cpp

@@ -63,8 +63,7 @@ static double s_thermalRot = 450;    // angle of stubs in thermal reliefs for ro
 static const bool s_DumpZonesWhenFilling = false;
 
 ZONE_FILLER::ZONE_FILLER(  BOARD* aBoard, COMMIT* aCommit ) :
-    m_board( aBoard ), m_commit( aCommit ), m_progressReporter( nullptr ),
+    m_board( aBoard ), m_commit( aCommit ), m_progressReporter( nullptr )
-    m_next( 0 ), m_count_done( 0 )
 {
 }
 
@@ -81,8 +80,6 @@ void ZONE_FILLER::SetProgressReporter( WX_PROGRESS_REPORTER* aReporter )
 
 bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
 {
-    int parallelThreadCount = std::max( ( int )std::thread::hardware_concurrency(), 2 );
-
     std::vector<CN_ZONE_ISOLATED_ISLAND_LIST> toFill;
     auto connectivity = m_board->GetConnectivity();
 
@@ -112,16 +109,20 @@ bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
         m_progressReporter->SetMaxProgress( toFill.size() );
     }
 
-    m_next = 0;
-    m_count_done = 0;
-    std::vector<std::thread> fillWorkers;
 
-    for( ssize_t ii = 0; ii < parallelThreadCount; ++ii )
+    std::atomic<size_t> nextItem( 0 );
+    std::atomic<size_t> threadsFinished( 0 );
+    size_t parallelThreadCount = std::min<size_t>(
+            std::max<size_t>( std::thread::hardware_concurrency(), 2 ),
+            toFill.size() );
+
+    for( size_t ii = 0; ii < parallelThreadCount; ++ii )
     {
-        fillWorkers.push_back( std::thread( [ this, toFill ]()
+        std::thread t = std::thread( [ & ]()
         {
-            size_t i = m_next.fetch_add( 1 );
+            for( size_t i = nextItem.fetch_add( 1 );
-            while( i < toFill.size() )
+                    i < toFill.size();
+                    i = nextItem.fetch_add( 1 ) )
             {
                 SHAPE_POLY_SET rawPolys, finalPolys;
                 ZONE_CONTAINER* zone = toFill[i].m_zone;
@@ -133,25 +134,25 @@ bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
 
                 if( m_progressReporter )
                     m_progressReporter->AdvanceProgress();
-
-                m_count_done.fetch_add( 1 );
-                i = m_next.fetch_add( 1 );
-            }
-        } ) );
             }
 
-    while( m_count_done.load() < toFill.size() )
+            threadsFinished++;
+        } );
+
+        t.detach();
+    }
+
+
+    // Finalize the triangulation threads
+    while( threadsFinished < parallelThreadCount )
     {
         if( m_progressReporter )
             m_progressReporter->KeepRefreshing();
 
-        wxMilliSleep( 20 );
+        std::this_thread::sleep_for( std::chrono::milliseconds( 10 ) );
     }
 
-    for( size_t ii = 0; ii < fillWorkers.size(); ++ii )
+    // Now update the connectivity to check for copper islands
-        fillWorkers[ ii ].join();
-
-    // Now remove insulated copper islands
     if( m_progressReporter )
     {
         m_progressReporter->AdvancePhase();
@@ -162,6 +163,14 @@ bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
     connectivity->SetProgressReporter( m_progressReporter );
     connectivity->FindIsolatedCopperIslands( toFill );
 
+    // Now remove insulated copper islands
+    if( m_progressReporter )
+    {
+        m_progressReporter->AdvancePhase();
+        m_progressReporter->SetMaxProgress( toFill.size() );
+        m_progressReporter->KeepRefreshing();
+    }
+
     bool outOfDate = false;
 
     for( auto& zone : toFill )
@@ -178,6 +187,12 @@ bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
 
         if( aCheck && zone.m_lastPolys.GetHash() != poly.GetHash() )
             outOfDate = true;
+
+        if( m_progressReporter )
+        {
+            m_progressReporter->AdvanceProgress();
+            m_progressReporter->KeepRefreshing();
+        }
     }
 
     if( aCheck )
@@ -211,9 +226,6 @@ bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
         }
     }
 
-    // Remove deprecaded segment zones (only found in very old boards)
-    m_board->m_SegZoneDeprecated.DeleteAll();
-
     if( m_progressReporter )
     {
         m_progressReporter->AdvancePhase();
@@ -221,82 +233,104 @@ bool ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones, bool aCheck )
         m_progressReporter->SetMaxProgress( toFill.size() );
     }
 
-    m_next = 0;
-    m_count_done = 0;
-    std::vector<std::thread> triangulationWorkers;
 
-    for( ssize_t ii = 0; ii < parallelThreadCount; ++ii )
+    nextItem = 0;
+    threadsFinished = 0;
+    for( size_t ii = 0; ii < parallelThreadCount; ++ii )
     {
-        triangulationWorkers.push_back( std::thread( [ this, toFill ]()
+        std::thread t = std::thread( [ & ]()
         {
-            size_t i = m_next.fetch_add( 1 );
+            for( size_t i = nextItem.fetch_add( 1 );
-            while( i < toFill.size() )
+                    i < toFill.size();
+                    i = nextItem.fetch_add( 1 ) )
             {
-                if( m_progressReporter )
-                    m_progressReporter->AdvanceProgress();
-
                 toFill[i].m_zone->CacheTriangulation();
 
-                m_count_done.fetch_add( 1 );
+                if( m_progressReporter )
-                i = m_next.fetch_add( 1 );
+                     m_progressReporter->AdvanceProgress();
-            }
-        } ) );
              }
 
-    while( m_count_done.load() < toFill.size() )
+             threadsFinished++;
+         } );
+
+         t.detach();
+     }
+
+
+     // Finalize the triangulation threads
+     while( threadsFinished < parallelThreadCount )
      {
          if( m_progressReporter )
              m_progressReporter->KeepRefreshing();
 
-        wxMilliSleep( 10 );
+         std::this_thread::sleep_for( std::chrono::milliseconds( 10 ) );
      }
 
-    for( size_t ii = 0; ii < triangulationWorkers.size(); ++ii )
+
-        triangulationWorkers[ ii ].join();
+    // Remove deprecaded segment zones (only found in very old boards)
+    m_board->m_SegZoneDeprecated.DeleteAll();
 
     // If some zones must be filled by segments, create the filling segments
     // (note, this is a outdated option, but it exists)
-    int zones_to_fill_count = 0;
+    if( int zone_count = std::count_if( toFill.begin(), toFill.end(),
-
+            []( CN_ZONE_ISOLATED_ISLAND_LIST& aList )
-    for( unsigned i = 0; i < toFill.size(); i++ )
+            { return aList.m_zone->GetFillMode() == ZFM_SEGMENTS; } ) > 0 )
-    {
-        if( toFill[i].m_zone->GetFillMode() == ZFM_SEGMENTS )
-            zones_to_fill_count++;
-    }
-
-    if( zones_to_fill_count )
     {
         if( m_progressReporter )
         {
             m_progressReporter->AdvancePhase();
             m_progressReporter->Report( _( "Performing segment fills..." ) );
-            m_progressReporter->SetMaxProgress( zones_to_fill_count );
+            m_progressReporter->SetMaxProgress( zone_count );
         }
 
-        // TODO: use thread pool to speedup calculations:
+        parallelThreadCount = std::min<size_t>( static_cast<size_t>( zone_count ), parallelThreadCount );
-        for( unsigned i = 0; i < toFill.size(); i++ )
+        nextItem = 0;
+        threadsFinished = 0;
+        for( size_t ii = 0; ii < parallelThreadCount; ++ii )
+        {
+            std::thread t = std::thread( [ & ]()
+            {
+                for( size_t i = nextItem.fetch_add( 1 );
+                        i < toFill.size();
+                        i = nextItem.fetch_add( 1 ) )
                 {
                     ZONE_CONTAINER* zone = toFill[i].m_zone;
 
-            if( zone->GetFillMode() != ZFM_SEGMENTS )
+                    if( zone->GetFillMode() == ZFM_SEGMENTS )
-                continue;
-
-            if( m_progressReporter )
                     {
-                m_progressReporter->AdvanceProgress();
-            }
-
                         ZONE_SEGMENT_FILL segFill;
 
                         fillZoneWithSegments( zone, zone->GetFilledPolysList(), segFill );
-            toFill[i].m_zone->SetFillSegments( segFill );
+                        zone->SetFillSegments( segFill );
+
+                        if( m_progressReporter )
+                             m_progressReporter->AdvanceProgress();
                     }
                  }
 
+                 threadsFinished++;
+             } );
+
+             t.detach();
+         }
+
+
+         // Finalize the triangulation threads
+         while( threadsFinished < parallelThreadCount )
+         {
+             if( m_progressReporter )
+                 m_progressReporter->KeepRefreshing();
+
+             std::this_thread::sleep_for( std::chrono::milliseconds( 10 ) );
+         }
+    }
+
+
     if( m_progressReporter )
     {
         m_progressReporter->AdvancePhase();
         m_progressReporter->Report( _( "Committing changes..." ) );
+        m_progressReporter->KeepRefreshing();
     }
 
     connectivity->SetProgressReporter( nullptr );

pcbnew/zone_filler.h

@@ -118,11 +118,6 @@ private:
     BOARD* m_board;
     COMMIT* m_commit;
     WX_PROGRESS_REPORTER* m_progressReporter;
-
-    std::atomic_size_t m_next;          // An index into the vector of zones to fill.
-                                        // Used by the variuos parallel thread sets during
-                                        // fill operations.
-    std::atomic_size_t m_count_done;
 };
 
 #endif

pcbnew/zones_by_polygon.cpp

@@ -968,7 +968,7 @@ void PCB_EDIT_FRAME::Edit_Zone_Params( wxDC* DC, ZONE_CONTAINER* aZone )
         wxString title;
         title.Printf( _( "Refill %d Zones" ), (int)zones_to_refill.size() );
         std::unique_ptr<WX_PROGRESS_REPORTER> progressReporter(
-                                new WX_PROGRESS_REPORTER( this, title, 3 ) );
+                                new WX_PROGRESS_REPORTER( this, title, 4 ) );
 
         filler.SetProgressReporter( progressReporter.get() );
         filler.Fill( zones_to_refill );

pcbnew/zones_by_polygon_fill_functions.cpp

@@ -117,7 +117,7 @@ void PCB_EDIT_FRAME::Check_All_Zones( wxWindow* aActiveWindow )
     BOARD_COMMIT commit( this );
 
     std::unique_ptr<WX_PROGRESS_REPORTER> progressReporter(
-            new WX_PROGRESS_REPORTER( aActiveWindow, _( "Checking Zones" ), 3 ) );
+            new WX_PROGRESS_REPORTER( aActiveWindow, _( "Checking Zones" ), 4 ) );
 
     ZONE_FILLER filler( GetBoard(), &commit );
     filler.SetProgressReporter( progressReporter.get() );