Fix of several issues related to gizmos updating and undo/redo

Common gizmos data cannot be used in on_set_state method
Also prevented calling render on empty GLVertexArrays

src/slic3r/GUI/Gizmos/GLGizmoFdmSupports.cpp

@@ -54,18 +54,35 @@ bool GLGizmoFdmSupports::on_init()
     return true;
 }
 
+
+void GLGizmoFdmSupports::activate_internal_undo_redo_stack(bool activate)
+{
+    if (activate && ! m_internal_stack_active) {
+        Plater::TakeSnapshot(wxGetApp().plater(), _L("FDM gizmo turned on"));
+        wxGetApp().plater()->enter_gizmos_stack();
+        m_internal_stack_active = true;
+    }
+    if (! activate && m_internal_stack_active) {
+        wxGetApp().plater()->leave_gizmos_stack();
+        Plater::TakeSnapshot(wxGetApp().plater(), _L("FDM gizmo turned off"));
+        m_internal_stack_active = false;
+    }
+}
+
 void GLGizmoFdmSupports::set_fdm_support_data(ModelObject* model_object, const Selection& selection)
 {
-    const ModelObject* mo = m_c->selection_info() ? m_c->selection_info()->model_object() : nullptr;
+    if (m_state != On)
-    if (! mo)
         return;
 
+    const ModelObject* mo = m_c->selection_info() ? m_c->selection_info()->model_object() : nullptr;
+
     if (mo && selection.is_from_single_instance()
-     && (mo->id() != m_old_mo_id || mo->volumes.size() != m_old_volumes_size))
+     && (m_schedule_update || mo->id() != m_old_mo_id || mo->volumes.size() != m_old_volumes_size))
     {
         update_from_model_object();
         m_old_mo_id = mo->id();
         m_old_volumes_size = mo->volumes.size();
+        m_schedule_update = false;
     }
 }
 
@@ -131,8 +148,10 @@ void GLGizmoFdmSupports::render_triangles(const Selection& selection) const
         // Now render both enforcers and blockers.
         for (int i=0; i<2; ++i) {
             glsafe(::glColor4f(i ? 1.f : 0.2f, 0.2f, i ? 0.2f : 1.0f, 0.5f));
-            for (const GLIndexedVertexArray& iva : m_ivas[mesh_id][i])
+            for (const GLIndexedVertexArray& iva : m_ivas[mesh_id][i]) {
-                iva.render();
+                if (iva.has_VBOs())
+                    iva.render();
+            }
         }
         glsafe(::glPopMatrix());
         if (is_left_handed)
