Performance

Cache VIEW_ITEM's bbox in VIEW_TREE for faster removals.

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

common/view/view.cpp

@@ -234,6 +234,8 @@ private:
     int                  m_groupsSize;
 
     std::vector<int>     m_layers;           /// Stores layer numbers used by the item.
+
+    BOX2I                m_bbox;             /// Cached inserted Bbox for faster removals.
 };
 
 
@@ -326,6 +328,8 @@ void VIEW::Add( VIEW_ITEM* aItem, int aDrawPriority )
 
     aItem->m_viewPrivData->m_view = this;
     aItem->m_viewPrivData->m_drawPriority = aDrawPriority;
+    const BOX2I bbox = aItem->ViewBBox();
+    aItem->m_viewPrivData->m_bbox = bbox;
 
     aItem->ViewGetLayers( layers, layers_count );
     aItem->viewPrivData()->saveLayers( layers, layers_count );
@@ -338,7 +342,7 @@ void VIEW::Add( VIEW_ITEM* aItem, int aDrawPriority )
                       continue, wxS( "Invalid layer" ) );
 
         VIEW_LAYER& l = m_layers[layers[i]];
-        l.items->Insert( aItem );
+        l.items->Insert( aItem, bbox );
         MarkTargetDirty( l.target );
     }
 
@@ -362,11 +366,12 @@ void VIEW::Remove( VIEW_ITEM* aItem )
 
         int layers[VIEW::VIEW_MAX_LAYERS], layers_count;
         aItem->m_viewPrivData->getLayers( layers, layers_count );
+        const BOX2I* bbox = &aItem->m_viewPrivData->m_bbox;
 
         for( int i = 0; i < layers_count; ++i )
         {
             VIEW_LAYER& l = m_layers[layers[i]];
-            l.items->Remove( aItem );
+            l.items->Remove( aItem, bbox );
             MarkTargetDirty( l.target );
 
             // Clear the GAL cache
@@ -1327,12 +1332,17 @@ void VIEW::updateBbox( VIEW_ITEM* aItem )
     int layers[VIEW_MAX_LAYERS], layers_count;
 
     aItem->ViewGetLayers( layers, layers_count );
+    wxASSERT( aItem->m_viewPrivData ); //must have a viewPrivData
+
+    const BOX2I  new_bbox = aItem->ViewBBox();
+    const BOX2I* old_bbox = &aItem->m_viewPrivData->m_bbox;
+    aItem->m_viewPrivData->m_bbox = new_bbox;
 
     for( int i = 0; i < layers_count; ++i )
     {
         VIEW_LAYER& l = m_layers[layers[i]];
-        l.items->Remove( aItem );
+        l.items->Remove( aItem, old_bbox );
-        l.items->Insert( aItem );
+        l.items->Insert( aItem, new_bbox );
         MarkTargetDirty( l.target );
     }
 }
@@ -1348,11 +1358,12 @@ void VIEW::updateLayers( VIEW_ITEM* aItem )
 
     // Remove the item from previous layer set
     viewData->getLayers( layers, layers_count );
+    const BOX2I* old_bbox = &aItem->m_viewPrivData->m_bbox;
 
     for( int i = 0; i < layers_count; ++i )
     {
         VIEW_LAYER& l = m_layers[layers[i]];
-        l.items->Remove( aItem );
+        l.items->Remove( aItem, old_bbox );
         MarkTargetDirty( l.target );
 
         if( IsCached( l.id ) )
@@ -1368,6 +1379,9 @@ void VIEW::updateLayers( VIEW_ITEM* aItem )
         }
     }
 
+    const BOX2I new_bbox = aItem->ViewBBox();
+    aItem->m_viewPrivData->m_bbox = new_bbox;
+
     // Add the item to new layer set
     aItem->ViewGetLayers( layers, layers_count );
     viewData->saveLayers( layers, layers_count );
@@ -1375,7 +1389,7 @@ void VIEW::updateLayers( VIEW_ITEM* aItem )
     for( int i = 0; i < layers_count; i++ )
     {
         VIEW_LAYER& l = m_layers[layers[i]];
-        l.items->Insert( aItem );
+        l.items->Insert( aItem, new_bbox );
         MarkTargetDirty( l.target );
     }
 }
@@ -1463,6 +1477,9 @@ void VIEW::UpdateItems()
         // and re-insert items from scratch
         for( VIEW_ITEM* item : allItems )
         {
+            const BOX2I bbox = item->ViewBBox();
+            item->m_viewPrivData->m_bbox = bbox;
+
             item->ViewGetLayers( layers, layers_count );
             item->viewPrivData()->saveLayers( layers, layers_count );
 
@@ -1471,7 +1488,7 @@ void VIEW::UpdateItems()
                 wxCHECK2_MSG( layers[i] >= 0 && static_cast<unsigned>( layers[i] ) < m_layers.size(),
                         continue, wxS( "Invalid layer" ) );
                 VIEW_LAYER& l = m_layers[layers[i]];
-                l.items->Insert( item );
+                l.items->Insert( item, bbox );
                 MarkTargetDirty( l.target );
             }
 

gerbview/gerber_file_image.cpp

@@ -30,6 +30,7 @@
 #include <algorithm>
 #include <map>
 #include <core/arraydim.h>
+#include <core/profile.h>
 
 
 /**
@@ -97,8 +98,10 @@ GERBER_FILE_IMAGE::GERBER_FILE_IMAGE( int aLayer ) :
 
 GERBER_FILE_IMAGE::~GERBER_FILE_IMAGE()
 {
-    for( GERBER_DRAW_ITEM* item : GetItems() )
+    // Reverse iteration to avoid O(N^2) memcpy in a vector erase downstream.
-        delete item;
+    // It results in a O(N^2) std::find, which is somewhat faster.
+    for( auto it = GetItems().rbegin(); it < GetItems().rend(); it++ )
+        delete *it;
 
     m_drawings.clear();
 

include/view/view_rtree.h

@@ -46,9 +46,8 @@ public:
      *
      * Item's bounding box is taken via its ViewBBox() method.
      */
-    void Insert( VIEW_ITEM* aItem )
+    void Insert( VIEW_ITEM* aItem, const BOX2I& bbox )
     {
-        const BOX2I&    bbox    = aItem->ViewBBox();
         const int       mmin[2] = { bbox.GetX(), bbox.GetY() };
         const int       mmax[2] = { bbox.GetRight(), bbox.GetBottom() };
 
@@ -60,10 +59,18 @@ public:
      *
      * Removal is done by comparing pointers, attempting to remove a copy of the item will fail.
      */
-    void Remove( VIEW_ITEM* aItem )
+    void Remove( VIEW_ITEM* aItem, const BOX2I* aBbox )
     {
         // const BOX2I&    bbox    = aItem->ViewBBox();
 
+        if( aBbox )
+        {
+            const int mmin[2] = { aBbox->GetX(), aBbox->GetY() };
+            const int mmax[2] = { aBbox->GetRight(), aBbox->GetBottom() };
+            VIEW_RTREE_BASE::Remove( mmin, mmax, aItem );
+            return;
+        }
+
         // FIXME: use cached bbox or ptr_map to speed up pointer <-> node lookups.
         const int       mmin[2] = { INT_MIN, INT_MIN };
         const int       mmax[2] = { INT_MAX, INT_MAX };