Fix some inconsistencies in clearance priorities.

Fixes https://gitlab.com/kicad/code/kicad/issues/4555

pcbnew/board_connected_item.cpp

@@ -84,22 +84,87 @@ NETCLASS* BOARD_CONNECTED_ITEM::GetEffectiveNetclass() const
 
 
 /*
- * Clearances exist in a hiearchy:
+ * Clearances exist in a hiearchy.  If a given level is specified then the remaining levels
- * 1) accumulated board & netclass constraints
+ * are NOT consulted.
- * 2) last rule whose condition evaluates to true
- * 4) footprint override
- * 5) pad override
  *
- * The base class handles (1) and (2).
+ * LEVEL 1: (highest priority) local overrides (pad, footprint, etc.)
+ * LEVEL 2: Rules
+ * LEVEL 3: Accumulated local settings, netclass settings, & board design settings
  */
 int BOARD_CONNECTED_ITEM::GetClearance( BOARD_ITEM* aItem, wxString* aSource ) const
 {
-    BOARD* board = GetBoard();
+    BOARD*                board = GetBoard();
+    int                   clearance = 0;
+    wxString              source;
+    wxString*             localSource = aSource ? &source : nullptr;
+    BOARD_CONNECTED_ITEM* second = dynamic_cast<BOARD_CONNECTED_ITEM*>( aItem );
 
     // No clearance if "this" is not (yet) linked to a board therefore no available netclass
     if( !board )
-        return 0;
+        return clearance;
 
+    // LEVEL 1: local overrides (pad, footprint, etc.)
+    //
+    if( GetLocalClearanceOverrides() > clearance )
+        clearance = GetLocalClearanceOverrides( localSource );
+
+    if( second && second->GetLocalClearanceOverrides() > clearance )
+        clearance = second->GetLocalClearanceOverrides( localSource );
+
+    if( clearance )
+    {
+        if( aSource )
+            *aSource = *localSource;
+
+        return clearance;
+    }
+
+    // LEVEL 2: Rules
+    //
+    if( GetRuleClearance( aItem, &clearance, aSource ) )
+        return clearance;
+
+    // LEVEL 3: Accumulated local settings, netclass settings, & board design settings
+    //
+    BOARD_DESIGN_SETTINGS& bds = board->GetDesignSettings();
+    NETCLASS*              netclass = GetEffectiveNetclass();
+    NETCLASS*              secondNetclass = second ? second->GetEffectiveNetclass() : nullptr;
+
+    if( bds.m_MinClearance > clearance )
+    {
+        if( aSource )
+            *aSource = _( "board minimum" );
+
+        clearance = bds.m_MinClearance;
+    }
+
+    if( netclass && netclass->GetClearance() > clearance )
+        clearance = netclass->GetClearance( aSource );
+
+    if( secondNetclass && secondNetclass->GetClearance() > clearance )
+        clearance = secondNetclass->GetClearance( aSource );
+
+    if( aItem && aItem->GetLayer() == Edge_Cuts && bds.m_CopperEdgeClearance > clearance )
+    {
+        if( aSource )
+            *aSource = _( "board edge" );
+
+        clearance = bds.m_CopperEdgeClearance;
+    }
+
+    if( GetLocalClearance() > clearance )
+        clearance = GetLocalClearance( aSource );
+
+    if( second && second->GetLocalClearance() > clearance )
+        clearance = second->GetLocalClearance( aSource );
+
+    return clearance;
+}
+
+
+bool BOARD_CONNECTED_ITEM::GetRuleClearance( BOARD_ITEM* aItem, int* aClearance,
+                                             wxString* aSource ) const
+{
     DRC_RULE* rule = GetRule( this, aItem, CLEARANCE_CONSTRAINT );
 
     if( rule )
@@ -107,47 +172,11 @@ int BOARD_CONNECTED_ITEM::GetClearance( BOARD_ITEM* aItem, wxString* aSource ) c
         if( aSource )
             *aSource = wxString::Format( _( "'%s' rule clearance" ), rule->m_Name );
 
-        return rule->m_Clearance.Min;
+        *aClearance = rule->m_Clearance.Min;
+        return true;
     }
 