@@ -493,6 +512,7 @@ bool GLGizmoFdmSupports::gizmo_event(SLAGizmoEventType action, const Vec2d& mous
                 : (m_button_down == Button::Left
                    ? _L("Add supports")
                    : _L("Block supports"));
+        activate_internal_undo_redo_stack(true);
         Plater::TakeSnapshot(wxGetApp().plater(), action_name);
         update_model_object();
 
@@ -588,6 +608,8 @@ void GLGizmoFdmSupports::select_facets_by_angle(float threshold_deg, bool overwr
         update_vertex_buffers(&mv->mesh(), mesh_id, FacetSupportType::BLOCKER);
     }
 
+    activate_internal_undo_redo_stack(true);
+
     Plater::TakeSnapshot(wxGetApp().plater(), block ? _L("Block supports by angle")
                                                     : _L("Add supports by angle"));
     update_model_object();
@@ -778,12 +800,9 @@ void GLGizmoFdmSupports::on_set_state()
         return;
 
     if (m_state == On && m_old_state != On) { // the gizmo was just turned on
-        {
-            Plater::TakeSnapshot snapshot(wxGetApp().plater(), _(L("FDM gizmo turned on")));
-        }
         if (! m_parent.get_gizmos_manager().is_serializing()) {
-            wxGetApp().CallAfter([]() {
+            wxGetApp().CallAfter([this]() {
-                wxGetApp().plater()->enter_gizmos_stack();
+                activate_internal_undo_redo_stack(true);
             });
         }
     }
@@ -793,11 +812,7 @@ void GLGizmoFdmSupports::on_set_state()
             m_setting_angle = false;
             m_parent.use_slope(false);
         }
-
+        activate_internal_undo_redo_stack(false);
-        wxGetApp().plater()->leave_gizmos_stack();
-        {
-            Plater::TakeSnapshot snapshot(wxGetApp().plater(), _(L("FDM gizmo turned off")));
-        }
         m_old_mo_id = -1;
         m_ivas.clear();
         m_selected_facets.clear();
@@ -820,14 +835,19 @@ void GLGizmoFdmSupports::on_stop_dragging()
 
 
 
-void GLGizmoFdmSupports::on_load(cereal::BinaryInputArchive& ar)
+void GLGizmoFdmSupports::on_load(cereal::BinaryInputArchive&)
 {
-    update_from_model_object();
+    // We should update the gizmo from current ModelObject, but it is not
+    // possible at this point. That would require having updated selection and
+    // common gizmos data, which is not done at this point. Instead, save
+    // a flag to do the update in set_fdm_support_data, which will be called
+    // soon after.
+    m_schedule_update = true;
 }
 
 
 
-void GLGizmoFdmSupports::on_save(cereal::BinaryOutputArchive& ar) const
+void GLGizmoFdmSupports::on_save(cereal::BinaryOutputArchive&) const
 {
 
 }

src/slic3r/GUI/Gizmos/GLGizmoFdmSupports.hpp

@@ -64,6 +64,7 @@ private:
 
     void update_model_object() const;
     void update_from_model_object();
+    void activate_internal_undo_redo_stack(bool activate);
 
     void select_facets_by_angle(float threshold, bool overwrite, bool block);
     bool m_overwrite_selected = false;
@@ -74,6 +75,8 @@ private:
     float m_clipping_plane_distance = 0.f;
     std::unique_ptr<ClippingPlane> m_clipping_plane;
     bool m_setting_angle = false;
+    bool m_internal_stack_active = false;
+    bool m_schedule_update = false;
 
     // This map holds all translated description texts, so they can be easily referenced during layout calculations
     // etc. When language changes, GUI is recreated and this class constructed again, so the change takes effect.

src/slic3r/GUI/Gizmos/GLGizmoFlatten.cpp

@@ -28,8 +28,7 @@ bool GLGizmoFlatten::on_init()
 
 void GLGizmoFlatten::on_set_state()
 {
-    if (m_state == On && is_plane_update_necessary())
+
-        update_planes();
 }
 
 CommonGizmosDataID GLGizmoFlatten::on_get_requirements() const
@@ -81,7 +80,8 @@ void GLGizmoFlatten::on_render() const
             else
                 glsafe(::glColor4f(0.9f, 0.9f, 0.9f, 0.5f));
 
-            m_planes[i].vbo.render();
+            if (m_planes[i].vbo.has_VBOs())
+                m_planes[i].vbo.render();
         }
         glsafe(::glPopMatrix());
     }

src/slic3r/GUI/Gizmos/GLGizmoHollow.cpp

@@ -59,7 +59,7 @@ void GLGizmoHollow::set_sla_support_data(ModelObject*, const Selection&)
         return;
 
     const ModelObject* mo = m_c->selection_info()->model_object();
