← Back to team overview

kicad-developers team mailing list archive

[PATCH] GerbView performance: in-place layer reordering for X2 sorting

 

Hi all,

A few more GerbView patches for tonight.

The larger one should get some testing if possible before merging -- last
time I tried something like this, JP said it killed performance on Windows.
On my Linux machine, this fixes some issues with X2 sorting behavior, and
is quite fast.

The smaller one is just a fix to redraw negative items when their
visibility is toggled.

-Jon
From 30664c1ef39a3e9898b7ca732d3b9c0a97264318 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@xxxxxxxxxxxxx>
Date: Mon, 26 Feb 2018 21:15:32 -0500
Subject: [PATCH] GerbView: Implement in-place GAL layer reordering (for X2
 sorting)

---
 common/view/view.cpp                 | 71 ++++++++++++++++++++++++++++++++++++
 gerbview/events_called_functions.cpp |  2 +-
 gerbview/files.cpp                   |  2 -
 gerbview/gerber_file_image_list.cpp  |  7 +++-
 gerbview/gerber_file_image_list.h    |  5 ++-
 gerbview/gerbview_layer_widget.cpp   | 14 +++----
 gerbview/job_file_reader.cpp         | 15 ++++++++
 include/view/view.h                  |  9 +++++
 8 files changed, 112 insertions(+), 13 deletions(-)

diff --git a/common/view/view.cpp b/common/view/view.cpp
index c42d4d0b9..a1ad88aa6 100644
--- a/common/view/view.cpp
+++ b/common/view/view.cpp
@@ -189,6 +189,31 @@ private:
         return m_groupsSize > 0;
     }
 
+
+    /**
+     * Reorders the stored groups (to facilitate reordering of layers)
+     * @see VIEW::ReorderLayerData
+     *
+     * @param aReorderMap is the mapping of old to new layer ids
+     */
+    void reorderGroups( std::unordered_map<int, int> aReorderMap )
+    {
+        for( int i = 0; i < m_groupsSize; ++i )
+        {
+            int orig_layer = m_groups[i].first;
+            int new_layer = orig_layer;
+
+            try
+            {
+                new_layer = aReorderMap.at( orig_layer );
+            }
+            catch( std::out_of_range ) {}
+
+            m_groups[i].first = new_layer;
+        }
+    }
+
+
     /// Stores layer numbers used by the item.
     std::vector<int> m_layers;
 
@@ -626,6 +651,52 @@ 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;
+    }
+
+    m_layers = new_map;
+
+    for( VIEW_ITEM* item : m_allItems )
+    {
+        auto viewData = item->viewPrivData();
+
+        if( !viewData )
+            continue;
+
+        int layers[VIEW::VIEW_MAX_LAYERS], layers_count;
+
+        item->ViewGetLayers( layers, layers_count );
+        viewData->saveLayers( layers, layers_count );
+
+        viewData->reorderGroups( aReorderMap );
+
+        viewData->m_requiredUpdate |= COLOR;
+    }
+
+    UpdateItems();
+}
+
+
 struct VIEW::updateItemsColor
 {
     updateItemsColor( int aLayer, PAINTER* aPainter, GAL* aGal ) :
diff --git a/gerbview/events_called_functions.cpp b/gerbview/events_called_functions.cpp
index fac8e99b7..3d1d0ae79 100644
--- a/gerbview/events_called_functions.cpp
+++ b/gerbview/events_called_functions.cpp
@@ -492,7 +492,7 @@ void GERBVIEW_FRAME::OnSelectOptionToolbar( wxCommandEvent& event )
 
     case ID_TB_OPTIONS_SHOW_NEGATIVE_ITEMS:
         SetElementVisibility( LAYER_NEGATIVE_OBJECTS, state );
-        m_canvas->Refresh( true );
+        needs_refresh = true;
         break;
 
     case ID_TB_OPTIONS_DIFF_MODE:
diff --git a/gerbview/files.cpp b/gerbview/files.cpp
index 76f7e06ed..381ade3f5 100644
--- a/gerbview/files.cpp
+++ b/gerbview/files.cpp
@@ -306,8 +306,6 @@ bool GERBVIEW_FRAME::loadListOfGerberFiles( const wxString& aPath,
 
     Zoom_Automatique( false );
 
-    GetImagesList()->SortImagesByZOrder();
-
     // Synchronize layers tools with actual active layer:
     ReFillLayerWidget();
     SetActiveLayer( GetActiveLayer() );
diff --git a/gerbview/gerber_file_image_list.cpp b/gerbview/gerber_file_image_list.cpp
index 364036acd..b6135c1b0 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_layer_widget.cpp b/gerbview/gerbview_layer_widget.cpp
index e5e037785..4b7407a86 100644
--- a/gerbview/gerbview_layer_widget.cpp
+++ b/gerbview/gerbview_layer_widget.cpp
@@ -209,19 +209,19 @@ 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() )
+        std::unordered_map<int, int> view_remapping;
+
+        for( auto it : remapping )
         {
-            for( int layer = 0; layer < GERBER_DRAWLAYERS_COUNT; ++layer )
-            {
-                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();
         break;
     }
diff --git a/gerbview/job_file_reader.cpp b/gerbview/job_file_reader.cpp
index c72de6b66..f6a4b470f 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>
 
 
 /**
@@ -196,6 +197,20 @@ bool GERBVIEW_FRAME::LoadGerberJobFile( const wxString& aFullFileName )
 
     Zoom_Automatique( false );
 
+    auto remapping = GetImagesList()->SortImagesByZOrder();
+
+    ReFillLayerWidget();
+    syncLayerBox( true );
+
+    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 );
+
     if( !msg.IsEmpty() )
     {
         wxSafeYield();  // Allows slice of time to redraw the screen
diff --git a/include/view/view.h b/include/view/view.h
index 9029b7be2..f7453de39 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.
-- 
2.14.1

From 02e4d4a2f96d89fe0bbf71066abc37d7425ee275 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@xxxxxxxxxxxxx>
Date: Mon, 26 Feb 2018 20:49:42 -0500
Subject: [PATCH] GerbView: redraw negative items when visibility changes

---
 gerbview/gerbview_frame.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gerbview/gerbview_frame.cpp b/gerbview/gerbview_frame.cpp
index 99bc24787..f73e2afd2 100644
--- a/gerbview/gerbview_frame.cpp
+++ b/gerbview/gerbview_frame.cpp
@@ -498,8 +498,20 @@ void GERBVIEW_FRAME::SetElementVisibility( GERBVIEW_LAYER_ID aItemIdVisible,
         break;
 
     case LAYER_NEGATIVE_OBJECTS:
+    {
         m_DisplayOptions.m_DisplayNegativeObjects = aNewState;
+
+        auto view = GetGalCanvas()->GetView();
+
+        view->UpdateAllItemsConditionally( KIGFX::REPAINT,
+                                           []( KIGFX::VIEW_ITEM* aItem ) {
+            auto item = static_cast<GERBER_DRAW_ITEM*>( aItem );
+
+            // GetLayerPolarity() returns true for negative items
+            return item->GetLayerPolarity();
+        } );
         break;
+    }
 
     case LAYER_GERBVIEW_GRID:
         SetGridVisibility( aNewState );
-- 
2.14.1


Follow ups