AllSpiceMirrors/kicad
7
Fork 0
mirror of https://gitlab.com/kicad/code/kicad.git synced 2025-04-19 23:31:40 +00:00
Files Issues Releases Activity

Fixup multiple issues with DP checks

Browse Source 
Handle arcs properly even when not exactly concentric.  Properly check
for interfering elements that would prevent coupling.  Avoiding those
that are directly connected

Fixes https://gitlab.com/kicad/code/kicad/-/issues/17967
This commit is contained in:
Seth Hillbrand 2025-01-13 16:02:26 -08:00
parent ce483c848a
commit 3e8791b88d
4 changed files with 137 additions and 52 deletions
libs/kimath
include/geometry
shape_arc.h
src/geometry
shape_arc.cpp
pcbnew/drc
drc_test_provider_diff_pair_coupling.cpp
qa/tests/pcbnew/drc
test_drc_regressions.cpp

11
libs/kimath/include/geometry/shape_arc.h
View File

@ -120,6 +120,17 @@ public:
VECTOR2I NearestPoint( const VECTOR2I& aP ) const;
/**
* Compute closest points between this arc and \a aArc.
*
* @param aPtA point on this arc (output)
* @param aPtB point on the other arc (output)
* @param aDistSq squared distance between points (output)
* @return true if the operation was successful
*/
bool NearestPoints( const SHAPE_ARC& aArc, VECTOR2I& aPtA, VECTOR2I& aPtB, int64_t& aDistSq ) const;
bool Collide( const SEG& aSeg, int aClearance = 0, int* aActual = nullptr,
VECTOR2I* aLocation = nullptr ) const override;
bool Collide( const VECTOR2I& aP, int aClearance = 0, int* aActual = nullptr,

71
libs/kimath/src/geometry/shape_arc.cpp
View File

@ -446,6 +446,77 @@ VECTOR2I SHAPE_ARC::NearestPoint( const VECTOR2I& aP ) const
}
bool SHAPE_ARC::NearestPoints( const SHAPE_ARC& aArc, VECTOR2I& aPtA, VECTOR2I& aPtB,
int64_t& aDistSq ) const
{
aDistSq = std::numeric_limits<int64_t>::max();
VECTOR2I center1 = GetCenter();
VECTOR2I center2 = aArc.GetCenter();
int64_t dist = center1.SquaredDistance( center2 );
// Start by checking endpoints
std::vector<VECTOR2I> pts1 = { m_start, m_end };
std::vector<VECTOR2I> pts2 = { aArc.GetP0(), aArc.GetP1() };
for( const VECTOR2I& pt1 : pts1 )
{
for( const VECTOR2I& pt2 : pts2 )
{
int64_t distSq = pt1.SquaredDistance( pt2 );
if( distSq < aDistSq )
{
aDistSq = distSq;
aPtA = pt1;
aPtB = pt2;
}
}
}
// If the centers are the same, the end points are always the closest
// or at least equidistant
if( dist > 0 )
{
CIRCLE circle1( center1, GetRadius() );
CIRCLE circle2( center2, aArc.GetRadius() );
// First check for intersections on the circles
std::vector<VECTOR2I> pts = circle1.Intersect( circle2 );
for( const VECTOR2I& pt : pts )
{
if( sliceContainsPoint( pt ) && aArc.sliceContainsPoint( pt ) )
{
aPtA = pt;
aPtB = pt;
aDistSq = 0;
return true;
}
}
// Check for the closest points on the circles
VECTOR2I pt1 = circle1.NearestPoint( center2 );
VECTOR2I pt2 = circle2.NearestPoint( center1 );
if( sliceContainsPoint( pt1 ) && aArc.sliceContainsPoint( pt2 ) )
{
int64_t distSq = pt1.SquaredDistance( pt2 );
if( distSq < aDistSq )
{
aDistSq = distSq;
aPtA = pt1;
aPtB = pt2;
}
}
}
return true;
}
bool SHAPE_ARC::Collide( const VECTOR2I& aP, int aClearance, int* aActual,
VECTOR2I* aLocation ) const
{

105
pcbnew/drc/drc_test_provider_diff_pair_coupling.cpp
View File

@ -193,6 +193,10 @@ static bool commonParallelProjection( const PCB_ARC& p, const PCB_ARC& n, SHAPE_
pClip = SHAPE_ARC( p_center, p_start_pt, clip_total_angle );
nClip = SHAPE_ARC( n_center, n_start_pt, clip_total_angle );
if( std::abs( pClip.GetP0().x - pClip.GetArcMid().x ) < 10
|| std::abs( nClip.GetP0().x - nClip.GetArcMid().x ) < 10 )
return false;
return true;
}
@ -240,7 +244,9 @@ struct DIFF_PAIR_COUPLED_SEGMENTS
SHAPE_ARC coupledArcP;
PCB_TRACK* parentN;
PCB_TRACK* parentP;
int computedGap;
int64_t computedGap;
VECTOR2I nearestN;
VECTOR2I nearestP;
PCB_LAYER_ID layer;
bool couplingFailMin;
bool couplingFailMax;
@ -272,8 +278,7 @@ static void extractDiffPairCoupledItems( DIFF_PAIR_ITEMS& aDp )
for( BOARD_CONNECTED_ITEM* itemP : aDp.itemsP )
{
PCB_TRACK* sp = dyn_cast<PCB_TRACK*>( itemP );
std::optional<DIFF_PAIR_COUPLED_SEGMENTS> bestCoupled;
int bestGap = std::numeric_limits<int>::max();
std::vector<std::optional<DIFF_PAIR_COUPLED_SEGMENTS>> coupled_vec;
if( !sp )
continue;
@ -293,7 +298,7 @@ static void extractDiffPairCoupledItems( DIFF_PAIR_ITEMS& aDp )
// Segments that are ~ 1 IU in length per side are approximately parallel (tolerance is 1 IU)
// with everything and their parallel projection is < 1 IU, leading to bad distance calculations
if( ssp.SquaredLength() > 2 && ssn.SquaredLength() > 2 && ssp.ApproxParallel(ssn) )
if( ssp.SquaredLength() > 2 && ssn.SquaredLength() > 2 && !ssp.Intersect( ssn, false, true ) )
{
DIFF_PAIR_COUPLED_SEGMENTS cpair;
bool coupled = commonParallelProjection( ssp, ssn, cpair.coupledP, cpair.coupledN );
@ -303,33 +308,33 @@ static void extractDiffPairCoupledItems( DIFF_PAIR_ITEMS& aDp )
cpair.parentP = sp;
cpair.parentN = sn;
cpair.layer = sp->GetLayer();
cpair.computedGap = (cpair.coupledP.A - cpair.coupledN.A).EuclideanNorm();
cpair.coupledP.NearestPoints( cpair.coupledN, cpair.nearestP, cpair.nearestN,
cpair.computedGap );
cpair.computedGap = std::sqrt( cpair.computedGap ); // NearestPoints returns squared distance
cpair.computedGap -= ( sp->GetWidth() + sn->GetWidth() ) / 2;
if( cpair.computedGap < bestGap )
{
bestGap = cpair.computedGap;
bestCoupled = cpair;
}
coupled_vec.push_back( cpair );
}
}
}
if( bestCoupled )
for( auto coupled : coupled_vec )
{
auto excludeSelf = [&]( BOARD_ITEM* aItem )
{
if( aItem == bestCoupled->parentN || aItem == bestCoupled->parentP )
if( aItem == coupled->parentN || aItem == coupled->parentP )
return false;
if( aItem->Type() == PCB_TRACE_T || aItem->Type() == PCB_VIA_T
|| aItem->Type() == PCB_ARC_T )
{
BOARD_CONNECTED_ITEM* bci = static_cast<BOARD_CONNECTED_ITEM*>( aItem );
PCB_TRACK* bci = static_cast<PCB_TRACK*>( aItem );
if( bci->GetNetCode() == bestCoupled->parentN->GetNetCode()
|| bci->GetNetCode() == bestCoupled->parentP->GetNetCode() )
// Directly connected items don't count
if( bci->HitTest( coupled->coupledN.A, 0 )
|| bci->HitTest( coupled->coupledN.B, 0 )
|| bci->HitTest( coupled->coupledP.A, 0 )
|| bci->HitTest( coupled->coupledP.B, 0 ) )
{
return false;
}
@ -338,17 +343,15 @@ static void extractDiffPairCoupledItems( DIFF_PAIR_ITEMS& aDp )
return true;
};
SHAPE_SEGMENT checkSegStart( bestCoupled->coupledP.A, bestCoupled->coupledN.A );
SHAPE_SEGMENT checkSegEnd( bestCoupled->coupledP.B, bestCoupled->coupledN.B );
DRC_RTREE* tree = bestCoupled->parentP->GetBoard()->m_CopperItemRTreeCache.get();
SHAPE_SEGMENT checkSeg( coupled->nearestN, coupled->nearestP );
DRC_RTREE* tree = coupled->parentP->GetBoard()->m_CopperItemRTreeCache.get();
// check if there's anything in between the segments suspected to be coupled. If
// there's nothing, assume they are really coupled.
if( !tree->CheckColliding( &checkSegStart, sp->GetLayer(), 0, excludeSelf )
&& !tree->CheckColliding( &checkSegEnd, sp->GetLayer(), 0, excludeSelf ) )
if( !tree->CheckColliding( &checkSeg, sp->GetLayer(), 0, excludeSelf ) )
{
aDp.coupled.push_back( *bestCoupled );
aDp.coupled.push_back( *coupled );
}
}
}
@ -356,8 +359,7 @@ static void extractDiffPairCoupledItems( DIFF_PAIR_ITEMS& aDp )
for( BOARD_CONNECTED_ITEM* itemP : aDp.itemsP )
{
PCB_ARC* sp = dyn_cast<PCB_ARC*>( itemP );
std::optional<DIFF_PAIR_COUPLED_SEGMENTS> bestCoupled;
int bestGap = std::numeric_limits<int>::max();
std::vector<std::optional<DIFF_PAIR_COUPLED_SEGMENTS>> coupled_vec;
if( !sp )
continue;
@ -374,7 +376,9 @@ static void extractDiffPairCoupledItems( DIFF_PAIR_ITEMS& aDp )
// Segments that are ~ 1 IU in length per side are approximately parallel (tolerance is 1 IU)
// with everything and their parallel projection is < 1 IU, leading to bad distance calculations
if( sp->GetLength() > 2 && sn->GetLength() > 2 && ( sp->GetCenter() - sn->GetCenter() ).SquaredEuclideanNorm() < 4 )
int64_t sqWidth = static_cast<int64_t>( sp->GetWidth() ) * sp->GetWidth();
if( sp->GetLength() > 2 && sn->GetLength() > 2 && sp->GetCenter().SquaredDistance( sn->GetCenter() ) < sqWidth )
{
DIFF_PAIR_COUPLED_SEGMENTS cpair;
cpair.isArc = true;
@ -385,34 +389,29 @@ static void extractDiffPairCoupledItems( DIFF_PAIR_ITEMS& aDp )
cpair.parentP = sp;
cpair.parentN = sn;
cpair.layer = sp->GetLayer();
cpair.computedGap = KiROUND( std::abs( cpair.coupledArcP.GetRadius()
- cpair.coupledArcN.GetRadius() ) );
cpair.coupledArcP.NearestPoints( cpair.coupledArcN, cpair.nearestP, cpair.nearestN,
cpair.computedGap );
cpair.computedGap = std::sqrt( cpair.computedGap ); // NearestPoints returns squared distance
cpair.computedGap -= ( sp->GetWidth() + sn->GetWidth() ) / 2;
if( cpair.computedGap < bestGap )
{
bestGap = cpair.computedGap;
bestCoupled = cpair;
}
coupled_vec.push_back( cpair );
}
}
}
if( bestCoupled )
for( auto coupled : coupled_vec )
{
auto excludeSelf =
[&] ( BOARD_ITEM *aItem )
{
if( aItem == bestCoupled->parentN || aItem == bestCoupled->parentP )
if( aItem == coupled->parentN || aItem == coupled->parentP )
return false;
if( aItem->Type() == PCB_TRACE_T || aItem->Type() == PCB_VIA_T || aItem->Type() == PCB_ARC_T )
{
BOARD_CONNECTED_ITEM* bci = static_cast<BOARD_CONNECTED_ITEM*>( aItem );
if( bci->GetNetCode() == bestCoupled->parentN->GetNetCode()
|| bci->GetNetCode() == bestCoupled->parentP->GetNetCode() )
if( bci->GetNetCode() == coupled->parentN->GetNetCode()
|| bci->GetNetCode() == coupled->parentP->GetNetCode() )
{
return false;
}
@ -421,17 +420,15 @@ static void extractDiffPairCoupledItems( DIFF_PAIR_ITEMS& aDp )
return true;
};
SHAPE_SEGMENT checkSegStart( bestCoupled->coupledP.A, bestCoupled->coupledN.A );
SHAPE_SEGMENT checkSegEnd( bestCoupled->coupledP.B, bestCoupled->coupledN.B );
DRC_RTREE* tree = bestCoupled->parentP->GetBoard()->m_CopperItemRTreeCache.get();
SHAPE_SEGMENT checkArcMid( coupled->coupledArcN.GetArcMid(), coupled->coupledArcP.GetArcMid() );
DRC_RTREE* tree = coupled->parentP->GetBoard()->m_CopperItemRTreeCache.get();
// check if there's anything in between the segments suspected to be coupled. If
// there's nothing, assume they are really coupled.
if( !tree->CheckColliding( &checkSegStart, sp->GetLayer(), 0, excludeSelf )
&& !tree->CheckColliding( &checkSegEnd, sp->GetLayer(), 0, excludeSelf ) )
if( !tree->CheckColliding( &checkArcMid, sp->GetLayer(), 0, excludeSelf ) )
{
aDp.coupled.push_back( *bestCoupled );
aDp.coupled.push_back( *coupled );
}
}
}
@ -536,22 +533,29 @@ bool test::DRC_TEST_PROVIDER_DIFF_PAIR_COUPLING::Run()
drc_dbg(10, wxT( " coupled prims : %d\n" ), (int) itemSet.coupled.size() );
std::set<BOARD_CONNECTED_ITEM*> allItems;
for( BOARD_CONNECTED_ITEM* item : itemSet.itemsN )
{
if( PCB_TRACK* track = dynamic_cast<PCB_TRACK*>( item ) )
itemSet.totalLengthN += track->GetLength();
{
if( allItems.insert( item ).second)
itemSet.totalLengthN += track->GetLength();
}
}
for( BOARD_CONNECTED_ITEM* item : itemSet.itemsP )
{
if( PCB_TRACK* track = dynamic_cast<PCB_TRACK*>( item ) )
itemSet.totalLengthP += track->GetLength();
{
if( allItems.insert( item ).second)
itemSet.totalLengthP += track->GetLength();
}
}
for( DIFF_PAIR_COUPLED_SEGMENTS& dp : itemSet.coupled )
{
int length = dp.coupledN.Length();
int length = dp.isArc ? dp.coupledArcN.GetLength() : dp.coupledN.Length();
wxCHECK2( dp.parentN && dp.parentP, continue );
std::shared_ptr<KIGFX::VIEW_OVERLAY> overlay = m_drcEngine->GetDebugOverlay();
@ -567,7 +571,7 @@ bool test::DRC_TEST_PROVIDER_DIFF_PAIR_COUPLING::Run()
overlay->Line( dp.coupledN );
}
drc_dbg( 10, wxT( " len %d gap %d l %d\n" ),
drc_dbg( 10, wxT( " len %d gap %ld l %d\n" ),
length,
dp.computedGap,
dp.parentP->GetLayer() );
@ -598,8 +602,7 @@ bool test::DRC_TEST_PROVIDER_DIFF_PAIR_COUPLING::Run()
MessageTextFromValue( itemSet.totalCoupled ),
MessageTextFromValue( totalLen ) ) );
int totalUncoupled = totalLen - itemSet.totalCoupled;
int totalUncoupled = totalLen - itemSet.totalCoupled;
bool uncoupledViolation = false;
if( key.uncoupledConstraint && ( !itemSet.itemsP.empty() || !itemSet.itemsN.empty() ) )

2
qa/tests/pcbnew/drc/test_drc_regressions.cpp
View File

@ -65,6 +65,7 @@ BOOST_FIXTURE_TEST_CASE( DRCFalsePositiveRegressions, DRC_REGRESSION_TEST_FIXTUR
"issue14412", // Solder mask bridge between pads in a net-tie pad group
"issue15280", // Very wide spokes mis-counted as being single spoke
"issue14008", // Net-tie clearance error
"issue17967/issue17967", // Arc dp coupling
"unconnected-netnames/unconnected-netnames", // Raised false schematic partity error
};
@ -149,7 +150,6 @@ BOOST_FIXTURE_TEST_CASE( DRCFalseNegativeRegressions, DRC_REGRESSION_TEST_FIXTUR
{ "reverse_via", 3, {} }, // Via/track ordering
{ "intersectingzones", 1, {} }, // zones are too close to each other
{ "fill_bad", 1, {} }, // zone max BBox was too small
{ "issue17967/issue17967", 1, {}}, // Arc dp coupling
{ "issue18878", 9, {} },
{ "issue19325/issue19325", 4, issue19325_ignore }, // Overlapping pad annular ring calculation
};