-    BOARD_DESIGN_SETTINGS& bds = board->GetDesignSettings();
+    return false;
-    int                    clearance = bds.m_MinClearance;
-
-    if( aSource )
-        *aSource = _( "board minimum" );
-
-    NETCLASS* netclass = GetEffectiveNetclass();
-
-    if( netclass && netclass->GetClearance() > clearance )
-    {
-        clearance = netclass->GetClearance();
-
-        if( aSource )
-            *aSource = wxString::Format( _( "'%s' netclass" ), netclass->GetName() );
-    }
-
-    if( aItem && aItem->IsConnected() )
-    {
-        netclass = static_cast<BOARD_CONNECTED_ITEM*>( aItem )->GetEffectiveNetclass();
-
-        if( netclass && netclass->GetClearance() > clearance )
-        {
-            clearance = netclass->GetClearance();
-
-            if( aSource )
-                *aSource = wxString::Format( _( "'%s' netclass" ), netclass->GetName() );
-        }
-    }
-
-    if( aItem && aItem->GetLayer() == Edge_Cuts && bds.m_CopperEdgeClearance > clearance )
-    {
-        clearance = bds.m_CopperEdgeClearance;
-
-        if( aSource )
-            *aSource = _( "board edge" );
-    }
-
-    return clearance;
 }
 
 

pcbnew/board_connected_item.h

@@ -163,6 +163,32 @@ public:
      */
     virtual int GetClearance( BOARD_ITEM* aItem = nullptr, wxString* aSource = nullptr ) const;
 
