Fix missing DRC check with via

When the via is first and not second in our ordering, the hole-copper
clearance was not checked as the track did not have a hole.

We also calculated the NPTH-via clearance incorrectly in the inspector

pcbnew/drc/drc_test_provider_copper_clearance.cpp

@@ -172,6 +172,8 @@ bool DRC_TEST_PROVIDER_COPPER_CLEARANCE::testTrackAgainstItem( PCB_TRACK* track,
     int            actual;
     VECTOR2I       pos;
 
+    std::shared_ptr<SHAPE> otherShape = other->GetEffectiveShape( layer );
+
     if( other->Type() == PCB_PAD_T )
     {
         PAD* pad = static_cast<PAD*>( other );
@@ -206,8 +208,6 @@ bool DRC_TEST_PROVIDER_COPPER_CLEARANCE::testTrackAgainstItem( PCB_TRACK* track,
             }
         }
 
-        std::shared_ptr<SHAPE> otherShape = other->GetEffectiveShape( layer );
-
         if( trackShape->Collide( otherShape.get(), clearance - m_drcEpsilon, &actual, &pos ) )
         {
             if( m_drcEngine->IsNetTieExclusion( track->GetNetCode(), layer, pos, other ) )
@@ -237,43 +237,54 @@ bool DRC_TEST_PROVIDER_COPPER_CLEARANCE::testTrackAgainstItem( PCB_TRACK* track,
         }
     }
 
-    if( testHoles && other->HasHole() )
+    if( testHoles && ( track->HasHole() || other->HasHole() ) )
     {
-        std::shared_ptr<SHAPE_SEGMENT> holeShape;
+        std::array<BOARD_ITEM*, 2> a{ track, other };
+        std::array<BOARD_ITEM*, 2> b{ other, track };
+        std::array<SHAPE*, 2>      a_shape{ trackShape, otherShape.get() };
 
-        if( other->Type() == PCB_VIA_T )
+        bool has_error = false;
-        {
-            if( other->GetLayerSet().Contains( layer ) )
-                holeShape = other->GetEffectiveHoleShape();
-        }
-        else
-        {
-            holeShape = other->GetEffectiveHoleShape();
-        }
 
-        if( holeShape )
+        for( size_t ii = 0; ii < 2 && !has_error; ++ii )
         {
-            constraint = m_drcEngine->EvalRules( HOLE_CLEARANCE_CONSTRAINT, other, track, layer );
+            std::shared_ptr<SHAPE_SEGMENT> holeShape;
+
+            // We only test a track item here against an item with a hole.
+            // If either case is not valid, simply move on
+            if( !( dynamic_cast<PCB_TRACK*>( a[ii] ) ) || !b[ii]->HasHole() )
+            {
+                continue;
+            }
+            if( b[ii]->Type() == PCB_VIA_T )
+            {
+                if( b[ii]->GetLayerSet().Contains( layer ) )
+                    holeShape = b[ii]->GetEffectiveHoleShape();
+            }
+            else
+            {
+                holeShape = b[ii]->GetEffectiveHoleShape();
+            }
+
+            constraint = m_drcEngine->EvalRules( HOLE_CLEARANCE_CONSTRAINT, b[ii], a[ii], layer );
             clearance = constraint.GetValue().Min();
 
             if( constraint.GetSeverity() != RPT_SEVERITY_IGNORE && clearance > 0 )
             {
-                if( trackShape->Collide( holeShape.get(), std::max( 0, clearance - m_drcEpsilon ),
+                if( a_shape[ii]->Collide( holeShape.get(), std::max( 0, clearance - m_drcEpsilon ),
-                                         &actual, &pos ) )
+                        &actual, &pos ) )
                 {
                     std::shared_ptr<DRC_ITEM> drce = DRC_ITEM::Create( DRCE_HOLE_CLEARANCE );
                     wxString msg;
 
-                    msg.Printf( _( "(%s clearance %s; actual %s)" ),
+                    msg.Printf( _( "(%s clearance %s; actual %s)" ), constraint.GetName(),
-                                  constraint.GetName(),
+                            MessageTextFromValue( clearance ), MessageTextFromValue( actual ) );
-                                  MessageTextFromValue( clearance ),
-                                  MessageTextFromValue( actual ) );
 
                     drce->SetErrorMessage( drce->GetErrorText() + wxS( " " ) + msg );
-                    drce->SetItems( track, other );
+                    drce->SetItems( a[ii], b[ii] );
                     drce->SetViolatingRule( constraint.GetParentRule() );
 
                     reportViolation( drce, pos, layer );
+                    has_error = true;
 
                     if( !m_drcEngine->GetReportAllTrackErrors() )
                         return false;

pcbnew/pcb_track.cpp

@@ -387,7 +387,7 @@ INSPECT_RESULT PCB_TRACK::Visit( INSPECTOR inspector, void* testData,
 
 std::shared_ptr<SHAPE_SEGMENT> PCB_VIA::GetEffectiveHoleShape() const
 {
-    return std::make_shared<SHAPE_SEGMENT>( SEG( m_Start, m_Start ), KiROUND( m_drill / 2.0 ) );
+    return std::make_shared<SHAPE_SEGMENT>( SEG( m_Start, m_Start ), m_drill );
 }
 
 

pcbnew/tools/pcb_control.cpp

@@ -1378,7 +1378,7 @@ int PCB_CONTROL::UpdateMessagePanel( const TOOL_EVENT& aEvent )
             }
         }
 
-        if( a->HasHole() || b->HasHole() )
+        if( ( a->HasHole() || b->HasHole() ) )
         {
             PCB_LAYER_ID active = m_frame->GetActiveLayer();
             PCB_LAYER_ID layer = UNDEFINED_LAYER;
@@ -1389,14 +1389,14 @@ int PCB_CONTROL::UpdateMessagePanel( const TOOL_EVENT& aEvent )
                 layer = active;
             else if( a->HasHole() && b->IsOnCopperLayer() )
                 layer = b->GetLayer();
-            else if( b->HasHole() && b->IsOnCopperLayer() )
+            else if( b->HasHole() && a->IsOnCopperLayer() )
                 layer = a->GetLayer();
 
-            if( layer >= 0 )
+            if( IsCopperLayer( layer ) )
             {
                 int actual = std::numeric_limits<int>::max();
 
-                if( a->HasHole() )
+                if( a->HasHole() && b->IsOnCopperLayer() )
                 {
                     std::shared_ptr<SHAPE_SEGMENT> hole = a->GetEffectiveHoleShape();
                     std::shared_ptr<SHAPE>         other( b->GetEffectiveShape( layer ) );
@@ -1404,7 +1404,7 @@ int PCB_CONTROL::UpdateMessagePanel( const TOOL_EVENT& aEvent )
                     actual = std::min( actual, hole->GetClearance( other.get() ) );
                 }
 
-                if( b->HasHole() )
+                if( b->HasHole() && a->IsOnCopperLayer() )
                 {
                     std::shared_ptr<SHAPE_SEGMENT> hole = b->GetEffectiveHoleShape();
                     std::shared_ptr<SHAPE>         other( a->GetEffectiveShape( layer ) );
@@ -1412,14 +1412,17 @@ int PCB_CONTROL::UpdateMessagePanel( const TOOL_EVENT& aEvent )
                     actual = std::min( actual, hole->GetClearance( other.get() ) );
                 }
 
-                constraint = drcEngine->EvalRules( HOLE_CLEARANCE_CONSTRAINT, a, b, layer );
+                if( actual < std::numeric_limits<int>::max() )
-                msgItems.emplace_back( _( "Resolved hole clearance" ),
-                                       m_frame->MessageTextFromValue( constraint.m_Value.Min() ) );
-
-                if( actual > -1 && actual < std::numeric_limits<int>::max() )
                 {
-                    msgItems.emplace_back( _( "Actual hole clearance" ),
+                    constraint = drcEngine->EvalRules( HOLE_CLEARANCE_CONSTRAINT, a, b, layer );
-                                           m_frame->MessageTextFromValue( actual ) );
+                    msgItems.emplace_back( _( "Resolved hole clearance" ),
+                            m_frame->MessageTextFromValue( constraint.m_Value.Min() ) );
+
+                    if( actual > -1 && actual < std::numeric_limits<int>::max() )
+                    {
+                        msgItems.emplace_back( _( "Actual hole clearance" ),
+                                m_frame->MessageTextFromValue( actual ) );
+                    }
                 }
             }
         }

qa/data/pcbnew/reverse_via.kicad_pro

@@ -0,0 +1,502 @@
+{
+  "board": {
+    "3dviewports": [],
+    "design_settings": {
+      "defaults": {
+        "board_outline_line_width": 0.09999999999999999,
+        "copper_line_width": 0.19999999999999998,
+        "copper_text_italic": false,
+        "copper_text_size_h": 1.5,
+        "copper_text_size_v": 1.5,
+        "copper_text_thickness": 0.3,
+        "copper_text_upright": false,
+        "courtyard_line_width": 0.049999999999999996,
+        "dimension_precision": 4,
+        "dimension_units": 3,
+        "dimensions": {
+          "arrow_length": 1270000,
+          "extension_offset": 500000,
+          "keep_text_aligned": true,
+          "suppress_zeroes": false,
+          "text_position": 0,
+          "units_format": 1
+        },
+        "fab_line_width": 0.09999999999999999,
+        "fab_text_italic": false,
+        "fab_text_size_h": 1.0,
+        "fab_text_size_v": 1.0,
+        "fab_text_thickness": 0.15,
+        "fab_text_upright": false,
+        "other_line_width": 0.15,
+        "other_text_italic": false,
+        "other_text_size_h": 1.0,
+        "other_text_size_v": 1.0,
+        "other_text_thickness": 0.15,
+        "other_text_upright": false,
+        "pads": {
+          "drill": 0.0,
+          "height": 1.8,
+          "width": 1.25
+        },
+        "silk_line_width": 0.15,
+        "silk_text_italic": false,
+        "silk_text_size_h": 1.0,
+        "silk_text_size_v": 1.0,
+        "silk_text_thickness": 0.15,
+        "silk_text_upright": false,
+        "zones": {
+          "45_degree_only": false,
+          "min_clearance": 0.15239999999999998
+        }
+      },
+      "diff_pair_dimensions": [
+        {
+          "gap": 0.0,
+          "via_gap": 0.0,
+          "width": 0.0
+        }
+      ],
+      "drc_exclusions": [],
+      "meta": {
+        "version": 2
+      },
+      "rule_severities": {
+        "annular_width": "error",
+        "clearance": "error",
+        "connection_width": "ignore",
+        "copper_edge_clearance": "error",
+        "copper_sliver": "warning",
+        "courtyards_overlap": "error",
+        "diff_pair_gap_out_of_range": "error",
+        "diff_pair_uncoupled_length_too_long": "error",
+        "drill_out_of_range": "error",
+        "duplicate_footprints": "warning",
+        "extra_footprint": "warning",
+        "footprint": "error",
+        "footprint_type_mismatch": "error",
+        "hole_clearance": "warning",
+        "hole_near_hole": "error",
+        "invalid_outline": "error",
+        "isolated_copper": "warning",
+        "item_on_disabled_layer": "error",
+        "items_not_allowed": "error",
+        "length_out_of_range": "error",
+        "lib_footprint_issues": "warning",
+        "lib_footprint_mismatch": "warning",
+        "malformed_courtyard": "error",
+        "microvia_drill_out_of_range": "error",
+        "missing_courtyard": "ignore",
+        "missing_footprint": "warning",
+        "net_conflict": "warning",
+        "npth_inside_courtyard": "ignore",
+        "overlapping_pads": "warning",
+        "padstack": "error",
+        "pth_inside_courtyard": "ignore",
+        "shorting_items": "error",
+        "silk_edge_clearance": "warning",
+        "silk_over_copper": "warning",
+        "silk_overlap": "warning",
+        "skew_out_of_range": "error",
+        "solder_mask_bridge": "error",
+        "starved_thermal": "error",
+        "text_height": "warning",
+        "text_thickness": "warning",
+        "through_hole_pad_without_hole": "error",
+        "too_many_vias": "error",
+        "track_dangling": "warning",
+        "track_width": "error",
+        "tracks_crossing": "error",
+        "unconnected_items": "error",
+        "unresolved_variable": "error",
+        "via_dangling": "warning",
+        "zones_intersect": "error"
+      },
+      "rules": {
+        "allow_blind_buried_vias": false,
+        "allow_microvias": false,
+        "max_error": 0.005,
+        "min_clearance": 0.12,
+        "min_connection": 0.0,
+        "min_copper_edge_clearance": 0.0,
+        "min_hole_clearance": 0.254,
+        "min_hole_to_hole": 0.25,
+        "min_microvia_diameter": 0.19999999999999998,
+        "min_microvia_drill": 0.09999999999999999,
+        "min_resolved_spokes": 2,
+        "min_silk_clearance": 0.0,
+        "min_text_height": 0.7999999999999999,
+        "min_text_thickness": 0.12,
+        "min_through_hole_diameter": 0.19999999999999998,
+        "min_track_width": 0.12,
+        "min_via_annular_width": 0.049999999999999996,
+        "min_via_diameter": 0.39999999999999997,
+        "solder_mask_clearance": 0.0,
+        "solder_mask_min_width": 0.0,
+        "solder_mask_to_copper_clearance": 0.0,
+        "use_height_for_length_calcs": true
+      },
+      "teardrop_options": [
+        {
+          "td_allow_use_two_tracks": true,
+          "td_curve_segcount": 5,
+          "td_on_pad_in_zone": false,
+          "td_onpadsmd": true,
+          "td_onroundshapesonly": false,
+          "td_ontrackend": false,
+          "td_onviapad": true
+        }
+      ],
+      "teardrop_parameters": [
+        {
+          "td_curve_segcount": 0,
+          "td_height_ratio": 1.0,
+          "td_length_ratio": 0.5,
+          "td_maxheight": 2.0,
+          "td_maxlen": 1.0,
+          "td_target_name": "td_round_shape",
+          "td_width_to_size_filter_ratio": 0.9
+        },
+        {
+          "td_curve_segcount": 0,
+          "td_height_ratio": 1.0,
+          "td_length_ratio": 0.5,
+          "td_maxheight": 2.0,
+          "td_maxlen": 1.0,
+          "td_target_name": "td_rect_shape",
+          "td_width_to_size_filter_ratio": 0.9
+        },
+        {
+          "td_curve_segcount": 0,
+          "td_height_ratio": 1.0,
+          "td_length_ratio": 0.5,
+          "td_maxheight": 2.0,
+          "td_maxlen": 1.0,
+          "td_target_name": "td_track_end",
+          "td_width_to_size_filter_ratio": 0.9
+        }
+      ],
+      "track_widths": [
+        0.0,
+        0.127,
+        0.2,
+        0.4,
+        0.6,
+        1.0,
+        1.5,
+        2.0,
+        3.55
+      ],
+      "via_dimensions": [
+        {
+          "diameter": 0.0,
+          "drill": 0.0
+        },
+        {
+          "diameter": 0.4,
+          "drill": 0.2
+        }
+      ],
+      "zones_allow_external_fillets": true,
+      "zones_use_no_outline": false
+    },
+    "layer_presets": [],
+    "viewports": []
+  },
+  "boards": [],
+  "cvpcb": {
+    "equivalence_files": []
+  },
+  "erc": {
+    "erc_exclusions": [],
+    "meta": {
+      "version": 0
+    },
+    "pin_map": [
+      [
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        1,
+        0,
+        0,
+        0,
+        0,
+        2
+      ],
+      [
+        0,
+        2,
+        0,
+        1,
+        0,
+        0,
+        1,
+        0,
+        2,
+        2,
+        2,
+        2
+      ],
+      [
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        1,
+        0,
+        1,
+        0,
+        1,
+        2
+      ],
+      [
+        0,
+        1,
+        0,
+        0,
+        0,
+        0,
+        1,
+        1,
+        2,
+        1,
+        1,
+        2
+      ],
+      [
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        1,
+        0,
+        0,
+        0,
+        0,
+        2
+      ],
+      [
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        2
+      ],
+      [
+        1,
+        1,
+        1,
+        1,
+        1,
+        0,
+        1,
+        1,
+        1,
+        1,
+        1,
+        2
+      ],
+      [
+        0,
+        0,
+        0,
+        1,
+        0,
+        0,
+        1,
+        0,
+        0,
+        0,
+        0,
+        2
+      ],
+      [
+        0,
+        2,
+        1,
+        2,
+        0,
+        0,
+        1,
+        0,
+        2,
+        2,
+        2,
+        2
+      ],
+      [
+        0,
+        2,
+        0,
+        1,
+        0,
+        0,
+        1,
+        0,
+        2,
+        0,
+        0,
+        2
+      ],
+      [
+        0,
+        2,
+        1,
+        1,
+        0,
+        0,
+        1,
+        0,
+        2,
+        0,
+        0,
+        2
+      ],
+      [
+        2,
+        2,
+        2,
+        2,
+        2,
+        2,
+        2,
+        2,
+        2,
+        2,
+        2,
+        2
+      ]
+    ],
+    "rule_severities": {
+      "bus_definition_conflict": "error",
+      "bus_entry_needed": "error",
+      "bus_label_syntax": "error",
+      "bus_to_bus_conflict": "error",
+      "bus_to_net_conflict": "error",
+      "different_unit_footprint": "error",
+      "different_unit_net": "error",
+      "duplicate_reference": "error",
+      "duplicate_sheet_names": "error",
+      "extra_units": "error",
+      "global_label_dangling": "warning",
+      "hier_label_mismatch": "error",
+      "label_dangling": "error",
+      "lib_symbol_issues": "warning",
+      "multiple_net_names": "warning",
+      "net_not_bus_member": "warning",
+      "no_connect_connected": "warning",
+      "no_connect_dangling": "warning",
+      "pin_not_connected": "error",
+      "pin_not_driven": "error",
+      "pin_to_pin": "error",
+      "power_pin_not_driven": "error",
+      "similar_labels": "warning",
+      "unannotated": "error",
+      "unit_value_mismatch": "error",
+      "unresolved_variable": "error",
+      "wire_dangling": "error"
+    }
+  },
+  "libraries": {
+    "pinned_footprint_libs": [],
+    "pinned_symbol_libs": []
+  },
+  "meta": {
+    "filename": "reverse_via.kicad_pro",
+    "version": 1
+  },
+  "net_settings": {
+    "classes": [
+      {
+        "bus_width": 12,
+        "clearance": 0.12,
+        "diff_pair_gap": 0.127,
+        "diff_pair_via_gap": 0.25,
+        "diff_pair_width": 0.21,
+        "line_style": 0,
+        "microvia_diameter": 0.3,
+        "microvia_drill": 0.1,
+        "name": "Default",
+        "pcb_color": "rgba(0, 0, 0, 0.000)",
+        "schematic_color": "rgba(0, 0, 0, 0.000)",
+        "track_width": 0.25,
+        "via_diameter": 0.4,
+        "via_drill": 0.2,
+        "wire_width": 6
+      }
+    ],
+    "meta": {
+      "version": 3
+    },
+    "net_colors": null,
+    "netclass_assignments": null,
+    "netclass_patterns": []
+  },
+  "pcbnew": {
+    "last_paths": {
+      "gencad": "",
+      "idf": "",
+      "netlist": "",
+      "specctra_dsn": "",
+      "step": "",
+      "vrml": ""
+    },
+    "page_layout_descr_file": ""
+  },
+  "schematic": {
+    "annotate_start_num": 0,
+    "drawing": {
+      "dashed_lines_dash_length_ratio": 12.0,
+      "dashed_lines_gap_length_ratio": 3.0,
+      "default_line_thickness": 6.0,
+      "default_text_size": 50.0,
+      "field_names": [],
+      "intersheets_ref_own_page": false,
+      "intersheets_ref_prefix": "",
+      "intersheets_ref_short": false,
+      "intersheets_ref_show": false,
+      "intersheets_ref_suffix": "",
+      "junction_size_choice": 3,
+      "label_size_ratio": 0.375,
+      "pin_symbol_size": 25.0,
+      "text_offset_ratio": 0.15
+    },
+    "legacy_lib_dir": "",
+    "legacy_lib_list": [],
+    "meta": {
+      "version": 1
+    },
+    "net_format_name": "",
+    "ngspice": {
+      "fix_include_paths": true,
+      "fix_passive_vals": false,
+      "meta": {
+        "version": 0
+      },
+      "model_mode": 0,
+      "workbook_filename": ""
+    },
+    "page_layout_descr_file": "",
+    "plot_directory": "",
+    "spice_adjust_passive_values": false,
+    "spice_external_command": "spice \"%I\"",
+    "spice_save_all_currents": false,
+    "spice_save_all_voltages": false,
+    "subpart_first_id": 65,
+    "subpart_id_separator": 0
+  }
+}

qa/unittests/pcbnew/drc/test_drc_regressions.cpp

@@ -131,7 +131,8 @@ BOOST_FIXTURE_TEST_CASE( DRCFalseNegativeRegressions, DRC_REGRESSION_TEST_FIXTUR
         { "issue7325",  2 },
         { "issue8003",  2 },
         { "issue9081",  2 },
-        { "issue12109", 8 }    // Pads fail annular width test
+        { "issue12109", 8 },    // Pads fail annular width test
+        { "reverse_via", 3 }    // Via/track ordering
     };
 
     for( const std::pair<wxString, int>& entry : tests )