kicad-developers team mailing list archive

[Jon Evans <jon@xxxxxxxxxxxxx>]

As JP has noted, the "sort layers by X2 attributes" feature is really slow
in GAL mode when you are looking at a complicated set of Gerber files.

This is because there was previously no way to swap around data between
layer IDs without removing and re-adding that data.

The attached patch implements a reordering mechanism and speeds up the X2
sorting by ~20x on my machine.

JP/Orson/others, please take a look and let me know if you have any
concerns with it.

Thanks,
Jon

From 90d98151de03453252708aff73a64fb2d2961c41 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@xxxxxxxxxxxxx>
Date: Sun, 4 Feb 2018 16:42:41 -0500
Subject: [PATCH] Allow low-cost reordering of view layers (for Gerber X2
 sorting)

---
 common/view/view.cpp                | 69 +++++++++++++++++++++++++++++++++++++
 gerbview/gerber_file_image_list.cpp |  7 ++--
 gerbview/gerber_file_image_list.h   |  5 ++-
 gerbview/gerbview_frame.cpp         |  3 +-
 gerbview/gerbview_layer_widget.cpp  | 11 +++---
 gerbview/job_file_reader.cpp        | 16 ++++++++-
 include/view/view.h                 | 13 +++++++
 7 files changed, 115 insertions(+), 9 deletions(-)

diff --git a/common/view/view.cpp b/common/view/view.cpp
index da11551d4..56de062db 100644
--- a/common/view/view.cpp
+++ b/common/view/view.cpp
@@ -626,6 +626,75 @@ void VIEW::SortLayers( int aLayers[], int& aCount ) const
 }
 
 
+void VIEW::ReorderLayerData( std::unordered_map<int, int> aReorderMap )
+{
+    LAYER_MAP new_map;
+
+    for( auto it : m_layers )
+    {
+        int orig_idx = it.first;
+        VIEW_LAYER layer = it.second;
+
+        int new_idx;
+
+        try
+        {
+            new_idx = aReorderMap.at( orig_idx );
+        }
+        catch( std::out_of_range )
+        {
+            new_idx = orig_idx;
+        }
+
+        layer.id = new_idx;
+        new_map[new_idx] = layer;
+
+        if( new_idx != orig_idx )
+            updateItemsLayer( new_idx );
+    }
+
+    m_layers = new_map;
+}
+
+
+struct VIEW::updateItemsLayerVisitor
+{
+    updateItemsLayerVisitor( int aLayer ) : layer( aLayer )
+    {
+    }
+
+    bool operator()( VIEW_ITEM* aItem )
+    {
+        int layers[VIEW_MAX_LAYERS], layers_count;
+
+        aItem->ViewGetLayers( layers, layers_count );
+        aItem->viewPrivData()->saveLayers( layers, layers_count );
+
+        return true;
+    }
+
+    int layer;
+};
+
+
+void VIEW::updateItemsLayer( int aLayer )
+{
+    // There is no point in updating non-cached layers
+    if( !IsCached( aLayer ) )
+        return;
+
+    BOX2I r;
+
+    r.SetMaximum();
+
+    m_gal->BeginUpdate();
+    updateItemsLayerVisitor visitor( aLayer );
+    m_layers[aLayer].items->Query( r, visitor );
+    MarkTargetDirty( m_layers[aLayer].target );
+    m_gal->EndUpdate();
+}
+
+
 struct VIEW::updateItemsColor
 {
     updateItemsColor( int aLayer, PAINTER* aPainter, GAL* aGal ) :
diff --git a/gerbview/gerber_file_image_list.cpp b/gerbview/gerber_file_image_list.cpp
index 364036acd..ecd6c769b 100644
--- a/gerbview/gerber_file_image_list.cpp
+++ b/gerbview/gerber_file_image_list.cpp
@@ -224,7 +224,8 @@ static bool sortZorder( const GERBER_FILE_IMAGE* const& ref, const GERBER_FILE_I
     return ref->m_FileFunction->GetZSubOrder() > test->m_FileFunction->GetZSubOrder();
 }
 
-void GERBER_FILE_IMAGE_LIST::SortImagesByZOrder()
+
+std::unordered_map<int, int> GERBER_FILE_IMAGE_LIST::SortImagesByZOrder()
 {
     std::sort( m_GERBER_List.begin(), m_GERBER_List.end(), sortZorder );
 
@@ -232,7 +233,7 @@ void GERBER_FILE_IMAGE_LIST::SortImagesByZOrder()
     // Graphic layer numbering must be updated to match the widgets layer order
 
     // Store the old/new graphic layer info:
-    std::map <int, int> tab_lyr;
+    std::unordered_map <int, int> tab_lyr;
 
     for( unsigned layer = 0; layer < m_GERBER_List.size(); ++layer )
     {
@@ -244,4 +245,6 @@ void GERBER_FILE_IMAGE_LIST::SortImagesByZOrder()
         tab_lyr[gerber->m_GraphicLayer] = layer;
         gerber->m_GraphicLayer = layer ;
     }
+
+    return tab_lyr;
 }
diff --git a/gerbview/gerber_file_image_list.h b/gerbview/gerber_file_image_list.h
index acfc369b6..6880a9187 100644
--- a/gerbview/gerber_file_image_list.h
+++ b/gerbview/gerber_file_image_list.h
@@ -27,6 +27,7 @@
 
 #include <vector>
 #include <set>
+#include <unordered_map>
 
 #include <gerber_draw_item.h>
 #include <am_primitive.h>
@@ -118,8 +119,10 @@ public:
     /**
      * Sort loaded images by Z order priority, if they have the X2 FileFormat info
      * (SortImagesByZOrder updates the graphic layer of these items)
+     *
+     * @return a mapping of old to new layer index
      */
-    void SortImagesByZOrder();
+    std::unordered_map<int, int> SortImagesByZOrder();
 
     #if defined(DEBUG)
 
diff --git a/gerbview/gerbview_frame.cpp b/gerbview/gerbview_frame.cpp
index 14aea7c04..7b86e98ab 100644
--- a/gerbview/gerbview_frame.cpp
+++ b/gerbview/gerbview_frame.cpp
@@ -521,7 +521,8 @@ void GERBVIEW_FRAME::applyDisplaySettingsToGAL()
 
     settings->ImportLegacyColors( m_colorsSettings );
 
-    view->RecacheAllItems();
+    view->UpdateAllLayersColor();
+    view->UpdateAllLayersOrder();
     view->MarkTargetDirty( KIGFX::TARGET_NONCACHED );
 }
 
