From 71a0b626a9d15a61dbd9a87f25c28001cb631292 Mon Sep 17 00:00:00 2001
From: YuSanka <yusanka@gmail.com>
Date: Wed, 7 Dec 2022 16:52:52 +0100
Subject: [PATCH] Fixed memory leaks on switch between parameter pages

---
 src/slic3r/GUI/Field.cpp        |   9 +++
 src/slic3r/GUI/Field.hpp        |   2 +-
 src/slic3r/GUI/OptionsGroup.cpp | 130 ++++++++++++++++----------------
 src/slic3r/GUI/OptionsGroup.hpp |   1 +
 src/slic3r/GUI/Tab.cpp          |   2 +
 5 files changed, 78 insertions(+), 66 deletions(-)

diff --git a/src/slic3r/GUI/Field.cpp b/src/slic3r/GUI/Field.cpp
index d3fef44d3..40cd7f889 100644
--- a/src/slic3r/GUI/Field.cpp
+++ b/src/slic3r/GUI/Field.cpp
@@ -1416,6 +1416,15 @@ void ColourPicker::sys_color_changed()
 #endif
 }
 
+PointCtrl::~PointCtrl()
+{
+    if (sizer) {
+        sizer->Clear();
+        delete sizer;
+        sizer = nullptr;
+    }
+}
+
 void PointCtrl::BUILD()
 {
 	auto temp = new wxBoxSizer(wxHORIZONTAL);
diff --git a/src/slic3r/GUI/Field.hpp b/src/slic3r/GUI/Field.hpp
index 60a06427d..eaa4fe481 100644
--- a/src/slic3r/GUI/Field.hpp
+++ b/src/slic3r/GUI/Field.hpp
@@ -425,7 +425,7 @@ class PointCtrl : public Field {
 public:
 	PointCtrl(const ConfigOptionDef& opt, const t_config_option_key& id) : Field(opt, id) {}
 	PointCtrl(wxWindow* parent, const ConfigOptionDef& opt, const t_config_option_key& id) : Field(parent, opt, id) {}
-	~PointCtrl() {}
+	~PointCtrl();
 
 	wxSizer*		sizer{ nullptr };
 	wxTextCtrl*		x_textctrl{ nullptr };
diff --git a/src/slic3r/GUI/OptionsGroup.cpp b/src/slic3r/GUI/OptionsGroup.cpp
index cfa2ef15a..0534446f0 100644
--- a/src/slic3r/GUI/OptionsGroup.cpp
+++ b/src/slic3r/GUI/OptionsGroup.cpp
@@ -118,6 +118,24 @@ OptionsGroup::OptionsGroup(	wxWindow* _parent, const wxString& title,
 {
 }
 
+void Line::clear()
+{
+    if (near_label_widget_win)
+        near_label_widget_win = nullptr;
+
+    if (widget_sizer) {
+        widget_sizer->Clear(true);
+        delete widget_sizer;
+        widget_sizer = nullptr;
+    }
+
+    if (extra_widget_sizer) {
+        extra_widget_sizer->Clear(true);
+        delete extra_widget_sizer;
+        extra_widget_sizer = nullptr;
+    }
+}
+
 wxWindow* OptionsGroup::ctrl_parent() const
 {
 	return this->custom_ctrl && m_use_custom_ctrl_as_parent ? static_cast<wxWindow*>(this->custom_ctrl) : (this->stb ? static_cast<wxWindow*>(this->stb) : this->parent());
@@ -231,7 +249,7 @@ void OptionsGroup::activate_line(Line& line)
 		}
     }
 
-	auto option_set = line.get_options();
+    const std::vector<Option>& option_set = line.get_options();
 	bool is_legend_line = option_set.front().opt.gui_type == ConfigOptionDef::GUIType::legend;
 
     if (!custom_ctrl && m_use_custom_ctrl) {
@@ -263,15 +281,13 @@ void OptionsGroup::activate_line(Line& line)
 		return;
 	}
 
-    auto grid_sizer = m_grid_sizer;
-
     if (custom_ctrl)
         m_use_custom_ctrl_as_parent = true;
 
     // if we have an extra column, build it
     if (extra_column) {
-		m_extra_column_item_ptrs.push_back(extra_column(this->ctrl_parent(), line));
-		grid_sizer->Add(m_extra_column_item_ptrs.back(), 0, wxALIGN_CENTER_VERTICAL | wxRIGHT, 3);
+        m_extra_column_item_ptrs.push_back(extra_column(this->ctrl_parent(), line));
+        m_grid_sizer->Add(m_extra_column_item_ptrs.back(), 0, wxALIGN_CENTER_VERTICAL | wxRIGHT, 3);
 	}
 
 	// Build a label if we have it
@@ -298,12 +314,12 @@ void OptionsGroup::activate_line(Line& line)
                 label->Wrap(label_width * wxGetApp().em_unit()); // avoid a Linux/GTK bug
             }
             if (!line.near_label_widget)
-                grid_sizer->Add(label, 0, (staticbox ? 0 : wxALIGN_RIGHT | wxRIGHT) | wxALIGN_CENTER_VERTICAL, line.label.IsEmpty() ? 0 : 5);
+                m_grid_sizer->Add(label, 0, (staticbox ? 0 : wxALIGN_RIGHT | wxRIGHT) | wxALIGN_CENTER_VERTICAL, line.label.IsEmpty() ? 0 : 5);
             else if (!line.label.IsEmpty()) {
                 // If we're here, we have some widget near the label
                 // so we need a horizontal sizer to arrange these things
                 auto sizer = new wxBoxSizer(wxHORIZONTAL);
-                grid_sizer->Add(sizer, 0, wxEXPAND | (staticbox ? wxALL : wxBOTTOM | wxTOP | wxLEFT), staticbox ? 0 : 1);
+                m_grid_sizer->Add(sizer, 0, wxEXPAND | (staticbox ? wxALL : wxBOTTOM | wxTOP | wxLEFT), staticbox ? 0 : 1);
                 sizer->Add(label, 0, (staticbox ? 0 : wxALIGN_RIGHT | wxRIGHT) | wxALIGN_CENTER_VERTICAL, 5);
             }
             if (label != nullptr && line.label_tooltip != "")
@@ -317,15 +333,19 @@ void OptionsGroup::activate_line(Line& line)
         if (custom_ctrl)
             line.widget_sizer = wgt;
         else
-            grid_sizer->Add(wgt, 0, wxEXPAND | wxBOTTOM | wxTOP, (wxOSX || line.label.IsEmpty()) ? 0 : 5);
+            m_grid_sizer->Add(wgt, 0, wxEXPAND | wxBOTTOM | wxTOP, (wxOSX || line.label.IsEmpty()) ? 0 : 5);
 		return;
 	}
 
 	// If we're here, we have more than one option or a single option with sidetext
     // so we need a horizontal sizer to arrange these things
