From cd1a5ed6fbe2c98207882243e3d7ba7a3dde6f3e Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 12 Oct 2020 23:59:01 +0100 Subject: [PATCH] Implement progress reporting for DRC RTree. Also fixes a bug in silk-to-mask checking which wasn't checking zones on the mask layer. Also a perfomance fix for the DRC RTree to use a hash table (std::map) instead of a std::set for keeping track of known collisions. Fixes https://gitlab.com/kicad/code/kicad/issues/5851 --- pcbnew/drc/drc_rtree.h | 69 +++++++++++++------ .../drc_test_provider_copper_clearance.cpp | 2 +- .../drc/drc_test_provider_silk_clearance.cpp | 14 ++-- pcbnew/drc/drc_test_provider_silk_to_mask.cpp | 25 +++---- 4 files changed, 69 insertions(+), 41 deletions(-) diff --git a/pcbnew/drc/drc_rtree.h b/pcbnew/drc/drc_rtree.h index ee606f1aae..58e8e0e8b7 100644 --- a/pcbnew/drc/drc_rtree.h +++ b/pcbnew/drc/drc_rtree.h @@ -322,24 +322,35 @@ public: typedef std::pair LAYER_PAIR; + struct PAIR_INFO + { + PAIR_INFO( LAYER_PAIR aPair, ITEM_WITH_SHAPE* aRef, ITEM_WITH_SHAPE* aTest ) : + layerPair( aPair ), + refItem( aRef ), + testItem( aTest ) + { }; + + LAYER_PAIR layerPair; + ITEM_WITH_SHAPE* refItem; + ITEM_WITH_SHAPE* testItem; + }; + int QueryCollidingPairs( DRC_RTREE* aRefTree, std::vector aLayers, std::function aVisitor, - int aMaxClearance ) + int aMaxClearance, + std::function aProgressReporter ) { - // keep track of BOARD_ITEMs pairs that have been already found to collide (some items - // might be build of COMPOUND/triangulated shapes and a single subshape collision - // means we have a hit) - std::set< std::pair> collidingCompounds; + std::vector< PAIR_INFO > pairsToVisit; - for( auto refLayerIter : aLayers ) + for( LAYER_PAIR& refLayerIter : aLayers ) { const PCB_LAYER_ID refLayer = refLayerIter.first; const PCB_LAYER_ID targetLayer = refLayerIter.second; - for( auto refItem : aRefTree->OnLayer( refLayer ) ) + for( ITEM_WITH_SHAPE* refItem : aRefTree->OnLayer( refLayer ) ) { BOX2I box = refItem->shape->BBox(); box.Inflate( aMaxClearance ); @@ -350,30 +361,44 @@ public: auto visit = [&]( ITEM_WITH_SHAPE* aItemToTest ) -> bool { - const std::pair - chkCompoundPair( refItem->parent, aItemToTest->parent ); - - // don't report multiple collisions for compound or triangulated shapes - if( alg::contains( collidingCompounds, chkCompoundPair ) ) - return true; - // don't collide items against themselves if( refLayer == targetLayer && aItemToTest->parent == refItem->parent ) return true; - bool collisionDetected = false; - bool continueSearch = aVisitor( refLayerIter, refItem, aItemToTest, - &collisionDetected ); - - if( collisionDetected ) - collidingCompounds.insert( chkCompoundPair ); - - return continueSearch; + pairsToVisit.emplace_back( refLayerIter, refItem, aItemToTest ); + return true; }; this->m_tree[targetLayer]->Search( min, max, visit ); }; } + + // keep track of BOARD_ITEMs pairs that have been already found to collide (some items + // might be build of COMPOUND/triangulated shapes and a single subshape collision + // means we have a hit) + std::map< std::pair, int> collidingCompounds; + + int progress = 0; + int count = pairsToVisit.size(); + + for( PAIR_INFO& pair : pairsToVisit ) + { + if( !aProgressReporter( progress++, count ) ) + break; + + // don't report multiple collisions for compound or triangulated shapes + if( collidingCompounds.count( { pair.refItem->parent, pair.testItem->parent } ) ) + continue; + + bool collisionDetected = false; + + if( !aVisitor( pair.layerPair, pair.refItem, pair.testItem, &collisionDetected ) ) + break; + + if( collisionDetected ) + collidingCompounds[ { pair.refItem->parent, pair.testItem->parent } ] = 1; + } + return 0; } diff --git a/pcbnew/drc/drc_test_provider_copper_clearance.cpp b/pcbnew/drc/drc_test_provider_copper_clearance.cpp index 9149d9d39b..c6e88028a3 100644 --- a/pcbnew/drc/drc_test_provider_copper_clearance.cpp +++ b/pcbnew/drc/drc_test_provider_copper_clearance.cpp @@ -314,7 +314,7 @@ void DRC_TEST_PROVIDER_COPPER_CLEARANCE::testCopperDrawItem( BOARD_ITEM* aItem ) void DRC_TEST_PROVIDER_COPPER_CLEARANCE::testTrackClearances() { // This is the number of tests between 2 calls to the progress bar - const int delta = m_drcEngine->GetTestTracksAgainstZones() ? 25 : 100; + const int delta = 25; int count = m_board->Tracks().size(); reportAux( "Testing %d tracks...", count ); diff --git a/pcbnew/drc/drc_test_provider_silk_clearance.cpp b/pcbnew/drc/drc_test_provider_silk_clearance.cpp index 8d9cbed1da..57dd444916 100644 --- a/pcbnew/drc/drc_test_provider_silk_clearance.cpp +++ b/pcbnew/drc/drc_test_provider_silk_clearance.cpp @@ -179,8 +179,7 @@ bool DRC_TEST_PROVIDER_SILK_CLEARANCE::Run() return true; }; - forEachGeometryItem( { PCB_SHAPE_T, PCB_FP_SHAPE_T, PCB_TEXT_T, PCB_FP_TEXT_T }, - LSET( 2, F_SilkS, B_SilkS ), addToSilkTree ); + forEachGeometryItem( {}, LSET( 2, F_SilkS, B_SilkS ), addToSilkTree ); forEachGeometryItem( {}, LSET::FrontMask() | LSET::BackMask(), addToTargetTree ); reportAux( _("Testing %d silkscreen features against %d board items."), @@ -204,10 +203,17 @@ bool DRC_TEST_PROVIDER_SILK_CLEARANCE::Run() DRC_RTREE::LAYER_PAIR( B_SilkS, B_CrtYd ), DRC_RTREE::LAYER_PAIR( B_SilkS, B_Fab ), DRC_RTREE::LAYER_PAIR( B_SilkS, B_Cu ), - DRC_RTREE::LAYER_PAIR( B_SilkS, Edge_Cuts ), + DRC_RTREE::LAYER_PAIR( B_SilkS, Edge_Cuts ) }; - targetTree.QueryCollidingPairs( &silkTree, layerPairs, checkClearance, m_largestClearance ); + // This is the number of tests between 2 calls to the progress bar + const int delta = 250; + + targetTree.QueryCollidingPairs( &silkTree, layerPairs, checkClearance, m_largestClearance, + [this]( int aCount, int aSize ) -> bool + { + return reportProgress( aCount, aSize, delta ); + } ); reportRuleStatistics(); diff --git a/pcbnew/drc/drc_test_provider_silk_to_mask.cpp b/pcbnew/drc/drc_test_provider_silk_to_mask.cpp index 1fd2e1cc9f..2341d2158c 100644 --- a/pcbnew/drc/drc_test_provider_silk_to_mask.cpp +++ b/pcbnew/drc/drc_test_provider_silk_to_mask.cpp @@ -161,20 +161,10 @@ bool DRC_TEST_PROVIDER_SILK_TO_MASK::Run() return true; }; - int numPads = forEachGeometryItem( { PCB_PAD_T, - PCB_SHAPE_T, - PCB_FP_SHAPE_T, - PCB_TEXT_T, - PCB_FP_TEXT_T }, - LSET( 2, F_Mask, B_Mask ), addMaskToTree ); + int numMask = forEachGeometryItem( {}, LSET( 2, F_Mask, B_Mask ), addMaskToTree ); + int numSilk = forEachGeometryItem( {}, LSET( 2, F_SilkS, B_SilkS ), addSilkToTree ); - int numSilk = forEachGeometryItem( { PCB_SHAPE_T, - PCB_FP_SHAPE_T, - PCB_TEXT_T, - PCB_FP_TEXT_T }, - LSET( 2, F_SilkS, B_SilkS ), addSilkToTree ); - - reportAux( _("Testing %d exposed copper against %d silkscreen features."), numPads, numSilk ); + reportAux( _("Testing %d mask apertures against %d silkscreen features."), numMask, numSilk ); const std::vector layerPairs = { @@ -182,7 +172,14 @@ bool DRC_TEST_PROVIDER_SILK_TO_MASK::Run() DRC_RTREE::LAYER_PAIR( B_SilkS, B_Mask ) }; - maskTree.QueryCollidingPairs( &silkTree, layerPairs, checkClearance, m_largestClearance ); + // This is the number of tests between 2 calls to the progress bar + const int delta = 250; + + maskTree.QueryCollidingPairs( &silkTree, layerPairs, checkClearance, m_largestClearance, + [this]( int aCount, int aSize ) -> bool + { + return reportProgress( aCount, aSize, delta ); + } ); reportRuleStatistics();