From 18c95c404c6c5d596c9eca5b99efb37d171ba4a4 Mon Sep 17 00:00:00 2001
From: Vojtech Kral <vojtech@kral.hk>
Date: Wed, 28 Nov 2018 19:26:11 +0100
Subject: [PATCH] Fix wxNotebook event incontinence

---
 src/slic3r/GUI/GUI_Utils.hpp | 89 ++++++++++++++++++++++++++----------
 src/slic3r/GUI/Plater.cpp    | 10 ++--
 2 files changed, 72 insertions(+), 27 deletions(-)

diff --git a/src/slic3r/GUI/GUI_Utils.hpp b/src/slic3r/GUI/GUI_Utils.hpp
index 9cee986b0c..256d472538 100644
--- a/src/slic3r/GUI/GUI_Utils.hpp
+++ b/src/slic3r/GUI/GUI_Utils.hpp
@@ -1,7 +1,7 @@
 #ifndef slic3r_GUI_Utils_hpp_
 #define slic3r_GUI_Utils_hpp_
 
-#include <functional>
+#include <memory>
 #include <string>
 
 #include <boost/optional.hpp>
@@ -10,6 +10,7 @@
 #include <wx/filedlg.h>
 #include <wx/gdicmn.h>
 #include <wx/panel.h>
+#include <wx/debug.h>
 
 class wxCheckBox;
 class wxTopLevelWindow;
@@ -25,40 +26,80 @@ wxTopLevelWindow* find_toplevel_parent(wxWindow *window);
 
 class EventGuard
 {
+    // This is a RAII-style smart-ptr-like guard that will bind any event to any event handler
+    // and unbind it as soon as it goes out of scope or unbind() is called.
+    // This can be used to solve the annoying problem of wx events being delivered to freed objects.
+
+private:
+    // This is a way to type-erase both the event type as well as the handler:
+
+    struct EventStorageBase {
+        virtual ~EventStorageBase() {}
+    };
+
+    template<class EvTag, class Fun>
+    struct EventStorageFun : EventStorageBase {
+        wxEvtHandler *emitter;
+        EvTag tag;
+        Fun fun;
+
+        EventStorageFun(wxEvtHandler *emitter, const EvTag &tag, Fun fun)
+            : emitter(emitter)
+            , tag(tag)
+            , fun(std::move(fun))
+        {
+            emitter->Bind(this->tag, this->fun);
+        }
+
+        virtual ~EventStorageFun() { emitter->Unbind(tag, fun); }
+    };
+
+    template<typename EvTag, typename Class, typename EvArg, typename EvHandler>
+    struct EventStorageMethod : EventStorageBase {
+        typedef void(Class::* MethodPtr)(EvArg &);
+
+        wxEvtHandler *emitter;
+        EvTag tag;
+        MethodPtr method;
+        EvHandler *handler;
+
+        EventStorageMethod(wxEvtHandler *emitter, const EvTag &tag, MethodPtr method, EvHandler *handler)
+            : emitter(emitter)
+            , tag(tag)
+            , method(method)
+            , handler(handler)
+        {
+            emitter->Bind(tag, method, handler);
+        }
+
+        virtual ~EventStorageMethod() { emitter->Unbind(tag, method, handler); }
+    };
+
+    std::unique_ptr<EventStorageBase> event_storage;
 public:
     EventGuard() {}
     EventGuard(const EventGuard&) = delete;
-    EventGuard(EventGuard &&other) : unbinder(std::move(other.unbinder)) {}
+    EventGuard(EventGuard &&other) : event_storage(std::move(other.event_storage)) {}
 
-    ~EventGuard() {
-        if (unbinder) {
-            unbinder(false);
-        }
-    }
+    template<class EvTag, class Fun>
+    EventGuard(wxEvtHandler *emitter, const EvTag &tag, Fun fun)
+        :event_storage(new EventStorageFun<EvTag, Fun>(emitter, tag, std::move(fun)))
+    {}
 
-    template<class EvTag, class Fun> void bind(wxEvtHandler *emitter, const EvTag &type, Fun fun)
-    {
-        // This is a way to type-erase both the event type as well as the handler:
-
-        unbinder = std::move([=](bool bind) {
-            if (bind) {
-                emitter->Bind(type, fun);
-            } else {
-                emitter->Unbind(type, fun);
-            }
-        });
-
-        unbinder(true);
-    }
+    template<typename EvTag, typename Class, typename EvArg, typename EvHandler>
+    EventGuard(wxEvtHandler *emitter, const EvTag &tag, void(Class::* method)(EvArg &), EvHandler *handler)
+        :event_storage(new EventStorageMethod<EvTag, Class, EvArg, EvHandler>(emitter, tag, method, handler))
+    {}
 
     EventGuard& operator=(const EventGuard&) = delete;
     EventGuard& operator=(EventGuard &&other)
     {
-        unbinder.swap(other.unbinder);
+        event_storage = std::move(other.event_storage);
         return *this;
     }
-private:
-    std::function<void(bool)> unbinder;
+
+    void unbind() { event_storage.reset(nullptr); }
+    explicit operator bool() const noexcept { return !!event_storage; }
 };
 
 
diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp
index 34b0aaeeb5..be4194768e 100644
--- a/src/slic3r/GUI/Plater.cpp
+++ b/src/slic3r/GUI/Plater.cpp
@@ -914,6 +914,10 @@ struct Plater::priv
 
     // GUI elements
     wxNotebook *notebook;
+    EventGuard guard_on_notebook_changed;
+    // Note: ^ The on_notebook_changed is guarded here because the wxNotebook d-tor tends to generate
+    // wxEVT_NOTEBOOK_PAGE_CHANGED events on some platforms, which causes them to be received by a freed Plater.
+    // EventGuard unbinds the handler in its d-tor.
     Sidebar *sidebar;
 #if !ENABLE_IMGUI
     wxPanel *panel3d;
@@ -1042,6 +1046,7 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame)
         "extruder_colour", "filament_colour", "max_print_height", "printer_model", "printer_technology"
         }))
     , notebook(new wxNotebook(q, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxNB_BOTTOM))
+    , guard_on_notebook_changed(notebook, wxEVT_NOTEBOOK_PAGE_CHANGED, &priv::on_notebook_changed, this)
     , sidebar(new Sidebar(q))
 #if ENABLE_IMGUI
     , canvas3Dwidget(GLCanvas3DManager::create_wxglcanvas(notebook))
@@ -1124,9 +1129,6 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame)
 
     // Events:
 
-    // Notebook page change event
-    notebook->Bind(wxEVT_NOTEBOOK_PAGE_CHANGED, &priv::on_notebook_changed, this);
-
     // Preset change event
     sidebar->Bind(wxEVT_COMBOBOX, &priv::on_select_preset, this);
 
@@ -1939,6 +1941,8 @@ void Plater::priv::fix_through_netfabb(const int obj_idx)
 
 void Plater::priv::on_notebook_changed(wxBookCtrlEvent&)
 {
+    wxCHECK_RET(canvas3D != nullptr, "on_notebook_changed on freed Plater");
+
     const auto current_id = notebook->GetCurrentPage()->GetId();
 #if ENABLE_IMGUI
     if (current_id == canvas3Dwidget->GetId()) {