-	auto sizer = new wxBoxSizer(wxHORIZONTAL);
-    if (!custom_ctrl)
-        grid_sizer->Add(sizer, 0, wxEXPAND | (staticbox ? wxALL : wxBOTTOM | wxTOP | wxLEFT), staticbox ? 0 : 1);
+    wxBoxSizer* h_sizer{ nullptr };
+    if (!custom_ctrl) {
+        // but this sizer is currently used just for NON-custom_ctrl cases
+        h_sizer = new wxBoxSizer(wxHORIZONTAL);
+        m_grid_sizer->Add(h_sizer, 0, wxEXPAND | (staticbox ? wxALL : wxBOTTOM | wxTOP | wxLEFT), staticbox ? 0 : 1);
+    }
+
     // If we have a single option with no sidetext just add it directly to the grid sizer
     if (option_set.size() == 1 && option_set.front().opt.sidetext.size() == 0 &&
 		option_set.front().side_widget == nullptr && line.get_extra_widgets().size() == 0) {
@@ -334,42 +354,37 @@ void OptionsGroup::activate_line(Line& line)
 
         if (!custom_ctrl) {
             if (is_window_field(field))
-                sizer->Add(field->getWindow(), option.opt.full_width ? 1 : 0,
+                h_sizer->Add(field->getWindow(), option.opt.full_width ? 1 : 0,
                     wxBOTTOM | wxTOP | (option.opt.full_width ? int(wxEXPAND) : int(wxALIGN_CENTER_VERTICAL)), (wxOSX || !staticbox) ? 0 : 2);
             if (is_sizer_field(field))
-                sizer->Add(field->getSizer(), 1, (option.opt.full_width ? int(wxEXPAND) : int(wxALIGN_CENTER_VERTICAL)), 0);
-        } else
-            delete sizer;
+                h_sizer->Add(field->getSizer(), 1, (option.opt.full_width ? int(wxEXPAND) : int(wxALIGN_CENTER_VERTICAL)), 0);
+        }
         return;
 	}
 
