From 00ed75b891fa29acf9bd89cef72407bace93f9ef Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Wed, 2 Jun 2021 14:00:11 +0100 Subject: [PATCH] Fix DRC performance with multi-layer keepout zones. The main issue was a parameter mismatch which caused On^2 behaviour for zone layers. But there are several other performance optimizations here, along with status bar updating for zones while running the dissallow test. Fixes https://gitlab.com/kicad/code/kicad/issues/8521 --- pcbnew/drc/drc_engine.cpp | 2 +- pcbnew/drc/drc_rtree.h | 90 ++++++++++--------- .../drc_test_provider_copper_clearance.cpp | 18 +++- .../drc_test_provider_diff_pair_coupling.cpp | 7 +- pcbnew/drc/drc_test_provider_disallow.cpp | 12 +++ .../drc/drc_test_provider_edge_clearance.cpp | 8 +- pcbnew/drc/drc_test_provider_hole_to_hole.cpp | 4 +- .../drc/drc_test_provider_silk_clearance.cpp | 11 ++- pcbnew/drc/drc_test_provider_silk_to_mask.cpp | 14 ++- pcbnew/pcb_expr_evaluator.cpp | 4 +- pcbnew/tracks_cleaner.cpp | 2 +- 11 files changed, 116 insertions(+), 56 deletions(-) diff --git a/pcbnew/drc/drc_engine.cpp b/pcbnew/drc/drc_engine.cpp index 9f853c71d5..3592599a9f 100644 --- a/pcbnew/drc/drc_engine.cpp +++ b/pcbnew/drc/drc_engine.cpp @@ -736,7 +736,7 @@ void DRC_ENGINE::RunTests( EDA_UNITS aUnits, bool aReportAllTrackErrors, bool aT m_board->m_CopperZoneRTrees[ zone ] = std::make_unique(); - for( int layer : zone->GetLayerSet().Seq() ) + for( PCB_LAYER_ID layer : zone->GetLayerSet().Seq() ) { if( IsCopperLayer( layer ) ) m_board->m_CopperZoneRTrees[ zone ]->Insert( zone, layer ); diff --git a/pcbnew/drc/drc_rtree.h b/pcbnew/drc/drc_rtree.h index ee3b38bbdf..0953d19ee5 100644 --- a/pcbnew/drc/drc_rtree.h +++ b/pcbnew/drc/drc_rtree.h @@ -81,60 +81,36 @@ public: /** * Function Insert() - * Inserts an item into the tree. + * Inserts an item into the tree on a particular layer with an optional worst clearance. */ - void Insert( BOARD_ITEM* aItem, int aWorstClearance = 0, int aLayer = UNDEFINED_LAYER ) + void Insert( BOARD_ITEM* aItem, PCB_LAYER_ID aLayer, int aWorstClearance = 0 ) { - std::vector subshapes; - - auto addLayer = - [&]( PCB_LAYER_ID layer ) - { - std::shared_ptr shape = aItem->GetEffectiveShape( layer ); - subshapes.clear(); - - if( shape->HasIndexableSubshapes() ) - shape->GetIndexableSubshapes( subshapes ); - else - subshapes.push_back( shape.get() ); - - for( SHAPE* subshape : subshapes ) - { - BOX2I bbox = subshape->BBox(); - - bbox.Inflate( aWorstClearance ); - - const int mmin[2] = { bbox.GetX(), bbox.GetY() }; - const int mmax[2] = { bbox.GetRight(), bbox.GetBottom() }; - - m_tree[layer]->Insert( mmin, mmax, new ITEM_WITH_SHAPE( aItem, subshape, - shape ) ); - m_count++; - } - }; + wxASSERT( aLayer != UNDEFINED_LAYER ); if( aItem->Type() == PCB_FP_TEXT_T && !static_cast( aItem )->IsVisible() ) return; - if( aLayer != UNDEFINED_LAYER ) - { - addLayer( (PCB_LAYER_ID) aLayer ); - } + std::vector subshapes; + std::shared_ptr shape = aItem->GetEffectiveShape( ToLAYER_ID( aLayer ) ); + subshapes.clear(); + + if( shape->HasIndexableSubshapes() ) + shape->GetIndexableSubshapes( subshapes ); else + subshapes.push_back( shape.get() ); + + for( SHAPE* subshape : subshapes ) { - LSET layers = aItem->GetLayerSet(); + BOX2I bbox = subshape->BBox(); - // Special-case pad holes which pierce all the copper layers - if( aItem->Type() == PCB_PAD_T ) - { - PAD* pad = static_cast( aItem ); + bbox.Inflate( aWorstClearance ); - if( pad->GetDrillSizeX() > 0 && pad->GetDrillSizeY() > 0 ) - layers |= LSET::AllCuMask(); - } + const int mmin[2] = { bbox.GetX(), bbox.GetY() }; + const int mmax[2] = { bbox.GetRight(), bbox.GetBottom() }; + ITEM_WITH_SHAPE* itemShape = new ITEM_WITH_SHAPE( aItem, subshape, shape ); - for( int layer : layers.Seq() ) - addLayer( (PCB_LAYER_ID) layer ); + m_tree[aLayer]->Insert( mmin, mmax, itemShape ); + m_count++; } } @@ -261,7 +237,7 @@ public: * position. */ bool QueryColliding( EDA_RECT aBox, SHAPE* aRefShape, PCB_LAYER_ID aLayer, int aClearance, - int* aActual = nullptr, VECTOR2I* aPos = nullptr ) const + int* aActual, VECTOR2I* aPos ) const { aBox.Inflate( aClearance ); @@ -308,6 +284,32 @@ public: return false; } + /** + * Quicker version of above that just reports a raw yes/no. + */ + bool QueryColliding( EDA_RECT aBox, SHAPE* aRefShape, PCB_LAYER_ID aLayer ) const + { + int min[2] = { aBox.GetX(), aBox.GetY() }; + int max[2] = { aBox.GetRight(), aBox.GetBottom() }; + bool collision = false; + + auto visit = + [&]( ITEM_WITH_SHAPE* aItem ) -> bool + { + if( aRefShape->Collide( aItem->shape, 0 ) ) + { + collision = true; + return false; + } + + return true; + }; + + this->m_tree[aLayer]->Search( min, max, visit ); + + return collision; + } + typedef std::pair LAYER_PAIR; struct PAIR_INFO diff --git a/pcbnew/drc/drc_test_provider_copper_clearance.cpp b/pcbnew/drc/drc_test_provider_copper_clearance.cpp index a0cbc109f7..4d2c811d41 100644 --- a/pcbnew/drc/drc_test_provider_copper_clearance.cpp +++ b/pcbnew/drc/drc_test_provider_copper_clearance.cpp @@ -165,7 +165,23 @@ bool DRC_TEST_PROVIDER_COPPER_CLEARANCE::Run() if( !reportProgress( ii++, count, delta ) ) return false; - m_copperTree.Insert( item, m_largestClearance ); + LSET layers = item->GetLayerSet(); + + // Special-case pad holes which pierce all the copper layers + if( item->Type() == PCB_PAD_T ) + { + PAD* pad = static_cast( item ); + + if( pad->GetDrillSizeX() > 0 && pad->GetDrillSizeY() > 0 ) + layers |= LSET::AllCuMask(); + } + + for( PCB_LAYER_ID layer : layers.Seq() ) + { + if( IsCopperLayer( layer ) ) + m_copperTree.Insert( item, layer, m_largestClearance ); + } + return true; }; diff --git a/pcbnew/drc/drc_test_provider_diff_pair_coupling.cpp b/pcbnew/drc/drc_test_provider_diff_pair_coupling.cpp index 31097b3a81..40a27a23f9 100644 --- a/pcbnew/drc/drc_test_provider_diff_pair_coupling.cpp +++ b/pcbnew/drc/drc_test_provider_diff_pair_coupling.cpp @@ -329,7 +329,12 @@ bool test::DRC_TEST_PROVIDER_DIFF_PAIR_COUPLING::Run() auto addToTree = [&copperTree]( BOARD_ITEM *item ) -> bool { - copperTree.Insert( item ); + for( PCB_LAYER_ID layer : item->GetLayerSet().Seq() ) + { + if( IsCopperLayer( layer ) ) + copperTree.Insert( item, layer ); + } + return true; }; diff --git a/pcbnew/drc/drc_test_provider_disallow.cpp b/pcbnew/drc/drc_test_provider_disallow.cpp index 07f122ac19..36055c3500 100644 --- a/pcbnew/drc/drc_test_provider_disallow.cpp +++ b/pcbnew/drc/drc_test_provider_disallow.cpp @@ -68,6 +68,12 @@ bool DRC_TEST_PROVIDER_DISALLOW::Run() if( !reportPhase( _( "Checking keepouts & disallow constraints..." ) ) ) return false; // DRC cancelled + // Zones can be expensive (particularly when multi-layer), so we run the progress bar on them + BOARD* board = m_drcEngine->GetBoard(); + int boardZoneCount = board->Zones().size(); + int delta = std::max( 1, boardZoneCount / board->GetCopperLayerCount() ); + int ii = 0; + auto doCheckItem = [&]( BOARD_ITEM* item ) { @@ -123,6 +129,12 @@ bool DRC_TEST_PROVIDER_DISALLOW::Run() if( zone->GetIsRuleArea() ) return true; + + if( item->Type() == PCB_ZONE_T ) + { + if( !reportProgress( ii++, boardZoneCount, delta ) ) + return false; // DRC cancelled + } } item->ClearFlags( HOLE_PROXY ); diff --git a/pcbnew/drc/drc_test_provider_edge_clearance.cpp b/pcbnew/drc/drc_test_provider_edge_clearance.cpp index 983566a053..7dbb85c2cf 100644 --- a/pcbnew/drc/drc_test_provider_edge_clearance.cpp +++ b/pcbnew/drc/drc_test_provider_edge_clearance.cpp @@ -208,7 +208,13 @@ bool DRC_TEST_PROVIDER_EDGE_CLEARANCE::Run() forEachGeometryItem( s_allBasicItemsButZones, LSET::AllCuMask(), queryBoardGeometryItems ); for( const std::unique_ptr& edge : edges ) - edgesTree.Insert( edge.get(), m_largestClearance ); + { + for( PCB_LAYER_ID layer : { Edge_Cuts, Margin } ) + { + if( edge->IsOnLayer( layer ) ) + edgesTree.Insert( edge.get(), layer, m_largestClearance ); + } + } wxString val; wxGetEnv( "WXTRACE", &val ); diff --git a/pcbnew/drc/drc_test_provider_hole_to_hole.cpp b/pcbnew/drc/drc_test_provider_hole_to_hole.cpp index 84f0c6665c..5f2c1287f1 100644 --- a/pcbnew/drc/drc_test_provider_hole_to_hole.cpp +++ b/pcbnew/drc/drc_test_provider_hole_to_hole.cpp @@ -151,7 +151,7 @@ bool DRC_TEST_PROVIDER_HOLE_TO_HOLE::Run() // We only care about drilled (ie: round) holes if( pad->GetDrillSize().x && pad->GetDrillSize().x == pad->GetDrillSize().y ) - m_holeTree.Insert( item, m_largestClearance, F_Cu ); + m_holeTree.Insert( item, F_Cu, m_largestClearance ); } else if( item->Type() == PCB_VIA_T ) { @@ -159,7 +159,7 @@ bool DRC_TEST_PROVIDER_HOLE_TO_HOLE::Run() // We only care about mechanically drilled (ie: non-laser) holes if( via->GetViaType() == VIATYPE::THROUGH ) - m_holeTree.Insert( item, m_largestClearance, F_Cu ); + m_holeTree.Insert( item, F_Cu, m_largestClearance ); } return true; diff --git a/pcbnew/drc/drc_test_provider_silk_clearance.cpp b/pcbnew/drc/drc_test_provider_silk_clearance.cpp index 42d33c6502..093dc6f702 100644 --- a/pcbnew/drc/drc_test_provider_silk_clearance.cpp +++ b/pcbnew/drc/drc_test_provider_silk_clearance.cpp @@ -112,7 +112,12 @@ bool DRC_TEST_PROVIDER_SILK_CLEARANCE::Run() auto addToSilkTree = [&silkTree]( BOARD_ITEM* item ) -> bool { - silkTree.Insert( item ); + for( PCB_LAYER_ID layer : { F_SilkS, B_SilkS } ) + { + if( item->IsOnLayer( layer ) ) + silkTree.Insert( item, layer ); + } + return true; }; @@ -129,7 +134,9 @@ bool DRC_TEST_PROVIDER_SILK_CLEARANCE::Run() if( !reportProgress( ii++, targets, delta ) ) return false; - targetTree.Insert( item ); + for( PCB_LAYER_ID layer : item->GetLayerSet().Seq() ) + targetTree.Insert( item, layer ); + return true; }; diff --git a/pcbnew/drc/drc_test_provider_silk_to_mask.cpp b/pcbnew/drc/drc_test_provider_silk_to_mask.cpp index ecf5d5db81..a4f5545cc1 100644 --- a/pcbnew/drc/drc_test_provider_silk_to_mask.cpp +++ b/pcbnew/drc/drc_test_provider_silk_to_mask.cpp @@ -104,14 +104,24 @@ bool DRC_TEST_PROVIDER_SILK_TO_MASK::Run() auto addMaskToTree = [&maskTree]( BOARD_ITEM *item ) -> bool { - maskTree.Insert( item ); + for( PCB_LAYER_ID layer : { F_Mask, B_Mask } ) + { + if( item->IsOnLayer( layer ) ) + maskTree.Insert( item, layer ); + } + return true; }; auto addSilkToTree = [&silkTree]( BOARD_ITEM *item ) -> bool { - silkTree.Insert( item ); + for( PCB_LAYER_ID layer : { F_SilkS, B_SilkS } ) + { + if( item->IsOnLayer( layer ) ) + silkTree.Insert( item, layer ); + } + return true; }; diff --git a/pcbnew/pcb_expr_evaluator.cpp b/pcbnew/pcb_expr_evaluator.cpp index f17aad041a..ca9612f66c 100644 --- a/pcbnew/pcb_expr_evaluator.cpp +++ b/pcbnew/pcb_expr_evaluator.cpp @@ -540,11 +540,13 @@ static void insideArea( LIBEVAL::CONTEXT* aCtx, void* self ) DRC_RTREE* zoneRTree = board->m_CopperZoneRTrees[ zone ].get(); + std::vector shapes; + if( zoneRTree ) { for( PCB_LAYER_ID layer : area->GetLayerSet().Seq() ) { - if( zoneRTree->QueryColliding( itemBBox, &areaOutline, layer, 0 ) ) + if( zoneRTree->QueryColliding( itemBBox, &areaOutline, layer ) ) return true; } } diff --git a/pcbnew/tracks_cleaner.cpp b/pcbnew/tracks_cleaner.cpp index be2d6e879f..ceedd26497 100644 --- a/pcbnew/tracks_cleaner.cpp +++ b/pcbnew/tracks_cleaner.cpp @@ -275,7 +275,7 @@ void TRACKS_CLEANER::cleanup( bool aDeleteDuplicateVias, bool aDeleteNullSegment for( TRACK* track : m_brd->Tracks() ) { track->ClearFlags( IS_DELETED | SKIP_STRUCT ); - rtree.Insert( track ); + rtree.Insert( track, track->GetLayer() ); } std::set toRemove;