Pcbnew: improve snapping: avoid cases where a valid snap is excluded

Trimming items that don't have an "involved" real item needs to be
done at the time that the nearest point is found. Otherwise, if
there are multiple nearest points at the same location, and an
'uninvolved' one is chosen, it will later be discarded, and the
grid snap will be the fallback.

The effect of this is that grid snaps can appear to be very
agressive and also inconsistent, as it sometimes uses the item
snap (when it happens to choose an "involved" one) and sometimes not.

pcbnew/tools/pcb_grid_helper.cpp

@@ -645,37 +645,14 @@ VECTOR2I PCB_GRID_HELPER::BestSnapAnchor( const VECTOR2I& aOrigin, const LSET& a
                 if( !anchorIsConstructed )
                     proposeConstructionForItems( nearest->items );
 
-                const auto shouldAcceptAnchor = [&]( const ANCHOR& aAnchor )
-                {
-                    // If no extension snaps are enabled, don't inhibit
-                    static const bool haveExtensions = ADVANCED_CFG::GetCfg().m_EnableExtensionSnaps;
-
-                    if( !haveExtensions )
-                        return true;
+                m_snapItem = *nearest;
 
-                    // Check that any involved real items are 'active'
-                    // (i.e. the user has moused over a key point previously)
-                    // If any are not real (e.g. snap lines), they are allowed to be involved
-                    //
-                    // This is an area most likely to be controversial/need tuning,
-                    // as some users will think it's fiddly; without 'activation', others will
-                    // think the snaps are intrusive.
-                    bool allRealAreInvolved = snapManager.GetConstructionManager().InvolvesAllGivenRealItems(
-                                    aAnchor.items );
-                    return allRealAreInvolved;
-                };
-
-                if( shouldAcceptAnchor( *nearest ) )
-                {
-                    m_snapItem = *nearest;
+                // Set the snap line origin or end as needed
+                snapLineManager.SetSnappedAnchor( m_snapItem->pos );
+                // Show the correct snap point marker
+                updateSnapPoint( { m_snapItem->pos, m_snapItem->pointTypes } );
 
-                    // Set the snap line origin or end as needed
-                    snapLineManager.SetSnappedAnchor( m_snapItem->pos );
-                    // Show the correct snap point marker
-                    updateSnapPoint( { m_snapItem->pos, m_snapItem->pointTypes } );
-
-                    return m_snapItem->pos;
-                }
+                return m_snapItem->pos;
             }
 
             snapValid = true;
@@ -1729,6 +1706,37 @@ PCB_GRID_HELPER::ANCHOR* PCB_GRID_HELPER::nearestAnchor( const VECTOR2I& aPos, i
         }
     }
 
+    // Check that any involved real items are 'active'
+    // (i.e. the user has moused over a key point previously)
+    // If any are not real (e.g. snap lines), they are allowed to be involved
+    //
+    // This is an area most likely to be controversial/need tuning,
+    // as some users will think it's fiddly; without 'activation', others will
+    // think the snaps are intrusive.
+    SNAP_MANAGER& snapManager = getSnapManager();
+    const auto    noRealItemsInAnchorAreInvolved = [&]( ANCHOR* aAnchor ) -> bool
+    {
+        // If no extension snaps are enabled, don't inhibit
+        static const bool haveExtensions = ADVANCED_CFG::GetCfg().m_EnableExtensionSnaps;
+
+        if( !haveExtensions )
+            return false;
+
+        // If the anchor is not constructed, it may be involved (because it is one
+        // of the nearest anchors). The items will only be activated later, but don't
+        // discard the anchor yet.
+        const bool anchorIsConstructed = aAnchor->flags & ANCHOR_FLAGS::CONSTRUCTED;
+
+        if( !anchorIsConstructed )
+            return false;
+
+        bool allRealAreInvolved = snapManager.GetConstructionManager().InvolvesAllGivenRealItems( aAnchor->items );
+        return !allRealAreInvolved;
+    };
+
+    // Trim out items that aren't involved
+    std::erase_if( anchorsAtMinDistance, noRealItemsInAnchorAreInvolved );
+
     // More than one anchor can be at the same distance, for example
     // two lines end-to-end each have the same endpoint anchor.
     // So, check which one has an involved item that's closest to the origin,