-    bool sizer_is_used       = false;
-    bool is_multioption_line = option_set.size() > 1;
-    for (auto opt : option_set) {
-		ConfigOptionDef option = opt.opt;
-        wxSizer* sizer_tmp = sizer;
-		// add label if any
-		if ((is_multioption_line || line.label.IsEmpty()) && !option.label.empty() && !custom_ctrl) {
-//!			To correct translation by context have to use wxGETTEXT_IN_CONTEXT macro from wxWidget 3.1.1
-			wxString str_label = (option.label == L_CONTEXT("Top", "Layers") || option.label == L_CONTEXT("Bottom", "Layers")) ?
-				_CTX(option.label, "Layers") :
-				_(option.label);
-			label = new wxStaticText(this->ctrl_parent(), wxID_ANY, str_label + ": ", wxDefaultPosition, //wxDefaultSize);
-				wxSize(sublabel_width != -1 ? sublabel_width * wxGetApp().em_unit() : -1, -1), wxALIGN_RIGHT);
-			label->SetBackgroundStyle(wxBG_STYLE_PAINT);
-            label->SetFont(wxGetApp().normal_font());
-            sizer_tmp->Add(label, 0, wxALIGN_CENTER_VERTICAL, 0);
-            sizer_is_used = true;
-        }
+    for (const Option& opt : option_set) {
+        // add field
+        auto& field = build_field(opt);
 
-		// add field
-		const Option& opt_ref = opt;
-		auto& field = build_field(opt_ref);
         if (!custom_ctrl) {
+            ConfigOptionDef option = opt.opt;
+            // add label if any
+            if ((option_set.size() > 1 || line.label.IsEmpty()) && !option.label.empty()) {
+                // To correct translation by context have to use wxGETTEXT_IN_CONTEXT macro from wxWidget 3.1.1
+                wxString str_label = (option.label == L_CONTEXT("Top", "Layers") || option.label == L_CONTEXT("Bottom", "Layers")) ?
+                    _CTX(option.label, "Layers") :
+                    _(option.label);
+                label = new wxStaticText(this->ctrl_parent(), wxID_ANY, str_label + ": ", wxDefaultPosition, //wxDefaultSize);
+                    wxSize(sublabel_width != -1 ? sublabel_width * wxGetApp().em_unit() : -1, -1), wxALIGN_RIGHT);
+                label->SetBackgroundStyle(wxBG_STYLE_PAINT);
+                label->SetFont(wxGetApp().normal_font());
+                h_sizer->Add(label, 0, wxALIGN_CENTER_VERTICAL, 0);
+            }
+
             if (option_set.size() == 1 && option_set.front().opt.full_width)
             {
                 const auto v_sizer = new wxBoxSizer(wxVERTICAL);
-                sizer_tmp->Add(v_sizer, 1, wxEXPAND);
+                h_sizer->Add(v_sizer, 1, wxEXPAND);
                 is_sizer_field(field) ?
                     v_sizer->Add(field->getSizer(), 0, wxEXPAND) :
                     v_sizer->Add(field->getWindow(), 0, wxEXPAND);
@@ -377,8 +392,8 @@ void OptionsGroup::activate_line(Line& line)
             }
 
             is_sizer_field(field) ?
-                sizer_tmp->Add(field->getSizer(), 0, wxALIGN_CENTER_VERTICAL, 0) :
-                sizer_tmp->Add(field->getWindow(), 0, wxALIGN_CENTER_VERTICAL, 0);
+                h_sizer->Add(field->getSizer(), 0, wxALIGN_CENTER_VERTICAL, 0) :
+                h_sizer->Add(field->getWindow(), 0, wxALIGN_CENTER_VERTICAL, 0);
 
             // add sidetext if any
             if (!option.sidetext.empty() || sidetext_width > 0) {
@@ -386,38 +401,35 @@ void OptionsGroup::activate_line(Line& line)
                     wxSize(sidetext_width != -1 ? sidetext_width * wxGetApp().em_unit() : -1, -1), wxALIGN_LEFT);
                 sidetext->SetBackgroundStyle(wxBG_STYLE_PAINT);
                 sidetext->SetFont(wxGetApp().normal_font());
-                sizer_tmp->Add(sidetext, 0, wxLEFT | wxALIGN_CENTER_VERTICAL, 4);
+                h_sizer->Add(sidetext, 0, wxLEFT | wxALIGN_CENTER_VERTICAL, 4);
             }
 
             // add side widget if any
             if (opt.side_widget != nullptr) {
-                sizer_tmp->Add(opt.side_widget(this->ctrl_parent())/*!.target<wxWindow>()*/, 0, wxLEFT | wxALIGN_CENTER_VERTICAL, 1);    //! requires verification
+                h_sizer->Add(opt.side_widget(this->ctrl_parent())/*!.target<wxWindow>()*/, 0, wxLEFT | wxALIGN_CENTER_VERTICAL, 1);    //! requires verification
             }
 
             if (opt.opt_id != option_set.back().opt_id) //! istead of (opt != option_set.back())
-                sizer_tmp->AddSpacer(6);
+                h_sizer->AddSpacer(6);
         }
-	}
+    }
 
