Better error reporting for DP uncoupled length.

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

include/pcb_base_frame.h

@@ -211,6 +211,7 @@ public:
     EDA_ITEM* GetItem( const KIID& aId ) const override;
 
     void FocusOnItem( BOARD_ITEM* aItem, PCB_LAYER_ID aLayer = UNDEFINED_LAYER );
+    void FocusOnItems( std::vector<BOARD_ITEM*> aItems, PCB_LAYER_ID aLayer = UNDEFINED_LAYER );
 
     void HideSolderMask();
     void ShowSolderMask();

include/rc_item.h

@@ -115,10 +115,12 @@ public:
         m_ids.push_back( dItem );
     }
 
-    KIID GetMainItemID() const { return m_ids.size() > 0 ? m_ids[0] : niluuid; }
-    KIID GetAuxItemID() const { return m_ids.size() > 1 ? m_ids[1] : niluuid;; }
-    KIID GetAuxItem2ID() const { return m_ids.size() > 2 ? m_ids[2] : niluuid;; }
-    KIID GetAuxItem3ID() const { return m_ids.size() > 3 ? m_ids[3] : niluuid;; }
+    virtual KIID GetMainItemID() const { return m_ids.size() > 0 ? m_ids[0] : niluuid; }
+    virtual KIID GetAuxItemID() const { return m_ids.size() > 1 ? m_ids[1] : niluuid; }
+    virtual KIID GetAuxItem2ID() const { return m_ids.size() > 2 ? m_ids[2] : niluuid; }
+    virtual KIID GetAuxItem3ID() const { return m_ids.size() > 3 ? m_ids[3] : niluuid; }
+
+    std::vector<KIID> GetIDs() const { return m_ids; }
 
     void SetParent( MARKER_BASE* aMarker ) { m_parent = aMarker; }
     MARKER_BASE* GetParent() const { return m_parent; }

pcbnew/dialogs/dialog_drc.cpp

@@ -440,6 +440,22 @@ void DIALOG_DRC::OnDRCItemSelected( wxDataViewEvent& aEvent )
                 }
             }
         }
+        else if( rc_item->GetErrorCode() == DRCE_DIFF_PAIR_UNCOUPLED_LENGTH_TOO_LONG )
+        {
+            PCB_TRACK* track = dynamic_cast<PCB_TRACK*>( item );
+            int        net = track->GetNetCode();
+            std::vector<BOARD_ITEM*> segs;
+
+            for( const KIID& id : rc_item->GetIDs() )
+            {
+                PCB_TRACK*  candidate = dynamic_cast<PCB_TRACK*>( board->GetItem( id ) );
+
+                if( candidate && candidate->GetNetCode() == net )
+                    segs.push_back( candidate );
+            }
+
+            m_frame->FocusOnItems( segs, principalLayer );
+        }
         else if( m_centerMarkerOnIdle )
         {
             // we already came from a cross-probe of the marker in the document; don't go

pcbnew/drc/drc_item.cpp

@@ -391,6 +391,30 @@ std::shared_ptr<DRC_ITEM> DRC_ITEM::Create( const wxString& aErrorKey )
 }
 
 
