Prevent lockup when tuning differential pair length

Fix freezes caused excessive branching.  Replaced the recursive
branching tracking with iterative joint mapping and a timeout fail-safe.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/22041
This commit is contained in:
Seth Hillbrand
2026-01-07 16:08:27 -08:00
parent 4d7c6f2f8d
commit bc37f184bb
5 changed files with 139 additions and 70 deletions
+5
View File
@@ -137,6 +137,7 @@ static const wxChar SimulatorMultiRunCombinationLimit[] = wxT( "SimulatorMultiRu
static const wxChar GitIconRefreshInterval[] = wxT( "GitIconRefreshInterval" );
static const wxChar MaxPastedTextLength[] = wxT( "MaxPastedTextLength" );
static const wxChar PNSProcessClusterTimeout[] = wxT( "PNSProcessClusterTimeout" );
static const wxChar FollowBranchTimeout[] = wxT( "FollowBranchTimeoutMs" );
static const wxChar ImportSkipComponentBodies[] = wxT( "ImportSkipComponentBodies" );
static const wxChar ScreenDPI[] = wxT( "ScreenDPI" );
static const wxChar EnableVariantsUI[] = wxT( "EnableVariantsUI" );
@@ -324,6 +325,7 @@ ADVANCED_CFG::ADVANCED_CFG()
m_MaxPastedTextLength = 100;
m_PNSProcessClusterTimeout = 100; // Default: 100 ms
m_FollowBranchTimeout = 500; // Default: 500 ms
m_ImportSkipComponentBodies = false;
@@ -630,6 +632,9 @@ void ADVANCED_CFG::loadSettings( wxConfigBase& aCfg )
m_entries.push_back( std::make_unique<PARAM_CFG_INT>( true, AC_KEYS::PNSProcessClusterTimeout,
&m_PNSProcessClusterTimeout, 100, 10, 10000 ) );
m_entries.push_back( std::make_unique<PARAM_CFG_INT>( true, AC_KEYS::FollowBranchTimeout,
&m_FollowBranchTimeout, 500, 50, 5000 ) );
m_entries.push_back( std::make_unique<PARAM_CFG_BOOL>( true, AC_KEYS::ImportSkipComponentBodies,
&m_ImportSkipComponentBodies, m_ImportSkipComponentBodies ) );
+12
View File
@@ -851,6 +851,18 @@ public:
*/
int m_PNSProcessClusterTimeout;
/**
* Timeout for the PNS router's followBranch path search, in milliseconds.
*
* This limits how long the router will spend searching for the longest path
* through a complex track topology before returning the best path found so far.
*
* Setting name: "FollowBranchTimeoutMs"
* Valid values: 50 to 5000
* Default value: 500
*/
int m_FollowBranchTimeout;
/**
* Skip importing component bodies when importing some format files, such as Altium.
*
+12
View File
@@ -220,6 +220,18 @@ bool DP_MEANDER_PLACER::Move( const VECTOR2I& aP, ITEM* aEndItem )
m_originPair.CP().Split( m_currentStart, aP, preP, tunedP, postP );
m_originPair.CN().Split( m_currentStart, aP, preN, tunedN, postN );
// Bail out early if the tuned sections are empty (issue #22041). This can happen when the
// split points are too close together or outside the line chain.
if( tunedP.PointCount() == 0 || tunedN.PointCount() == 0 )
{
m_finalShapeP = m_originPair.CP();
m_finalShapeN = m_originPair.CN();
m_lastLength = origPathLength();
m_lastStatus = TOO_SHORT;
return false;
}
auto updateStatus =
[&]()
{
+109 -69
View File
@@ -21,6 +21,11 @@
#include <wx/log.h>
#include <chrono>
#include <stack>
#include <advanced_config.h>
#include "pns_line.h"
#include "pns_segment.h"
#include "pns_arc.h"
@@ -203,106 +208,141 @@ ITEM* TOPOLOGY::NearestUnconnectedItem( const JOINT* aStart, int* aAnchor, int a
}
TOPOLOGY::PATH_RESULT TOPOLOGY::followBranch( const JOINT* aJoint, LINKED_ITEM* aPrev,
TOPOLOGY::PATH_RESULT TOPOLOGY::followBranch( const JOINT* aStartJoint, LINKED_ITEM* aPrev,
std::set<ITEM*>& aVisited,
bool aFollowLockedSegments )
{
static int depth = 0;
depth++;
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: joint at (%d,%d), prev=%p" ),
depth, aJoint->Pos().x, aJoint->Pos().y, aPrev );
using clock = std::chrono::steady_clock;
PATH_RESULT best;
best.m_end = aStartJoint;
ITEM* via = nullptr;
ITEM_SET links( aJoint->CLinks() );
const int timeoutMs = ADVANCED_CFG::GetCfg().m_FollowBranchTimeout;
auto startTime = clock::now();
for( ITEM* link : links )
// State for iterative DFS: current joint, previous item, accumulated path items,
// accumulated length, and the set of visited joints for this path
struct STATE
{
if( link->OfKind( ITEM::VIA_T ) && !aVisited.contains( link ) )
via = link;
}
const JOINT* joint;
LINKED_ITEM* prev;
ITEM_SET pathItems;
int pathLength;
std::set<const JOINT*> visitedJoints;
ITEM* via;
};
int branchCount = 0;
for( ITEM* link : links )
std::stack<STATE> stateStack;
// Initialize with starting state
STATE initial;
initial.joint = aStartJoint;
initial.prev = aPrev;
initial.pathLength = 0;
initial.visitedJoints.insert( aStartJoint );
initial.via = nullptr;
stateStack.push( std::move( initial ) );
while( !stateStack.empty() )
{
if( link->OfKind( ITEM::SEGMENT_T | ITEM::ARC_T )
&& link != aPrev && !aVisited.contains( link ) )
// Check timeout
auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
clock::now() - startTime ).count();
if( elapsed > timeoutMs )
{
branchCount++;
wxLogTrace( wxT( "PNS_TUNE" ),
wxT( "followBranch: timeout after %lld ms, returning best path found" ),
elapsed );
break;
}
STATE current = std::move( stateStack.top() );
stateStack.pop();
const JOINT* joint = current.joint;
ITEM_SET links( joint->CLinks() );
// Check for via at this joint
ITEM* via = nullptr;
for( ITEM* link : links )
{
if( link->OfKind( ITEM::VIA_T ) && !aVisited.contains( link ) )
{
via = link;
break;
}
}
// Find all unvisited branches from this joint
bool foundBranch = false;
for( ITEM* link : links )
{
if( !link->OfKind( ITEM::SEGMENT_T | ITEM::ARC_T ) )
continue;
if( link == current.prev )
continue;
if( aVisited.contains( link ) )
continue;
LINE l = m_world->AssembleLine( static_cast<LINKED_ITEM*>( link ), nullptr,
false, aFollowLockedSegments );
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: branch %d - assembled line with %d segments" ),
depth, branchCount, l.SegmentCount() );
if( l.CPoint( 0 ) != aJoint->Pos() )
{
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: branch %d - reversing line" ),
depth, branchCount );
if( l.CPoint( 0 ) != joint->Pos() )
l.Reverse();
}
const JOINT* next = m_world->FindJoint( l.CLastPoint(), &l );
const JOINT* nextJoint = m_world->FindJoint( l.CLastPoint(), &l );
for( LINKED_ITEM* ll : l.Links() )
aVisited.insert( ll );
// Skip if we've already visited this joint in the current path
if( current.visitedJoints.count( nextJoint ) )
continue;
foundBranch = true;
// Build new state for this branch
STATE nextState;
nextState.joint = nextJoint;
nextState.prev = l.Links().back();
nextState.pathItems = current.pathItems;
nextState.pathLength = current.pathLength + l.CLine().Length();
nextState.visitedJoints = current.visitedJoints;
nextState.visitedJoints.insert( nextJoint );
nextState.via = via;
// Add via and line to path
if( via )
aVisited.insert( via );
nextState.pathItems.Add( via );
int lineLength = l.CLine().Length();
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: branch %d - line length=%d, recursing..." ),
depth, branchCount, lineLength );
nextState.pathItems.Add( l );
PATH_RESULT sub = followBranch( next, l.Links().back(), aVisited, aFollowLockedSegments );
stateStack.push( std::move( nextState ) );
}
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: branch %d - sub-path returned with length=%d, %d items" ),
depth, branchCount, sub.m_length, sub.m_items.Size() );
ITEM_SET tmp;
if( via )
tmp.Add( via );
tmp.Add( l );
for( ITEM* it : sub.m_items )
tmp.Add( it );
int len = l.CLine().Length() + sub.m_length;
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: branch %d - total length=%d (line=%d + sub=%d), best so far=%d" ),
depth, branchCount, len, lineLength, sub.m_length, best.m_length );
if( len > best.m_length )
// If no branches found, this is a terminal joint - check if it's the best path
if( !foundBranch )
{
if( current.pathLength > best.m_length )
{
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: branch %d - NEW BEST! Replacing best path" ),
depth, branchCount );
best.m_length = len;
best.m_end = sub.m_end;
best.m_items = tmp;
best.m_length = current.pathLength;
best.m_end = joint;
best.m_items = current.pathItems;
}
for( LINKED_ITEM* ll : l.Links() )
aVisited.erase( ll );
if( via )
aVisited.erase( via );
}
}
if( !best.m_end )
{
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: no branches found, terminal joint" ), depth );
best.m_end = aJoint;
}
wxLogTrace( wxT( "PNS_TUNE" ),
wxT( "followBranch: completed with best path length=%d, %d items" ),
best.m_length, best.m_items.Size() );
wxLogTrace( wxT( "PNS_TUNE" ), wxT( "followBranch[depth=%d]: returning best path with length=%d, %d items" ),
depth, best.m_length, best.m_items.Size() );
depth--;
return best;
}
ITEM_SET TOPOLOGY::followTrivialPath( LINE* aLine2, const JOINT** aTerminalJointA,
const JOINT** aTerminalJointB,
bool aFollowLockedSegments )
+1 -1
View File
@@ -113,7 +113,7 @@ private:
PATH_RESULT() : m_end( nullptr ), m_length( 0 ) {}
};
PATH_RESULT followBranch( const JOINT* aJoint, LINKED_ITEM* aPrev,
PATH_RESULT followBranch( const JOINT* aStartJoint, LINKED_ITEM* aPrev,
std::set<ITEM*>& aVisited, bool aFollowLockedSegments );
ITEM_SET followTrivialPath( LINE* aLine, const JOINT** aTerminalJointA,