diff --git a/gerbview/gerbview_layer_widget.cpp b/gerbview/gerbview_layer_widget.cpp
index 415cfc011..f82367e89 100644
--- a/gerbview/gerbview_layer_widget.cpp
+++ b/gerbview/gerbview_layer_widget.cpp
@@ -209,17 +209,20 @@ void GERBER_LAYER_WIDGET::onPopupSelection( wxCommandEvent& event )
         break;
 
     case ID_SORT_GBR_LAYERS:
-        GetImagesList()->SortImagesByZOrder();
+        auto remapping = GetImagesList()->SortImagesByZOrder();
         myframe->ReFillLayerWidget();
         myframe->syncLayerBox( true );
 
         if( myframe->IsGalCanvasActive() )
         {
-            for( int layer = 0; layer < GERBER_DRAWLAYERS_COUNT; ++layer )
+            std::unordered_map<int, int> view_remapping;
+
+            for( auto it : remapping )
             {
-                myframe->SetLayerColor( GERBER_DRAW_LAYER( layer ),
-                                        GetLayerColor( GERBER_DRAW_LAYER( layer ) ) );
+                view_remapping[ GERBER_DRAW_LAYER( it.first) ] = GERBER_DRAW_LAYER( it.second );
             }
+
+            myframe->GetGalCanvas()->GetView()->ReorderLayerData( view_remapping );
         }
 
         myframe->GetCanvas()->Refresh();
diff --git a/gerbview/job_file_reader.cpp b/gerbview/job_file_reader.cpp
index 80f54ee38..fde1814ab 100644
--- a/gerbview/job_file_reader.cpp
+++ b/gerbview/job_file_reader.cpp
@@ -39,6 +39,7 @@
 #include <reporter.h>
 #include <plot_auxiliary_data.h>
 #include <html_messagebox.h>
+#include <view/view.h>
 
 
 /**
@@ -220,9 +221,22 @@ bool GERBVIEW_FRAME::LoadGerberJobFile( const wxString& aFullFileName )
                 }
             }
 
-            GetImagesList()->SortImagesByZOrder();
+            auto remapping = GetImagesList()->SortImagesByZOrder();
             ReFillLayerWidget();
             syncLayerBox( true );
+
+            if( IsGalCanvasActive() )
+            {
+                std::unordered_map<int, int> view_remapping;
+
+                for( auto it : remapping )
+                {
+                    view_remapping[ GERBER_DRAW_LAYER( it.first) ] = GERBER_DRAW_LAYER( it.second );
+                }
+
+                GetGalCanvas()->GetView()->ReorderLayerData( view_remapping );
+            }
+
             GetCanvas()->Refresh();
         }
     }
diff --git a/include/view/view.h b/include/view/view.h
index 30bb6df39..693c582bc 100644
--- a/include/view/view.h
+++ b/include/view/view.h
@@ -454,6 +454,15 @@ public:
      */
     void SortLayers( int aLayers[], int& aCount ) const;
 
+    /**
+     * Remaps the data between layer ids without invalidating that data
+     *
+     * Used by GerbView for the "Sort by X2" functionality
+     *
+     * @param aReorderMap is a mapping of old to new layer ids
+     */
+    void ReorderLayerData( std::unordered_map<int, int> aReorderMap );
+
     /**
      * Function UpdateLayerColor()
      * Applies the new coloring scheme held by RENDER_SETTINGS in case that it has changed.
@@ -678,6 +687,7 @@ private:
     struct updateItemsColor;
     struct changeItemsDepth;
     struct extentsVisitor;
+    struct updateItemsLayerVisitor;
 
 
     ///* Redraws contents within rect aRect
@@ -749,6 +759,9 @@ private:
     /// Updates set of layers that an item occupies
     void updateLayers( VIEW_ITEM* aItem );
 
+    /// Updates the cached layer id for all items on the given layer
+    void updateItemsLayer( int aLayer );
+
     /// Determines rendering order of layers. Used in display order sorting function.
     static bool compareRenderingOrder( VIEW_LAYER* aI, VIEW_LAYER* aJ )
     {
-- 
2.14.1