+    /**
+     * Function GetRuleClearance
+     * returns any rule-based clearance.
+     * @param aClearance [out] the clearance value in internal units
+     * @param aSource [out] reports the source as a user-readable string
+     * @return true if a rule was fired
+     */
+    virtual bool GetRuleClearance( BOARD_ITEM* aItem, int* aClearance, wxString* aSource ) const;
+
+    /**
+     * Function GetLocalClearanceOverrides
+     * returns any local clearance overrides set in the "classic" (ie: pre-rule) system.
+     * @param aSource [out] optionally reports the source as a user-readable string
+     * @return int - the clearance in internal units.
+     */
+    virtual int GetLocalClearanceOverrides( wxString* aSource = nullptr ) const { return 0; }
+
+    /**
+     * Function GetLocalClearance
+     * returns any local clearances set in the "classic" (ie: pre-rule) system.  These are
+     * things like zone clearance which are NOT an override.
+     * @param aSource [out] optionally reports the source as a user-readable string
+     * @return int - the clearance in internal units.
+     */
+    virtual int GetLocalClearance( wxString* aSource = nullptr ) const { return 0; }
+
     /**
      * Function GetNetClassPtr
      * returns the NETCLASS for this item.

pcbnew/class_module.h

@@ -227,7 +227,14 @@ public:
     int GetLocalSolderMaskMargin() const { return m_LocalSolderMaskMargin; }
     void SetLocalSolderMaskMargin( int aMargin ) { m_LocalSolderMaskMargin = aMargin; }
 
-    int GetLocalClearance() const { return m_LocalClearance; }
+    int GetLocalClearance( wxString* aSource = nullptr ) const
+    {
+        if( aSource )
+            *aSource = wxString::Format( _( "footprint %s" ), GetReference() );
+
+        return m_LocalClearance;
+    }
+
     void SetLocalClearance( int aClearance ) { m_LocalClearance = aClearance; }
 
     int GetLocalSolderPasteMargin() const { return m_LocalSolderPasteMargin; }

pcbnew/class_pad.cpp

@@ -613,27 +613,26 @@ wxPoint D_PAD::ShapePos() const
 }
 
 
-int D_PAD::GetClearance( BOARD_ITEM* aItem, wxString* aSource ) const
+int D_PAD::GetLocalClearanceOverrides( wxString* aSource ) const
 {
     // A pad can have specific clearance that overrides its NETCLASS clearance value
-    if( m_LocalClearance )
+    if( GetLocalClearance() )
-    {
+        return GetLocalClearance( aSource );
-        if( aSource )
-            *aSource = wxString::Format( _( "pad %s" ), GetName() );
-
-        return m_LocalClearance;
-    }
 
     // A footprint can have a specific clearance value
     if( GetParent() && GetParent()->GetLocalClearance() )
-    {
+        return GetParent()->GetLocalClearance( aSource );
-        if( aSource )
-            *aSource = wxString::Format( _( "%s footprint" ), GetParent()->GetReference() );
 
-        return GetParent()->GetLocalClearance();
+    return 0;
-    }
+}
 
-    return BOARD_CONNECTED_ITEM::GetClearance( aItem, aSource );
+
+int D_PAD::GetLocalClearance( wxString* aSource ) const
+{
+    if( aSource )
+        *aSource = wxString::Format( _( "pad %s" ), GetName() );
+
+    return m_LocalClearance;
 }
 
 

pcbnew/class_pad.h

@@ -450,7 +450,7 @@ public:
     int GetLocalSolderMaskMargin() const        { return m_LocalSolderMaskMargin; }
     void SetLocalSolderMaskMargin( int aMargin ) { m_LocalSolderMaskMargin = aMargin; }
 
-    int GetLocalClearance() const               { return m_LocalClearance; }
+    int GetLocalClearance( wxString* aSource = nullptr ) const override;
     void SetLocalClearance( int aClearance )    { m_LocalClearance = aClearance; }
 
     int GetLocalSolderPasteMargin() const       { return m_LocalSolderPasteMargin; }
@@ -475,16 +475,12 @@ public:
             int aMaxError = ARC_HIGH_DEF, bool ignoreLineWidth = false ) const override;
 
     /**
-     * Function GetClearance
+     * Function GetLocalClearanceOverrides
-     * returns the clearance in internal units.  If \a aItem is not NULL then the
+     * returns any local clearance overrides set in the "classic" (ie: pre-rule) system.
-     * returned clearance is the greater of this object's clearance and
-     * aItem's clearance.  If \a aItem is NULL, then this objects clearance
-     * is returned.
-     * @param aItem is an optional BOARD_ITEM
      * @param aSource [out] optionally reports the source as a user-readable string
      * @return int - the clearance in internal units.
      */
-    int GetClearance( BOARD_ITEM* aItem = nullptr, wxString* aSource = nullptr ) const override;
+    int GetLocalClearanceOverrides( wxString* aSource = nullptr ) const override;
 
    // Mask margins handling:
 

pcbnew/class_track.cpp

@@ -117,21 +117,28 @@ BITMAP_DEF VIA::GetMenuImage() const
 }
 
 