-    if (mo) {
+    if (m_state == On && mo) {
         if (m_old_mo_id != mo->id()) {
             reload_cache();
             m_old_mo_id = mo->id();
@@ -810,11 +810,6 @@ void GLGizmoHollow::on_set_state()
     if (m_state == m_old_state)
         return;
 
-    if (m_state == On && m_old_state != On) { // the gizmo was just turned on
-        // we'll now reload support points:
-        if (m_c->selection_info()->model_object())
-            reload_cache();
-    }
     if (m_state == Off && m_old_state != Off) // the gizmo was just turned Off
         m_parent.post_event(SimpleEvent(EVT_GLCANVAS_FORCE_UPDATE));
     m_old_state = m_state;

src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp

@@ -67,10 +67,11 @@ void GLGizmoSlaSupports::set_sla_support_data(ModelObject* model_object, const S
 
     ModelObject* mo = m_c->selection_info()->model_object();
 
-    if (mo && mo->id() != m_old_mo_id) {
+    if (m_state == On && mo && mo->id() != m_old_mo_id) {
         disable_editing_mode();
         reload_cache();
         m_old_mo_id = mo->id();
+        m_c->instances_hider()->show_supports(true);
     }
 
     // If we triggered autogeneration before, check backend and fetch results if they are there
@@ -884,25 +885,23 @@ CommonGizmosDataID GLGizmoSlaSupports::on_get_requirements() const
 
 void GLGizmoSlaSupports::on_set_state()
 {
-    const ModelObject* mo = m_c->selection_info()->model_object();
-
     if (m_state == m_old_state)
         return;
 
     if (m_state == On && m_old_state != On) { // the gizmo was just turned on
-        Plater::TakeSnapshot snapshot(wxGetApp().plater(), _(L("SLA gizmo turned on")));
+        // This function can be called from undo/redo, when selection (and hence
+        // common gizmos data are not yet deserialized. The CallAfter should put
+        // this off until after the update is done.
+        wxGetApp().CallAfter([this]() {
+            Plater::TakeSnapshot snapshot(wxGetApp().plater(), _(L("SLA gizmo turned on")));
 
-        // we'll now reload support points:
+            // Set default head diameter from config.
-        if (mo)
+            const DynamicPrintConfig& cfg = wxGetApp().preset_bundle->sla_prints.get_edited_preset().config;
-            reload_cache();
+            m_new_point_head_diameter = static_cast<const ConfigOptionFloat*>(cfg.option("support_head_front_diameter"))->value;
-
+        });
-        // Set default head diameter from config.
-        const DynamicPrintConfig& cfg = wxGetApp().preset_bundle->sla_prints.get_edited_preset().config;
-        m_new_point_head_diameter = static_cast<const ConfigOptionFloat*>(cfg.option("support_head_front_diameter"))->value;
-        m_c->instances_hider()->show_supports(true);
     }
     if (m_state == Off && m_old_state != Off) { // the gizmo was just turned Off
-        bool will_ask = mo && m_editing_mode && unsaved_changes();
+        bool will_ask = m_editing_mode && unsaved_changes();
         if (will_ask) {
             wxGetApp().CallAfter([this]() {
                 // Following is called through CallAfter, because otherwise there was a problem
@@ -922,7 +921,7 @@ void GLGizmoSlaSupports::on_set_state()
             disable_editing_mode(); // so it is not active next time the gizmo opens
             Plater::TakeSnapshot snapshot(wxGetApp().plater(), _(L("SLA gizmo turned off")));
             m_normal_cache.clear();
-
+            m_old_mo_id = -1;
         }
     }
     m_old_state = m_state;

src/slic3r/GUI/Gizmos/GLGizmosManager.cpp

@@ -1104,9 +1104,16 @@ void GLGizmosManager::activate_gizmo(EType type)
     }
 
     m_current = type;
-    m_common_gizmos_data->update(get_current()
+
-                           ? get_current()->get_requirements()
+    // Updating common data should be left to the update_data function, which
-                           : CommonGizmosDataID(0));
+    // is always called after this one. activate_gizmo can be called by undo/redo,
+    // when selection is not yet deserialized, so the common data would update
+    // incorrectly (or crash if relying on unempty selection). Undo/redo stack
+    // will also call update_data, after selection is restored.
+
+    //m_common_gizmos_data->update(get_current()
+    //                       ? get_current()->get_requirements()
+    //                       : CommonGizmosDataID(0));
 
     if (type != Undefined)
         m_gizmos[type]->set_state(GLGizmoBase::On);

src/slic3r/GUI/Gizmos/GLGizmosManager.hpp

@@ -141,11 +141,6 @@ public:
         EType new_current = m_current;
         m_current = old_current;
 
-        // Update common data. They should be updated when activate_gizmo is
-        // called, so it can be used in on_set_state which is called from there.
-        if (new_current != Undefined)
-            m_common_gizmos_data->update(m_gizmos[new_current]->get_requirements());
-
         // activate_gizmo call sets m_current and calls set_state for the gizmo
         // it does nothing in case the gizmo is already activated
         // it can safely be called for Undefined gizmo

src/slic3r/GUI/MeshUtils.cpp

@@ -49,7 +49,8 @@ void MeshClipper::render_cut()
     if (! m_triangles_valid)
         recalculate_triangles();
 
-    m_vertex_array.render();
+    if (m_vertex_array.has_VBOs())
+        m_vertex_array.render();
 }