-	// add extra sizers if any
-	for (auto extra_widget : line.get_extra_widgets())
+    // add extra sizers if any
+    for (auto extra_widget : line.get_extra_widgets())
     {
         if (line.get_extra_widgets().size() == 1 && !staticbox)
         {
             // extra widget for non-staticbox option group (like for the frequently used parameters on the sidebar) should be wxALIGN_RIGHT
             const auto v_sizer = new wxBoxSizer(wxVERTICAL);
-            sizer->Add(v_sizer, option_set.size() == 1 ? 0 : 1, wxEXPAND);
+            h_sizer->Add(v_sizer, option_set.size() == 1 ? 0 : 1, wxEXPAND);
             v_sizer->Add(extra_widget(this->ctrl_parent()), 0, wxALIGN_RIGHT);
             return;
         }
 
         line.extra_widget_sizer = extra_widget(this->ctrl_parent());
         if (!custom_ctrl)
-            sizer->Add(line.extra_widget_sizer, 0, wxLEFT | wxALIGN_CENTER_VERTICAL, 4);        //! requires verification
-	}
-
-    if (custom_ctrl && !sizer_is_used)
-        delete sizer;
+            h_sizer->Add(line.extra_widget_sizer, 0, wxLEFT | wxALIGN_CENTER_VERTICAL, 4);        //! requires verification
+    }
 }
 
 // create all controls for the option group from the m_lines
@@ -484,20 +496,8 @@ void OptionsGroup::clear(bool destroy_custom_ctrl)
 	m_grid_sizer = nullptr;
 	sizer = nullptr;
 
-	for (Line& line : m_lines) {
-        if (line.near_label_widget_win)
-            line.near_label_widget_win = nullptr;
-
-        if (line.widget_sizer) {
-            line.widget_sizer->Clear(true);
-            line.widget_sizer = nullptr;
-        }
-
-        if (line.extra_widget_sizer) {
-            line.extra_widget_sizer->Clear(true);
-            line.extra_widget_sizer = nullptr;
-        }
-	}
+    for (Line& line : m_lines)
+        line.clear();
 
     if (custom_ctrl) {
         for (auto const &item : m_fields) {
diff --git a/src/slic3r/GUI/OptionsGroup.hpp b/src/slic3r/GUI/OptionsGroup.hpp
index f74da354d..88d0ff8bf 100644
--- a/src/slic3r/GUI/OptionsGroup.hpp
+++ b/src/slic3r/GUI/OptionsGroup.hpp
@@ -86,6 +86,7 @@ public:
 
 	bool is_separator() const { return m_is_separator; }
 	bool has_only_option(const std::string& opt_key) const { return m_options.size() == 1 && m_options[0].opt_id == opt_key; }
+	void clear();
 
     const std::vector<widget_t>&	get_extra_widgets() const {return m_extra_widgets;}
     const std::vector<Option>&		get_options() const { return m_options; }
diff --git a/src/slic3r/GUI/Tab.cpp b/src/slic3r/GUI/Tab.cpp
index d5b909f37..58678104b 100644
--- a/src/slic3r/GUI/Tab.cpp
+++ b/src/slic3r/GUI/Tab.cpp
@@ -1784,6 +1784,8 @@ void TabPrint::clear_pages()
     m_recommended_thin_wall_thickness_description_line = nullptr;
     m_top_bottom_shell_thickness_explanation = nullptr;
     m_post_process_explanation = nullptr;
+
+    m_del_all_substitutions_btn = nullptr;
 }
 
 bool Tab::validate_custom_gcode(const wxString& title, const std::string& gcode)