From 2a91ba35c1a1e443714c4a33334236be1f60413e Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Fri, 3 Jun 2022 17:20:47 -0400 Subject: [PATCH] PNS: Use exact hulls for walkaround path generation When we generate hulls, by default we subtract the clearance epsilon to prevent false collisions in the router that wouldn't be flagged by DRC. However, we need to use the actual hull with no epsilon when generating hulls for pathfinding in the walkaround system. Without this change, it is possible for the walkaround to generate a valid-seeming path that results in a DRC violation, for example when dragging a drag against a board edge. Fixes https://gitlab.com/kicad/code/kicad/-/issues/10536 Fixes https://gitlab.com/kicad/code/kicad/-/issues/11365 Fixes https://gitlab.com/kicad/code/kicad/-/issues/10710 --- pcbnew/router/pns_joint.h | 16 +++++++--- pcbnew/router/pns_kicad_iface.cpp | 52 +++++++++++++++++++++---------- pcbnew/router/pns_node.cpp | 25 ++++++++------- pcbnew/router/pns_node.h | 20 +++++++----- pcbnew/router/pns_walkaround.cpp | 2 +- 5 files changed, 74 insertions(+), 41 deletions(-) diff --git a/pcbnew/router/pns_joint.h b/pcbnew/router/pns_joint.h index fdcb3015ac..a1c32a84ab 100644 --- a/pcbnew/router/pns_joint.h +++ b/pcbnew/router/pns_joint.h @@ -108,24 +108,30 @@ public: { return false; } - else if( m_linkedItems.Size() == 3 - && m_linkedItems.Count( SEGMENT_T | ARC_T ) == 2 - && m_linkedItems.Count( VIA_T ) == 1 ) + // There will be multiple VVIAs on joints between two locked segments, because we + // naively add a VVIA to each end of a locked segment. + else if( ( m_linkedItems.Size() - m_linkedItems.Count( SEGMENT_T | ARC_T ) ) + == m_linkedItems.Count( VIA_T ) ) { const VIA* via = nullptr; + bool hasNonVirtualVia = false; for( const ITEM* item : m_linkedItems.CItems() ) { if( item->Kind() == VIA_T ) { via = static_cast( item ); - break; + + hasNonVirtualVia = !via->IsVirtual(); + + if( hasNonVirtualVia ) + break; } } assert( via ); - if( !via || !via->IsVirtual() ) + if( !via || hasNonVirtualVia ) return false; } else diff --git a/pcbnew/router/pns_kicad_iface.cpp b/pcbnew/router/pns_kicad_iface.cpp index 7b8159357c..6b396ce1f3 100644 --- a/pcbnew/router/pns_kicad_iface.cpp +++ b/pcbnew/router/pns_kicad_iface.cpp @@ -72,9 +72,12 @@ public: PNS_PCBNEW_RULE_RESOLVER( BOARD* aBoard, PNS::ROUTER_IFACE* aRouterIface ); virtual ~PNS_PCBNEW_RULE_RESOLVER(); - virtual int Clearance( const PNS::ITEM* aA, const PNS::ITEM* aB ) override; - virtual int HoleClearance( const PNS::ITEM* aA, const PNS::ITEM* aB ) override; - virtual int HoleToHoleClearance( const PNS::ITEM* aA, const PNS::ITEM* aB ) override; + virtual int Clearance( const PNS::ITEM* aA, const PNS::ITEM* aB, + bool aUseClearanceEpsilon = true ) override; + virtual int HoleClearance( const PNS::ITEM* aA, const PNS::ITEM* aB, + bool aUseClearanceEpsilon = true ) override; + virtual int HoleToHoleClearance( const PNS::ITEM* aA, const PNS::ITEM* aB, + bool aUseClearanceEpsilon = true ) override; virtual int DpCoupledNet( int aNet ) override; virtual int DpNetPolarity( int aNet ) override; @@ -112,9 +115,11 @@ private: PCB_VIA m_dummyVias[2]; int m_clearanceEpsilon; - std::map, int> m_clearanceCache; - std::map, int> m_holeClearanceCache; - std::map, int> m_holeToHoleClearanceCache; + typedef std::tuple CLEARANCE_CACHE_KEY; + + std::map m_clearanceCache; + std::map m_holeClearanceCache; + std::map m_holeToHoleClearanceCache; }; @@ -298,13 +303,15 @@ bool PNS_PCBNEW_RULE_RESOLVER::QueryConstraint( PNS::CONSTRAINT_TYPE aType, void PNS_PCBNEW_RULE_RESOLVER::ClearCacheForItem( const PNS::ITEM* aItem ) { - m_clearanceCache.erase( std::make_pair( aItem, nullptr ) ); + m_clearanceCache.erase( std::make_tuple( aItem, nullptr, false ) ); + m_clearanceCache.erase( std::make_tuple( aItem, nullptr, true ) ); } -int PNS_PCBNEW_RULE_RESOLVER::Clearance( const PNS::ITEM* aA, const PNS::ITEM* aB ) +int PNS_PCBNEW_RULE_RESOLVER::Clearance( const PNS::ITEM* aA, const PNS::ITEM* aB, + bool aUseClearanceEpsilon ) { - std::pair key( aA, aB ); + CLEARANCE_CACHE_KEY key( aA, aB, aUseClearanceEpsilon ); auto it = m_clearanceCache.find( key ); if( it != m_clearanceCache.end() ) @@ -322,7 +329,7 @@ int PNS_PCBNEW_RULE_RESOLVER::Clearance( const PNS::ITEM* aA, const PNS::ITEM* a if( isCopper( aA ) && ( !aB || isCopper( aB ) ) ) { if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_CLEARANCE, aA, aB, layer, &constraint ) ) - rv = constraint.m_Value.Min() - m_clearanceEpsilon; + rv = constraint.m_Value.Min(); } if( isEdge( aA ) || ( aB && isEdge( aB ) ) ) @@ -330,18 +337,22 @@ int PNS_PCBNEW_RULE_RESOLVER::Clearance( const PNS::ITEM* aA, const PNS::ITEM* a if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_EDGE_CLEARANCE, aA, aB, layer, &constraint ) ) { if( constraint.m_Value.Min() > rv ) - rv = constraint.m_Value.Min() - m_clearanceEpsilon; + rv = constraint.m_Value.Min(); } } + if( aUseClearanceEpsilon ) + rv -= m_clearanceEpsilon; + m_clearanceCache[ key ] = rv; return rv; } -int PNS_PCBNEW_RULE_RESOLVER::HoleClearance( const PNS::ITEM* aA, const PNS::ITEM* aB ) +int PNS_PCBNEW_RULE_RESOLVER::HoleClearance( const PNS::ITEM* aA, const PNS::ITEM* aB, + bool aUseClearanceEpsilon ) { - std::pair key( aA, aB ); + CLEARANCE_CACHE_KEY key( aA, aB, aUseClearanceEpsilon ); auto it = m_holeClearanceCache.find( key ); if( it != m_holeClearanceCache.end() ) @@ -357,16 +368,20 @@ int PNS_PCBNEW_RULE_RESOLVER::HoleClearance( const PNS::ITEM* aA, const PNS::ITE layer = aB->Layer(); if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_HOLE_CLEARANCE, aA, aB, layer, &constraint ) ) - rv = constraint.m_Value.Min() - m_clearanceEpsilon; + rv = constraint.m_Value.Min(); + + if( aUseClearanceEpsilon ) + rv -= m_clearanceEpsilon; m_holeClearanceCache[ key ] = rv; return rv; } -int PNS_PCBNEW_RULE_RESOLVER::HoleToHoleClearance( const PNS::ITEM* aA, const PNS::ITEM* aB ) +int PNS_PCBNEW_RULE_RESOLVER::HoleToHoleClearance( const PNS::ITEM* aA, const PNS::ITEM* aB, + bool aUseClearanceEpsilon ) { - std::pair key( aA, aB ); + CLEARANCE_CACHE_KEY key( aA, aB, aUseClearanceEpsilon ); auto it = m_holeToHoleClearanceCache.find( key ); if( it != m_holeToHoleClearanceCache.end() ) @@ -382,7 +397,10 @@ int PNS_PCBNEW_RULE_RESOLVER::HoleToHoleClearance( const PNS::ITEM* aA, const PN layer = aB->Layer(); if( QueryConstraint( PNS::CONSTRAINT_TYPE::CT_HOLE_TO_HOLE, aA, aB, layer, &constraint ) ) - rv = constraint.m_Value.Min() - m_clearanceEpsilon; + rv = constraint.m_Value.Min(); + + if( aUseClearanceEpsilon ) + rv -= m_clearanceEpsilon; m_holeToHoleClearanceCache[ key ] = rv; return rv; diff --git a/pcbnew/router/pns_node.cpp b/pcbnew/router/pns_node.cpp index f87a1fd559..e076fe6f71 100644 --- a/pcbnew/router/pns_node.cpp +++ b/pcbnew/router/pns_node.cpp @@ -99,7 +99,7 @@ NODE::~NODE() } -int NODE::GetClearance( const ITEM* aA, const ITEM* aB ) const +int NODE::GetClearance( const ITEM* aA, const ITEM* aB, bool aUseClearanceEpsilon ) const { if( !m_ruleResolver ) return 100000; @@ -107,11 +107,11 @@ int NODE::GetClearance( const ITEM* aA, const ITEM* aB ) const if( aA->IsVirtual() || aB->IsVirtual() ) return 0; - return m_ruleResolver->Clearance( aA, aB ); + return m_ruleResolver->Clearance( aA, aB, aUseClearanceEpsilon ); } -int NODE::GetHoleClearance( const ITEM* aA, const ITEM* aB ) const +int NODE::GetHoleClearance( const ITEM* aA, const ITEM* aB, bool aUseClearanceEpsilon ) const { if( !m_ruleResolver ) return 0; @@ -119,11 +119,11 @@ int NODE::GetHoleClearance( const ITEM* aA, const ITEM* aB ) const if( aA->IsVirtual() || aB->IsVirtual() ) return 0; - return m_ruleResolver->HoleClearance( aA, aB ); + return m_ruleResolver->HoleClearance( aA, aB, aUseClearanceEpsilon ); } -int NODE::GetHoleToHoleClearance( const ITEM* aA, const ITEM* aB ) const +int NODE::GetHoleToHoleClearance( const ITEM* aA, const ITEM* aB, bool aUseClearanceEpsilon ) const { if( !m_ruleResolver ) return 0; @@ -131,7 +131,7 @@ int NODE::GetHoleToHoleClearance( const ITEM* aA, const ITEM* aB ) const if( aA->IsVirtual() || aB->IsVirtual() ) return 0; - return m_ruleResolver->HoleToHoleClearance( aA, aB ); + return m_ruleResolver->HoleToHoleClearance( aA, aB, aUseClearanceEpsilon ); } @@ -295,7 +295,8 @@ int NODE::QueryColliding( const ITEM* aItem, NODE::OBSTACLES& aObstacles, int aK NODE::OPT_OBSTACLE NODE::NearestObstacle( const LINE* aLine, int aKindMask, - const std::set* aRestrictedSet ) + const std::set* aRestrictedSet, + bool aUseClearanceEpsilon ) { OBSTACLES obstacleList; obstacleList.reserve( 100 ); @@ -345,8 +346,10 @@ NODE::OPT_OBSTACLE NODE::NearestObstacle( const LINE* aLine, int aKindMask, if( aRestrictedSet && aRestrictedSet->find( obstacle.m_item ) == aRestrictedSet->end() ) continue; - int clearance = GetClearance( obstacle.m_item, aLine ) + aLine->Width() / 2; - obstacleHull = obstacle.m_item->Hull( clearance + PNS_HULL_MARGIN, 0, layer ); + int clearance = + GetClearance( obstacle.m_item, aLine, aUseClearanceEpsilon ) + aLine->Width() / 2; + + obstacleHull = obstacle.m_item->Hull( clearance, 0, layer ); //debugDecorator->AddLine( obstacleHull, 2, 40000, "obstacle-hull-test" ); //debugDecorator->AddLine( aLine->CLine(), 5, 40000, "obstacle-test-line" ); @@ -373,7 +376,7 @@ NODE::OPT_OBSTACLE NODE::NearestObstacle( const LINE* aLine, int aKindMask, if( holeClearance > viaClearance ) viaClearance = holeClearance; - obstacleHull = obstacle.m_item->Hull( viaClearance + PNS_HULL_MARGIN, 0, layer ); + obstacleHull = obstacle.m_item->Hull( viaClearance, 0, layer ); //debugDecorator->AddLine( obstacleHull, 3 ); intersectingPts.clear(); @@ -388,7 +391,7 @@ NODE::OPT_OBSTACLE NODE::NearestObstacle( const LINE* aLine, int aKindMask, if( obstacle.m_item->Hole() ) { clearance = GetHoleClearance( obstacle.m_item, aLine ) + aLine->Width() / 2; - obstacleHull = obstacle.m_item->HoleHull( clearance + PNS_HULL_MARGIN, 0, layer ); + obstacleHull = obstacle.m_item->HoleHull( clearance, 0, layer ); //debugDecorator->AddLine( obstacleHull, 4 ); intersectingPts.clear(); diff --git a/pcbnew/router/pns_node.h b/pcbnew/router/pns_node.h index d6388b3da2..816e5834a2 100644 --- a/pcbnew/router/pns_node.h +++ b/pcbnew/router/pns_node.h @@ -79,9 +79,11 @@ class RULE_RESOLVER public: virtual ~RULE_RESOLVER() {} - virtual int Clearance( const ITEM* aA, const ITEM* aB ) = 0; - virtual int HoleClearance( const ITEM* aA, const ITEM* aB ) = 0; - virtual int HoleToHoleClearance( const ITEM* aA, const ITEM* aB ) = 0; + virtual int Clearance( const ITEM* aA, const ITEM* aB, bool aUseClearanceEpsilon = true ) = 0; + virtual int HoleClearance( const ITEM* aA, const ITEM* aB, + bool aUseClearanceEpsilon = true ) = 0; + virtual int HoleToHoleClearance( const ITEM* aA, const ITEM* aB, + bool aUseClearanceEpsilon = true ) = 0; virtual int DpCoupledNet( int aNet ) = 0; virtual int DpNetPolarity( int aNet ) = 0; @@ -154,9 +156,10 @@ public: ~NODE(); ///< Return the expected clearance between items a and b. - int GetClearance( const ITEM* aA, const ITEM* aB ) const; - int GetHoleClearance( const ITEM* aA, const ITEM* aB ) const; - int GetHoleToHoleClearance( const ITEM* aA, const ITEM* aB ) const; + int GetClearance( const ITEM* aA, const ITEM* aB, bool aUseClearanceEpsilon = true ) const; + int GetHoleClearance( const ITEM* aA, const ITEM* aB, bool aUseClearanceEpsilon = true ) const; + int GetHoleToHoleClearance( const ITEM* aA, const ITEM* aB, + bool aUseClearanceEpsilon = true ) const; ///< Return the pre-set worst case clearance between any pair of items. int GetMaxClearance() const @@ -214,10 +217,13 @@ public: * * @param aLine the item to find collisions with * @param aKindMask mask of obstacle types to take into account + * @param aRestrictedSet is an optional set of items that should be considered as obstacles + * @param aUseClearanceEpsilon determines if the epsilon is subtracted from the hull size * @return the obstacle, if found, otherwise empty. */ OPT_OBSTACLE NearestObstacle( const LINE* aLine, int aKindMask = ITEM::ANY_T, - const std::set* aRestrictedSet = nullptr ); + const std::set* aRestrictedSet = nullptr, + bool aUseClearanceEpsilon = true ); /** * Check if the item collides with anything else in the world, and if found, returns the diff --git a/pcbnew/router/pns_walkaround.cpp b/pcbnew/router/pns_walkaround.cpp index df3614a848..26c2ccd987 100644 --- a/pcbnew/router/pns_walkaround.cpp +++ b/pcbnew/router/pns_walkaround.cpp @@ -41,7 +41,7 @@ void WALKAROUND::start( const LINE& aInitialPath ) NODE::OPT_OBSTACLE WALKAROUND::nearestObstacle( const LINE& aPath ) { NODE::OPT_OBSTACLE obs = m_world->NearestObstacle( - &aPath, m_itemMask, m_restrictedSet.empty() ? nullptr : &m_restrictedSet ); + &aPath, m_itemMask, m_restrictedSet.empty() ? nullptr : &m_restrictedSet, false ); if( m_restrictedSet.empty() ) return obs;