From 3a9ef02fdfde83b787db2d124567b545a1961e93 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@craftyjon.com>
Date: Sat, 21 Jan 2023 17:36:39 -0500
Subject: [PATCH] Clean up handling of selection changes

Fixes https://gitlab.com/kicad/code/kicad/-/issues/13620
Fixes https://gitlab.com/kicad/code/kicad/-/issues/13609
---
 common/tool/selection.cpp               | 10 ++++++++++
 common/widgets/properties_panel.cpp     | 12 ++++++------
 common/widgets/properties_panel.h       |  4 ++--
 include/tool/selection.h                |  2 ++
 pcbnew/widgets/pcb_properties_panel.cpp |  9 +++++----
 5 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/common/tool/selection.cpp b/common/tool/selection.cpp
index 535d8030a0..db233335eb 100644
--- a/common/tool/selection.cpp
+++ b/common/tool/selection.cpp
@@ -29,6 +29,16 @@
 #include <tool/selection.h>
 
 
+bool SELECTION::operator==( const SELECTION& aOther )
+{
+    return ( m_items == aOther.m_items
+             && m_itemsOrders == aOther.m_itemsOrders
+             && m_isHover == aOther.m_isHover
+             && m_lastAddedItem == aOther.m_lastAddedItem
+             && m_orderCounter == aOther.m_orderCounter );
+}
+
+
 void SELECTION::Add( EDA_ITEM* aItem )
 {
     // We're not sorting here; this is just a time-optimized way to do an
diff --git a/common/widgets/properties_panel.cpp b/common/widgets/properties_panel.cpp
index 7229c10599..e21d834bf5 100644
--- a/common/widgets/properties_panel.cpp
+++ b/common/widgets/properties_panel.cpp
@@ -39,8 +39,7 @@ extern APIIMPORT wxPGGlobalVarsClass* wxPGGlobalVars;
 PROPERTIES_PANEL::PROPERTIES_PANEL( wxWindow* aParent, EDA_BASE_FRAME* aFrame ) :
         wxPanel( aParent ),
         m_frame( aFrame ),
-        m_splitter_key_proportion( -1 ),
-        m_skipNextUpdate( false )
+        m_splitter_key_proportion( -1 )
 {
     wxBoxSizer* mainSizer = new wxBoxSizer( wxVERTICAL );
 
@@ -66,6 +65,8 @@ PROPERTIES_PANEL::PROPERTIES_PANEL( wxWindow* aParent, EDA_BASE_FRAME* aFrame )
     delete wxPGGlobalVars->m_defaultRenderer;
     wxPGGlobalVars->m_defaultRenderer = new PG_CELL_RENDERER();
 
+    m_cachedSelection = std::make_unique<SELECTION>();
+
     m_caption = new wxStaticText( this, wxID_ANY, _( "No objects selected" ) );
     mainSizer->Add( m_caption, 0, wxALL | wxEXPAND, 5 );
 
@@ -130,11 +131,10 @@ void PROPERTIES_PANEL::OnLanguageChanged()
 
 void PROPERTIES_PANEL::update( const SELECTION& aSelection )
 {
-    if( m_skipNextUpdate )
-    {
-        m_skipNextUpdate = false;
+    if( *m_cachedSelection == aSelection )
         return;
-    }
+
+    *m_cachedSelection = aSelection;
 
     if( m_grid->IsEditorFocused() )
         m_grid->CommitChangesFromEditor();
diff --git a/common/widgets/properties_panel.h b/common/widgets/properties_panel.h
index 513c2182e4..03ba80b029 100644
--- a/common/widgets/properties_panel.h
+++ b/common/widgets/properties_panel.h
@@ -28,6 +28,7 @@
 #include <wx/propgrid/propgrid.h>
 
 #include <vector>
+#include <memory>
 
 class EDA_BASE_FRAME;
 class SELECTION;
@@ -88,8 +89,7 @@ protected:
     /// Proportion of the grid column splitter that is used for the key column (0.0 - 1.0)
     float m_splitter_key_proportion;
 
-    /// Skips one call to update()
-    bool m_skipNextUpdate;
+    std::unique_ptr<SELECTION> m_cachedSelection;
 };
 
 #endif /* PROPERTIES_PANEL_H */
diff --git a/include/tool/selection.h b/include/tool/selection.h
index 8494cc0a8c..8989d838cc 100644
--- a/include/tool/selection.h
+++ b/include/tool/selection.h
@@ -65,6 +65,8 @@ public:
         return *this;
     }
 
+    bool operator==( const SELECTION& aOther );
+
     using ITER = std::deque<EDA_ITEM*>::iterator;
     using CITER = std::deque<EDA_ITEM*>::const_iterator;
 
diff --git a/pcbnew/widgets/pcb_properties_panel.cpp b/pcbnew/widgets/pcb_properties_panel.cpp
index fc265e229b..7cc96ab211 100644
--- a/pcbnew/widgets/pcb_properties_panel.cpp
+++ b/pcbnew/widgets/pcb_properties_panel.cpp
@@ -145,10 +145,14 @@ void PCB_PROPERTIES_PANEL::valueChanged( wxPropertyGridEvent& aEvent )
     const SELECTION& selection = selectionTool->GetSelection();
     BOARD_ITEM* firstItem = static_cast<BOARD_ITEM*>( selection.Front() );
 
-    wxCHECK( firstItem, /* void */ );
+    wxCHECK_MSG( firstItem, /* void */,
+                 wxT( "valueChanged for a property with nothing selected!") );
 
     PROPERTY_BASE* property = m_propMgr.GetProperty( TYPE_HASH( *firstItem ),
                                                      aEvent.GetPropertyName() );
+    wxCHECK_MSG( property, /* void */,
+                 wxT( "valueChanged for a property not found on the selected item!" ) );
+
     wxVariant newValue = aEvent.GetPropertyValue();
     BOARD_COMMIT changes( m_frame );
 
@@ -159,9 +163,6 @@ void PCB_PROPERTIES_PANEL::valueChanged( wxPropertyGridEvent& aEvent )
         item->Set( property, newValue );
     }
 
-    // Pushing the commit will result in a SelectedItemsModified event, which we want to skip
-    m_skipNextUpdate = true;
-
     changes.Push( _( "Change property" ) );
     m_frame->Refresh();