Lazily re-evaluate worst-clearance cache.

This prevents crashes when trying to re-evaluate
during destruction, etc. and is a cleaner solution
than trying to keep a flag updated.

It should also be a performance win for very large
documents.

Also implements proper threadlocking for the cache.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/17950

pcbnew/board.cpp

@@ -81,8 +81,6 @@ BOARD::BOARD() :
         m_project( nullptr ),
         m_userUnits( EDA_UNITS::MILLIMETRES ),
         m_designSettings( new BOARD_DESIGN_SETTINGS( nullptr, "board.design_settings" ) ),
-        m_skipMaxClearanceCacheUpdate( false ),
-        m_maxClearanceValue( 0 ),
         m_NetInfo( this )
 {
     // A too small value do not allow connecting 2 shapes (i.e. segments) not exactly connected
@@ -134,8 +132,6 @@ BOARD::BOARD() :
 
 BOARD::~BOARD()
 {
-    m_skipMaxClearanceCacheUpdate = true;
-
     // Untangle group parents before doing any deleting
     for( PCB_GROUP* group : m_groups )
     {
@@ -250,8 +246,6 @@ void BOARD::IncrementTimeStamp()
 {
     m_timeStamp++;
 
-    UpdateMaxClearanceCache();
-
     if( !m_IntersectsAreaCache.empty()
         || !m_EnclosedByAreaCache.empty()
         || !m_IntersectsCourtyardCache.empty()
@@ -259,7 +253,8 @@ void BOARD::IncrementTimeStamp()
         || !m_IntersectsBCourtyardCache.empty()
         || !m_LayerExpressionCache.empty()
         || !m_ZoneBBoxCache.empty()
-        || m_CopperItemRTreeCache )
+        || m_CopperItemRTreeCache
+        || m_maxClearanceValue.has_value() )
     {
         std::unique_lock<std::shared_mutex> writeLock( m_CachesMutex );
 
@@ -282,6 +277,8 @@ void BOARD::IncrementTimeStamp()
         m_DRCCopperZones.clear();
         m_ZoneIsolatedIslandsMap.clear();
         m_CopperZoneRTreeCache.clear();
+
+        m_maxClearanceValue.reset();
     }
 }
 
@@ -800,33 +797,36 @@ BOARD_DESIGN_SETTINGS& BOARD::GetDesignSettings() const
 }
 
 
-void BOARD::UpdateMaxClearanceCache()
+int BOARD::GetMaxClearanceValue() const
 {
-    // in destructor or otherwise reasonable to skip
+    if( !m_maxClearanceValue.has_value() )
-    if( m_skipMaxClearanceCacheUpdate )
-        return;
-
-    int worstClearance = m_designSettings->GetBiggestClearanceValue();
-
-    for( ZONE* zone : m_zones )
-        worstClearance = std::max( worstClearance, zone->GetLocalClearance().value() );
-
-    for( FOOTPRINT* footprint : m_footprints )
     {
-        for( PAD* pad : footprint->Pads() )
+        std::unique_lock<std::shared_mutex> writeLock( m_CachesMutex );
-        {
-            std::optional<int> override = pad->GetClearanceOverrides( nullptr );
 
-            if( override.has_value() )
+        int worstClearance = m_designSettings->GetBiggestClearanceValue();
-                worstClearance = std::max( worstClearance, override.value() );
+
+        for( ZONE* zone : m_zones )
+            worstClearance = std::max( worstClearance, zone->GetLocalClearance().value() );
+
+        for( FOOTPRINT* footprint : m_footprints )
+        {
+            for( PAD* pad : footprint->Pads() )
+            {
+                std::optional<int> override = pad->GetClearanceOverrides( nullptr );
+
+                if( override.has_value() )
+                    worstClearance = std::max( worstClearance, override.value() );
+            }
+
+            for( ZONE* zone : footprint->Zones() )
+                worstClearance = std::max( worstClearance, zone->GetLocalClearance().value() );
         }
 
-        for( ZONE* zone : footprint->Zones() )
+        m_maxClearanceValue = worstClearance;
-            worstClearance = std::max( worstClearance, zone->GetLocalClearance().value() );
     }
 
-    m_maxClearanceValue = worstClearance;
+    return m_maxClearanceValue.value_or( 0 );
-}
+};
 
 
 void BOARD::CacheTriangulation( PROGRESS_REPORTER* aReporter, const std::vector<ZONE*>& aZones )
@@ -1276,8 +1276,6 @@ void BOARD::DeleteMARKERs( bool aWarningsAndErrors, bool aExclusions )
 
 void BOARD::DeleteAllFootprints()
 {
-    m_skipMaxClearanceCacheUpdate = true;
-
     for( FOOTPRINT* footprint : m_footprints )
     {
         m_itemByIdCache.erase( footprint->m_Uuid );
@@ -1285,8 +1283,7 @@ void BOARD::DeleteAllFootprints()
     }
 
     m_footprints.clear();
-    m_skipMaxClearanceCacheUpdate = false;
+    IncrementTimeStamp();
-    UpdateMaxClearanceCache();
 }
 
 

pcbnew/board.h

@@ -1154,10 +1154,7 @@ public:
      * the clearances from board design settings as well as embedded clearances in footprints,
      * pads and zones.  Includes electrical, physical, hole and edge clearances.
     */
-    int GetMaxClearanceValue() const
+    int GetMaxClearanceValue() const;
-    {
-        return m_maxClearanceValue;
-    };
 
     /**
      * Map all nets in the given board to nets with the same name (if any) in the destination
@@ -1258,6 +1255,7 @@ public:
         bool operator()( const BOARD_ITEM* aFirst, const BOARD_ITEM* aSecond ) const;
     };
 
+public:
     // ------------ Run-time caches -------------
     mutable std::shared_mutex                             m_CachesMutex;
     std::unordered_map<PTR_PTR_CACHE_KEY, bool>           m_IntersectsCourtyardCache;
@@ -1269,6 +1267,7 @@ public:
     std::unordered_map<ZONE*, std::unique_ptr<DRC_RTREE>> m_CopperZoneRTreeCache;
     std::shared_ptr<DRC_RTREE>                            m_CopperItemRTreeCache;
     mutable std::unordered_map<const ZONE*, BOX2I>        m_ZoneBBoxCache;
+    mutable std::optional<int>                            m_maxClearanceValue;
 
     // ------------ DRC caches -------------
     std::vector<ZONE*>    m_DRCZones;
@@ -1292,14 +1291,10 @@ private:
             ( l->*aFunc )( std::forward<Args>( args )... );
     }
 
-
-    void UpdateMaxClearanceCache();
-
     friend class PCB_EDIT_FRAME;
 
-
     /// the max distance between 2 end point to see them connected when building the board outlines
-    int m_outlinesChainingEpsilon;
+    int                 m_outlinesChainingEpsilon;
 
     /// What is this board being used for
     BOARD_USE           m_boardUse;
@@ -1354,9 +1349,6 @@ private:
      */
     bool                         m_legacyTeardrops = false;
 
-    bool                         m_skipMaxClearanceCacheUpdate;
-    int                          m_maxClearanceValue;  // cached value
-
     NETINFO_LIST                 m_NetInfo;         // net info list (name, design constraints...
 
     std::vector<BOARD_LISTENER*> m_listeners;