electron-rare/kicad-source-mirror
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki 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

(cherry picked from commit 3e8791b88d)
This commit is contained in:
Seth Hillbrand
2025-01-29 09:59:26 -06:00
parent a8d09657a8
commit 17dcfdbf7a
4 changed files with 141 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
libs/kimath/include/geometry/shape_arc.h
+11
View File
@@ -118,6 +118,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,
libs/kimath/src/geometry/shape_arc.cpp
+71
View File
@@ -395,6 +395,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
{
pcbnew/drc/drc_test_provider_diff_pair_coupling.cpp
+56 -53
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,53 +389,46 @@ 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 )
{
auto 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;
}
}
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() ) )
qa/tests/pcbnew/drc/test_drc_regressions.cpp
+3 -1
View File
@@ -65,6 +65,7 @@ BOOST_FIXTURE_TEST_CASE( DRCFalsePositiveRegressions, DRC_REGRESSION_TEST_FIXTUR
"issue15280", // Very wide spokes mis-counted as being single spoke
"issue14008", // Net-tie clearance error
"issue19521/issue19521", // oval annular ring size
"issue17967/issue17967", // Arc dp coupling
"unconnected-netnames/unconnected-netnames", // Raised false schematic partity error
};
@@ -128,7 +129,7 @@ BOOST_FIXTURE_TEST_CASE( DRCFalseNegativeRegressions, DRC_REGRESSION_TEST_FIXTUR
{
{ "issue1358", 2 },
{ "issue2512", 5 },
{ "issue2528", 1 },
{ "issue2528", 1 },
{ "issue5750", 4 }, // Shorting zone fills pass DRC in some cases
{ "issue5854", 3 },
{ "issue6879", 6 },
@@ -144,6 +145,7 @@ 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
{ "issue18878", 9 },
{ "issue19325/issue19325", 4 }, // Overlapping pad annular ring calculation
};