+KIID DRC_ITEM::GetAuxItem2ID() const
+{
+    if( m_errorCode == DRCE_DIFF_PAIR_UNCOUPLED_LENGTH_TOO_LONG )
+    {
+        // we have lots of segments, but it's enough to show the first P and the first N
+        return niluuid;
+    }
+
+    return m_ids.size() > 2 ? m_ids[2] : niluuid;
+}
+
+
+KIID DRC_ITEM::GetAuxItem3ID() const
+{
+    if( m_errorCode == DRCE_DIFF_PAIR_UNCOUPLED_LENGTH_TOO_LONG )
+    {
+        // we have lots of segments, but it's enough to show the first P and the first N
+        return niluuid;
+    }
+
+    return m_ids.size() > 3 ? m_ids[3] : niluuid;
+}
+
+
 wxString DRC_ITEM::GetViolatingRuleDesc() const
 {
     if( m_violatingRule )

pcbnew/drc/drc_item.h

@@ -131,6 +131,9 @@ public:
     void SetViolatingTest( DRC_TEST_PROVIDER *aProvider ) { m_violatingTest = aProvider; }
     DRC_TEST_PROVIDER* GetViolatingTest() const { return m_violatingTest; }
 
+    KIID GetAuxItem2ID() const override;
+    KIID GetAuxItem3ID() const override;
+
 private:
     DRC_ITEM( int aErrorCode = 0, const wxString& aTitle = "", const wxString& aSettingsKey = "" )
     {

pcbnew/drc/drc_test_provider_diff_pair_coupling.cpp

@@ -456,11 +456,17 @@ bool test::DRC_TEST_PROVIDER_DIFF_PAIR_COUPLING::Run()
 
                 drce->SetErrorMessage( drce->GetErrorText() + wxS( " " ) + m_msg );
 
-                for( BOARD_CONNECTED_ITEM* offendingTrack : it.second.itemsP )
-                    drce->AddItem( offendingTrack );
+                auto pit = it.second.itemsP.begin();
+                auto nit = it.second.itemsN.begin();
 
-                for( BOARD_CONNECTED_ITEM* offendingTrack : it.second.itemsN )
-                    drce->AddItem( offendingTrack );
+                drce->AddItem( *pit );
+                drce->AddItem( *nit );
+
+                for( pit++; pit != it.second.itemsP.end(); pit++ )
+                    drce->AddItem( *pit );
+
+                for( nit++; nit != it.second.itemsN.end(); nit++ )
+                    drce->AddItem( *nit );
 
                 uncoupledViolation = true;
 

pcbnew/pcb_base_frame.cpp

@@ -250,227 +250,251 @@ EDA_ITEM* PCB_BASE_FRAME::GetItem( const KIID& aId ) const
 
 void PCB_BASE_FRAME::FocusOnItem( BOARD_ITEM* aItem, PCB_LAYER_ID aLayer )
 {
-    static KIID lastBrightenedItemID( niluuid );
+    std::vector<BOARD_ITEM*> items;
 
-    BOARD_ITEM* lastItem = nullptr;
+    if( aItem )
+        items.push_back( aItem );
 
-    /// @todo The Boost entropy exception does not exist prior to 1.67. Once the minimum Boost
-    ///       version is raise to 1.67 or greater, this version check can be removed.
-#if BOOST_VERSION >= 106700
-    try
-    {
-        lastItem = GetBoard()->GetItem( lastBrightenedItemID );
-    }
-    catch( const boost::uuids::entropy_error& )
-    {
-        wxLogError( wxT( "A Boost UUID entropy exception was thrown in %s:%s." ),
-                    __FILE__, __FUNCTION__ );
-    }
-#else
-    lastItem = GetBoard()->GetItem( lastBrightenedItemID );
-#endif
+    FocusOnItems( items, aLayer );
+}
 
-    if( lastItem && lastItem != aItem && lastItem != DELETED_BOARD_ITEM::GetInstance() )
-    {
-        lastItem->ClearBrightened();
 
-        if( lastItem->Type() == PCB_FOOTPRINT_T )
+void PCB_BASE_FRAME::FocusOnItems( std::vector<BOARD_ITEM*> aItems, PCB_LAYER_ID aLayer )
+{
+    static std::vector<KIID> lastBrightenedItemIDs;
+
+    BOARD_ITEM* lastItem = nullptr;
+
+    for( KIID lastBrightenedItemID : lastBrightenedItemIDs )
+    {
+        /// @todo The Boost entropy exception does not exist prior to 1.67. Once the minimum Boost
+        ///       version is raise to 1.67 or greater, this version check can be removed.
+    #if BOOST_VERSION >= 106700
+        try
         {
-            static_cast<FOOTPRINT*>( lastItem )->RunOnChildren(
-                    [&]( BOARD_ITEM* child )
-                    {
-                        child->ClearBrightened();
-                    } );
+            lastItem = GetBoard()->GetItem( lastBrightenedItemID );
         }
-        else if( lastItem->Type() == PCB_GROUP_T )
+        catch( const boost::uuids::entropy_error& )
         {
-            static_cast<PCB_GROUP*>( lastItem )->RunOnChildren(
-                    [&]( BOARD_ITEM* child )
-                    {
-                        child->ClearBrightened();
-                    } );
+            wxLogError( wxT( "A Boost UUID entropy exception was thrown in %s:%s." ),
+                        __FILE__, __FUNCTION__ );
         }
+    #else
+        lastItem = GetBoard()->GetItem( lastBrightenedItemID );
+    #endif
 
-        GetCanvas()->GetView()->Update( lastItem );
-        lastBrightenedItemID = niluuid;
-        GetCanvas()->Refresh();
+        if( lastItem && lastItem != DELETED_BOARD_ITEM::GetInstance() )
+        {
+            lastItem->ClearBrightened();
+
+            if( lastItem->Type() == PCB_FOOTPRINT_T )
+            {
+                static_cast<FOOTPRINT*>( lastItem )->RunOnChildren(
+                        [&]( BOARD_ITEM* child )
+                        {
+                            child->ClearBrightened();
+                        } );
+            }
+            else if( lastItem->Type() == PCB_GROUP_T )
+            {
+                static_cast<PCB_GROUP*>( lastItem )->RunOnChildren(
+                        [&]( BOARD_ITEM* child )
+                        {
+                            child->ClearBrightened();
+                        } );
+            }
+
+            GetCanvas()->GetView()->Update( lastItem );
+            lastBrightenedItemID = niluuid;
+            GetCanvas()->Refresh();
+        }
     }
 
-    if( aItem && aItem != DELETED_BOARD_ITEM::GetInstance() )
+    lastBrightenedItemIDs.clear();
+
+    if( aItems.empty() )
+        return;
+
+    VECTOR2I       focusPt;
+    KIGFX::VIEW*   view = GetCanvas()->GetView();
+    SHAPE_POLY_SET viewportPoly( view->GetViewport() );
+
+    for( wxWindow* dialog : findDialogs() )
     {
-        aItem->SetBrightened();
+        wxPoint dialogPos = GetCanvas()->ScreenToClient( dialog->GetScreenPosition() );
+        SHAPE_POLY_SET dialogPoly( BOX2D( view->ToWorld( dialogPos, true ),
+                                          view->ToWorld( dialog->GetSize(), false ) ) );
 
-        if( aItem->Type() == PCB_FOOTPRINT_T )
+        try
         {
-            static_cast<FOOTPRINT*>( aItem )->RunOnChildren(
-                    [&]( BOARD_ITEM* child )
-                    {
-                        child->SetBrightened();
-                    });
+            viewportPoly.BooleanSubtract( dialogPoly, SHAPE_POLY_SET::PM_FAST );
         }
-        else if( aItem->Type() == PCB_GROUP_T )
+        catch( const ClipperLib::clipperException& exc )
         {
-            static_cast<PCB_GROUP*>( aItem )->RunOnChildren(
-                    [&]( BOARD_ITEM* child )
-                    {
-                        child->SetBrightened();
-                    });
+            // This may be overkill and could be an assertion but we are more likely to
+            // find any clipper errors this way.
+            wxLogError( wxT( "Clipper library exception '%s' occurred." ), exc.what() );
         }
+    }
 
-        GetCanvas()->GetView()->Update( aItem );
-        lastBrightenedItemID = aItem->m_Uuid;
-
-        // Focus on the object's location.  Prefer a visible part of the object to its anchor
-        // in order to keep from scrolling around.
-
-        VECTOR2I       focusPt = aItem->GetFocusPosition();
-        KIGFX::VIEW*   view = GetCanvas()->GetView();
-        SHAPE_POLY_SET viewportPoly( view->GetViewport() );
+    SHAPE_POLY_SET itemPoly, clippedPoly;
 
-        for( wxWindow* dialog : findDialogs() )
+    for( BOARD_ITEM* item : aItems )
+    {
+        if( item && item != DELETED_BOARD_ITEM::GetInstance() )
         {
-            wxPoint        dialogPos = GetCanvas()->ScreenToClient( dialog->GetScreenPosition() );
-            SHAPE_POLY_SET dialogPoly( BOX2D( view->ToWorld( dialogPos, true ),
-                                              view->ToWorld( dialog->GetSize(), false ) ) );
+            item->SetBrightened();
 
-            try
+            if( item->Type() == PCB_FOOTPRINT_T )
             {
-                viewportPoly.BooleanSubtract( dialogPoly, SHAPE_POLY_SET::PM_FAST );
+                static_cast<FOOTPRINT*>( item )->RunOnChildren(
+                        [&]( BOARD_ITEM* child )
+                        {
+                            child->SetBrightened();
+                        });
             }
-            catch( const ClipperLib::clipperException& exc )
+            else if( item->Type() == PCB_GROUP_T )
             {
-                // This may be overkill and could be an assertion but we are more likely to find
-                // any clipper errors this way.
-                wxLogError( wxT( "Clipper library exception '%s' occurred." ), exc.what() );
+                static_cast<PCB_GROUP*>( item )->RunOnChildren(
+                        [&]( BOARD_ITEM* child )
+                        {
+                            child->SetBrightened();
+                        });
             }
-        }
 
-        SHAPE_POLY_SET itemPoly, clippedPoly;
+            GetCanvas()->GetView()->Update( item );
+            lastBrightenedItemIDs.push_back( item->m_Uuid );
 
-        if( aLayer == UNDEFINED_LAYER )
-            aLayer = aItem->GetLayerSet().Seq()[0];
+            // Focus on the object's location.  Prefer a visible part of the object to its anchor
+            // in order to keep from scrolling around.
+
+            focusPt = item->GetPosition();
+
+            if( aLayer == UNDEFINED_LAYER )
+                aLayer = item->GetLayerSet().Seq()[0];
+
+            switch( item->Type() )
+            {
+            case PCB_FOOTPRINT_T:
+                try
+                {
+                    itemPoly = static_cast<FOOTPRINT*>( item )->GetBoundingHull();
+                }
+                catch( const ClipperLib::clipperException& exc )
+                {
+                    // This may be overkill and could be an assertion but we are more likely to
+                    // find any clipper errors this way.
+                    wxLogError( wxT( "Clipper library exception '%s' occurred." ), exc.what() );
+                }
+
+                break;
+
+            case PCB_PAD_T:
+            case PCB_MARKER_T:
+            case PCB_VIA_T:
+                FocusOnLocation( item->GetFocusPosition() );
+                GetCanvas()->Refresh();
+                return;
+
+            case PCB_SHAPE_T:
+            case PCB_TEXT_T:
+            case PCB_TEXTBOX_T:
+            case PCB_FP_TEXT_T:
+            case PCB_FP_TEXTBOX_T:
+            case PCB_FP_SHAPE_T:
+            case PCB_FP_ZONE_T:
+            case PCB_TRACE_T:
+            case PCB_ARC_T:
+            case PCB_DIM_ALIGNED_T:
+            case PCB_DIM_LEADER_T:
+            case PCB_DIM_CENTER_T:
+            case PCB_DIM_RADIAL_T:
+            case PCB_DIM_ORTHOGONAL_T:
+            case PCB_FP_DIM_ALIGNED_T:
+            case PCB_FP_DIM_LEADER_T:
+            case PCB_FP_DIM_CENTER_T:
+            case PCB_FP_DIM_RADIAL_T:
+            case PCB_FP_DIM_ORTHOGONAL_T:
+                item->TransformShapeWithClearanceToPolygon( itemPoly, aLayer, 0,
+                                                            Millimeter2iu( 0.1 ), ERROR_INSIDE );
+                break;
+
+            case PCB_ZONE_T:
+            {
+                ZONE* zone = static_cast<ZONE*>( item );
+                #if 0
+                // Using the filled area shapes to find a Focus point can give good results, but
+                // unfortunately the calculations are highly time consuming, even for not very
+                // large areas (can be easily a few minutes for large areas).
+                // so we used only the zone outline that usually do not have too many vertices.
+                zone->TransformShapeWithClearanceToPolygon( itemPoly, aLayer, 0,
+                                                            Millimeter2iu( 0.1 ), ERROR_INSIDE );
+
+                if( itemPoly.IsEmpty() )
+                    itemPoly = *zone->Outline();
+                #else
+                // much faster calculation time when using only the zone outlines
+                itemPoly = *zone->Outline();
+                #endif
+
+                break;
+            }
+
+            default:
+            {
+                BOX2I item_bbox = item->GetBoundingBox();
+                itemPoly.NewOutline();
+                itemPoly.Append( item_bbox.GetOrigin() );
+                itemPoly.Append( item_bbox.GetOrigin() + VECTOR2I( item_bbox.GetWidth(), 0 ) );
+                itemPoly.Append( item_bbox.GetOrigin() + VECTOR2I( 0, item_bbox.GetHeight() ) );
+                itemPoly.Append( item_bbox.GetOrigin() + VECTOR2I( item_bbox.GetWidth(),
+                                                                   item_bbox.GetHeight() ) );
+                break;
+            }
+            }
 
-        switch( aItem->Type() )
-        {
-        case PCB_FOOTPRINT_T:
             try
             {
-                itemPoly = static_cast<FOOTPRINT*>( aItem )->GetBoundingHull();
+                clippedPoly.BooleanIntersection( itemPoly, viewportPoly, SHAPE_POLY_SET::PM_FAST );
             }
             catch( const ClipperLib::clipperException& exc )
             {
-                // This may be overkill and could be an assertion but we are more likely to find
-                // any clipper errors this way.
+                // This may be overkill and could be an assertion but we are more likely to
+                // find any clipper errors this way.
                 wxLogError( wxT( "Clipper library exception '%s' occurred." ), exc.what() );
             }
 
-            break;
+            if( !clippedPoly.IsEmpty() )
+                itemPoly = clippedPoly;
+        }
+    }
 
-        case PCB_PAD_T:
-        case PCB_MARKER_T:
-        case PCB_VIA_T:
-            FocusOnLocation( focusPt );
-            GetCanvas()->Refresh();
-            return;
-
-        case PCB_SHAPE_T:
-        case PCB_TEXT_T:
-        case PCB_TEXTBOX_T:
-        case PCB_FP_TEXT_T:
-        case PCB_FP_TEXTBOX_T:
-        case PCB_FP_SHAPE_T:
-        case PCB_FP_ZONE_T:
-        case PCB_TRACE_T:
-        case PCB_ARC_T:
-        case PCB_DIM_ALIGNED_T:
-        case PCB_DIM_LEADER_T:
-        case PCB_DIM_CENTER_T:
-        case PCB_DIM_RADIAL_T:
-        case PCB_DIM_ORTHOGONAL_T:
-        case PCB_FP_DIM_ALIGNED_T:
-        case PCB_FP_DIM_LEADER_T:
-        case PCB_FP_DIM_CENTER_T:
-        case PCB_FP_DIM_RADIAL_T:
-        case PCB_FP_DIM_ORTHOGONAL_T:
-            aItem->TransformShapeWithClearanceToPolygon( itemPoly, aLayer, 0, Millimeter2iu( 0.1 ),
-                                                         ERROR_INSIDE );
-            break;
-
-        case PCB_ZONE_T:
-        {
-            ZONE* zone = static_cast<ZONE*>( aItem );
-            #if 0
-            // Using the filled area shapes to find a Focus point can give good results, but
-            // unfortunately the calculations are highly time consuming, even for not very
-            // large areas (can be easily a few minutes for large areas).
-            // so we used only the zone outline that usually do not have too many vertices.
-            zone->TransformShapeWithClearanceToPolygon( itemPoly, aLayer, 0, Millimeter2iu( 0.1 ),
-                                                        ERROR_INSIDE );
-
-            if( itemPoly.IsEmpty() )
-                itemPoly = *zone->Outline();
-            #else
-            // much faster calculation time when using only the zone outlines
-            itemPoly = *zone->Outline();
-            #endif
+    /*
+     * Perform a step-wise deflate to find the visual-center-of-mass
+     */
 
-            break;
-        }
+    BOX2I    bbox = itemPoly.BBox();
+    int      step = std::min( bbox.GetWidth(), bbox.GetHeight() ) / 10;
 
-        default:
-        {
-            BOX2I item_bbox = aItem->GetBoundingBox();
-            itemPoly.NewOutline();
-            itemPoly.Append( item_bbox.GetOrigin() );
-            itemPoly.Append( item_bbox.GetOrigin() + VECTOR2I( item_bbox.GetWidth(), 0 ) );
-            itemPoly.Append( item_bbox.GetOrigin() + VECTOR2I( 0, item_bbox.GetHeight() ) );
-            itemPoly.Append( item_bbox.GetOrigin() + VECTOR2I( item_bbox.GetWidth(),
-                                                               item_bbox.GetHeight() ) );
-            break;
-        }
-        }
+    while( !itemPoly.IsEmpty() )
+    {
+        focusPt = (wxPoint) itemPoly.BBox().Centre();
 
         try
         {
-            clippedPoly.BooleanIntersection( itemPoly, viewportPoly, SHAPE_POLY_SET::PM_FAST );
+            itemPoly.Deflate( step, 4, SHAPE_POLY_SET::CHAMFER_ACUTE_CORNERS );
         }
         catch( const ClipperLib::clipperException& exc )
         {
-            // This may be overkill and could be an assertion but we are more likely to find
-            // any clipper errors this way.
+            // This may be overkill and could be an assertion but we are more likely to
+            // find any clipper errors this way.
             wxLogError( wxT( "Clipper library exception '%s' occurred." ), exc.what() );
         }
+    }
 
-        if( !clippedPoly.IsEmpty() )
-            itemPoly = clippedPoly;
-
-        /*
-         * Perform a step-wise deflate to find the visual-center-of-mass
-         */
-
-        BOX2I bbox = itemPoly.BBox();
-        int   step = std::min( bbox.GetWidth(), bbox.GetHeight() ) / 10;
-
-        while( !itemPoly.IsEmpty() )
-        {
-            focusPt = (wxPoint) itemPoly.BBox().Centre();
-
-            try
-            {
-                itemPoly.Deflate( step, 4, SHAPE_POLY_SET::CHAMFER_ACUTE_CORNERS );
-            }
-            catch( const ClipperLib::clipperException& exc )
-            {
-                // This may be overkill and could be an assertion but we are more likely to find
-                // any clipper errors this way.
-                wxLogError( wxT( "Clipper library exception '%s' occurred." ), exc.what() );
-            }
-        }
-
-        FocusOnLocation( focusPt );
+    FocusOnLocation( focusPt );
 
-        GetCanvas()->Refresh();
-    }
+    GetCanvas()->Refresh();
 }