Skip EvalRules when no custom rules exist
GetSolderMaskExpansion() on PAD, PCB_TRACK, and PCB_SHAPE called EvalRules(SOLDER_MASK_EXPANSION_CONSTRAINT) unconditionally during DRC, even when no custom rules existed for that constraint type. Now check HasRulesForConstraintType() first and fall back to the direct property lookup chain when no custom rules exist. Also fix a bug where PCB_TRACK and PCB_SHAPE returned 0 instead of the board default mask expansion when no local override was set. Apply the same optimization to solder paste margin lookups. Add per-provider DRC timing via the existing KICAD_DRC_PROFILE trace mask. Fixes https://gitlab.com/kicad/code/kicad/-/issues/23213
This commit is contained in:
@@ -909,8 +909,14 @@ void DRC_ENGINE::RunTests( EDA_UNITS aUnits, bool aReportAllTrackErrors, bool aT
|
||||
if( m_logReporter )
|
||||
m_logReporter->Report( wxString::Format( wxT( "Run DRC provider: '%s'" ), provider->GetName() ) );
|
||||
|
||||
PROF_TIMER providerTimer;
|
||||
|
||||
if( !provider->RunTests( aUnits ) )
|
||||
break;
|
||||
|
||||
providerTimer.Stop();
|
||||
wxLogTrace( traceDrcProfile, "DRC provider '%s' took %0.3f ms",
|
||||
provider->GetName(), providerTimer.msecs() );
|
||||
}
|
||||
|
||||
timer.Stop();
|
||||
|
||||
+54
-12
@@ -1653,7 +1653,9 @@ int PAD::GetSolderMaskExpansion( PCB_LAYER_ID aLayer ) const
|
||||
|
||||
std::optional<int> margin;
|
||||
|
||||
if( GetBoard() && GetBoard()->GetDesignSettings().m_DRCEngine )
|
||||
if( GetBoard() && GetBoard()->GetDesignSettings().m_DRCEngine
|
||||
&& GetBoard()->GetDesignSettings().m_DRCEngine->HasRulesForConstraintType(
|
||||
SOLDER_MASK_EXPANSION_CONSTRAINT ) )
|
||||
{
|
||||
DRC_CONSTRAINT constraint;
|
||||
std::shared_ptr<DRC_ENGINE> drcEngine = GetBoard()->GetDesignSettings().m_DRCEngine;
|
||||
@@ -1672,6 +1674,12 @@ int PAD::GetSolderMaskExpansion( PCB_LAYER_ID aLayer ) const
|
||||
if( FOOTPRINT* parentFootprint = GetParentFootprint() )
|
||||
margin = parentFootprint->GetLocalSolderMaskMargin();
|
||||
}
|
||||
|
||||
if( !margin.has_value() )
|
||||
{
|
||||
if( const BOARD* brd = GetBoard() )
|
||||
margin = brd->GetDesignSettings().m_SolderMaskExpansion;
|
||||
}
|
||||
}
|
||||
|
||||
int marginValue = margin.value_or( 0 );
|
||||
@@ -1709,25 +1717,42 @@ VECTOR2I PAD::GetSolderPasteMargin( PCB_LAYER_ID aLayer ) const
|
||||
std::optional<int> margin;
|
||||
std::optional<double> mratio;
|
||||
|
||||
if( GetBoard() && GetBoard()->GetDesignSettings().m_DRCEngine )
|
||||
std::shared_ptr<DRC_ENGINE> drcEngine;
|
||||
|
||||
if( GetBoard() )
|
||||
drcEngine = GetBoard()->GetDesignSettings().m_DRCEngine;
|
||||
|
||||
bool hasAbsRules = drcEngine
|
||||
&& drcEngine->HasRulesForConstraintType( SOLDER_PASTE_ABS_MARGIN_CONSTRAINT );
|
||||
bool hasRelRules = drcEngine
|
||||
&& drcEngine->HasRulesForConstraintType( SOLDER_PASTE_REL_MARGIN_CONSTRAINT );
|
||||
|
||||
if( hasAbsRules || hasRelRules )
|
||||
{
|
||||
DRC_CONSTRAINT constraint;
|
||||
std::shared_ptr<DRC_ENGINE> drcEngine = GetBoard()->GetDesignSettings().m_DRCEngine;
|
||||
DRC_CONSTRAINT constraint;
|
||||
|
||||
constraint = drcEngine->EvalRules( SOLDER_PASTE_ABS_MARGIN_CONSTRAINT, this, nullptr, aLayer );
|
||||
if( hasAbsRules )
|
||||
{
|
||||
constraint = drcEngine->EvalRules( SOLDER_PASTE_ABS_MARGIN_CONSTRAINT, this, nullptr,
|
||||
aLayer );
|
||||
|
||||
if( constraint.m_Value.HasOpt() )
|
||||
margin = constraint.m_Value.Opt();
|
||||
if( constraint.m_Value.HasOpt() )
|
||||
margin = constraint.m_Value.Opt();
|
||||
}
|
||||
|
||||
constraint = drcEngine->EvalRules( SOLDER_PASTE_REL_MARGIN_CONSTRAINT, this, nullptr, aLayer );
|
||||
if( hasRelRules )
|
||||
{
|
||||
constraint = drcEngine->EvalRules( SOLDER_PASTE_REL_MARGIN_CONSTRAINT, this, nullptr,
|
||||
aLayer );
|
||||
|
||||
if( constraint.m_Value.HasOpt() )
|
||||
mratio = constraint.m_Value.Opt() / 1000.0;
|
||||
if( constraint.m_Value.HasOpt() )
|
||||
mratio = constraint.m_Value.Opt() / 1000.0;
|
||||
}
|
||||
}
|
||||
else
|
||||
|
||||
if( !margin.has_value() )
|
||||
{
|
||||
margin = m_padStack.SolderPasteMargin( aLayer );
|
||||
mratio = m_padStack.SolderPasteMarginRatio( aLayer );
|
||||
|
||||
if( !margin.has_value() )
|
||||
{
|
||||
@@ -1735,11 +1760,28 @@ VECTOR2I PAD::GetSolderPasteMargin( PCB_LAYER_ID aLayer ) const
|
||||
margin = parentFootprint->GetLocalSolderPasteMargin();
|
||||
}
|
||||
|
||||
if( !margin.has_value() )
|
||||
{
|
||||
if( const BOARD* brd = GetBoard() )
|
||||
margin = brd->GetDesignSettings().m_SolderPasteMargin;
|
||||
}
|
||||
}
|
||||
|
||||
if( !mratio.has_value() )
|
||||
{
|
||||
mratio = m_padStack.SolderPasteMarginRatio( aLayer );
|
||||
|
||||
if( !mratio.has_value() )
|
||||
{
|
||||
if( FOOTPRINT* parentFootprint = GetParentFootprint() )
|
||||
mratio = parentFootprint->GetLocalSolderPasteMarginRatio();
|
||||
}
|
||||
|
||||
if( !mratio.has_value() )
|
||||
{
|
||||
if( const BOARD* brd = GetBoard() )
|
||||
mratio = brd->GetDesignSettings().m_SolderPasteMarginRatio;
|
||||
}
|
||||
}
|
||||
|
||||
PCB_LAYER_ID cuLayer = ( aLayer == B_Paste ) ? B_Cu : F_Cu;
|
||||
|
||||
@@ -189,7 +189,9 @@ int PCB_SHAPE::GetSolderMaskExpansion() const
|
||||
{
|
||||
int margin = 0;
|
||||
|
||||
if( GetBoard() && GetBoard()->GetDesignSettings().m_DRCEngine )
|
||||
if( GetBoard() && GetBoard()->GetDesignSettings().m_DRCEngine
|
||||
&& GetBoard()->GetDesignSettings().m_DRCEngine->HasRulesForConstraintType(
|
||||
SOLDER_MASK_EXPANSION_CONSTRAINT ) )
|
||||
{
|
||||
DRC_CONSTRAINT constraint;
|
||||
std::shared_ptr<DRC_ENGINE> drcEngine = GetBoard()->GetDesignSettings().m_DRCEngine;
|
||||
@@ -203,6 +205,10 @@ int PCB_SHAPE::GetSolderMaskExpansion() const
|
||||
{
|
||||
margin = m_solderMaskMargin.value();
|
||||
}
|
||||
else if( const BOARD* board = GetBoard() )
|
||||
{
|
||||
margin = board->GetDesignSettings().m_SolderMaskExpansion;
|
||||
}
|
||||
|
||||
// Ensure the resulting mask opening has a non-negative size
|
||||
if( margin < 0 && !IsSolidFill() )
|
||||
|
||||
@@ -1433,7 +1433,9 @@ int PCB_TRACK::GetSolderMaskExpansion() const
|
||||
{
|
||||
int margin = 0;
|
||||
|
||||
if( GetBoard() && GetBoard()->GetDesignSettings().m_DRCEngine )
|
||||
if( GetBoard() && GetBoard()->GetDesignSettings().m_DRCEngine
|
||||
&& GetBoard()->GetDesignSettings().m_DRCEngine->HasRulesForConstraintType(
|
||||
SOLDER_MASK_EXPANSION_CONSTRAINT ) )
|
||||
{
|
||||
DRC_CONSTRAINT constraint;
|
||||
std::shared_ptr<DRC_ENGINE> drcEngine = GetBoard()->GetDesignSettings().m_DRCEngine;
|
||||
@@ -1447,6 +1449,10 @@ int PCB_TRACK::GetSolderMaskExpansion() const
|
||||
{
|
||||
margin = m_solderMaskMargin.value();
|
||||
}
|
||||
else if( const BOARD* board = GetBoard() )
|
||||
{
|
||||
margin = board->GetDesignSettings().m_SolderMaskExpansion;
|
||||
}
|
||||
|
||||
// Ensure the resulting mask opening has a non-negative size
|
||||
if( margin < 0 )
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -26,8 +26,11 @@
|
||||
#include <board.h>
|
||||
#include <board_design_settings.h>
|
||||
#include <pad.h>
|
||||
#include <pcb_track.h>
|
||||
#include <pcb_shape.h>
|
||||
#include <footprint.h>
|
||||
#include <drc/drc_engine.h>
|
||||
#include <core/profile.h>
|
||||
#include <settings/settings_manager.h>
|
||||
|
||||
|
||||
@@ -129,3 +132,80 @@ BOOST_FIXTURE_TEST_CASE( FootprintLevelSolderMaskExpansion, DRC_SOLDER_MASK_EXPA
|
||||
int expansionWithPadMargin = padWithOwnMargin->GetSolderMaskExpansion( F_Mask );
|
||||
BOOST_CHECK_EQUAL( expansionWithPadMargin, 200000 );
|
||||
}
|
||||
|
||||
|
||||
BOOST_FIXTURE_TEST_CASE( SolderMaskExpansionPerformance, DRC_SOLDER_MASK_EXPANSION_TEST_FIXTURE )
|
||||
{
|
||||
// Verify that GetSolderMaskExpansion is fast when no custom DRC rules exist.
|
||||
// Regression test for https://gitlab.com/kicad/code/kicad/-/issues/23213
|
||||
//
|
||||
// Before the fix, GetSolderMaskExpansion called EvalRules for every item even when no
|
||||
// custom rules existed, causing a 10-14x DRC slowdown vs KiCad 9.
|
||||
|
||||
KI_TEST::LoadBoard( m_settingsManager, "stonehenge", m_board );
|
||||
|
||||
BOARD_DESIGN_SETTINGS& bds = m_board->GetDesignSettings();
|
||||
bds.m_DRCEngine->InitEngine( wxFileName() );
|
||||
|
||||
BOOST_REQUIRE( !bds.m_DRCEngine->HasRulesForConstraintType( SOLDER_MASK_EXPANSION_CONSTRAINT ) );
|
||||
|
||||
// Collect all items that would be queried during the solder mask test
|
||||
std::vector<PAD*> pads;
|
||||
std::vector<PCB_TRACK*> tracks;
|
||||
std::vector<PCB_SHAPE*> shapes;
|
||||
|
||||
for( FOOTPRINT* fp : m_board->Footprints() )
|
||||
{
|
||||
for( PAD* pad : fp->Pads() )
|
||||
pads.push_back( pad );
|
||||
|
||||
for( BOARD_ITEM* item : fp->GraphicalItems() )
|
||||
{
|
||||
if( item->Type() == PCB_SHAPE_T )
|
||||
shapes.push_back( static_cast<PCB_SHAPE*>( item ) );
|
||||
}
|
||||
}
|
||||
|
||||
for( PCB_TRACK* track : m_board->Tracks() )
|
||||
tracks.push_back( track );
|
||||
|
||||
for( BOARD_ITEM* item : m_board->Drawings() )
|
||||
{
|
||||
if( item->Type() == PCB_SHAPE_T )
|
||||
shapes.push_back( static_cast<PCB_SHAPE*>( item ) );
|
||||
}
|
||||
|
||||
// Simulate the access pattern from the solder mask bridge test: call GetSolderMaskExpansion
|
||||
// many times for each item (once per potential collision pair).
|
||||
const int iterations = 1000;
|
||||
|
||||
PROF_TIMER timer;
|
||||
|
||||
for( int i = 0; i < iterations; ++i )
|
||||
{
|
||||
for( PAD* pad : pads )
|
||||
pad->GetSolderMaskExpansion( F_Mask );
|
||||
|
||||
for( PCB_TRACK* track : tracks )
|
||||
track->GetSolderMaskExpansion();
|
||||
|
||||
for( PCB_SHAPE* shape : shapes )
|
||||
shape->GetSolderMaskExpansion();
|
||||
}
|
||||
|
||||
timer.Stop();
|
||||
|
||||
int totalCalls = iterations * ( pads.size() + tracks.size() + shapes.size() );
|
||||
|
||||
BOOST_TEST_MESSAGE( wxString::Format( "%d calls to GetSolderMaskExpansion took %0.1f ms "
|
||||
"(%0.0f ns/call)",
|
||||
totalCalls, timer.msecs(),
|
||||
timer.msecs() * 1e6 / totalCalls ) );
|
||||
|
||||
// With the fix (direct property lookup), this should be well under 100ms.
|
||||
// Without the fix (EvalRules for every call), this was ~500ms+ even in debug builds.
|
||||
// Use a generous threshold to avoid flakiness on slow CI machines.
|
||||
BOOST_CHECK_MESSAGE( timer.msecs() < 500.0,
|
||||
wxString::Format( "GetSolderMaskExpansion too slow: %0.1f ms for %d calls",
|
||||
timer.msecs(), totalCalls ) );
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user