-int TRACK::GetClearance( BOARD_ITEM* aItem, wxString* aSource ) const
+int TRACK::GetLocalClearance( wxString* aSource ) const
 {
-    // Currently tracks have no specific clearance parameter on a per track or per
+    // Not currently implemented
-    // segment basis.  The NETCLASS clearance is used.
+    return 0;
-    return BOARD_CONNECTED_ITEM::GetClearance( aItem, aSource );
 }
 
 
 /*
- * Width constraints exist in a hiearchy:
+ * Width constraints exist in a hiearchy.  If a given level is specified then the remaining
- * 1) accumulated board & netclass constraints
+ * levels are NOT consulted.
- * 2) last rule whose condition evaluates to true
+ *
+ * LEVEL 1: (highest priority) local overrides (not currently implemented.)
+ * LEVEL 2: Rules
+ * LEVEL 3: Accumulated local settings, netclass settings, & board design settings
  */
 void TRACK::GetWidthConstraints( int* aMin, int* aMax, wxString* aSource ) const
 {
+    // LEVEL 1: local overrides
+    //
+    // Not currently implemented
+
+    // LEVEL 2: Rules
     DRC_RULE* rule = GetRule( this, nullptr, TRACK_CONSTRAINT );
 
     if( rule )
@@ -141,15 +148,21 @@ void TRACK::GetWidthConstraints( int* aMin, int* aMax, wxString* aSource ) const
 
         if( aSource )
             *aSource = wxString::Format( _( "'%s' rule" ), rule->m_Name );
-    }
-    else
-    {
-        *aMin = GetBoard()->GetDesignSettings().m_TrackMinWidth;
-        *aMax = INT_MAX / 2;
 
-        if( aSource )
+        return;
-            *aSource = _( "board minimum" );
     }
+
+    // LEVEL 3: Netclasses & board design settings
+    //
+    // Note that local settings aren't currently implemented, and netclasses don't contain a
+    // minimum width (only a default width), so only the board design settings are relevant
+    // here.
+    //
+    *aMin = GetBoard()->GetDesignSettings().m_TrackMinWidth;
+    *aMax = INT_MAX / 2;
+
+    if( aSource )
+        *aSource = _( "board minimum" );
 }
 
 

pcbnew/class_track.h

@@ -201,16 +201,12 @@ public:
     }
 
     /**
-     * Function GetClearance
+     * Function GetLocalClearance
-     * returns the clearance in internal units.  If \a aItem is not NULL then the
+     * returns any local clearance overrides set in the "classic" (ie: pre-rule) system.
-     * returned clearance is the greater of this object's clearance and
-     * aItem's clearance.  If \a aItem is NULL, then this objects clearance
-     * is returned.
-     * @param aItem is an optional BOARD_ITEM
      * @param aSource [out] optionally reports the source as a user-readable string
      * @return int - the clearance in internal units.
      */
-    int GetClearance( BOARD_ITEM* aItem = nullptr, wxString* aSource = nullptr ) const override;
+    int GetLocalClearance( wxString* aSource = nullptr ) const override;
 
     void GetWidthConstraints( int* aMin, int* aMax, wxString* aSource ) const;
 

pcbnew/class_zone.cpp

@@ -534,73 +534,15 @@ bool ZONE_CONTAINER::HitTest( const EDA_RECT& aRect, bool aContained, int aAccur
 }
 
 
-int ZONE_CONTAINER::GetClearance( BOARD_ITEM* aItem, wxString* aSource ) const
+int ZONE_CONTAINER::GetLocalClearance( wxString* aSource ) const
 {
     if( m_isKeepout )
         return 0;
 
-    BOARD* board = GetBoard();
-
-    // No clearance if "this" is not (yet) linked to a board
-    if( !board )
-        return 0;
-
-    DRC_RULE* rule = GetRule( this, aItem, CLEARANCE_CONSTRAINT );
-
-    if( rule )
-    {
-        if( aSource )
-            *aSource = wxString::Format( _( "'%s' rule clearance" ), rule->m_Name );
-
-        return rule->m_Clearance.Min;
-    }
-
-    BOARD_DESIGN_SETTINGS& bds = board->GetDesignSettings();
-    int                    clearance = bds.m_MinClearance;
-
     if( aSource )
-        *aSource = _( "board minimum" );
+        *aSource = _( "zone" );
 
-    NETCLASS* netclass = GetEffectiveNetclass();
+    return m_ZoneClearance;
-
-    if( netclass && netclass->GetClearance() > clearance )
-    {
-        clearance = netclass->GetClearance();
-
-        if( aSource )
-            *aSource = wxString::Format( _( "'%s' netclass" ), netclass->GetName() );
-    }
-
-    if( aItem && aItem->IsConnected() )
-    {
-        netclass = static_cast<BOARD_CONNECTED_ITEM*>( aItem )->GetEffectiveNetclass();
-
-        if( netclass && netclass->GetClearance() > clearance )
-        {
-            clearance = netclass->GetClearance();
-
-            if( aSource )
-                *aSource = wxString::Format( _( "'%s' netclass" ), netclass->GetName() );
-        }
-    }
-
-    if( aItem && aItem->GetLayer() == Edge_Cuts && bds.m_CopperEdgeClearance > clearance )
-    {
-        clearance = bds.m_CopperEdgeClearance;
-
-        if( aSource )
-            *aSource = _( "board edge" );
-    }
-
-    if( m_ZoneClearance > clearance )
-    {
-        clearance = m_ZoneClearance;
-
-        if( aSource )
-            *aSource = _( "zone" );
-    }
-
-    return clearance;
 }
 
 

