From 6adebb9c78eaf32b44f83d09cda1562db7bee5fb Mon Sep 17 00:00:00 2001 From: bubnikv Date: Fri, 23 Aug 2019 15:53:45 +0200 Subject: [PATCH] When synchronizing the front end with the back end after Undo / Redo jump, postpone error messages, so they are displayed after the Undo / Redo jump has been fully performed. Otherwise there would be a message box opening, taking over the message queue, and possibly performing actions as rendering on an inconsistent application state. --- src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp | 4 +- src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.hpp | 2 +- src/slic3r/GUI/Gizmos/GLGizmosManager.cpp | 2 +- src/slic3r/GUI/Plater.cpp | 58 +++++++++++++------- src/slic3r/GUI/Plater.hpp | 2 +- 5 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp index 693c3c074d..28d569761b 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp @@ -1320,9 +1320,9 @@ bool GLGizmoSlaSupports::has_backend_supports() const return false; } -void GLGizmoSlaSupports::reslice_SLA_supports() const +void GLGizmoSlaSupports::reslice_SLA_supports(bool postpone_error_messages) const { - wxGetApp().CallAfter([this]() { wxGetApp().plater()->reslice_SLA_supports(*m_model_object); }); + wxGetApp().CallAfter([this, postpone_error_messages]() { wxGetApp().plater()->reslice_SLA_supports(*m_model_object, postpone_error_messages); }); } void GLGizmoSlaSupports::get_data_from_backend() diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.hpp index e331cef53e..feb2455cbc 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.hpp @@ -87,7 +87,7 @@ public: bool is_in_editing_mode() const { return m_editing_mode; } bool is_selection_rectangle_dragging() const { return m_selection_rectangle.is_dragging(); } bool has_backend_supports() const; - void reslice_SLA_supports() const; + void reslice_SLA_supports(bool postpone_error_messages = false) const; private: bool on_init(); diff --git a/src/slic3r/GUI/Gizmos/GLGizmosManager.cpp b/src/slic3r/GUI/Gizmos/GLGizmosManager.cpp index c21af45f0d..1528a4bcbd 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmosManager.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmosManager.cpp @@ -882,7 +882,7 @@ void GLGizmosManager::update_after_undo_redo(const UndoRedo::Snapshot& snapshot) m_serializing = false; if (m_current == SlaSupports && snapshot.snapshot_data.flags & UndoRedo::SnapshotData::RECALCULATE_SLA_SUPPORTS) - dynamic_cast(m_gizmos[SlaSupports])->reslice_SLA_supports(); + dynamic_cast(m_gizmos[SlaSupports])->reslice_SLA_supports(true); } void GLGizmosManager::reset() diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index f4cd7f0cc8..d03ac918a3 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -1457,7 +1457,7 @@ struct Plater::priv // Do a full refresh of scene tree, including regenerating // all the GLVolumes. FIXME The update function shall just // reload the modified matrices. - if (!was_canceled()) plater().update(true); + if (!was_canceled()) plater().update((unsigned int)UpdateParams::FORCE_FULL_SCREEN_REFRESH); } public: @@ -1676,7 +1676,7 @@ struct Plater::priv // Apply the arrange result to all selected objects for (ArrangePolygon &ap : m_selected) ap.apply(); - plater().update(false /*dont force_full_scene_refresh*/); + plater().update(); } }; @@ -1759,7 +1759,12 @@ struct Plater::priv priv(Plater *q, MainFrame *main_frame); ~priv(); - void update(bool force_full_scene_refresh = false, bool force_background_processing_update = false); + enum class UpdateParams { + FORCE_FULL_SCREEN_REFRESH = 1, + FORCE_BACKGROUND_PROCESSING_UPDATE = 2, + POSTPONE_VALIDATION_ERROR_MESSAGE = 4, + }; + void update(unsigned int flags = 0); void select_view(const std::string& direction); void select_view_3D(const std::string& name); void select_next_view_3D(); @@ -1837,11 +1842,18 @@ struct Plater::priv UPDATE_BACKGROUND_PROCESS_FORCE_EXPORT = 16, }; // returns bit mask of UpdateBackgroundProcessReturnState - unsigned int update_background_process(bool force_validation = false); + unsigned int update_background_process(bool force_validation = false, bool postpone_error_messages = false); // Restart background processing thread based on a bitmask of UpdateBackgroundProcessReturnState. bool restart_background_process(unsigned int state); // returns bit mask of UpdateBackgroundProcessReturnState unsigned int update_restart_background_process(bool force_scene_update, bool force_preview_update); + void show_delayed_error_message() { + if (!this->delayed_error_message.empty()) { + std::string msg = std::move(this->delayed_error_message); + this->delayed_error_message.clear(); + GUI::show_error(this->q, msg); + } + } void export_gcode(fs::path output_path, PrintHostJob upload_job); void reload_from_disk(); void fix_through_netfabb(const int obj_idx, const int vol_idx = -1); @@ -2082,7 +2094,7 @@ Plater::priv::~priv() delete config; } -void Plater::priv::update(bool force_full_scene_refresh, bool force_background_processing_update) +void Plater::priv::update(unsigned int flags) { // the following line, when enabled, causes flickering on NVIDIA graphics cards // wxWindowUpdateLocker freeze_guard(q); @@ -2095,11 +2107,11 @@ void Plater::priv::update(bool force_full_scene_refresh, bool force_background_p } unsigned int update_status = 0; - if (this->printer_technology == ptSLA || force_background_processing_update) + if (this->printer_technology == ptSLA || (flags & (unsigned int)UpdateParams::FORCE_BACKGROUND_PROCESSING_UPDATE)) // Update the SLAPrint from the current Model, so that the reload_scene() // pulls the correct data. - update_status = this->update_background_process(false); - this->view3D->reload_scene(false, force_full_scene_refresh); + update_status = this->update_background_process(false, flags & (unsigned int)UpdateParams::POSTPONE_VALIDATION_ERROR_MESSAGE); + this->view3D->reload_scene(false, flags & (unsigned int)UpdateParams::FORCE_FULL_SCREEN_REFRESH); this->preview->reload_print(); if (this->printer_technology == ptSLA) this->restart_background_process(update_status); @@ -2871,7 +2883,7 @@ void Plater::priv::update_print_volume_state() // Update background processing thread from the current config and Model. // Returns a bitmask of UpdateBackgroundProcessReturnState. -unsigned int Plater::priv::update_background_process(bool force_validation) +unsigned int Plater::priv::update_background_process(bool force_validation, bool postpone_error_messages) { // bitmap of enum UpdateBackgroundProcessReturnState unsigned int return_state = 0; @@ -2881,8 +2893,6 @@ unsigned int Plater::priv::update_background_process(bool force_validation) this->background_process_timer.Stop(); // Update the "out of print bed" state of ModelInstances. this->update_print_volume_state(); - // The delayed error message is no more valid. - this->delayed_error_message.clear(); // Apply new config to the possibly running background task. bool was_running = this->background_process.running(); Print::ApplyStatus invalidated = this->background_process.apply(this->q->model(), wxGetApp().preset_bundle->full_config()); @@ -2908,7 +2918,9 @@ unsigned int Plater::priv::update_background_process(bool force_validation) } if ((invalidated != Print::APPLY_STATUS_UNCHANGED || force_validation) && ! this->background_process.empty()) { - // The state of the Print changed, and it is non-zero. Let's validate it and give the user feedback on errors. + // The delayed error message is no more valid. + this->delayed_error_message.clear(); + // The state of the Print changed, and it is non-zero. Let's validate it and give the user feedback on errors. std::string err = this->background_process.validate(); if (err.empty()) { if (invalidated != Print::APPLY_STATUS_UNCHANGED && this->background_processing_enabled()) @@ -2920,7 +2932,7 @@ unsigned int Plater::priv::update_background_process(bool force_validation) while (p->GetParent()) p = p->GetParent(); auto *top_level_wnd = dynamic_cast(p); - if (top_level_wnd && top_level_wnd->IsActive()) { + if (! postpone_error_messages && top_level_wnd && top_level_wnd->IsActive()) { // The error returned from the Print needs to be translated into the local language. GUI::show_error(this->q, _(err)); } else { @@ -2929,6 +2941,9 @@ unsigned int Plater::priv::update_background_process(bool force_validation) } return_state |= UPDATE_BACKGROUND_PROCESS_INVALID; } + } else if (! this->delayed_error_message.empty()) { + // Reusing the old state. + return_state |= UPDATE_BACKGROUND_PROCESS_INVALID; } if (invalidated != Print::APPLY_STATUS_UNCHANGED && was_running && ! this->background_process.running() && @@ -3970,7 +3985,7 @@ void Plater::priv::update_after_undo_redo(const UndoRedo::Snapshot& snapshot, bo { this->view3D->get_canvas3d()->get_selection().clear(); // Update volumes from the deserializd model, always stop / update the background processing (for both the SLA and FFF technologies). - this->update(false, true); + this->update((unsigned int)UpdateParams::FORCE_BACKGROUND_PROCESSING_UPDATE | (unsigned int)UpdateParams::POSTPONE_VALIDATION_ERROR_MESSAGE); // Release old snapshots if the memory allocated is excessive. This may remove the top most snapshot if jumping to the very first snapshot. //if (temp_snapshot_was_taken) // Release the old snapshots always, as it may have happened, that some of the triangle meshes got deserialized from the snapshot, while some @@ -3989,6 +4004,11 @@ void Plater::priv::update_after_undo_redo(const UndoRedo::Snapshot& snapshot, bo view3D->set_as_dirty(); } + // this->update() above was called with POSTPONE_VALIDATION_ERROR_MESSAGE, so that if an error message was generated when updating the back end, it would not open immediately, + // but it would be saved to be show later. Let's do it now. We do not want to display the message box earlier, because on Windows & OSX the message box takes over the message + // queue pump, which in turn executes the rendering function before a full update after the Undo / Redo jump. + this->show_delayed_error_message(); + //FIXME what about the state of the manipulators? //FIXME what about the focus? Cursor in the side panel? @@ -4507,11 +4527,11 @@ void Plater::reslice() p->preview->update_view_type(); } -void Plater::reslice_SLA_supports(const ModelObject &object) +void Plater::reslice_SLA_supports(const ModelObject &object, bool postpone_error_messages) { //FIXME Don't reslice if export of G-code or sending to OctoPrint is running. // bitmask of UpdateBackgroundProcessReturnState - unsigned int state = this->p->update_background_process(true); + unsigned int state = this->p->update_background_process(true, postpone_error_messages); if (state & priv::UPDATE_BACKGROUND_PROCESS_REFRESH_SCENE) this->p->view3D->reload_scene(false); @@ -4735,11 +4755,7 @@ void Plater::on_activate() this->p->preview->get_wxglcanvas()->SetFocus(); #endif - if (! this->p->delayed_error_message.empty()) { - std::string msg = std::move(this->p->delayed_error_message); - this->p->delayed_error_message.clear(); - GUI::show_error(this, msg); - } + this->p->show_delayed_error_message(); } wxString Plater::get_project_filename(const wxString& extension) const diff --git a/src/slic3r/GUI/Plater.hpp b/src/slic3r/GUI/Plater.hpp index 0bd01835e7..c2d7545e9c 100644 --- a/src/slic3r/GUI/Plater.hpp +++ b/src/slic3r/GUI/Plater.hpp @@ -186,7 +186,7 @@ public: bool has_toolpaths_to_export() const; void export_toolpaths_to_obj() const; void reslice(); - void reslice_SLA_supports(const ModelObject &object); + void reslice_SLA_supports(const ModelObject &object, bool postpone_error_messages = false); void changed_object(int obj_idx); void changed_objects(const std::vector& object_idxs); void schedule_background_process(bool schedule = true);