From 89f065b57e2a8c69a3183814502a92659d05438c Mon Sep 17 00:00:00 2001
From: YuSanka <yusanka@gmail.com>
Date: Thu, 25 Feb 2021 12:55:06 +0100
Subject: [PATCH] Fix of #5510: ctrlsub.cpp(231): assert "IsValid(n)" failed in
 GetClientData(): Invalid index passed to GetClientData()

BitmapComboBox: Use virtual OnSelect() on wxEVT_COMBO event
Don't save information about preset combobox type to the evt.SetInt(). This information can be received from BitmapComboBox::get_type() now.
---
 src/slic3r/GUI/Plater.cpp           |   8 +-
 src/slic3r/GUI/PresetComboBoxes.cpp | 154 +++++++++++++++-------------
 src/slic3r/GUI/PresetComboBoxes.hpp |   3 +
 3 files changed, 86 insertions(+), 79 deletions(-)

diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp
index 6400bbbcc..815cf3912 100644
--- a/src/slic3r/GUI/Plater.cpp
+++ b/src/slic3r/GUI/Plater.cpp
@@ -3336,16 +3336,14 @@ void Plater::priv::set_current_panel(wxPanel* panel)
 
 void Plater::priv::on_select_preset(wxCommandEvent &evt)
 {
-    auto preset_type = static_cast<Preset::Type>(evt.GetInt());
-    auto *combo = static_cast<PlaterPresetComboBox*>(evt.GetEventObject());
+    PlaterPresetComboBox* combo = static_cast<PlaterPresetComboBox*>(evt.GetEventObject());
+    Preset::Type preset_type    = combo->get_type();
 
     // see https://github.com/prusa3d/PrusaSlicer/issues/3889
     // Under OSX: in case of use of a same names written in different case (like "ENDER" and "Ender"),
     // m_presets_choice->GetSelection() will return first item, because search in PopupListCtrl is case-insensitive.
     // So, use GetSelection() from event parameter 
-    // But in this function we couldn't use evt.GetSelection(), because m_commandInt is used for preset_type
-    // Thus, get selection in this way:
-    int selection = combo->FindString(evt.GetString(), true);
+    int selection = evt.GetSelection();
 
     auto idx = combo->get_extruder_idx();
 
diff --git a/src/slic3r/GUI/PresetComboBoxes.cpp b/src/slic3r/GUI/PresetComboBoxes.cpp
index 0754df571..da723a36b 100644
--- a/src/slic3r/GUI/PresetComboBoxes.cpp
+++ b/src/slic3r/GUI/PresetComboBoxes.cpp
@@ -121,23 +121,26 @@ PresetComboBox::PresetComboBox(wxWindow* parent, Preset::Type preset_type, const
     Bind(wxEVT_COMBOBOX_DROPDOWN, [this](wxCommandEvent&) { m_suppress_change = false; });
     Bind(wxEVT_COMBOBOX_CLOSEUP,  [this](wxCommandEvent&) { m_suppress_change = true;  });
 
-    Bind(wxEVT_COMBOBOX, [this](wxCommandEvent& evt) {
-        // see https://github.com/prusa3d/PrusaSlicer/issues/3889
-        // Under OSX: in case of use of a same names written in different case (like "ENDER" and "Ender")
-        // m_presets_choice->GetSelection() will return first item, because search in PopupListCtrl is case-insensitive.
-        // So, use GetSelection() from event parameter 
-        auto selected_item = evt.GetSelection();
+    Bind(wxEVT_COMBOBOX, &PresetComboBox::OnSelect, this);
+}
 
-        auto marker = reinterpret_cast<Marker>(this->GetClientData(selected_item));
-        if (marker >= LABEL_ITEM_DISABLED && marker < LABEL_ITEM_MAX)
-            this->SetSelection(this->m_last_selected);
-        else if (on_selection_changed && (m_last_selected != selected_item || m_collection->current_is_dirty())) {
-            m_last_selected = selected_item;
-            on_selection_changed(selected_item);
-            evt.StopPropagation();
-        }
-        evt.Skip();
-    });
+void PresetComboBox::OnSelect(wxCommandEvent& evt)
+{
+    // see https://github.com/prusa3d/PrusaSlicer/issues/3889
+    // Under OSX: in case of use of a same names written in different case (like "ENDER" and "Ender")
+    // m_presets_choice->GetSelection() will return first item, because search in PopupListCtrl is case-insensitive.
+    // So, use GetSelection() from event parameter 
+    auto selected_item = evt.GetSelection();
+
+    auto marker = reinterpret_cast<Marker>(this->GetClientData(selected_item));
+    if (marker >= LABEL_ITEM_DISABLED && marker < LABEL_ITEM_MAX)
+        this->SetSelection(this->m_last_selected);
+    else if (on_selection_changed && (m_last_selected != selected_item || m_collection->current_is_dirty())) {
+        m_last_selected = selected_item;
+        on_selection_changed(selected_item);
+        evt.StopPropagation();
+    }
+    evt.Skip();
 }
 
 PresetComboBox::~PresetComboBox()
@@ -602,34 +605,6 @@ void PresetComboBox::OnDrawItem(wxDC& dc,
 PlaterPresetComboBox::PlaterPresetComboBox(wxWindow *parent, Preset::Type preset_type) :
     PresetComboBox(parent, preset_type, wxSize(15 * wxGetApp().em_unit(), -1))
 {
-    Bind(wxEVT_COMBOBOX, [this](wxCommandEvent &evt) {
-        auto selected_item = evt.GetSelection();
-
-        auto marker = reinterpret_cast<Marker>(this->GetClientData(selected_item));
-        if (marker >= LABEL_ITEM_MARKER && marker < LABEL_ITEM_MAX) {
-            this->SetSelection(this->m_last_selected);
-            evt.StopPropagation();
-            if (marker == LABEL_ITEM_WIZARD_PRINTERS)
-                show_add_menu();
-            else
-            {
-                ConfigWizard::StartPage sp = ConfigWizard::SP_WELCOME;
-                switch (marker) {
-                case LABEL_ITEM_WIZARD_FILAMENTS: sp = ConfigWizard::SP_FILAMENTS; break;
-                case LABEL_ITEM_WIZARD_MATERIALS: sp = ConfigWizard::SP_MATERIALS; break;
-                default: break;
-                }
-                wxTheApp->CallAfter([sp]() { wxGetApp().run_wizard(ConfigWizard::RR_USER, sp); });
-            }
-        } else if (marker == LABEL_ITEM_PHYSICAL_PRINTER || this->m_last_selected != selected_item || m_collection->current_is_dirty() ) {
-            this->m_last_selected = selected_item;
-            evt.SetInt(this->m_type);
-            evt.Skip();
-        } else {
-            evt.StopPropagation();
-        }
-    });
-
     if (m_type == Preset::TYPE_FILAMENT)
     {
         Bind(wxEVT_LEFT_DOWN, [this](wxMouseEvent &event) {
@@ -717,6 +692,35 @@ PlaterPresetComboBox::~PlaterPresetComboBox()
         edit_btn->Destroy();
 }
 
+void PlaterPresetComboBox::OnSelect(wxCommandEvent &evt)
+{
+    auto selected_item = evt.GetSelection();
+
+    auto marker = reinterpret_cast<Marker>(this->GetClientData(selected_item));
+    if (marker >= LABEL_ITEM_MARKER && marker < LABEL_ITEM_MAX) {
+        this->SetSelection(this->m_last_selected);
+        evt.StopPropagation();
+        if (marker == LABEL_ITEM_MARKER)
+            return;
+        if (marker == LABEL_ITEM_WIZARD_PRINTERS)
+            show_add_menu();
+        else {
+            ConfigWizard::StartPage sp = ConfigWizard::SP_WELCOME;
+            switch (marker) {
+            case LABEL_ITEM_WIZARD_FILAMENTS: sp = ConfigWizard::SP_FILAMENTS; break;
+            case LABEL_ITEM_WIZARD_MATERIALS: sp = ConfigWizard::SP_MATERIALS; break;
+            default: break;
+            }
+            wxTheApp->CallAfter([sp]() { wxGetApp().run_wizard(ConfigWizard::RR_USER, sp); });
+        }
+        return;
+    }
+    else if (marker == LABEL_ITEM_PHYSICAL_PRINTER || this->m_last_selected != selected_item || m_collection->current_is_dirty())
+        this->m_last_selected = selected_item;
+        
+    evt.Skip();
+}
+
 bool PlaterPresetComboBox::switch_to_tab()
 {
     Tab* tab = wxGetApp().get_tab(m_type);
@@ -957,40 +961,42 @@ void PlaterPresetComboBox::msw_rescale()
 TabPresetComboBox::TabPresetComboBox(wxWindow* parent, Preset::Type preset_type) :
     PresetComboBox(parent, preset_type, wxSize(35 * wxGetApp().em_unit(), -1))
 {
-    Bind(wxEVT_COMBOBOX, [this](wxCommandEvent& evt) {
-        // see https://github.com/prusa3d/PrusaSlicer/issues/3889
-        // Under OSX: in case of use of a same names written in different case (like "ENDER" and "Ender")
-        // m_presets_choice->GetSelection() will return first item, because search in PopupListCtrl is case-insensitive.
-        // So, use GetSelection() from event parameter 
-        auto selected_item = evt.GetSelection();
+}
 
-        auto marker = reinterpret_cast<Marker>(this->GetClientData(selected_item));
-        if (marker >= LABEL_ITEM_DISABLED && marker < LABEL_ITEM_MAX) {
-            this->SetSelection(this->m_last_selected);
-            if (marker == LABEL_ITEM_WIZARD_PRINTERS)
-                wxTheApp->CallAfter([this]() {
-                wxGetApp().run_wizard(ConfigWizard::RR_USER, ConfigWizard::SP_PRINTERS);
+void TabPresetComboBox::OnSelect(wxCommandEvent &evt)
+{
+    // see https://github.com/prusa3d/PrusaSlicer/issues/3889
+    // Under OSX: in case of use of a same names written in different case (like "ENDER" and "Ender")
+    // m_presets_choice->GetSelection() will return first item, because search in PopupListCtrl is case-insensitive.
+    // So, use GetSelection() from event parameter 
+    auto selected_item = evt.GetSelection();
 
-                // update combobox if its parent is a PhysicalPrinterDialog
-                PhysicalPrinterDialog* parent = dynamic_cast<PhysicalPrinterDialog*>(this->GetParent());
-                if (parent != nullptr)
-                    update();
-            });
-        }
-        else if (on_selection_changed && (m_last_selected != selected_item || m_collection->current_is_dirty()) ) {
-            m_last_selected = selected_item;
-            on_selection_changed(selected_item);
-        }
+    auto marker = reinterpret_cast<Marker>(this->GetClientData(selected_item));
+    if (marker >= LABEL_ITEM_DISABLED && marker < LABEL_ITEM_MAX) {
+        this->SetSelection(this->m_last_selected);
+        if (marker == LABEL_ITEM_WIZARD_PRINTERS)
+            wxTheApp->CallAfter([this]() {
+            wxGetApp().run_wizard(ConfigWizard::RR_USER, ConfigWizard::SP_PRINTERS);
 
-        evt.StopPropagation();
+            // update combobox if its parent is a PhysicalPrinterDialog
+            PhysicalPrinterDialog* parent = dynamic_cast<PhysicalPrinterDialog*>(this->GetParent());
+            if (parent != nullptr)
+                update();
+        });
+    }
+    else if (on_selection_changed && (m_last_selected != selected_item || m_collection->current_is_dirty())) {
+        m_last_selected = selected_item;
+        on_selection_changed(selected_item);
+    }
+
+    evt.StopPropagation();
 #ifdef __WXMSW__
-        // From the Win 2004 preset combobox lose a focus after change the preset selection
-        // and that is why the up/down arrow doesn't work properly
-        // (see https://github.com/prusa3d/PrusaSlicer/issues/5531 ).
-        // So, set the focus to the combobox explicitly
-        this->SetFocus();
-#endif    
-    });
+    // From the Win 2004 preset combobox lose a focus after change the preset selection
+    // and that is why the up/down arrow doesn't work properly
+    // (see https://github.com/prusa3d/PrusaSlicer/issues/5531 ).
+    // So, set the focus to the combobox explicitly
+    this->SetFocus();
+#endif
 }
 
 wxString TabPresetComboBox::get_preset_name(const Preset& preset)
diff --git a/src/slic3r/GUI/PresetComboBoxes.hpp b/src/slic3r/GUI/PresetComboBoxes.hpp
index 6f41c95f4..efcbec370 100644
--- a/src/slic3r/GUI/PresetComboBoxes.hpp
+++ b/src/slic3r/GUI/PresetComboBoxes.hpp
@@ -71,6 +71,7 @@ public:
     void             show_all(bool show_all);
     virtual void update();
     virtual void msw_rescale();
+    virtual void OnSelect(wxCommandEvent& evt);
 
 protected:
     typedef std::size_t Marker;
@@ -167,6 +168,7 @@ public:
     wxString get_preset_name(const Preset& preset) override;
     void update() override;
     void msw_rescale() override;
+    void OnSelect(wxCommandEvent& evt) override;
 
 private:
     int     m_extruder_idx = -1;
@@ -193,6 +195,7 @@ public:
     void update() override;
     void update_dirty();
     void msw_rescale() override;
+    void OnSelect(wxCommandEvent& evt) override;
 
     void set_enable_all(bool enable=true) { m_enable_all = enable; }