Fix of a crash on OSX after changing a layer range value and thenvb_add_layer_range_osx

pressing the "add layer" button.

5 files changed, 131 insertions, 85 deletions

diff --git a/src/slic3r/GUI/GUI_ObjectLayers.cpp b/src/slic3r/GUI/GUI_ObjectLayers.cpp
index d209214ae..d62bb2727 100644
--- a/src/slic3r/GUI/GUI_ObjectLayers.cpp
+++ b/src/slic3r/GUI/GUI_ObjectLayers.cpp
@@ -58,7 +58,7 @@ void ObjectLayers::select_editor(LayerRangeEditor* editor, const bool is_last_ed
     }    
 }
 
-wxSizer* ObjectLayers::create_layer(const t_layer_height_range& range) 
+wxSizer* ObjectLayers::create_layer(const t_layer_height_range& range, PlusMinusButton *delete_button, PlusMinusButton *add_button) 
 {
     const bool is_last_edited_range = range == m_selectable_range;
 
@@ -79,8 +79,8 @@ wxSizer* ObjectLayers::create_layer(const t_layer_height_range& range)
 
     // Add control for the "Min Z"
 
-    auto editor = new LayerRangeEditor(this, double_to_string(range.first), etMinZ,
-                                       set_focus_data, [range, update_focus_data, this](coordf_t min_z, bool enter_pressed)
+    auto editor = new LayerRangeEditor(this, double_to_string(range.first), etMinZ, set_focus_data, 
+        [range, update_focus_data, this, delete_button, add_button](coordf_t min_z, bool enter_pressed, bool dont_update_ui) 
     {
         if (fabs(min_z - range.first) < EPSILON) {
             m_selection_type = etUndef;
@@ -89,10 +89,14 @@ wxSizer* ObjectLayers::create_layer(const t_layer_height_range& range)
 
         // data for next focusing
         coordf_t max_z = min_z < range.second ? range.second : min_z + 0.5;
-        const t_layer_height_range& new_range = { min_z, max_z };
+        const t_layer_height_range new_range = { min_z, max_z };
+        if (delete_button)
+            delete_button->range = new_range;
+        if (add_button)
+            add_button->range = new_range;
         update_focus_data(new_range, etMinZ, enter_pressed);
 
-        return wxGetApp().obj_list()->edit_layer_range(range, new_range);
+        return wxGetApp().obj_list()->edit_layer_range(range, new_range, dont_update_ui);
     });
 
     select_editor(editor, is_last_edited_range);
@@ -100,8 +104,8 @@ wxSizer* ObjectLayers::create_layer(const t_layer_height_range& range)
 
     // Add control for the "Max Z"
 
-    editor = new LayerRangeEditor(this, double_to_string(range.second), etMaxZ,
-                                  set_focus_data, [range, update_focus_data, this](coordf_t max_z, bool enter_pressed)
+    editor = new LayerRangeEditor(this, double_to_string(range.second), etMaxZ, set_focus_data, 
+        [range, update_focus_data, this, delete_button, add_button](coordf_t max_z, bool enter_pressed, bool dont_update_ui)
     {
         if (fabs(max_z - range.second) < EPSILON || range.first > max_z) {
             m_selection_type = etUndef;
@@ -110,9 +114,13 @@ wxSizer* ObjectLayers::create_layer(const t_layer_height_range& range)
 
         // data for next focusing
         const t_layer_height_range& new_range = { range.first, max_z };
+        if (delete_button)
+            delete_button->range = new_range;
+        if (add_button)
+            add_button->range = new_range;
         update_focus_data(new_range, etMaxZ, enter_pressed);
 
-        return wxGetApp().obj_list()->edit_layer_range(range, new_range);
+        return wxGetApp().obj_list()->edit_layer_range(range, new_range, dont_update_ui);
     });
 
     select_editor(editor, is_last_edited_range);
@@ -120,9 +128,8 @@ wxSizer* ObjectLayers::create_layer(const t_layer_height_range& range)
 
     // Add control for the "Layer height"
 
-    editor = new LayerRangeEditor(this,
-                                double_to_string(m_object->layer_config_ranges[range].option("layer_height")->getFloat()),
-                             etLayerHeight, set_focus_data, [range, this](coordf_t layer_height, bool)
+    editor = new LayerRangeEditor(this, double_to_string(m_object->layer_config_ranges[range].option("layer_height")->getFloat()), etLayerHeight, set_focus_data,
+        [range, this](coordf_t layer_height, bool, bool)
     {
         return wxGetApp().obj_list()->edit_layer_range(range, layer_height);
     });
@@ -141,30 +148,27 @@ wxSizer* ObjectLayers::create_layer(const t_layer_height_range& range)
 
     return sizer;
 }
-
+    
 void ObjectLayers::create_layers_list()
 {
     for (const auto layer : m_object->layer_config_ranges)
     {
         const t_layer_height_range& range = layer.first;
-        auto sizer = create_layer(range);
-
-        auto del_btn = new ScalableButton(m_parent, wxID_ANY, m_bmp_delete);
+        auto del_btn = new PlusMinusButton(m_parent, m_bmp_delete, range);
         del_btn->SetToolTip(_(L("Remove layer range")));
+        auto add_btn = new PlusMinusButton(m_parent, m_bmp_add, range);
+        add_btn->SetToolTip(_(L("Add layer range")));
 
+        auto sizer = create_layer(range, del_btn, add_btn);
         sizer->Add(del_btn, 0, wxRIGHT | wxLEFT, em_unit(m_parent));
+        sizer->Add(add_btn);
 
-        del_btn->Bind(wxEVT_BUTTON, [range](wxEvent &) {
-            wxGetApp().obj_list()->del_layer_range(range);
+        del_btn->Bind(wxEVT_BUTTON, [del_btn](wxEvent &) {
+            wxGetApp().obj_list()->del_layer_range(del_btn->range);
         });
 
-        auto add_btn = new ScalableButton(m_parent, wxID_ANY, m_bmp_add);
-        add_btn->SetToolTip(_(L("Add layer range")));
-
-        sizer->Add(add_btn);
-
-        add_btn->Bind(wxEVT_BUTTON, [range](wxEvent &) {
-            wxGetApp().obj_list()->add_layer_range_after_current(range);
+        add_btn->Bind(wxEVT_BUTTON, [add_btn](wxEvent &) {
+            wxGetApp().obj_list()->add_layer_range_after_current(add_btn->range);
         });
     }
 }
@@ -204,7 +208,7 @@ void ObjectLayers::update_layers_list()
     if (type & itLayerRoot)
         create_layers_list();
     else
-        create_layer(objects_ctrl->GetModel()->GetLayerRangeByItem(item));
+        create_layer(objects_ctrl->GetModel()->GetLayerRangeByItem(item), nullptr, nullptr);
 
     m_parent->Layout();
 }
@@ -255,7 +259,7 @@ void ObjectLayers::msw_rescale()
             {
                 wxSizerItem* b_item = item->GetSizer()->GetItem(btn);
                 if (b_item->IsWindow()) {
-                    ScalableButton* button = dynamic_cast<ScalableButton*>(b_item->GetWindow());
+                    auto button = dynamic_cast<PlusMinusButton*>(b_item->GetWindow());
                     if (button != nullptr)
                         button->msw_rescale();
                 }                
@@ -275,7 +279,7 @@ LayerRangeEditor::LayerRangeEditor( ObjectLayers* parent,
                                     const wxString& value,
                                     EditorType type,
                                     std::function<void(EditorType)> set_focus_data_fn,
-                                    std::function<bool(coordf_t, bool enter_pressed)>   edit_fn
+                                    std::function<bool(coordf_t, bool, bool)>   edit_fn
                                     ) :
     m_valid_value(value),
     m_type(type),
@@ -293,13 +297,13 @@ LayerRangeEditor::LayerRangeEditor( ObjectLayers* parent,
         m_enter_pressed     = true;
         // If LayersList wasn't updated/recreated, we can call wxEVT_KILL_FOCUS.Skip()
         if (m_type&etLayerHeight) {
-            if (!edit_fn(get_value(), true))
+            if (!edit_fn(get_value(), true, false))
                 SetValue(m_valid_value);
             else
                 m_valid_value = double_to_string(get_value());
             m_call_kill_focus = true;
         }
-        else if (!edit_fn(get_value(), true)) {
+        else if (!edit_fn(get_value(), true, false)) {
             SetValue(m_valid_value);
             m_call_kill_focus = true;
         }
@@ -319,13 +323,13 @@ LayerRangeEditor::LayerRangeEditor( ObjectLayers* parent,
 #endif // not __WXGTK__
             // If LayersList wasn't updated/recreated, we should call e.Skip()
             if (m_type & etLayerHeight) {
-                if (!edit_fn(get_value(), false))
+                if (!edit_fn(get_value(), false, dynamic_cast<ObjectLayers::PlusMinusButton*>(e.GetWindow()) != nullptr))
                     SetValue(m_valid_value);
                 else
                     m_valid_value = double_to_string(get_value());
                 e.Skip();
             }
-            else if (!edit_fn(get_value(), false)) {
+            else if (!edit_fn(get_value(), false, dynamic_cast<ObjectLayers::PlusMinusButton*>(e.GetWindow()) != nullptr)) {
                 SetValue(m_valid_value);
                 e.Skip();
             } 
@@ -387,4 +391,4 @@ void LayerRangeEditor::msw_rescale()
 }
 
 } //namespace GUI
-} //namespace Slic3r 
\ No newline at end of file
+} //namespace Slic3r 
diff --git a/src/slic3r/GUI/GUI_ObjectLayers.hpp b/src/slic3r/GUI/GUI_ObjectLayers.hpp
index c0de3be4c..08b594910 100644
--- a/src/slic3r/GUI/GUI_ObjectLayers.hpp
+++ b/src/slic3r/GUI/GUI_ObjectLayers.hpp
@@ -43,7 +43,8 @@ public:
                         const wxString& value = wxEmptyString,
                         EditorType type = etUndef,
                         std::function<void(EditorType)>     set_focus_data_fn   = [](EditorType)      {;},
-                        std::function<bool(coordf_t, bool)> edit_fn             = [](coordf_t, bool) {return false; }
+                        // callback parameters: new value, from enter, dont't update panel UI (when called from edit field's kill focus handler for the PlusMinusButton)
+                        std::function<bool(coordf_t, bool, bool)> edit_fn       = [](coordf_t, bool, bool) {return false; }
                         );
     ~LayerRangeEditor() {}
 
@@ -69,8 +70,23 @@ public:
     ObjectLayers(wxWindow* parent);
     ~ObjectLayers() {}
 
+    
+    // Button remembers the layer height range, for which it has been created.
+    // The layer height range for this button is updated when the low or high boundary of the layer height range is updated
+    // by the respective text edit field, so that this button emits an action for an up to date layer height range value.
+    class PlusMinusButton : public ScalableButton
+    {
+    public:
+        PlusMinusButton(wxWindow *parent, const ScalableBitmap &bitmap, std::pair<coordf_t, coordf_t> range) : ScalableButton(parent, wxID_ANY, bitmap), range(range) {}
+        // updated when the text edit field loses focus for any PlusMinusButton.
+        std::pair<coordf_t, coordf_t> range;
+    };
+
     void        select_editor(LayerRangeEditor* editor, const bool is_last_edited_range);
-    wxSizer*    create_layer(const t_layer_height_range& range);    // without_buttons
+    // Create sizer with layer height range and layer height text edit fields, without buttons.
+    // If the delete and add buttons are provided, the respective text edit fields will modify the layer height ranges of thes buttons
+    // on value change, so that these buttons work with up to date values.
+    wxSizer*    create_layer(const t_layer_height_range& range, PlusMinusButton *delete_button, PlusMinusButton *add_button);
     void        create_layers_list();
     void        update_layers_list();
 
diff --git a/src/slic3r/GUI/GUI_ObjectList.cpp b/src/slic3r/GUI/GUI_ObjectList.cpp
index 961fc2077..4db7b06fe 100644
--- a/src/slic3r/GUI/GUI_ObjectList.cpp
+++ b/src/slic3r/GUI/GUI_ObjectList.cpp
@@ -2881,76 +2881,84 @@ static double get_max_layer_height(const int extruder_idx)
     return max_layer_height;
 }
 
-void ObjectList::add_layer_range_after_current(const t_layer_height_range& current_range)
+void ObjectList::add_layer_range_after_current(const t_layer_height_range current_range)
 {
     const int obj_idx = get_selected_obj_idx();
-    if (obj_idx < 0) return;
+    if (obj_idx < 0) 
+        // This should not happen.
+        return;
 
     const wxDataViewItem layers_item = GetSelection();
 
     t_layer_config_ranges& ranges = object(obj_idx)->layer_config_ranges;
+    auto it_range = ranges.find(current_range);
+    assert(it_range != ranges.end());
+    if (it_range == ranges.end())
+        // This shoudl not happen.
+        return;
 
-    const t_layer_height_range& last_range = (--ranges.end())->first;
-    
-    if (current_range == last_range)
+    auto it_next_range = it_range;
+    bool changed = false;
+    if (++ it_next_range == ranges.end())
     {
+        // Adding a new layer height range after the last one.
         take_snapshot(_(L("Add Height Range")));
+        changed = true;
 
-        const t_layer_height_range& new_range = { last_range.second, last_range.second + 2. };
+        const t_layer_height_range new_range = { current_range.second, current_range.second + 2. };
         ranges[new_range] = get_default_layer_config(obj_idx);
         add_layer_item(new_range, layers_item);
     }
-    else
+    else if (const std::pair<coordf_t, coordf_t> &next_range = it_next_range->first; current_range.second <= next_range.first)
     {
-        const t_layer_height_range& next_range = (++ranges.find(current_range))->first;
-
-        if (current_range.second > next_range.first)
-            return; // range division has no sense
-        
         const int layer_idx = m_objects_model->GetItemIdByLayerRange(obj_idx, next_range);
-        if (layer_idx < 0)
-            return;
-
-        if (current_range.second == next_range.first)
+        assert(layer_idx >= 0);
+        if (layer_idx >= 0) 
         {
-            const auto old_config = ranges.at(next_range);
-
-            const coordf_t delta = (next_range.second - next_range.first);
-            if (delta < get_min_layer_height(old_config.opt_int("extruder"))/*0.05f*/) // next range division has no sense 
-                return; 
-
-            const coordf_t midl_layer = next_range.first + 0.5 * delta;
-            
-            t_layer_height_range new_range = { midl_layer, next_range.second };
+            if (current_range.second == next_range.first)
+            {
+                // Splitting the currnet layer heigth range to two.
+                const auto old_config = ranges.at(next_range);
+                const coordf_t delta = (next_range.second - next_range.first);
+                if (delta >= get_min_layer_height(old_config.opt_int("extruder"))/*0.05f*/) {
+                    const coordf_t midl_layer = next_range.first + 0.5 * delta;
+                    t_layer_height_range new_range = { midl_layer, next_range.second };
 
-            Plater::TakeSnapshot snapshot(wxGetApp().plater(), _(L("Add Height Range")));
+                    Plater::TakeSnapshot snapshot(wxGetApp().plater(), _(L("Add Height Range")));
+                    changed = true;
 
-            // create new 2 layers instead of deleted one
+                    // create new 2 layers instead of deleted one
+                    // delete old layer
 
-            // delete old layer
+                    wxDataViewItem layer_item = m_objects_model->GetItemByLayerRange(obj_idx, next_range);
+                    del_subobject_item(layer_item);
 
-            wxDataViewItem layer_item = m_objects_model->GetItemByLayerRange(obj_idx, next_range);
-            del_subobject_item(layer_item);
+                    ranges[new_range] = old_config;
+                    add_layer_item(new_range, layers_item, layer_idx);
 
-            ranges[new_range] = old_config;
-            add_layer_item(new_range, layers_item, layer_idx);
+                    new_range = { current_range.second, midl_layer };
+                    ranges[new_range] = get_default_layer_config(obj_idx);
+                    add_layer_item(new_range, layers_item, layer_idx);
+                }
+            }
+            else
+            {
+                // Filling in a gap between the current and a new layer height range with a new one.
+                take_snapshot(_(L("Add Height Range")));
+                changed = true;
 
-            new_range = { current_range.second, midl_layer };
-            ranges[new_range] = get_default_layer_config(obj_idx);
-            add_layer_item(new_range, layers_item, layer_idx);
+                const t_layer_height_range new_range = { current_range.second, next_range.first };
+                ranges[new_range] = get_default_layer_config(obj_idx);
+                add_layer_item(new_range, layers_item, layer_idx);
+            }
         }
-        else
-        {
-            take_snapshot(_(L("Add Height Range")));
-
-            const t_layer_height_range new_range = { current_range.second, next_range.first };
-            ranges[new_range] = get_default_layer_config(obj_idx);
-            add_layer_item(new_range, layers_item, layer_idx);
-        }        
     }
 
-    changed_object(obj_idx);
+    if (changed)
+        changed_object(obj_idx);
 
+    // The layer range panel is updated even if this function does not change the layer ranges, as the panel update
+    // may have been postponed from the "kill focus" event of a text field, if the focus was lost for the "add layer" button.
     // select item to update layers sizer
     select_item(layers_item);
 }
@@ -2996,7 +3004,7 @@ bool ObjectList::edit_layer_range(const t_layer_height_range& range, coordf_t la
     return false;
 }
 
-bool ObjectList::edit_layer_range(const t_layer_height_range& range, const t_layer_height_range& new_range)
+bool ObjectList::edit_layer_range(const t_layer_height_range& range, const t_layer_height_range& new_range, bool dont_update_ui)
 {
     const int obj_idx = get_selected_obj_idx();
     if (obj_idx < 0) return false;
@@ -3012,17 +3020,21 @@ bool ObjectList::edit_layer_range(const t_layer_height_range& range, const t_lay
     ranges.erase(range);
     ranges[new_range] = config;
     changed_object(obj_idx);
-
+    
     wxDataViewItem root_item = m_objects_model->GetLayerRootItem(m_objects_model->GetItemById(obj_idx));
     // To avoid update selection after deleting of a selected item (under GTK)
     // set m_prevent_list_events to true
     m_prevent_list_events = true;
     m_objects_model->DeleteChildren(root_item);
 
-    if (root_item.IsOk())
+    if (root_item.IsOk()) {
         // create Layer item(s) according to the layer_config_ranges
         for (const auto& r : ranges)
             add_layer_item(r.first, root_item);
+    }
+
+    if (dont_update_ui)
+        return true;
 
     select_item(sel_type&itLayer ? m_objects_model->GetItemByLayerRange(obj_idx, new_range) : root_item);
     Expand(root_item);
diff --git a/src/slic3r/GUI/GUI_ObjectList.hpp b/src/slic3r/GUI/GUI_ObjectList.hpp
index edb7d800e..fa39c8b0c 100644
--- a/src/slic3r/GUI/GUI_ObjectList.hpp
+++ b/src/slic3r/GUI/GUI_ObjectList.hpp
@@ -320,13 +320,26 @@ public:
     // Remove objects/sub-object from the list
     void remove();
     void del_layer_range(const t_layer_height_range& range);
-    void add_layer_range_after_current(const t_layer_height_range& current_range);
+    // Add a new layer height after the current range if possible.
+    // The current range is shortened and the new range is entered after the shortened current range if it fits.
+    // If no range fits after the current range, then no range is inserted.
+    // The layer range panel is updated even if this function does not change the layer ranges, as the panel update
+    // may have been postponed from the "kill focus" event of a text field, if the focus was lost for the "add layer" button.
+    // Rather providing the range by a value than by a reference, so that the memory referenced cannot be invalidated.
+    void add_layer_range_after_current(const t_layer_height_range current_range);
     void add_layer_item (const t_layer_height_range& range, 
                          const wxDataViewItem layers_item, 
                          const int layer_idx = -1);
     bool edit_layer_range(const t_layer_height_range& range, coordf_t layer_height);
+    // This function may be called when a text field loses focus for a "add layer" or "remove layer" button.
+    // In that case we don't want to destroy the panel with that "add layer" or "remove layer" buttons, as some messages
+    // are already planned for them and destroying these widgets leads to crashes at least on OSX.
+    // In that case the "add layer" or "remove layer" button handlers are responsible for always rebuilding the panel
+    // even if the "add layer" or "remove layer" buttons did not update the layer spans or layer heights.
     bool edit_layer_range(const t_layer_height_range& range, 
-                          const t_layer_height_range& new_range);
+                          const t_layer_height_range& new_range,
+                          // Don't destroy the panel with the "add layer" or "remove layer" buttons.
+                          bool suppress_ui_update = false);
 
     void init_objects();
     bool multiple_selection() const ;
diff --git a/src/slic3r/GUI/Mouse3DHandlerMac.mm b/src/slic3r/GUI/Mouse3DHandlerMac.mm
index eaf580908..b2d07a4e0 100644
--- a/src/slic3r/GUI/Mouse3DHandlerMac.mm
+++ b/src/slic3r/GUI/Mouse3DHandlerMac.mm
@@ -152,8 +152,9 @@ static void DeviceAdded(uint32_t unused)
 static void DeviceRemoved(uint32_t unused)
 {
   BOOST_LOG_TRIVIAL(info) << "3dx device removed\n";
-  assert(m_connected);
-  assert(! m_device_str.empty());
+// not accessible in a free function
+//  assert(m_connected);
+//  assert(! m_device_str.empty());
   mouse_3d_controller->disconnected();
 }