pcbnew/class_zone.h

@@ -110,7 +110,14 @@ public:
      */
     const EDA_RECT GetBoundingBox() const override;
 
-    int GetClearance( BOARD_ITEM* aItem = nullptr, wxString* aSource = nullptr ) const override;
+    /**
+     * Function GetLocalClearance
+     * returns any local clearances set in the "classic" (ie: pre-rule) system.  These are
+     * things like zone clearance which are NOT an override.
+     * @param aSource [out] optionally reports the source as a user-readable string
+     * @return int - the clearance in internal units.
+     */
+    int GetLocalClearance( wxString* aSource = nullptr ) const override;
 
     /**
      * Function IsOnCopperLayer

pcbnew/dialogs/panel_modedit_color_settings.cpp

@@ -35,8 +35,7 @@ PANEL_MODEDIT_COLOR_SETTINGS::PANEL_MODEDIT_COLOR_SETTINGS( FOOTPRINT_EDIT_FRAME
         : PANEL_COLOR_SETTINGS( aParent ),
           m_frame( aFrame ),
           m_page( nullptr ),
-          m_titleBlock( nullptr ),
+          m_titleBlock( nullptr )
-          m_ws( nullptr )
 {
      // Currently this only applies to eeschema
     m_optOverrideColors->Hide();

pcbnew/dialogs/panel_modedit_color_settings.h

@@ -61,8 +61,6 @@ private:
 
     TITLE_BLOCK* m_titleBlock;
 
-    KIGFX::WS_PROXY_VIEW_ITEM* m_ws;
-
     void createSwatches();
 };
 

pcbnew/drc/drc.cpp

@@ -706,16 +706,14 @@ void DRC::testPadClearances( BOARD_COMMIT& aCommit )
     {
         if( !bds.Ignore( DRCE_PAD_NEAR_EDGE ) && m_board_outline_valid )
         {
+            int minClearance = bds.m_CopperEdgeClearance;
+            m_clearanceSource = _( "board edge" );
+
             static DRAWSEGMENT dummyEdge;
             dummyEdge.SetLayer( Edge_Cuts );
 
-            int minClearance = pad->GetClearance( &dummyEdge, &m_clearanceSource );
+            if( pad->GetRuleClearance( &dummyEdge, &minClearance, &m_clearanceSource ) )
-
+                /* minClearance and m_clearanceSource set in GetRuleClearance() */;
-            if( bds.m_CopperEdgeClearance > minClearance )
-            {
-                minClearance = bds.m_CopperEdgeClearance;
-                m_clearanceSource = _( "board edge" );
-            }
 
             for( auto it = m_board_outlines.IterateSegmentsWithHoles(); it; it++ )
             {

pcbnew/drc/drc_clearance_test_functions.cpp

@@ -598,18 +598,16 @@ void DRC::doTrackDrc( BOARD_COMMIT& aCommit, TRACK* aRefSeg, TRACKS::iterator aS
     /***********************************************/
     if( m_board_outline_valid )
     {
+        int minClearance = bds.m_CopperEdgeClearance;
+        m_clearanceSource = _( "board edge" );
+
         static DRAWSEGMENT dummyEdge;
         dummyEdge.SetLayer( Edge_Cuts );
 
+        if( aRefSeg->GetRuleClearance( &dummyEdge, &minClearance, &m_clearanceSource ) )
+            /* minClearance and m_clearanceSource set in GetRuleClearance() */;
+
         SEG testSeg( aRefSeg->GetStart(), aRefSeg->GetEnd() );
-        int minClearance = aRefSeg->GetClearance( &dummyEdge, &m_clearanceSource );
-
-        if( bds.m_CopperEdgeClearance > minClearance )
-        {
-            minClearance = bds.m_CopperEdgeClearance;
-            m_clearanceSource = _( "board edge" );
-        }
-
         int halfWidth = refSegWidth / 2;
         int center2centerAllowed = minClearance + halfWidth;
 

pcbnew/exporters/gerber_jobfile_writer.cpp

@@ -452,12 +452,8 @@ void GERBER_JOBFILE_WRITER::addJSONDesignRules()
     bool                         hasInnerLayers = m_pcb->GetCopperLayerCount() > 2;
 
     // Search a smaller clearance in other net classes, if any.
-    for( NETCLASSES::const_iterator it = dsnSettings.m_NetClasses.begin();
+    for( const std::pair<wxString, NETCLASSPTR>& entry : dsnSettings.m_NetClasses )
-            it != dsnSettings.m_NetClasses.end(); ++it )
+        minclearanceOuter = std::min( minclearanceOuter, entry.second->GetClearance() );
-    {
-        NETCLASS netclass = *it->second;
-        minclearanceOuter = std::min( minclearanceOuter, netclass.GetClearance() );
-    }
 
     // job file knows different clearance types.
     // Kicad knows only one clearance for pads and tracks

pcbnew/netclass.h

@@ -157,9 +157,16 @@ public:
     STRINGSET& NetNames()                   { return m_Members; }       ///< for SWIG
 
     const wxString& GetDescription() const  { return m_Description; }
-    void    SetDescription( const wxString& aDesc ) { m_Description = aDesc; }
+    void  SetDescription( const wxString& aDesc ) { m_Description = aDesc; }
+
+    int GetClearance( wxString* aSource = nullptr ) const
+    {
+        if( aSource )
+            *aSource = wxString::Format( _( "'%s' netclass" ), m_Name );
+
+        return m_Clearance;
+    }
 
-    int     GetClearance() const            { return m_Clearance; }
     void    SetClearance( int aClearance )  { m_Clearance = aClearance; }
 
     int     GetTrackWidth() const           { return m_TrackWidth; }

pcbnew/zone_filler.cpp

@@ -526,7 +526,7 @@ void ZONE_FILLER::buildCopperItemClearances( const ZONE_CONTAINER* aZone, SHAPE_
     int extra_margin = Millimeter2iu( 0.002 );
 
     BOARD_DESIGN_SETTINGS& bds = m_board->GetDesignSettings();
-    int                    zone_clearance = aZone->GetClearance();
+    int                    zone_clearance = aZone->GetLocalClearance();
     EDA_RECT               zone_boundingbox = aZone->GetBoundingBox();
 
     // items outside the zone bounding box are skipped, so it needs to be inflated by

pcbnew/zones_test_and_combine_areas.cpp

@@ -205,7 +205,7 @@ bool BOARD::TestAreaIntersections( ZONE_CONTAINER* area_to_test )
         // Filled zone specific tests
         else
         {
-            if( area_to_test->GetClearance() != area2->GetClearance() )
+            if( area_to_test->GetLocalClearance() != area2->GetLocalClearance() )
                 continue;
 
             if( area_to_test->GetThermalReliefGap() != area2->GetThermalReliefGap() )