From 36ceb8075e084207b50cce45d49cb002f1e926a3 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Sun, 4 Oct 2020 12:46:35 +0100
Subject: [PATCH] Fix logic bug in DRC RTree handler.

Return value from visitor is whether or not to keep searching, not
whether or not there was a collision.
---
 pcbnew/drc/drc_rtree.h                        | 46 +++++++++----------
 pcbnew/drc/drc_test_provider_silk_to_pad.cpp  |  6 +--
 pcbnew/drc/drc_test_provider_silk_to_silk.cpp |  8 ++--
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/pcbnew/drc/drc_rtree.h b/pcbnew/drc/drc_rtree.h
index cbed4b5a32..0b5fca708f 100644
--- a/pcbnew/drc/drc_rtree.h
+++ b/pcbnew/drc/drc_rtree.h
@@ -328,13 +328,11 @@ public:
     typedef std::pair<PCB_LAYER_ID, PCB_LAYER_ID> LAYER_PAIR;
 
     int QueryCollidingPairs( DRC_RTREE* aRefTree,
-                                std::vector<LAYER_PAIR> aLayers,
-                                std::function<bool( const LAYER_PAIR&, ITEM_WITH_SHAPE*, ITEM_WITH_SHAPE* )> aVisitor,
-    
-    //                         std::function<bool( const LAYER_PAIR&, BOARD_ITEM*, BOARD_ITEM*, int&, DRC_CONSTRAINT& )> aResolver,
-      //                       std::function<bool( BOARD_ITEM*, BOARD_ITEM*, int, int&, DRC_CONSTRAINT&)> aVisitor,
-                             int aMaxClearance
-                        )
+                             std::vector<LAYER_PAIR> aLayers,
+                             std::function<bool( const LAYER_PAIR&,
+                                                 ITEM_WITH_SHAPE*, ITEM_WITH_SHAPE*,
+                                                 bool* aCollision )> aVisitor,
+                             int aMaxClearance )
     {
         // keep track of BOARD_ITEMs pairs that have been already found to collide (some items
         // might be build of COMPOUND/triangulated shapes and a single subshape collision
@@ -354,27 +352,29 @@ public:
                 int min[2] = { box.GetX(),         box.GetY() };
                 int max[2] = { box.GetRight(),     box.GetBottom() };
 
-                auto visit = [&] ( ITEM_WITH_SHAPE* aItemToTest ) -> bool
-                {
-                    const std::pair<BOARD_ITEM*, BOARD_ITEM*> chkCompoundPair( refItem->parent, aItemToTest->parent );
+                auto visit =
+                        [&]( ITEM_WITH_SHAPE* aItemToTest ) -> bool
+                        {
+                            const std::pair<BOARD_ITEM*, BOARD_ITEM*>
+                                    chkCompoundPair( refItem->parent, aItemToTest->parent );
 
-                    // don't report multiple collisions for compound or triangulated shapes
-                    if( collidingCompounds.find( chkCompoundPair ) != collidingCompounds.end() )
-                        return true;
+                            // don't report multiple collisions for compound or triangulated shapes
+                            if( alg::contains( collidingCompounds, chkCompoundPair ) )
+                                return true;
 
-                    // don't collide items against themselves
-                    if( refLayer == targetLayer && aItemToTest->parent == refItem->parent )
-                        return true;
+                            // don't collide items against themselves
+                            if( refLayer == targetLayer && aItemToTest->parent == refItem->parent )
+                                return true;
 
-                    bool colliding = aVisitor( refLayerIter, refItem, aItemToTest );
+                            bool collisionDetected = false;
+                            bool continueSearch = aVisitor( refLayerIter, refItem, aItemToTest,
+                                                            &collisionDetected );
 
-                    if( colliding )
-                    {
-                        collidingCompounds.insert( chkCompoundPair );
-                    }
+                            if( collisionDetected )
+                                collidingCompounds.insert( chkCompoundPair );
 
-                    return true;
-                };
+                            return continueSearch;
+                        };
 
                 this->m_tree[targetLayer]->Search( min, max, visit );
             };
diff --git a/pcbnew/drc/drc_test_provider_silk_to_pad.cpp b/pcbnew/drc/drc_test_provider_silk_to_pad.cpp
index 9b680d7e74..6b5bb4a8ad 100644
--- a/pcbnew/drc/drc_test_provider_silk_to_pad.cpp
+++ b/pcbnew/drc/drc_test_provider_silk_to_pad.cpp
@@ -122,9 +122,8 @@ bool test::DRC_TEST_PROVIDER_SILK_TO_PAD::Run()
             };
 
     auto checkClearance =
-            [&]( const DRC_RTREE::LAYER_PAIR& aLayers,
-                 DRC_RTREE::ITEM_WITH_SHAPE* aRefItem,
-                 DRC_RTREE::ITEM_WITH_SHAPE* aTestItem ) -> bool
+            [&]( const DRC_RTREE::LAYER_PAIR& aLayers, DRC_RTREE::ITEM_WITH_SHAPE* aRefItem,
+                 DRC_RTREE::ITEM_WITH_SHAPE* aTestItem, bool* aCollisionDetected ) -> bool
             {
                 if( m_drcEngine->IsErrorLimitExceeded( DRCE_SILK_OVER_PAD ) )
                     return false;
@@ -156,6 +155,7 @@ bool test::DRC_TEST_PROVIDER_SILK_TO_PAD::Run()
 
                 reportViolation( drcItem, (wxPoint) pos );
 
+                *aCollisionDetected = true;
                 return true;
             };
 
diff --git a/pcbnew/drc/drc_test_provider_silk_to_silk.cpp b/pcbnew/drc/drc_test_provider_silk_to_silk.cpp
index 6170dcb154..ac513b22d6 100644
--- a/pcbnew/drc/drc_test_provider_silk_to_silk.cpp
+++ b/pcbnew/drc/drc_test_provider_silk_to_silk.cpp
@@ -107,9 +107,8 @@ bool DRC_TEST_PROVIDER_SILK_TO_SILK::Run()
             };
 
     auto checkClearance =
-            [&]( const DRC_RTREE::LAYER_PAIR& aLayers,
-                 DRC_RTREE::ITEM_WITH_SHAPE* aRefItem,
-                 DRC_RTREE::ITEM_WITH_SHAPE* aTestItem ) -> bool
+            [&]( const DRC_RTREE::LAYER_PAIR& aLayers, DRC_RTREE::ITEM_WITH_SHAPE* aRefItem,
+                 DRC_RTREE::ITEM_WITH_SHAPE* aTestItem, bool* aCollisionDetected ) -> bool
             {
                 if( m_drcEngine->IsErrorLimitExceeded( DRCE_SILK_CLEARANCE ) )
                     return false;
@@ -123,7 +122,7 @@ bool DRC_TEST_PROVIDER_SILK_TO_SILK::Run()
 
                 accountCheck( constraint );
 
-                // only check for silkscreen collisions belonging to different modules or
+                // only check for silkscreen collisions belonging to different footprints or
                 // overlapping texts
 
                 KICAD_T typeRef = aRefItem->parent->Type();
@@ -180,6 +179,7 @@ bool DRC_TEST_PROVIDER_SILK_TO_SILK::Run()
 
                 reportViolation( drcItem, (wxPoint) pos );
 
+                *aCollisionDetected = true;
                 return true;
             };