From eb492724c73fb1567efa6d374d1f41c4095cf531 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Fri, 7 Apr 2023 13:40:52 +0100 Subject: [PATCH] Further simplify PNS::ITEM::collideSimple(). Also brings text_pns_basic's mocks into line with their "real" counterparts. (cherry picked from commit 2f198bdcb274a902711229ec765755d09373e22a) --- pcbnew/router/pns_item.cpp | 52 +++------ pcbnew/router/pns_kicad_iface.cpp | 3 +- qa/unittests/pcbnew/test_pns_basics.cpp | 147 +++++++++++++++--------- 3 files changed, 110 insertions(+), 92 deletions(-) diff --git a/pcbnew/router/pns_item.cpp b/pcbnew/router/pns_item.cpp index 999b0f9c9c..6bb07b8258 100644 --- a/pcbnew/router/pns_item.cpp +++ b/pcbnew/router/pns_item.cpp @@ -48,8 +48,11 @@ static void dumpObstacles( const PNS::NODE::OBSTACLES &obstacles ) bool ITEM::collideSimple( const ITEM* aHead, const NODE* aNode, COLLISION_SEARCH_CONTEXT* aCtx ) const { + // Note: if 'this' is a pad or a via then its hole is a separate PNS::ITEM in the node's + // index and we don't need to deal with holeI here. The same is *not* true of the routing + // "head", so we do need to handle holeH. + const SHAPE* shapeI = Shape(); - const HOLE* holeI = Hole(); int lineWidthI = 0; const SHAPE* shapeH = aHead->Shape(); @@ -62,21 +65,7 @@ bool ITEM::collideSimple( const ITEM* aHead, const NODE* aNode, //printf( "h %p n %p t %p ctx %p\n", aHead, aNode, this, aCtx ); - if( aHead == this ) // we cannot be self-colliding - return false; - - // prevent bogus collisions between the item and its own hole. - // FIXME: figure out a cleaner way of doing that - if( holeI && aHead == holeI->ParentPadVia() ) - return false; - - if( holeH && this == holeH->ParentPadVia() ) - return false; - - if( holeH && this == holeH ) - return false; - - if( holeI && aHead == holeI ) + if( this == aHead || this == holeH ) // we cannot be self-colliding return false; // Special cases for "head" lines with vias attached at the end. Note that this does not @@ -95,6 +84,11 @@ bool ITEM::collideSimple( const ITEM* aHead, const NODE* aNode, collisionsFound |= line->Via().collideSimple( this, aNode, aCtx ); } + // And a special case for the "head" via's hole. + + if( holeH ) + collisionsFound |= collideSimple( holeH, aNode, aCtx ); + // Sadly collision routines ignore SHAPE_POLY_LINE widths so we have to pass them in as part // of the clearance value. if( m_kind == LINE_T ) @@ -107,34 +101,16 @@ bool ITEM::collideSimple( const ITEM* aHead, const NODE* aNode, if( !m_layers.Overlaps( aHead->m_layers ) ) return false; - if( holeH ) - collisionsFound |= collideSimple( holeH, aNode, aCtx ); - - if( holeI ) - collisionsFound |= holeI->collideSimple( aHead, aNode, aCtx ); - - if( holeH && holeI ) - { - if( aCtx ) - { - // Hole to hole collsions are never net-specific - COLLISION_SEARCH_OPTIONS holeToHoleOptions( aCtx->options ); - holeToHoleOptions.m_differentNetsOnly = false; - COLLISION_SEARCH_CONTEXT holeToHoleCtx( aCtx->obstacles, holeToHoleOptions ); - collisionsFound |= holeI->collideSimple( holeH, aNode, &holeToHoleCtx ); - } - else - { - collisionsFound |= holeI->collideSimple( holeH, aNode, nullptr ); - } - } - // fixme: this f***ing singleton must go... ROUTER* router = ROUTER::GetInstance(); ROUTER_IFACE* iface = router ? router->GetInterface() : nullptr; bool differentNetsOnly = aCtx && aCtx->options.m_differentNetsOnly; int clearance; + // Hole-to-hole collisions don't have anything to do with nets + if( Kind() == HOLE_T && aHead->Kind() == HOLE_T ) + differentNetsOnly = false; + if( differentNetsOnly && m_net == aHead->m_net && m_net >= 0 && aHead->m_net >= 0 ) { // same nets? no clearance! diff --git a/pcbnew/router/pns_kicad_iface.cpp b/pcbnew/router/pns_kicad_iface.cpp index de237bba5f..dcf215014c 100644 --- a/pcbnew/router/pns_kicad_iface.cpp +++ b/pcbnew/router/pns_kicad_iface.cpp @@ -440,8 +440,7 @@ int PNS_PCBNEW_RULE_RESOLVER::Clearance( const PNS::ITEM* aA, const PNS::ITEM* a rv = constraint.m_Value.Min(); } } - - if( isHole( aA ) || isHole( aB ) ) + else if( isHole( aA ) || isHole( aB ) ) { if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_HOLE_CLEARANCE, aA, aB, layer, &constraint ) ) { diff --git a/qa/unittests/pcbnew/test_pns_basics.cpp b/qa/unittests/pcbnew/test_pns_basics.cpp index d48cde11b5..f95446e466 100644 --- a/qa/unittests/pcbnew/test_pns_basics.cpp +++ b/qa/unittests/pcbnew/test_pns_basics.cpp @@ -40,15 +40,26 @@ static bool isCopper( const PNS::ITEM* aItem ) BOARD_ITEM* parent = aItem->Parent(); - if ( !parent ) - { - return LSET::AllCuMask().Contains( (PCB_LAYER_ID) aItem->Layer() ); - } - if( parent && parent->Type() == PCB_PAD_T ) { PAD* pad = static_cast( parent ); - return pad->IsOnCopperLayer() && pad->GetAttribute() != PAD_ATTRIB::NPTH; + + if( !pad->IsOnCopperLayer() ) + return false; + + if( pad->GetAttribute() != PAD_ATTRIB::NPTH ) + return true; + + // round NPTH with a hole size >= pad size are not on a copper layer + // All other NPTH are seen on copper layers + // This is a basic criteria, but probably enough for a NPTH + if( pad->GetShape() == PAD_SHAPE::CIRCLE ) + { + if( pad->GetSize().x <= pad->GetDrillSize().x ) + return false; + } + + return true; } return true; @@ -69,12 +80,7 @@ static bool isEdge( const PNS::ITEM* aItem ) if( !aItem ) return false; - const BOARD_ITEM* parent = aItem->Parent(); - - if ( !parent ) - { - return aItem->Layer() == Edge_Cuts; - } + const BOARD_ITEM *parent = aItem->BoardItem(); return parent && ( parent->IsOnLayer( Edge_Cuts ) || parent->IsOnLayer( Margin ) ); } @@ -93,51 +99,68 @@ public: bool aUseClearanceEpsilon = true ) override { PNS::CONSTRAINT constraint; - int rv = -1; - int layer; + int rv = 0; + LAYER_RANGE layers; - if( !aA->Layers().IsMultilayer() || !aB || aB->Layers().IsMultilayer() ) - layer = aA->Layer(); + if( !aB ) + layers = aA->Layers(); + else if( isEdge( aA ) ) + layers = aB->Layers(); + else if( isEdge( aB ) ) + layers = aA->Layers(); else - layer = aB->Layer(); + layers = aA->Layers().Intersection( aB->Layers() ); - if( isHole( aA ) && isHole( aB ) ) + // Normalize layer range (no -1 magic numbers) + layers = layers.Intersection( LAYER_RANGE( PCBNEW_LAYER_ID_START, PCB_LAYER_ID_COUNT - 1 ) ); + + for( int layer = layers.Start(); layer <= layers.End(); ++layer ) { - if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_HOLE_TO_HOLE, aA, aB, layer, - &constraint ) ) - rv = constraint.m_Value.Min(); - } - else if( isHole( aA ) || isHole( aB ) ) - { - if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_HOLE_CLEARANCE, aA, aB, layer, - &constraint ) ) - rv = constraint.m_Value.Min(); - } - else if( isCopper( aA ) && ( !aB || isCopper( aB ) ) ) - { - if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_CLEARANCE, aA, aB, layer, &constraint ) ) - rv = constraint.m_Value.Min(); - } - else if( isEdge( aA ) || ( aB && isEdge( aB ) ) ) - { - if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_EDGE_CLEARANCE, aA, aB, layer, - &constraint ) ) + if( isHole( aA ) && isHole( aB) ) { - if( constraint.m_Value.Min() > rv ) - rv = constraint.m_Value.Min(); + if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_HOLE_TO_HOLE, aA, aB, layer, &constraint ) ) + { + if( constraint.m_Value.Min() > rv ) + rv = constraint.m_Value.Min(); + } + } + else if( isHole( aA ) || isHole( aB ) ) + { + if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_HOLE_CLEARANCE, aA, aB, layer, &constraint ) ) + { + if( constraint.m_Value.Min() > rv ) + rv = constraint.m_Value.Min(); + } + } + else if( isCopper( aA ) && ( !aB || isCopper( aB ) ) ) + { + if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_CLEARANCE, aA, aB, layer, &constraint ) ) + { + if( constraint.m_Value.Min() > rv ) + rv = constraint.m_Value.Min(); + } + } + else if( isEdge( aA ) || ( aB && isEdge( aB ) ) ) + { + if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_EDGE_CLEARANCE, aA, aB, layer, &constraint ) ) + { + if( constraint.m_Value.Min() > rv ) + rv = constraint.m_Value.Min(); + } } } return rv; } + virtual int DpCoupledNet( int aNet ) override { return -1; } + virtual int DpNetPolarity( int aNet ) override { return -1; } - virtual int DpCoupledNet( int aNet ) override { return -1; } - virtual int DpNetPolarity( int aNet ) override { return -1; } virtual bool DpNetPair( const PNS::ITEM* aItem, int& aNetP, int& aNetN ) override { return false; } + virtual bool QueryConstraint( PNS::CONSTRAINT_TYPE aType, const PNS::ITEM* aItemA, const PNS::ITEM* aItemB, int aLayer, PNS::CONSTRAINT* aConstraint ) override @@ -149,13 +172,14 @@ public: key.type = aType; auto it = m_ruleMap.find( key ); + if( it == m_ruleMap.end() ) { int cl; switch( aType ) { - case PNS::CONSTRAINT_TYPE::CT_CLEARANCE: cl = m_defaultClearance; break; - case PNS::CONSTRAINT_TYPE::CT_HOLE_TO_HOLE: cl = m_defaultHole2Hole; break; + case PNS::CONSTRAINT_TYPE::CT_CLEARANCE: cl = m_defaultClearance; break; + case PNS::CONSTRAINT_TYPE::CT_HOLE_TO_HOLE: cl = m_defaultHole2Hole; break; case PNS::CONSTRAINT_TYPE::CT_HOLE_CLEARANCE: cl = m_defaultHole2Copper; break; default: return false; } @@ -166,7 +190,9 @@ public: aConstraint->m_Value.SetMin( cl ); return true; - } else { + } + else + { *aConstraint = it->second; } @@ -294,8 +320,8 @@ static void dumpObstacles( const PNS::NODE::OBSTACLES &obstacles ) BOOST_FIXTURE_TEST_CASE( PNSHoleCollisions, PNS_TEST_FIXTURE ) { - PNS::VIA* v1 = new PNS::VIA( VECTOR2I( 0, 1000000 ), LAYER_RANGE( F_Cu, B_Cu ), 500000, 100000 ); - PNS::VIA* v2 = new PNS::VIA( VECTOR2I( 0, 2000000 ), LAYER_RANGE( F_Cu, B_Cu ), 500000, 100000 ); + PNS::VIA* v1 = new PNS::VIA( VECTOR2I( 0, 1000000 ), LAYER_RANGE( F_Cu, B_Cu ), 50000, 10000 ); + PNS::VIA* v2 = new PNS::VIA( VECTOR2I( 0, 2000000 ), LAYER_RANGE( F_Cu, B_Cu ), 50000, 10000 ); std::unique_ptr world ( new PNS::NODE ); @@ -328,10 +354,28 @@ BOOST_FIXTURE_TEST_CASE( PNSHoleCollisions, PNS_TEST_FIXTURE ) BOOST_CHECK_EQUAL( first.m_clearance, m_ruleResolver.m_defaultClearance ); } - BOOST_TEST_MESSAGE( "via to via, forced copper to hole violation" ); + BOOST_TEST_MESSAGE( "via to via, forced hole to hole violation" ); { PNS::NODE::OBSTACLES obstacles; m_ruleResolver.m_defaultClearance = 200000; + m_ruleResolver.m_defaultHole2Hole = 1000000; + + world->QueryColliding( v1, obstacles ); + dumpObstacles( obstacles ); + + BOOST_CHECK_EQUAL( obstacles.size(), 1 ); + auto iter = obstacles.begin(); + const auto first = *iter++; + + BOOST_CHECK_EQUAL( first.m_head, v1->Hole() ); + BOOST_CHECK_EQUAL( first.m_item, v2->Hole() ); + BOOST_CHECK_EQUAL( first.m_clearance, m_ruleResolver.m_defaultHole2Hole ); + } + + BOOST_TEST_MESSAGE( "via to via, forced copper to hole violation" ); + { + PNS::NODE::OBSTACLES obstacles; + m_ruleResolver.m_defaultHole2Hole = 220000; m_ruleResolver.m_defaultHole2Copper = 1000000; world->QueryColliding( v1, obstacles ); @@ -340,13 +384,12 @@ BOOST_FIXTURE_TEST_CASE( PNSHoleCollisions, PNS_TEST_FIXTURE ) BOOST_CHECK_EQUAL( obstacles.size(), 2 ); auto iter = obstacles.begin(); const auto first = *iter++; - const auto second = *iter; - BOOST_CHECK_EQUAL( first.m_head, v1 ); - BOOST_CHECK_EQUAL( first.m_item, v2 ); + // There is no guarantee on what order the two collisions will be in... + BOOST_CHECK( ( first.m_head == v1 && first.m_item == v2->Hole() ) + || ( first.m_head == v1->Hole() && first.m_item == v2 ) ); + BOOST_CHECK_EQUAL( first.m_clearance, m_ruleResolver.m_defaultHole2Copper ); } - - }