From 50ea144b84a6a20318d28557b93d210d949230dc Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Thu, 28 Oct 2021 00:45:14 +0200 Subject: [PATCH 01/11] Minor improvements: - const corectness - volatile -> std::atomic - GUI::format - encoding issues --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 38 +++++++++++------------ src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 10 +++--- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index 9c1c4789f2..aecf564188 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -74,14 +74,10 @@ void GLGizmoSimplify::add_simplify_suggestion_notification( if (big_ids.empty()) return; for (size_t object_id : big_ids) { - std::string t = _u8L( - "Processing model '@object_name' with more than 1M triangles " + std::string t = GUI::format(_u8L( + "Processing model '%1%' with more than 1M triangles " "could be slow. It is highly recommend to reduce " - "amount of triangles."); - t.replace(t.find("@object_name"), sizeof("@object_name") - 1, - objects[object_id]->name); - // std::stringstream text; - // text << t << "\n"; + "amount of triangles."), objects[object_id]->name); std::string hypertext = _u8L("Simplify model"); std::function open_simplify = @@ -277,7 +273,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi bool is_canceling = m_state == State::canceling; m_imgui->disabled_begin(is_canceling); - if (m_imgui->button(_u8L("Cancel"))) { + if (m_imgui->button(_L("Cancel"))) { if (m_state == State::settings) { if (m_original_its.has_value()) { set_its(*m_original_its); @@ -296,7 +292,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi bool is_processing = m_state != State::settings; m_imgui->disabled_begin(is_processing); - if (m_imgui->button(_u8L("Apply"))) { + if (m_imgui->button(_L("Apply"))) { if (!m_is_valid_result) { m_state = State::close_on_end; process(); @@ -315,8 +311,9 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::SameLine(m_gui_cfg->bottom_left_width); // draw progress bar char buf[32]; - sprintf(buf, L("Process %d / 100"), m_progress); - ImGui::ProgressBar(m_progress / 100., ImVec2(m_gui_cfg->input_width, 0.f), buf); + int progress = m_progress; + sprintf(buf, L("Process %d / 100"), progress); + ImGui::ProgressBar(progress / 100., ImVec2(m_gui_cfg->input_width, 0.f), buf); } m_imgui->end(); @@ -454,7 +451,7 @@ void GLGizmoSimplify::process() }); } -void GLGizmoSimplify::set_its(indexed_triangle_set &its) { +void GLGizmoSimplify::set_its(const indexed_triangle_set &its) { if (m_volume == nullptr) return; // could appear after process m_volume->set_mesh(its); m_volume->calculate_convex_hull(); @@ -543,12 +540,12 @@ void GLGizmoSimplify::set_center_position() { m_move_to_center = true; } -bool GLGizmoSimplify::exist_volume(ModelVolume *volume) { - auto objs = wxGetApp().plater()->model().objects; - for (const auto &obj : objs) { - const auto &vlms = obj->volumes; +bool GLGizmoSimplify::exist_volume(const ModelVolume *volume) { + for (const ModelObject* obj : wxGetApp().plater()->model().objects) { + const auto & vlms = obj->volumes; auto item = std::find(vlms.begin(), vlms.end(), volume); - if (item != vlms.end()) return true; + if (item != vlms.end()) + return true; } return false; } @@ -556,11 +553,12 @@ bool GLGizmoSimplify::exist_volume(ModelVolume *volume) { ModelVolume * GLGizmoSimplify::get_volume(const Selection &selection, Model &model) { const Selection::IndicesList& idxs = selection.get_volume_idxs(); - if (idxs.empty()) return nullptr; // only one selected volume - if (idxs.size() != 1) return nullptr; + if (idxs.size() != 1) + return nullptr; const GLVolume *selected_volume = selection.get_volume(*idxs.begin()); - if (selected_volume == nullptr) return nullptr; + if (selected_volume == nullptr) + return nullptr; const GLVolume::CompositeID &cid = selected_volume->composite_id; const ModelObjectPtrs& objs = model.objects; diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index b978e9356d..ad954e29bb 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -53,7 +53,7 @@ private: void close(); void live_preview(); void process(); - void set_its(indexed_triangle_set &its); + void set_its(const indexed_triangle_set &its); void create_gui_cfg(); void request_rerender(); @@ -63,21 +63,21 @@ private: static const ModelVolume *get_volume(const GLVolume::CompositeID &cid, const Model &model); // return false when volume was deleted - static bool exist_volume(ModelVolume *volume); + static bool exist_volume(const ModelVolume *volume); std::atomic_bool m_is_valid_result; // differ what to do in apply std::atomic_bool m_exist_preview; // set when process end bool m_move_to_center; // opening gizmo - volatile int m_progress; // percent of done work + std::atomic m_progress; // percent of done work ModelVolume *m_volume; // keep pointer to actual working volume size_t m_obj_index; std::optional m_original_its; bool m_show_wireframe; - volatile bool m_need_reload; // after simplify, glReload must be on main thread + std::atomic m_need_reload; // after simplify, glReload must be on main thread std::thread m_worker; // wait before process @@ -90,7 +90,7 @@ private: close_on_end, // simplify with close on end canceling // after button click, before canceled }; - volatile State m_state; + std::atomic m_state; struct Configuration { From ba56a79795e4d28ca88a9972f7f5f003b942ac8f Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Fri, 29 Oct 2021 10:22:41 +0200 Subject: [PATCH 02/11] Simplify gizmo now renders the volume by itself, it does not rely on the usual GLVolume rendering. GLCanvas3D::toggle_model_object_visibility was extended to hide a single volume. Rendering the model and wireframe uses the same vertex buffer, which is now used through GLModel class. GLGizmoRenderTransparent class should no longer be needed. GLCanvas3D::reload_scene calls replaced with request_rerender. --- src/slic3r/GUI/GLCanvas3D.cpp | 11 +- src/slic3r/GUI/GLCanvas3D.hpp | 2 +- src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp | 17 ++- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 112 +++++++------------ src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 18 ++- src/slic3r/GUI/Gizmos/GLGizmosManager.cpp | 2 +- 6 files changed, 63 insertions(+), 99 deletions(-) diff --git a/src/slic3r/GUI/GLCanvas3D.cpp b/src/slic3r/GUI/GLCanvas3D.cpp index 90e67cd8ac..3fde99119c 100644 --- a/src/slic3r/GUI/GLCanvas3D.cpp +++ b/src/slic3r/GUI/GLCanvas3D.cpp @@ -24,6 +24,7 @@ #include "slic3r/GUI/Plater.hpp" #include "slic3r/GUI/MainFrame.hpp" #include "slic3r/Utils/UndoRedo.hpp" +#include "slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp" #include "GUI_App.hpp" #include "GUI_ObjectList.hpp" @@ -1143,7 +1144,7 @@ void GLCanvas3D::toggle_sla_auxiliaries_visibility(bool visible, const ModelObje } } -void GLCanvas3D::toggle_model_objects_visibility(bool visible, const ModelObject* mo, int instance_idx) +void GLCanvas3D::toggle_model_objects_visibility(bool visible, const ModelObject* mo, int instance_idx, const ModelVolume* mv) { for (GLVolume* vol : m_volumes.volumes) { if (vol->composite_id.object_id == 1000) { // wipe tower @@ -1151,7 +1152,8 @@ void GLCanvas3D::toggle_model_objects_visibility(bool visible, const ModelObject } else { if ((mo == nullptr || m_model->objects[vol->composite_id.object_id] == mo) - && (instance_idx == -1 || vol->composite_id.instance_id == instance_idx)) { + && (instance_idx == -1 || vol->composite_id.instance_id == instance_idx) + && (mv == nullptr || m_model->objects[vol->composite_id.object_id]->volumes[vol->composite_id.volume_id] == mv)) { vol->is_active = visible; if (instance_idx == -1) { @@ -5239,10 +5241,7 @@ void GLCanvas3D::_render_objects(GLVolumeCollection::ERenderType type) { const GLGizmosManager& gm = get_gizmos_manager(); GLGizmosManager::EType type = gm.get_current_type(); - if (type == GLGizmosManager::FdmSupports - || type == GLGizmosManager::Seam - || type == GLGizmosManager::MmuSegmentation - || type == GLGizmosManager::Simplify ) { + if (dynamic_cast(gm.get_current())) { shader->stop_using(); gm.render_painter_gizmo(); shader->start_using(); diff --git a/src/slic3r/GUI/GLCanvas3D.hpp b/src/slic3r/GUI/GLCanvas3D.hpp index 71f7131003..2020360298 100644 --- a/src/slic3r/GUI/GLCanvas3D.hpp +++ b/src/slic3r/GUI/GLCanvas3D.hpp @@ -635,7 +635,7 @@ public: void update_gcode_sequential_view_current(unsigned int first, unsigned int last) { m_gcode_viewer.update_sequential_view_current(first, last); } void toggle_sla_auxiliaries_visibility(bool visible, const ModelObject* mo = nullptr, int instance_idx = -1); - void toggle_model_objects_visibility(bool visible, const ModelObject* mo = nullptr, int instance_idx = -1); + void toggle_model_objects_visibility(bool visible, const ModelObject* mo = nullptr, int instance_idx = -1, const ModelVolume* mv = nullptr); void update_instance_printable_state_for_object(size_t obj_idx); void update_instance_printable_state_for_objects(const std::vector& object_idxs); diff --git a/src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp b/src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp index 819550c89d..2705f82d32 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp @@ -99,20 +99,11 @@ protected: GLPaintContour m_paint_contour; }; -class GLGizmoTransparentRender -{ -public: - // Following function renders the triangles and cursor. Having this separated - // from usual on_render method allows to render them before transparent - // objects, so they can be seen inside them. The usual on_render is called - // after all volumes (including transparent ones) are rendered. - virtual void render_painter_gizmo() const = 0; -}; // Following class is a base class for a gizmo with ability to paint on mesh // using circular blush (such as FDM supports gizmo and seam painting gizmo). // The purpose is not to duplicate code related to mesh painting. -class GLGizmoPainterBase : public GLGizmoTransparentRender, public GLGizmoBase +class GLGizmoPainterBase : public GLGizmoBase { private: ObjectID m_old_mo_id; @@ -125,6 +116,12 @@ public: virtual void set_painter_gizmo_data(const Selection& selection); virtual bool gizmo_event(SLAGizmoEventType action, const Vec2d& mouse_position, bool shift_down, bool alt_down, bool control_down); + // Following function renders the triangles and cursor. Having this separated + // from usual on_render method allows to render them before transparent + // objects, so they can be seen inside them. The usual on_render is called + // after all volumes (including transparent ones) are rendered. + virtual void render_painter_gizmo() const = 0; + protected: virtual void render_triangles(const Selection& selection) const; void render_cursor() const; diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index aecf564188..cbca708329 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -31,16 +31,12 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, , tr_preview(_u8L("Preview")) , tr_detail_level(_u8L("Detail level")) , tr_decimate_ratio(_u8L("Decimate ratio")) - // for wireframe - , m_wireframe_VBO_id(0) - , m_wireframe_IBO_id(0) - , m_wireframe_IBO_size(0) {} GLGizmoSimplify::~GLGizmoSimplify() { m_state = State::canceling; if (m_worker.joinable()) m_worker.join(); - free_gpu(); + m_glmodel.reset(); } bool GLGizmoSimplify::on_esc_key_down() { @@ -143,7 +139,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi m_configuration.fix_count_by_ratio(m_volume->mesh().its.indices.size()); m_is_valid_result = false; m_exist_preview = false; - init_wireframe(); + init_model(); live_preview(); // set window position @@ -266,10 +262,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::Text(_u8L("%d triangles").c_str(), m_configuration.wanted_count); m_imgui->disabled_end(); // use_count - if (ImGui::Checkbox(_u8L("Show wireframe").c_str(), &m_show_wireframe)) { - if (m_show_wireframe) init_wireframe(); - else free_gpu(); - } + ImGui::Checkbox(_u8L("Show wireframe").c_str(), &m_show_wireframe); bool is_canceling = m_state == State::canceling; m_imgui->disabled_begin(is_canceling); @@ -322,11 +315,11 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi m_need_reload = false; bool close_on_end = (m_state == State::close_on_end); // Reload visualization of mesh - change VBO, FBO on GPU - m_parent.reload_scene(true); + request_rerender(); // set m_state must be before close() !!! m_state = State::settings; if (close_on_end) after_apply(); - else init_wireframe(); + else init_model(); // Fix warning icon in object list wxGetApp().obj_list()->update_item_error_icon(m_obj_index, -1); } @@ -469,6 +462,8 @@ void GLGizmoSimplify::on_set_state() { // Closing gizmo. e.g. selecting another one if (GLGizmoBase::m_state == GLGizmoBase::Off) { + m_parent.toggle_model_objects_visibility(true); + // can appear when delete objects bool empty_selection = m_parent.get_selection().is_empty(); @@ -495,7 +490,7 @@ void GLGizmoSimplify::on_set_state() m_exist_preview = false; if (exist_volume(m_volume)) { set_its(*m_original_its); - m_parent.reload_scene(false); + request_rerender(); m_need_reload = false; } } @@ -581,44 +576,25 @@ const ModelVolume *GLGizmoSimplify::get_volume(const GLVolume::CompositeID &cid, return obj->volumes[cid.volume_id]; } -void GLGizmoSimplify::init_wireframe() +void GLGizmoSimplify::init_model() { - if (!m_show_wireframe) return; const indexed_triangle_set &its = m_volume->mesh().its; - free_gpu(); if (its.indices.empty()) return; - // vertices - glsafe(::glGenBuffers(1, &m_wireframe_VBO_id)); - glsafe(::glBindBuffer(GL_ARRAY_BUFFER, m_wireframe_VBO_id)); - glsafe(::glBufferData(GL_ARRAY_BUFFER, - its.vertices.size() * 3 * sizeof(float), - its.vertices.data(), GL_STATIC_DRAW)); + m_glmodel.reset(); + m_glmodel.init_from(its); + m_parent.toggle_model_objects_visibility(false, m_c->selection_info()->model_object(), + m_c->selection_info()->get_active_instance(), m_volume); - // indices - std::vector contour_indices; - contour_indices.reserve((its.indices.size() * 3) / 2); - for (const auto &triangle : its.indices) { - for (size_t ti1 = 0; ti1 < 3; ++ti1) { - size_t ti2 = (ti1 == 2) ? 0 : (ti1 + 1); - if (triangle[ti1] > triangle[ti2]) continue; - contour_indices.emplace_back(triangle[ti1], triangle[ti2]); - } - } - glsafe(::glGenBuffers(1, &m_wireframe_IBO_id)); - glsafe(::glBindBuffer(GL_ARRAY_BUFFER, m_wireframe_IBO_id)); - glsafe(::glBufferData(GL_ARRAY_BUFFER, - 2*contour_indices.size() * sizeof(coord_t), - contour_indices.data(), GL_STATIC_DRAW)); - m_wireframe_IBO_size = contour_indices.size() * 2; - glsafe(::glBindBuffer(GL_ARRAY_BUFFER, 0)); + if (const Selection&sel = m_parent.get_selection(); sel.get_volume_idxs().size() == 1) + m_glmodel.set_color(-1, sel.get_volume(*sel.get_volume_idxs().begin())->color); } -void GLGizmoSimplify::render_wireframe() const +void GLGizmoSimplify::on_render() { // is initialized? - if (m_wireframe_VBO_id == 0 || m_wireframe_IBO_id == 0) return; - if (!m_show_wireframe) return; + if (! m_glmodel.is_initialized()) + return; const auto& selection = m_parent.get_selection(); const auto& volume_idxs = selection.get_volume_idxs(); @@ -633,39 +609,35 @@ void GLGizmoSimplify::render_wireframe() const glsafe(::glPushMatrix()); glsafe(::glMultMatrixd(trafo_matrix.data())); - auto *contour_shader = wxGetApp().get_shader("mm_contour"); - contour_shader->start_using(); - glsafe(::glDepthFunc(GL_LEQUAL)); - glsafe(::glLineWidth(1.0f)); + auto *gouraud_shader = wxGetApp().get_shader("gouraud_light"); + glsafe(::glPushAttrib(GL_DEPTH_TEST)); + glsafe(::glEnable(GL_DEPTH_TEST)); + gouraud_shader->start_using(); + m_glmodel.render(); + gouraud_shader->stop_using(); - glsafe(::glBindBuffer(GL_ARRAY_BUFFER, m_wireframe_VBO_id)); - glsafe(::glVertexPointer(3, GL_FLOAT, 3 * sizeof(float), nullptr)); - glsafe(::glEnableClientState(GL_VERTEX_ARRAY)); + if (m_show_wireframe) { + auto* contour_shader = wxGetApp().get_shader("mm_contour"); + contour_shader->start_using(); + glsafe(::glLineWidth(1.0f)); + glsafe(::glPolygonMode(GL_FRONT_AND_BACK, GL_LINE)); + //ScopeGuard offset_fill_guard([]() { glsafe(::glDisable(GL_POLYGON_OFFSET_FILL)); }); + //glsafe(::glEnable(GL_POLYGON_OFFSET_FILL)); + //glsafe(::glPolygonOffset(5.0, 5.0)); + m_glmodel.render(); + glsafe(::glPolygonMode(GL_FRONT_AND_BACK, GL_FILL)); + contour_shader->stop_using(); + } - glsafe(::glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_wireframe_IBO_id)); - glsafe(::glDrawElements(GL_LINES, m_wireframe_IBO_size, GL_UNSIGNED_INT, nullptr)); - glsafe(::glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0)); - - glsafe(::glDisableClientState(GL_VERTEX_ARRAY)); - - glsafe(::glBindBuffer(GL_ARRAY_BUFFER, 0)); - glsafe(::glDepthFunc(GL_LESS)); - - glsafe(::glPopMatrix()); // pop trafo - contour_shader->stop_using(); + glsafe(::glPopAttrib()); + glsafe(::glPopMatrix()); } -void GLGizmoSimplify::free_gpu() -{ - if (m_wireframe_VBO_id != 0) { - glsafe(::glDeleteBuffers(1, &m_wireframe_VBO_id)); - m_wireframe_VBO_id = 0; - } - if (m_wireframe_IBO_id != 0) { - glsafe(::glDeleteBuffers(1, &m_wireframe_IBO_id)); - m_wireframe_IBO_id = 0; - } +CommonGizmosDataID GLGizmoSimplify::on_get_requirements() const +{ + return CommonGizmosDataID( + int(CommonGizmosDataID::SelectionInfo)); } } // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index ad954e29bb..979adfeba1 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -4,6 +4,7 @@ // Include GLGizmoBase.hpp before I18N.hpp as it includes some libigl code, // which overrides our localization "L" macro. #include "GLGizmoBase.hpp" +#include "slic3r/GUI/GLModel.hpp" #include "GLGizmoPainterBase.hpp" // for render wireframe #include "admesh/stl.h" // indexed_triangle_set #include @@ -24,7 +25,7 @@ class ModelVolume; namespace GUI { class NotificationManager; // for simplify suggestion -class GLGizmoSimplify: public GLGizmoBase, public GLGizmoTransparentRender // GLGizmoBase +class GLGizmoSimplify: public GLGizmoBase { public: GLGizmoSimplify(GLCanvas3D& parent, const std::string& icon_filename, unsigned int sprite_id); @@ -43,11 +44,10 @@ protected: // must implement virtual bool on_init() override { return true;}; - virtual void on_render() override{}; + virtual void on_render() override; virtual void on_render_for_picking() override{}; - // GLGizmoPainterBase - virtual void render_painter_gizmo() const override{ render_wireframe(); } + virtual CommonGizmosDataID on_get_requirements() const; private: void after_apply(); void close(); @@ -75,7 +75,8 @@ private: size_t m_obj_index; std::optional m_original_its; - bool m_show_wireframe; + bool m_show_wireframe; + GLModel m_glmodel; std::atomic m_need_reload; // after simplify, glReload must be on main thread @@ -142,12 +143,7 @@ private: const std::string tr_detail_level; const std::string tr_decimate_ratio; - // rendering wireframe - void render_wireframe() const; - void init_wireframe(); - void free_gpu(); - GLuint m_wireframe_VBO_id, m_wireframe_IBO_id; - size_t m_wireframe_IBO_size; + void init_model(); // cancel exception class SimplifyCanceledException: public std::exception diff --git a/src/slic3r/GUI/Gizmos/GLGizmosManager.cpp b/src/slic3r/GUI/Gizmos/GLGizmosManager.cpp index 2ff554ec88..3104a8595c 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmosManager.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmosManager.cpp @@ -483,7 +483,7 @@ void GLGizmosManager::render_painter_gizmo() const if (!m_enabled || m_current == Undefined) return; - auto *gizmo = dynamic_cast(get_current()); + auto *gizmo = dynamic_cast(get_current()); assert(gizmo); // check the precondition gizmo->render_painter_gizmo(); } From 6661967f9f27a8f0309fe9b098cf8639a5cacdad Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Fri, 29 Oct 2021 12:35:57 +0200 Subject: [PATCH 03/11] Shared data packed in a struct --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 104 ++++++++-------------- src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 29 +++--- 2 files changed, 55 insertions(+), 78 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index cbca708329..d150483008 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -16,10 +16,9 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, const std::string &icon_filename, unsigned int sprite_id) : GLGizmoBase(parent, icon_filename, -1) - , m_state(State::settings) + , m_state({ State::settings, 0 }) , m_is_valid_result(false) , m_exist_preview(false) - , m_progress(0) , m_volume(nullptr) , m_obj_index(0) , m_need_reload(false) @@ -34,16 +33,16 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, {} GLGizmoSimplify::~GLGizmoSimplify() { - m_state = State::canceling; + m_state.status = State::canceling; if (m_worker.joinable()) m_worker.join(); m_glmodel.reset(); } bool GLGizmoSimplify::on_esc_key_down() { - if (m_state == State::settings || m_state == State::canceling) + if (m_state.status == State::settings || m_state.status == State::canceling) return false; - m_state = State::canceling; + m_state.status = State::canceling; return true; } @@ -111,17 +110,17 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi int obj_index = selection.get_object_idx(); ModelVolume *act_volume = get_volume(selection, wxGetApp().plater()->model()); if (act_volume == nullptr) { - switch (m_state) { + switch (m_state.status) { case State::settings: close(); break; case State::canceling: break; - default: m_state = State::canceling; + default: m_state.status = State::canceling; } return; } // Check selection of new volume // Do not reselect object when processing - if (act_volume != m_volume && m_state == State::settings) { + if (act_volume != m_volume && m_state.status == State::settings) { bool change_window_position = (m_volume == nullptr); // select different model if (m_volume != nullptr && m_original_its.has_value()) { @@ -264,18 +263,18 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::Checkbox(_u8L("Show wireframe").c_str(), &m_show_wireframe); - bool is_canceling = m_state == State::canceling; + bool is_canceling = m_state.status == State::canceling; m_imgui->disabled_begin(is_canceling); if (m_imgui->button(_L("Cancel"))) { - if (m_state == State::settings) { + if (m_state.status == State::settings) { if (m_original_its.has_value()) { set_its(*m_original_its); - m_state = State::close_on_end; + m_state.status = State::close_on_end; } else { close(); } } else { - m_state = State::canceling; + m_state.status = State::canceling; } } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && is_canceling) ImGui::SetTooltip("%s", _u8L("Operation already canceling. Please wait few seconds.").c_str()); @@ -283,11 +282,11 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::SameLine(); - bool is_processing = m_state != State::settings; + bool is_processing = m_state.status != State::settings; m_imgui->disabled_begin(is_processing); if (m_imgui->button(_L("Apply"))) { if (!m_is_valid_result) { - m_state = State::close_on_end; + m_state.status = State::close_on_end; process(); } else if (m_exist_preview) { // use preview and close @@ -304,20 +303,18 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::SameLine(m_gui_cfg->bottom_left_width); // draw progress bar char buf[32]; - int progress = m_progress; - sprintf(buf, L("Process %d / 100"), progress); - ImGui::ProgressBar(progress / 100., ImVec2(m_gui_cfg->input_width, 0.f), buf); + sprintf(buf, L("Process %d / 100"), m_state.progress); + ImGui::ProgressBar(m_state.progress / 100., ImVec2(m_gui_cfg->input_width, 0.f), buf); } m_imgui->end(); // refresh view when needed if (m_need_reload) { m_need_reload = false; - bool close_on_end = (m_state == State::close_on_end); - // Reload visualization of mesh - change VBO, FBO on GPU + bool close_on_end = (m_state.status == State::close_on_end); request_rerender(); - // set m_state must be before close() !!! - m_state = State::settings; + // set m_state.status must be before close() !!! + m_state.status = State::settings; if (close_on_end) after_apply(); else init_model(); // Fix warning icon in object list @@ -330,6 +327,7 @@ void GLGizmoSimplify::after_apply() { m_exist_preview = false; // fix hollowing, sla support points, modifiers, ... auto plater = wxGetApp().plater(); + plater->take_snapshot(_u8L("Simplify ") + m_volume->name); plater->changed_mesh(m_obj_index); close(); } @@ -342,19 +340,12 @@ void GLGizmoSimplify::close() { void GLGizmoSimplify::live_preview() { m_is_valid_result = false; - if (m_state != State::settings) { + if (m_state.status != State::settings) { // already canceling process - if (m_state == State::canceling) return; - - // wait until cancel - if (m_worker.joinable()) { - m_state = State::canceling; - m_dealy_process_cv.notify_one(); - m_worker.join(); - } + if (m_state.status == State::canceling) return; } - m_state = State::preview; + m_state.status = State::preview; process(); } @@ -387,48 +378,29 @@ void GLGizmoSimplify::process() // store previous state auto plater = wxGetApp().plater(); - plater->take_snapshot(_u8L("Simplify ") + m_volume->name); + // LUKAS: ??? plater->clear_before_change_mesh(m_obj_index); } - m_progress = 0; + m_state.progress = 0; if (m_worker.joinable()) m_worker.join(); - m_worker = std::thread([this]() { - {// delay before process - std::unique_lock lk(m_state_mutex); - auto is_modify = [this]() { return m_state == State::canceling; }; - if (m_dealy_process_cv.wait_for(lk, m_gui_cfg->prcess_delay, is_modify)) { - // exist modification - m_state = State::settings; - request_rerender(); - return; - } - } + m_worker = std::thread([this]() { - // store original triangles - uint32_t triangle_count = (m_configuration.use_count) ? m_configuration.wanted_count : 0; - float max_error = (!m_configuration.use_count) ? m_configuration.max_error : std::numeric_limits::max(); - - std::function throw_on_cancel = [&]() { - if (m_state == State::canceling) { + // Checks that the UI thread did not request cancellation, throw if so. + std::function throw_on_cancel = [this]() { + if (m_state.status == State::canceling) throw SimplifyCanceledException(); - } }; - int64_t last = 0; - std::function statusfn = [this, &last](int percent) { - m_progress = percent; - - // check max 4fps - int64_t now = m_parent.timestamp_now(); - if ((now - last) < 250) return; - last = now; - - request_rerender(); + // Called by worker thread, + std::function statusfn = [this](int percent) { + m_state.progress = percent; }; indexed_triangle_set collapsed = *m_original_its; // copy + uint32_t triangle_count = (m_configuration.use_count) ? m_configuration.wanted_count : 0; + float max_error = (!m_configuration.use_count) ? m_configuration.max_error : std::numeric_limits::max(); try { its_quadric_edge_collapse(collapsed, triangle_count, &max_error, throw_on_cancel, statusfn); @@ -437,7 +409,7 @@ void GLGizmoSimplify::process() m_exist_preview = true; } catch (SimplifyCanceledException &) { // set state out of main thread - m_state = State::settings; + m_state.status = State::settings; } // need to render last status fn to change bar graph to buttons request_rerender(); @@ -469,13 +441,13 @@ void GLGizmoSimplify::on_set_state() // cancel processing if (empty_selection && - m_state != State::settings && - m_state != State::canceling) - m_state = State::canceling; + m_state.status != State::settings && + m_state.status != State::canceling) + m_state.status = State::canceling; // refuse outgoing during simlification // object is not selected when it is deleted(cancel and close gizmo) - if (m_state != State::settings && !empty_selection) { + if (m_state.status != State::settings && !empty_selection) { GLGizmoBase::m_state = GLGizmoBase::On; auto notification_manager = wxGetApp().plater()->get_notification_manager(); notification_manager->push_notification( diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index 979adfeba1..3373ca0b23 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -4,13 +4,11 @@ // Include GLGizmoBase.hpp before I18N.hpp as it includes some libigl code, // which overrides our localization "L" macro. #include "GLGizmoBase.hpp" -#include "slic3r/GUI/GLModel.hpp" #include "GLGizmoPainterBase.hpp" // for render wireframe +#include "slic3r/GUI/GLModel.hpp" #include "admesh/stl.h" // indexed_triangle_set #include #include -#include -#include #include #include @@ -35,6 +33,7 @@ public: const std::vector &object_ids, const ModelObjectPtrs & objects, NotificationManager & manager); + protected: virtual std::string on_get_name() const override; virtual void on_render_input_window(float x, float y, float bottom_limit) override; @@ -48,6 +47,7 @@ protected: virtual void on_render_for_picking() override{}; virtual CommonGizmosDataID on_get_requirements() const; + private: void after_apply(); void close(); @@ -70,7 +70,7 @@ private: bool m_move_to_center; // opening gizmo - std::atomic m_progress; // percent of done work + ModelVolume *m_volume; // keep pointer to actual working volume size_t m_obj_index; @@ -81,17 +81,22 @@ private: std::atomic m_need_reload; // after simplify, glReload must be on main thread std::thread m_worker; - // wait before process std::mutex m_state_mutex; - std::condition_variable m_dealy_process_cv; - enum class State { - settings, - preview, // simplify to show preview - close_on_end, // simplify with close on end - canceling // after button click, before canceled + struct State { + enum Status { + settings, + preview, // simplify to show preview + close_on_end, // simplify with close on end + canceling // after button click, before canceled + }; + + Status status; + int progress; // percent of done work + indexed_triangle_set result; }; - std::atomic m_state; + + State m_state; struct Configuration { From ab260d005e5ea50d224595617c2d0e8eb4b72ae6 Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Fri, 29 Oct 2021 12:45:30 +0200 Subject: [PATCH 04/11] More adjustments, still working with Model directly --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 49 +++++++++-------------- src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 7 +--- 2 files changed, 21 insertions(+), 35 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index d150483008..6c4cce9ee7 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -27,7 +27,6 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, // translation for GUI size , tr_mesh_name(_u8L("Mesh name")) , tr_triangles(_u8L("Triangles")) - , tr_preview(_u8L("Preview")) , tr_detail_level(_u8L("Detail level")) , tr_decimate_ratio(_u8L("Decimate ratio")) {} @@ -139,7 +138,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi m_is_valid_result = false; m_exist_preview = false; init_model(); - live_preview(); + process(); // set window position if (m_move_to_center && change_window_position) { @@ -185,20 +184,13 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi m_imgui->text_colored(ImGuiWrapper::COL_ORANGE_LIGHT, tr_triangles + ":"); ImGui::SameLine(m_gui_cfg->top_left_width); m_imgui->text(std::to_string(triangle_count)); - /* - m_imgui->text_colored(ImGuiWrapper::COL_ORANGE_LIGHT, tr_preview + ":"); - ImGui::SameLine(m_gui_cfg->top_left_width); - if (m_exist_preview) { - m_imgui->text(std::to_string(m_volume->mesh().its.indices.size())); - } else { - m_imgui->text("---"); - }*/ + ImGui::Separator(); if(ImGui::RadioButton("##use_error", !m_configuration.use_count)) { m_configuration.use_count = !m_configuration.use_count; - live_preview(); + process(); } ImGui::SameLine(); m_imgui->disabled_begin(m_configuration.use_count); @@ -223,13 +215,13 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi case 3: m_configuration.max_error = 0.5f; break; case 4: m_configuration.max_error = 1.f; break; } - live_preview(); + process(); } m_imgui->disabled_end(); // !use_count if (ImGui::RadioButton("##use_count", m_configuration.use_count)) { m_configuration.use_count = !m_configuration.use_count; - live_preview(); + process(); } ImGui::SameLine(); @@ -253,7 +245,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi if (m_configuration.decimate_ratio > 100.f) m_configuration.decimate_ratio = 100.f; m_configuration.fix_count_by_ratio(triangle_count); - live_preview(); + process(); } ImGui::NewLine(); @@ -338,16 +330,6 @@ void GLGizmoSimplify::close() { gizmos_mgr.open_gizmo(GLGizmosManager::EType::Simplify); } -void GLGizmoSimplify::live_preview() { - m_is_valid_result = false; - if (m_state.status != State::settings) { - // already canceling process - if (m_state.status == State::canceling) return; - } - - m_state.status = State::preview; - process(); -} void GLGizmoSimplify::process() { @@ -383,28 +365,35 @@ void GLGizmoSimplify::process() } m_state.progress = 0; - if (m_worker.joinable()) m_worker.join(); + if (m_worker.joinable()) + m_worker.join(); - m_worker = std::thread([this]() { + // Create a copy of current mesh to pass to the worker thread. + // Using unique_ptr instead of pass-by-value to avoid an extra + // copy (which would happen when passing to std::thread). + auto its = std::make_unique(*m_original_its); + + m_worker = std::thread([this](std::unique_ptr its) { // Checks that the UI thread did not request cancellation, throw if so. std::function throw_on_cancel = [this]() { + std::lock_guard lk(m_state_mutex); if (m_state.status == State::canceling) throw SimplifyCanceledException(); }; // Called by worker thread, std::function statusfn = [this](int percent) { + std::lock_guard lk(m_state_mutex); m_state.progress = percent; }; - indexed_triangle_set collapsed = *m_original_its; // copy uint32_t triangle_count = (m_configuration.use_count) ? m_configuration.wanted_count : 0; float max_error = (!m_configuration.use_count) ? m_configuration.max_error : std::numeric_limits::max(); try { - its_quadric_edge_collapse(collapsed, triangle_count, &max_error, throw_on_cancel, statusfn); - set_its(collapsed); + its_quadric_edge_collapse(*its, triangle_count, &max_error, throw_on_cancel, statusfn); + set_its(*its); m_is_valid_result = true; m_exist_preview = true; } catch (SimplifyCanceledException &) { @@ -413,7 +402,7 @@ void GLGizmoSimplify::process() } // need to render last status fn to change bar graph to buttons request_rerender(); - }); + }, std::move(its)); } void GLGizmoSimplify::set_its(const indexed_triangle_set &its) { diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index 3373ca0b23..3e0ea6706e 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -51,7 +51,6 @@ protected: private: void after_apply(); void close(); - void live_preview(); void process(); void set_its(const indexed_triangle_set &its); void create_gui_cfg(); @@ -80,9 +79,6 @@ private: std::atomic m_need_reload; // after simplify, glReload must be on main thread - std::thread m_worker; - std::mutex m_state_mutex; - struct State { enum Status { settings, @@ -96,6 +92,8 @@ private: indexed_triangle_set result; }; + std::thread m_worker; + std::mutex m_state_mutex; // guards m_state State m_state; struct Configuration @@ -144,7 +142,6 @@ private: // translations used for calc window size const std::string tr_mesh_name; const std::string tr_triangles; - const std::string tr_preview; const std::string tr_detail_level; const std::string tr_decimate_ratio; From 7bcab6f7952873539cee6f745b5a168964e5a548 Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Fri, 29 Oct 2021 17:58:05 +0200 Subject: [PATCH 05/11] Simplify does not touch ModelVolume all the time (runs, but needs polishing) --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 276 ++++++++++------------ src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 97 ++++---- 2 files changed, 173 insertions(+), 200 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index 6c4cce9ee7..14bfcd020a 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -16,12 +16,7 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, const std::string &icon_filename, unsigned int sprite_id) : GLGizmoBase(parent, icon_filename, -1) - , m_state({ State::settings, 0 }) - , m_is_valid_result(false) - , m_exist_preview(false) , m_volume(nullptr) - , m_obj_index(0) - , m_need_reload(false) , m_show_wireframe(false) , m_move_to_center(false) // translation for GUI size @@ -32,16 +27,15 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, {} GLGizmoSimplify::~GLGizmoSimplify() { - m_state.status = State::canceling; + m_state.status = State::cancelling; if (m_worker.joinable()) m_worker.join(); m_glmodel.reset(); } bool GLGizmoSimplify::on_esc_key_down() { - if (m_state.status == State::settings || m_state.status == State::canceling) + if (m_state.status != State::running) return false; - - m_state.status = State::canceling; + m_state.status = State::cancelling; return true; } @@ -106,38 +100,27 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi { create_gui_cfg(); const Selection &selection = m_parent.get_selection(); - int obj_index = selection.get_object_idx(); ModelVolume *act_volume = get_volume(selection, wxGetApp().plater()->model()); if (act_volume == nullptr) { - switch (m_state.status) { - case State::settings: close(); break; - case State::canceling: break; - default: m_state.status = State::canceling; - } + stop_worker_thread(false); + close(); return; } // Check selection of new volume // Do not reselect object when processing - if (act_volume != m_volume && m_state.status == State::settings) { + if (act_volume != m_volume && m_state.status != State::Status::running) { bool change_window_position = (m_volume == nullptr); // select different model - if (m_volume != nullptr && m_original_its.has_value()) { - set_its(*m_original_its); - } // close suggestion notification auto notification_manager = wxGetApp().plater()->get_notification_manager(); notification_manager->remove_simplify_suggestion_with_id(act_volume->get_object()->id()); - m_obj_index = obj_index; // to remember correct object m_volume = act_volume; - m_original_its = {}; m_configuration.decimate_ratio = 50.; // default value m_configuration.fix_count_by_ratio(m_volume->mesh().its.indices.size()); - m_is_valid_result = false; - m_exist_preview = false; - init_model(); + init_model(m_volume->mesh().its); process(); // set window position @@ -169,12 +152,6 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi int flag = ImGuiWindowFlags_AlwaysAutoResize | ImGuiWindowFlags_NoResize | ImGuiWindowFlags_NoCollapse; m_imgui->begin(on_get_name(), flag); - - size_t triangle_count = m_volume->mesh().its.indices.size(); - // already reduced mesh - if (m_original_its.has_value()) - triangle_count = m_original_its->indices.size(); - m_imgui->text_colored(ImGuiWrapper::COL_ORANGE_LIGHT, tr_mesh_name + ":"); ImGui::SameLine(m_gui_cfg->top_left_width); std::string name = m_volume->name; @@ -183,7 +160,9 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi m_imgui->text(name); m_imgui->text_colored(ImGuiWrapper::COL_ORANGE_LIGHT, tr_triangles + ":"); ImGui::SameLine(m_gui_cfg->top_left_width); - m_imgui->text(std::to_string(triangle_count)); + + size_t orig_triangle_count = m_volume->mesh().its.indices.size(); + m_imgui->text(std::to_string(orig_triangle_count)); ImGui::Separator(); @@ -226,10 +205,11 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::SameLine(); // show preview result triangle count (percent) - if (m_need_reload && !m_configuration.use_count) { - m_configuration.wanted_count = static_cast(m_volume->mesh().its.indices.size()); + // lm_FIXME + if (/*m_need_reload &&*/ !m_configuration.use_count) { + m_configuration.wanted_count = static_cast(m_triangle_count); m_configuration.decimate_ratio = - (1.0f - (m_configuration.wanted_count / (float) triangle_count)) * 100.f; + (1.0f - (m_configuration.wanted_count / (float) orig_triangle_count)) * 100.f; } m_imgui->disabled_begin(!m_configuration.use_count); @@ -244,7 +224,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi m_configuration.decimate_ratio = 0.01f; if (m_configuration.decimate_ratio > 100.f) m_configuration.decimate_ratio = 100.f; - m_configuration.fix_count_by_ratio(triangle_count); + m_configuration.fix_count_by_ratio(orig_triangle_count); process(); } @@ -255,37 +235,20 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::Checkbox(_u8L("Show wireframe").c_str(), &m_show_wireframe); - bool is_canceling = m_state.status == State::canceling; - m_imgui->disabled_begin(is_canceling); + bool is_cancelling = m_state.status == State::cancelling; + m_imgui->disabled_begin(is_cancelling); if (m_imgui->button(_L("Cancel"))) { - if (m_state.status == State::settings) { - if (m_original_its.has_value()) { - set_its(*m_original_its); - m_state.status = State::close_on_end; - } else { - close(); - } - } else { - m_state.status = State::canceling; - } - } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && is_canceling) - ImGui::SetTooltip("%s", _u8L("Operation already canceling. Please wait few seconds.").c_str()); - m_imgui->disabled_end(); // state canceling + close(); + } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && is_cancelling) + ImGui::SetTooltip("%s", _u8L("Operation already cancelling. Please wait few seconds.").c_str()); + m_imgui->disabled_end(); // state cancelling ImGui::SameLine(); - bool is_processing = m_state.status != State::settings; + bool is_processing = m_state.status == State::running; m_imgui->disabled_begin(is_processing); if (m_imgui->button(_L("Apply"))) { - if (!m_is_valid_result) { - m_state.status = State::close_on_end; - process(); - } else if (m_exist_preview) { - // use preview and close - after_apply(); - } else { // no changes made - close(); - } + apply_simplify(); } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && is_processing) ImGui::SetTooltip("%s", _u8L("Can't apply when proccess preview.").c_str()); m_imgui->disabled_end(); // state !settings @@ -301,28 +264,20 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi m_imgui->end(); // refresh view when needed - if (m_need_reload) { + // lm_FIXME + /*if (m_need_reload) { m_need_reload = false; bool close_on_end = (m_state.status == State::close_on_end); request_rerender(); // set m_state.status must be before close() !!! m_state.status = State::settings; if (close_on_end) after_apply(); - else init_model(); + else init_model(m_volume->mesh().its); // Fix warning icon in object list wxGetApp().obj_list()->update_item_error_icon(m_obj_index, -1); - } + }*/ } -void GLGizmoSimplify::after_apply() { - // set flag to NOT revert changes when switch GLGizmoBase::m_state - m_exist_preview = false; - // fix hollowing, sla support points, modifiers, ... - auto plater = wxGetApp().plater(); - plater->take_snapshot(_u8L("Simplify ") + m_volume->name); - plater->changed_mesh(m_obj_index); - close(); -} void GLGizmoSimplify::close() { // close gizmo == open it again @@ -330,55 +285,58 @@ void GLGizmoSimplify::close() { gizmos_mgr.open_gizmo(GLGizmosManager::EType::Simplify); } +void GLGizmoSimplify::stop_worker_thread(bool wait) +{ + if (! m_is_worker_running) + return; + + std::lock_guard lk(m_state_mutex); + if (m_state.status == State::Status::running) { + assert(m_worker.joinable()); + m_state.status = State::Status::cancelling; + if (wait) + m_worker.join(); + } +} + + +// Following is called from a UI thread when the worker terminates +// worker calls it through a CallAfter. +void GLGizmoSimplify::worker_finished() +{ + assert(m_worker.joinable()); + m_worker.join(); + m_is_worker_running = false; + if (m_state.result) + init_model(*m_state.result); + request_rerender(); +} void GLGizmoSimplify::process() { - if (m_volume == nullptr) return; - if (m_volume->mesh().its.indices.empty()) return; - size_t count_triangles = m_volume->mesh().its.indices.size(); - // Is neccessary simplification - if ((m_configuration.use_count && m_configuration.wanted_count >= count_triangles) || - (!m_configuration.use_count && m_configuration.max_error <= 0.f)) { - - // Exist different original volume? - if (m_original_its.has_value() && - m_original_its->indices.size() != count_triangles) { - indexed_triangle_set its = *m_original_its; // copy - set_its(its); - } - m_is_valid_result = true; - - // re-render bargraph - set_dirty(); - m_parent.schedule_extra_frame(0); + if (m_is_worker_running || m_volume == nullptr || m_volume->mesh().its.indices.empty()) return; - } - // when not store original volume store it for cancelation - if (!m_original_its.has_value()) { - m_original_its = m_volume->mesh().its; // copy + assert(! m_worker.joinable()); - // store previous state - auto plater = wxGetApp().plater(); - // LUKAS: ??? - plater->clear_before_change_mesh(m_obj_index); - } + // Worker is not running now. No synchronization needed. + if (m_state.result && m_state.config == m_configuration) + return; // The result is still valid. - m_state.progress = 0; - if (m_worker.joinable()) - m_worker.join(); + // We are about to actually start simplification. + m_state.config = m_configuration; // Create a copy of current mesh to pass to the worker thread. // Using unique_ptr instead of pass-by-value to avoid an extra // copy (which would happen when passing to std::thread). - auto its = std::make_unique(*m_original_its); + auto its = std::make_unique(m_volume->mesh().its); m_worker = std::thread([this](std::unique_ptr its) { // Checks that the UI thread did not request cancellation, throw if so. std::function throw_on_cancel = [this]() { std::lock_guard lk(m_state_mutex); - if (m_state.status == State::canceling) + if (m_state.status == State::cancelling) throw SimplifyCanceledException(); }; @@ -388,30 +346,61 @@ void GLGizmoSimplify::process() m_state.progress = percent; }; - uint32_t triangle_count = (m_configuration.use_count) ? m_configuration.wanted_count : 0; - float max_error = (!m_configuration.use_count) ? m_configuration.max_error : std::numeric_limits::max(); + // Initialize. + uint32_t triangle_count = 0; + float max_error = std::numeric_limits::max(); + { + std::lock_guard lk(m_state_mutex); + if (m_configuration.use_count) + triangle_count = m_configuration.wanted_count; + if (! m_configuration.use_count) + max_error = m_configuration.max_error; + m_state.progress = 0; + m_state.result.reset(); + m_state.status = State::Status::running; + } + // Start the actual calculation. try { its_quadric_edge_collapse(*its, triangle_count, &max_error, throw_on_cancel, statusfn); - set_its(*its); - m_is_valid_result = true; - m_exist_preview = true; } catch (SimplifyCanceledException &) { - // set state out of main thread - m_state.status = State::settings; + std::lock_guard lk(m_state_mutex); + m_state.status = State::idle; } - // need to render last status fn to change bar graph to buttons - request_rerender(); + + std::lock_guard lk(m_state_mutex); + if (m_state.status == State::Status::running) { + // We were not cancelled, the result is valid. + m_state.status = State::Status::idle; + m_state.result = std::move(its); + } + + // Update UI. Use CallAfter so the function is run on UI thread. + wxGetApp().CallAfter([this]() { worker_finished(); }); }, std::move(its)); + + m_is_worker_running = true; } -void GLGizmoSimplify::set_its(const indexed_triangle_set &its) { - if (m_volume == nullptr) return; // could appear after process - m_volume->set_mesh(its); +void GLGizmoSimplify::apply_simplify() { + assert(! m_is_worker_running); + + const Selection& selection = m_parent.get_selection(); + int object_idx = selection.get_object_idx(); + + auto plater = wxGetApp().plater(); + plater->take_snapshot(_u8L("Simplify ") + m_volume->name); + plater->clear_before_change_mesh(object_idx); + + m_volume->set_mesh(*m_state.result); m_volume->calculate_convex_hull(); m_volume->set_new_unique_id(); m_volume->get_object()->invalidate_bounding_box(); - m_need_reload = true; + + // fix hollowing, sla support points, modifiers, ... + + plater->changed_mesh(object_idx); + close(); } bool GLGizmoSimplify::on_is_activable() const @@ -425,40 +414,14 @@ void GLGizmoSimplify::on_set_state() if (GLGizmoBase::m_state == GLGizmoBase::Off) { m_parent.toggle_model_objects_visibility(true); - // can appear when delete objects - bool empty_selection = m_parent.get_selection().is_empty(); - - // cancel processing - if (empty_selection && - m_state.status != State::settings && - m_state.status != State::canceling) - m_state.status = State::canceling; - - // refuse outgoing during simlification - // object is not selected when it is deleted(cancel and close gizmo) - if (m_state.status != State::settings && !empty_selection) { - GLGizmoBase::m_state = GLGizmoBase::On; - auto notification_manager = wxGetApp().plater()->get_notification_manager(); - notification_manager->push_notification( - NotificationType::CustomNotification, - NotificationManager::NotificationLevel::PrintInfoNotificationLevel, - _u8L("ERROR: Wait until Simplification ends or Cancel process.")); - return; - } - - // revert preview - if (m_exist_preview) { - m_exist_preview = false; - if (exist_volume(m_volume)) { - set_its(*m_original_its); - request_rerender(); - m_need_reload = false; - } - } + // Stop worker but do not wait to join it. + stop_worker_thread(false); // invalidate selected model m_volume = nullptr; } else if (GLGizmoBase::m_state == GLGizmoBase::On) { + // Make sure the worker is not running and join it. + stop_worker_thread(true); // when open by hyperlink it needs to show up request_rerender(); } @@ -537,10 +500,10 @@ const ModelVolume *GLGizmoSimplify::get_volume(const GLVolume::CompositeID &cid, return obj->volumes[cid.volume_id]; } -void GLGizmoSimplify::init_model() +void GLGizmoSimplify::init_model(const indexed_triangle_set& its) { - const indexed_triangle_set &its = m_volume->mesh().its; - if (its.indices.empty()) return; + if (its.indices.empty()) + return; m_glmodel.reset(); m_glmodel.init_from(its); @@ -549,6 +512,7 @@ void GLGizmoSimplify::init_model() if (const Selection&sel = m_parent.get_selection(); sel.get_volume_idxs().size() == 1) m_glmodel.set_color(-1, sel.get_volume(*sel.get_volume_idxs().begin())->color); + m_triangle_count = its.indices.size(); } void GLGizmoSimplify::on_render() @@ -601,4 +565,16 @@ CommonGizmosDataID GLGizmoSimplify::on_get_requirements() const int(CommonGizmosDataID::SelectionInfo)); } + +void GLGizmoSimplify::Configuration::fix_count_by_ratio(size_t triangle_count) +{ + if (decimate_ratio <= 0.f) + wanted_count = static_cast(triangle_count); + else if (decimate_ratio >= 100.f) + wanted_count = 0; + else + wanted_count = static_cast(std::round( + triangle_count * (100.f - decimate_ratio) / 100.f)); +} + } // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index 3e0ea6706e..573b1ea518 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -49,12 +49,16 @@ protected: virtual CommonGizmosDataID on_get_requirements() const; private: - void after_apply(); + void apply_simplify(); void close(); + void process(); - void set_its(const indexed_triangle_set &its); + void stop_worker_thread(bool wait); + void worker_finished(); + void create_gui_cfg(); void request_rerender(); + void init_model(const indexed_triangle_set& its); void set_center_position(); // move to global functions @@ -64,59 +68,54 @@ private: // return false when volume was deleted static bool exist_volume(const ModelVolume *volume); - std::atomic_bool m_is_valid_result; // differ what to do in apply - std::atomic_bool m_exist_preview; // set when process end - - bool m_move_to_center; // opening gizmo - - - ModelVolume *m_volume; // keep pointer to actual working volume - size_t m_obj_index; - - std::optional m_original_its; - bool m_show_wireframe; - GLModel m_glmodel; - - std::atomic m_need_reload; // after simplify, glReload must be on main thread - - struct State { - enum Status { - settings, - preview, // simplify to show preview - close_on_end, // simplify with close on end - canceling // after button click, before canceled - }; - - Status status; - int progress; // percent of done work - indexed_triangle_set result; - }; - - std::thread m_worker; - std::mutex m_state_mutex; // guards m_state - State m_state; struct Configuration { bool use_count = false; - // minimal triangle count float decimate_ratio = 50.f; // in percent - uint32_t wanted_count = 0; // initialize by percents + uint32_t wanted_count = 0; // initialize by percents + float max_error = 1.; // maximal quadric error - // maximal quadric error - float max_error = 1.; - - void fix_count_by_ratio(size_t triangle_count) - { - if (decimate_ratio <= 0.f) - wanted_count = static_cast(triangle_count); - else if (decimate_ratio >= 100.f) - wanted_count = 0; - else - wanted_count = static_cast(std::round( - triangle_count * (100.f - decimate_ratio) / 100.f)); + void fix_count_by_ratio(size_t triangle_count); + bool operator==(const Configuration& rhs) { + return (use_count == rhs.use_count && decimate_ratio == rhs.decimate_ratio + && wanted_count == rhs.wanted_count && max_error == rhs.max_error); } - } m_configuration; + }; + + Configuration m_configuration; + + bool m_move_to_center; // opening gizmo + + ModelVolume *m_volume; // keep pointer to actual working volume + + bool m_show_wireframe; + GLModel m_glmodel; + size_t m_triangle_count; // triangle count of the model currently shown + + // Following struct is accessed by both UI and worker thread. + // Accesses protected by a mutex. + struct State { + enum Status { + idle, + running, + cancelling + }; + + Status status = idle; + int progress = 0; // percent of done work + Configuration config; // Configuration we started with. + std::unique_ptr result; + }; + + std::thread m_worker; + std::mutex m_state_mutex; // guards m_state + State m_state; // accessed by both threads + + // Following variable is accessed by UI only. + bool m_is_worker_running = false; + + // This configs holds GUI layout size given by translated texts. // etc. When language changes, GUI is recreated and this class constructed again, @@ -145,8 +144,6 @@ private: const std::string tr_detail_level; const std::string tr_decimate_ratio; - void init_model(); - // cancel exception class SimplifyCanceledException: public std::exception { From be04751776b8f60b4db8f22780f14fb8b01149df Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Sat, 30 Oct 2021 00:47:36 +0200 Subject: [PATCH 06/11] Many small fixes --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 133 ++++++++++++++-------- src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 17 +-- 2 files changed, 89 insertions(+), 61 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index 14bfcd020a..c5eada4a79 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -1,5 +1,4 @@ #include "GLGizmoSimplify.hpp" -#include "slic3r/GUI/3DScene.hpp" #include "slic3r/GUI/GLCanvas3D.hpp" #include "slic3r/GUI/GUI_App.hpp" #include "slic3r/GUI/GUI_ObjectManipulation.hpp" @@ -10,6 +9,10 @@ #include "libslic3r/Model.hpp" #include "libslic3r/QuadricEdgeCollapse.hpp" +#include + +#include + namespace Slic3r::GUI { GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, @@ -27,22 +30,22 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, {} GLGizmoSimplify::~GLGizmoSimplify() { - m_state.status = State::cancelling; - if (m_worker.joinable()) m_worker.join(); + stop_worker_thread(true); m_glmodel.reset(); } bool GLGizmoSimplify::on_esc_key_down() { - if (m_state.status != State::running) + return false; + /*if (!m_is_worker_running) return false; - m_state.status = State::cancelling; - return true; + stop_worker_thread(false); + return true;*/ } // while opening needs GLGizmoSimplify to set window position void GLGizmoSimplify::add_simplify_suggestion_notification( const std::vector &object_ids, - const ModelObjectPtrs & objects, + const std::vector& objects, NotificationManager & manager) { std::vector big_ids; @@ -107,9 +110,18 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi return; } + bool is_cancelling = false; + int progress = 0; + { + std::lock_guard lk(m_state_mutex); + is_cancelling = m_state.status == State::cancelling; + progress = m_state.progress; + } + + // Check selection of new volume // Do not reselect object when processing - if (act_volume != m_volume && m_state.status != State::Status::running) { + if (act_volume != m_volume && ! m_is_worker_running) { bool change_window_position = (m_volume == nullptr); // select different model @@ -235,9 +247,8 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::Checkbox(_u8L("Show wireframe").c_str(), &m_show_wireframe); - bool is_cancelling = m_state.status == State::cancelling; m_imgui->disabled_begin(is_cancelling); - if (m_imgui->button(_L("Cancel"))) { + if (m_imgui->button(_L("Close"))) { close(); } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && is_cancelling) ImGui::SetTooltip("%s", _u8L("Operation already cancelling. Please wait few seconds.").c_str()); @@ -245,21 +256,20 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::SameLine(); - bool is_processing = m_state.status == State::running; - m_imgui->disabled_begin(is_processing); + m_imgui->disabled_begin(m_is_worker_running); if (m_imgui->button(_L("Apply"))) { apply_simplify(); - } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && is_processing) + } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && m_is_worker_running) ImGui::SetTooltip("%s", _u8L("Can't apply when proccess preview.").c_str()); m_imgui->disabled_end(); // state !settings // draw progress bar - if (is_processing) { // apply or preview + if (m_is_worker_running) { // apply or preview ImGui::SameLine(m_gui_cfg->bottom_left_width); // draw progress bar char buf[32]; - sprintf(buf, L("Process %d / 100"), m_state.progress); - ImGui::ProgressBar(m_state.progress / 100., ImVec2(m_gui_cfg->input_width, 0.f), buf); + sprintf(buf, L("Process %d / 100"), progress); + ImGui::ProgressBar(progress / 100., ImVec2(m_gui_cfg->input_width, 0.f), buf); } m_imgui->end(); @@ -287,15 +297,14 @@ void GLGizmoSimplify::close() { void GLGizmoSimplify::stop_worker_thread(bool wait) { - if (! m_is_worker_running) - return; - - std::lock_guard lk(m_state_mutex); - if (m_state.status == State::Status::running) { - assert(m_worker.joinable()); - m_state.status = State::Status::cancelling; - if (wait) - m_worker.join(); + { + std::lock_guard lk(m_state_mutex); + if (m_state.status == State::running) + m_state.status = State::Status::cancelling; + } + if (wait && m_worker.joinable()) { + m_worker.join(); + m_is_worker_running = false; } } @@ -304,26 +313,53 @@ void GLGizmoSimplify::stop_worker_thread(bool wait) // worker calls it through a CallAfter. void GLGizmoSimplify::worker_finished() { - assert(m_worker.joinable()); - m_worker.join(); m_is_worker_running = false; + if (! m_worker.joinable()) { + // Probably stop_worker_thread waited after cancel. + // Nobody cares about the result apparently. + assert(! m_state.result); + return; + } + m_worker.join(); + if (GLGizmoBase::m_state == Off) + return; if (m_state.result) init_model(*m_state.result); + if (m_state.config != m_configuration) { + // Settings were changed, restart the worker immediately. + process(); + } request_rerender(); } void GLGizmoSimplify::process() { - if (m_is_worker_running || m_volume == nullptr || m_volume->mesh().its.indices.empty()) + if (m_volume == nullptr || m_volume->mesh().its.indices.empty()) return; - assert(! m_worker.joinable()); + bool configs_match = false; + bool result_valid = false; + { + std::lock_guard lk(m_state_mutex); + configs_match = m_state.config == m_configuration; + result_valid = bool(m_state.result); + } - // Worker is not running now. No synchronization needed. - if (m_state.result && m_state.config == m_configuration) - return; // The result is still valid. - - // We are about to actually start simplification. + if ((result_valid || m_is_worker_running) && configs_match) { + // Either finished or waiting for result already. Nothing to do. + return; + } + + if (m_is_worker_running && m_state.config != m_configuration) { + // Worker is running with outdated config. Stop it. It will + // restart itself when cancellation is done. + stop_worker_thread(false); + return; + } + + assert(! m_is_worker_running && ! m_worker.joinable()); + + // Copy configuration that will be used. m_state.config = m_configuration; // Create a copy of current mesh to pass to the worker thread. @@ -333,14 +369,14 @@ void GLGizmoSimplify::process() m_worker = std::thread([this](std::unique_ptr its) { - // Checks that the UI thread did not request cancellation, throw if so. + // Checks that the UI thread did not request cancellation, throws if so. std::function throw_on_cancel = [this]() { std::lock_guard lk(m_state_mutex); if (m_state.status == State::cancelling) throw SimplifyCanceledException(); }; - // Called by worker thread, + // Called by worker thread, updates progress bar. std::function statusfn = [this](int percent) { std::lock_guard lk(m_state_mutex); m_state.progress = percent; @@ -351,10 +387,10 @@ void GLGizmoSimplify::process() float max_error = std::numeric_limits::max(); { std::lock_guard lk(m_state_mutex); - if (m_configuration.use_count) - triangle_count = m_configuration.wanted_count; - if (! m_configuration.use_count) - max_error = m_configuration.max_error; + if (m_state.config.use_count) + triangle_count = m_state.config.wanted_count; + if (! m_state.config.use_count) + max_error = m_state.config.max_error; m_state.progress = 0; m_state.result.reset(); m_state.status = State::Status::running; @@ -392,13 +428,13 @@ void GLGizmoSimplify::apply_simplify() { plater->take_snapshot(_u8L("Simplify ") + m_volume->name); plater->clear_before_change_mesh(object_idx); - m_volume->set_mesh(*m_state.result); + m_volume->set_mesh(std::move(*m_state.result)); + m_state.result.reset(); m_volume->calculate_convex_hull(); m_volume->set_new_unique_id(); m_volume->get_object()->invalidate_bounding_box(); - // fix hollowing, sla support points, modifiers, ... - + // fix hollowing, sla support points, modifiers, ... plater->changed_mesh(object_idx); close(); } @@ -414,14 +450,10 @@ void GLGizmoSimplify::on_set_state() if (GLGizmoBase::m_state == GLGizmoBase::Off) { m_parent.toggle_model_objects_visibility(true); - // Stop worker but do not wait to join it. - stop_worker_thread(false); - - // invalidate selected model - m_volume = nullptr; + stop_worker_thread(false); // Stop worker, don't wait for it. + m_volume = nullptr; // invalidate selected model + m_glmodel.reset(); } else if (GLGizmoBase::m_state == GLGizmoBase::On) { - // Make sure the worker is not running and join it. - stop_worker_thread(true); // when open by hyperlink it needs to show up request_rerender(); } @@ -507,6 +539,7 @@ void GLGizmoSimplify::init_model(const indexed_triangle_set& its) m_glmodel.reset(); m_glmodel.init_from(its); + m_parent.toggle_model_objects_visibility(true); // selected volume may have changed m_parent.toggle_model_objects_visibility(false, m_c->selection_info()->model_object(), m_c->selection_info()->get_active_instance(), m_volume); diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index 573b1ea518..b055f458f1 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -4,21 +4,13 @@ // Include GLGizmoBase.hpp before I18N.hpp as it includes some libigl code, // which overrides our localization "L" macro. #include "GLGizmoBase.hpp" -#include "GLGizmoPainterBase.hpp" // for render wireframe -#include "slic3r/GUI/GLModel.hpp" +#include "slic3r/GUI/3DScene.hpp" #include "admesh/stl.h" // indexed_triangle_set -#include #include -#include -#include - -#include // GLUint - -// for simplify suggestion -class ModelObjectPtrs; // std::vector namespace Slic3r { class ModelVolume; +class Model; namespace GUI { class NotificationManager; // for simplify suggestion @@ -31,7 +23,7 @@ public: bool on_esc_key_down(); static void add_simplify_suggestion_notification( const std::vector &object_ids, - const ModelObjectPtrs & objects, + const std::vector & objects, NotificationManager & manager); protected: @@ -81,6 +73,9 @@ private: return (use_count == rhs.use_count && decimate_ratio == rhs.decimate_ratio && wanted_count == rhs.wanted_count && max_error == rhs.max_error); } + bool operator!=(const Configuration& rhs) { + return ! (*this == rhs); + } }; Configuration m_configuration; From 9ad54ab4db769ed809a7ce2305b4a5c28d564d34 Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Sat, 30 Oct 2021 01:14:52 +0200 Subject: [PATCH 07/11] Some more cleanup --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 82 +++++++++-------------- src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 9 +-- 2 files changed, 31 insertions(+), 60 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index c5eada4a79..727fc07788 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -15,6 +15,26 @@ namespace Slic3r::GUI { +static ModelVolume* get_model_volume(const Selection& selection, Model& model) +{ + const Selection::IndicesList& idxs = selection.get_volume_idxs(); + // only one selected volume + if (idxs.size() != 1) + return nullptr; + const GLVolume* selected_volume = selection.get_volume(*idxs.begin()); + if (selected_volume == nullptr) + return nullptr; + + const GLVolume::CompositeID& cid = selected_volume->composite_id; + const ModelObjectPtrs& objs = model.objects; + if (cid.object_id < 0 || objs.size() <= static_cast(cid.object_id)) + return nullptr; + const ModelObject* obj = objs[cid.object_id]; + if (cid.volume_id < 0 || obj->volumes.size() <= static_cast(cid.volume_id)) + return nullptr; + return obj->volumes[cid.volume_id]; +} + GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, const std::string &icon_filename, unsigned int sprite_id) @@ -103,7 +123,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi { create_gui_cfg(); const Selection &selection = m_parent.get_selection(); - ModelVolume *act_volume = get_volume(selection, wxGetApp().plater()->model()); + const ModelVolume *act_volume = get_model_volume(selection, wxGetApp().plater()->model()); if (act_volume == nullptr) { stop_worker_thread(false); close(); @@ -428,11 +448,14 @@ void GLGizmoSimplify::apply_simplify() { plater->take_snapshot(_u8L("Simplify ") + m_volume->name); plater->clear_before_change_mesh(object_idx); - m_volume->set_mesh(std::move(*m_state.result)); + ModelVolume* mv = get_model_volume(selection, wxGetApp().model()); + assert(mv == m_volume); + + mv->set_mesh(std::move(*m_state.result)); m_state.result.reset(); - m_volume->calculate_convex_hull(); - m_volume->set_new_unique_id(); - m_volume->get_object()->invalidate_bounding_box(); + mv->calculate_convex_hull(); + mv->set_new_unique_id(); + mv->get_object()->invalidate_bounding_box(); // fix hollowing, sla support points, modifiers, ... plater->changed_mesh(object_idx); @@ -481,56 +504,16 @@ void GLGizmoSimplify::create_gui_cfg() { } void GLGizmoSimplify::request_rerender() { - wxGetApp().plater()->CallAfter([this]() { + //wxGetApp().plater()->CallAfter([this]() { set_dirty(); m_parent.schedule_extra_frame(0); - }); + //}); } void GLGizmoSimplify::set_center_position() { m_move_to_center = true; } -bool GLGizmoSimplify::exist_volume(const ModelVolume *volume) { - for (const ModelObject* obj : wxGetApp().plater()->model().objects) { - const auto & vlms = obj->volumes; - auto item = std::find(vlms.begin(), vlms.end(), volume); - if (item != vlms.end()) - return true; - } - return false; -} - -ModelVolume * GLGizmoSimplify::get_volume(const Selection &selection, Model &model) -{ - const Selection::IndicesList& idxs = selection.get_volume_idxs(); - // only one selected volume - if (idxs.size() != 1) - return nullptr; - const GLVolume *selected_volume = selection.get_volume(*idxs.begin()); - if (selected_volume == nullptr) - return nullptr; - - const GLVolume::CompositeID &cid = selected_volume->composite_id; - const ModelObjectPtrs& objs = model.objects; - if (cid.object_id < 0 || objs.size() <= static_cast(cid.object_id)) - return nullptr; - const ModelObject* obj = objs[cid.object_id]; - if (cid.volume_id < 0 || obj->volumes.size() <= static_cast(cid.volume_id)) - return nullptr; - return obj->volumes[cid.volume_id]; -} - -const ModelVolume *GLGizmoSimplify::get_volume(const GLVolume::CompositeID &cid, const Model &model) -{ - const ModelObjectPtrs &objs = model.objects; - if (cid.object_id < 0 || objs.size() <= static_cast(cid.object_id)) - return nullptr; - const ModelObject *obj = objs[cid.object_id]; - if (cid.volume_id < 0 || obj->volumes.size() <= static_cast(cid.volume_id)) - return nullptr; - return obj->volumes[cid.volume_id]; -} void GLGizmoSimplify::init_model(const indexed_triangle_set& its) { @@ -550,7 +533,6 @@ void GLGizmoSimplify::init_model(const indexed_triangle_set& its) void GLGizmoSimplify::on_render() { - // is initialized? if (! m_glmodel.is_initialized()) return; @@ -558,10 +540,6 @@ void GLGizmoSimplify::on_render() const auto& volume_idxs = selection.get_volume_idxs(); if (volume_idxs.empty() || volume_idxs.size() != 1) return; const GLVolume *selected_volume = selection.get_volume(*volume_idxs.begin()); - - // check that selected model is wireframe initialized - if (m_volume != get_volume(selected_volume->composite_id, *m_parent.get_model())) - return; const Transform3d trafo_matrix = selected_volume->world_matrix(); glsafe(::glPushMatrix()); diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index b055f458f1..10e89e3c8f 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -53,13 +53,6 @@ private: void init_model(const indexed_triangle_set& its); void set_center_position(); - // move to global functions - static ModelVolume *get_volume(const Selection &selection, Model &model); - static const ModelVolume *get_volume(const GLVolume::CompositeID &cid, const Model &model); - - // return false when volume was deleted - static bool exist_volume(const ModelVolume *volume); - struct Configuration { @@ -82,7 +75,7 @@ private: bool m_move_to_center; // opening gizmo - ModelVolume *m_volume; // keep pointer to actual working volume + const ModelVolume *m_volume; // keep pointer to actual working volume bool m_show_wireframe; GLModel m_glmodel; From 0bfa81be56ed316567076825ed07db9497114405 Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Mon, 1 Nov 2021 09:36:44 +0100 Subject: [PATCH 08/11] Several more fixes: - fixed crash on close when worker is running - refresh percentage in the UI by requesting extra frames - get rid of extra m_is_worker_running variable --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 78 ++++++++++++++--------- src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 9 ++- 2 files changed, 51 insertions(+), 36 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index 727fc07788..8718f67ce5 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -15,6 +15,19 @@ namespace Slic3r::GUI { +// Following flag and function allow to schedule a function call through CallAfter, +// but only run it when the gizmo is still alive. Scheduling a CallAfter from the +// background thread may trigger the code after the gizmo is destroyed. Having +// both the gizmo and the checking function static should solve it. +bool s_simplify_gizmo_destructor_run = false; +void call_after_if_alive(std::function fn) +{ + wxGetApp().CallAfter([fn]() { + if (! s_simplify_gizmo_destructor_run) + fn(); + }); +} + static ModelVolume* get_model_volume(const Selection& selection, Model& model) { const Selection::IndicesList& idxs = selection.get_volume_idxs(); @@ -49,8 +62,12 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, , tr_decimate_ratio(_u8L("Decimate ratio")) {} -GLGizmoSimplify::~GLGizmoSimplify() { - stop_worker_thread(true); +GLGizmoSimplify::~GLGizmoSimplify() +{ + s_simplify_gizmo_destructor_run = true; + stop_worker_thread_request(); + if (m_worker.joinable()) + m_worker.join(); m_glmodel.reset(); } @@ -58,7 +75,7 @@ bool GLGizmoSimplify::on_esc_key_down() { return false; /*if (!m_is_worker_running) return false; - stop_worker_thread(false); + stop_worker_thread_request(); return true;*/ } @@ -125,23 +142,25 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi const Selection &selection = m_parent.get_selection(); const ModelVolume *act_volume = get_model_volume(selection, wxGetApp().plater()->model()); if (act_volume == nullptr) { - stop_worker_thread(false); + stop_worker_thread_request(); close(); return; } bool is_cancelling = false; + bool is_worker_running = false; int progress = 0; { std::lock_guard lk(m_state_mutex); is_cancelling = m_state.status == State::cancelling; + is_worker_running = m_state.status == State::running; progress = m_state.progress; } // Check selection of new volume // Do not reselect object when processing - if (act_volume != m_volume && ! m_is_worker_running) { + if (act_volume != m_volume && ! is_worker_running) { bool change_window_position = (m_volume == nullptr); // select different model @@ -276,15 +295,15 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::SameLine(); - m_imgui->disabled_begin(m_is_worker_running); + m_imgui->disabled_begin(is_worker_running); if (m_imgui->button(_L("Apply"))) { apply_simplify(); - } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && m_is_worker_running) + } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && is_worker_running) ImGui::SetTooltip("%s", _u8L("Can't apply when proccess preview.").c_str()); m_imgui->disabled_end(); // state !settings // draw progress bar - if (m_is_worker_running) { // apply or preview + if (is_worker_running) { // apply or preview ImGui::SameLine(m_gui_cfg->bottom_left_width); // draw progress bar char buf[32]; @@ -315,17 +334,12 @@ void GLGizmoSimplify::close() { gizmos_mgr.open_gizmo(GLGizmosManager::EType::Simplify); } -void GLGizmoSimplify::stop_worker_thread(bool wait) +void GLGizmoSimplify::stop_worker_thread_request() { - { - std::lock_guard lk(m_state_mutex); - if (m_state.status == State::running) - m_state.status = State::Status::cancelling; - } - if (wait && m_worker.joinable()) { - m_worker.join(); - m_is_worker_running = false; - } + + std::lock_guard lk(m_state_mutex); + if (m_state.status == State::running) + m_state.status = State::Status::cancelling; } @@ -333,9 +347,8 @@ void GLGizmoSimplify::stop_worker_thread(bool wait) // worker calls it through a CallAfter. void GLGizmoSimplify::worker_finished() { - m_is_worker_running = false; if (! m_worker.joinable()) { - // Probably stop_worker_thread waited after cancel. + // Somebody already joined. // Nobody cares about the result apparently. assert(! m_state.result); return; @@ -359,25 +372,27 @@ void GLGizmoSimplify::process() bool configs_match = false; bool result_valid = false; + bool is_worker_running = false; { std::lock_guard lk(m_state_mutex); configs_match = m_state.config == m_configuration; result_valid = bool(m_state.result); + is_worker_running = m_state.status == State::running; } - if ((result_valid || m_is_worker_running) && configs_match) { + if ((result_valid || is_worker_running) && configs_match) { // Either finished or waiting for result already. Nothing to do. return; } - if (m_is_worker_running && m_state.config != m_configuration) { + if (is_worker_running && m_state.config != m_configuration) { // Worker is running with outdated config. Stop it. It will // restart itself when cancellation is done. - stop_worker_thread(false); + stop_worker_thread_request(); return; } - assert(! m_is_worker_running && ! m_worker.joinable()); + assert(! is_worker_running && ! m_worker.joinable()); // Copy configuration that will be used. m_state.config = m_configuration; @@ -397,9 +412,11 @@ void GLGizmoSimplify::process() }; // Called by worker thread, updates progress bar. + // Using CallAfter so the rerequest function is run in UI thread. std::function statusfn = [this](int percent) { std::lock_guard lk(m_state_mutex); m_state.progress = percent; + call_after_if_alive([this]() { request_rerender(); }); }; // Initialize. @@ -432,14 +449,11 @@ void GLGizmoSimplify::process() } // Update UI. Use CallAfter so the function is run on UI thread. - wxGetApp().CallAfter([this]() { worker_finished(); }); + call_after_if_alive([this]() { worker_finished(); }); }, std::move(its)); - - m_is_worker_running = true; } void GLGizmoSimplify::apply_simplify() { - assert(! m_is_worker_running); const Selection& selection = m_parent.get_selection(); int object_idx = selection.get_object_idx(); @@ -473,7 +487,7 @@ void GLGizmoSimplify::on_set_state() if (GLGizmoBase::m_state == GLGizmoBase::Off) { m_parent.toggle_model_objects_visibility(true); - stop_worker_thread(false); // Stop worker, don't wait for it. + stop_worker_thread_request(); m_volume = nullptr; // invalidate selected model m_glmodel.reset(); } else if (GLGizmoBase::m_state == GLGizmoBase::On) { @@ -504,10 +518,12 @@ void GLGizmoSimplify::create_gui_cfg() { } void GLGizmoSimplify::request_rerender() { - //wxGetApp().plater()->CallAfter([this]() { + int64_t now = m_parent.timestamp_now(); + if (now > m_last_rerender_timestamp + 250) { // 250 ms set_dirty(); m_parent.schedule_extra_frame(0); - //}); + m_last_rerender_timestamp = now; + } } void GLGizmoSimplify::set_center_position() { diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index 10e89e3c8f..71907917b9 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -45,7 +45,7 @@ private: void close(); void process(); - void stop_worker_thread(bool wait); + void stop_worker_thread_request(); void worker_finished(); void create_gui_cfg(); @@ -81,6 +81,9 @@ private: GLModel m_glmodel; size_t m_triangle_count; // triangle count of the model currently shown + // Timestamp of the last rerender request. Only accessed from UI thread. + int64_t m_last_rerender_timestamp = std::numeric_limits::min(); + // Following struct is accessed by both UI and worker thread. // Accesses protected by a mutex. struct State { @@ -100,10 +103,6 @@ private: std::mutex m_state_mutex; // guards m_state State m_state; // accessed by both threads - // Following variable is accessed by UI only. - bool m_is_worker_running = false; - - // This configs holds GUI layout size given by translated texts. // etc. When language changes, GUI is recreated and this class constructed again, From a61c892c0588d14aca98794cca00374f6db65b6c Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Mon, 1 Nov 2021 11:21:50 +0100 Subject: [PATCH 09/11] Some more fixes when switching objects while simplifying --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 85 +++++++++++++---------- src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp | 3 +- 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index 8718f67ce5..e817fe9607 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -19,7 +19,7 @@ namespace Slic3r::GUI { // but only run it when the gizmo is still alive. Scheduling a CallAfter from the // background thread may trigger the code after the gizmo is destroyed. Having // both the gizmo and the checking function static should solve it. -bool s_simplify_gizmo_destructor_run = false; +static bool s_simplify_gizmo_destructor_run = false; void call_after_if_alive(std::function fn) { wxGetApp().CallAfter([fn]() { @@ -149,18 +149,23 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi bool is_cancelling = false; bool is_worker_running = false; + bool is_result_ready = false; int progress = 0; { std::lock_guard lk(m_state_mutex); is_cancelling = m_state.status == State::cancelling; is_worker_running = m_state.status == State::running; + is_result_ready = bool(m_state.result); progress = m_state.progress; } + + // Whether to trigger calculation after rendering is done. + bool start_process = false; // Check selection of new volume // Do not reselect object when processing - if (act_volume != m_volume && ! is_worker_running) { + if (act_volume != m_volume) { bool change_window_position = (m_volume == nullptr); // select different model @@ -172,7 +177,10 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi m_configuration.decimate_ratio = 50.; // default value m_configuration.fix_count_by_ratio(m_volume->mesh().its.indices.size()); init_model(m_volume->mesh().its); - process(); + + // Start processing. If we switched from another object, process will + // stop the background thread and it will restart itself later. + start_process = true; // set window position if (m_move_to_center && change_window_position) { @@ -220,7 +228,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi if(ImGui::RadioButton("##use_error", !m_configuration.use_count)) { m_configuration.use_count = !m_configuration.use_count; - process(); + start_process = true; } ImGui::SameLine(); m_imgui->disabled_begin(m_configuration.use_count); @@ -245,19 +253,18 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi case 3: m_configuration.max_error = 0.5f; break; case 4: m_configuration.max_error = 1.f; break; } - process(); + start_process = true; } m_imgui->disabled_end(); // !use_count if (ImGui::RadioButton("##use_count", m_configuration.use_count)) { m_configuration.use_count = !m_configuration.use_count; - process(); + start_process = true; } ImGui::SameLine(); // show preview result triangle count (percent) - // lm_FIXME - if (/*m_need_reload &&*/ !m_configuration.use_count) { + if (!m_configuration.use_count) { m_configuration.wanted_count = static_cast(m_triangle_count); m_configuration.decimate_ratio = (1.0f - (m_configuration.wanted_count / (float) orig_triangle_count)) * 100.f; @@ -276,7 +283,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi if (m_configuration.decimate_ratio > 100.f) m_configuration.decimate_ratio = 100.f; m_configuration.fix_count_by_ratio(orig_triangle_count); - process(); + start_process = true; } ImGui::NewLine(); @@ -295,7 +302,7 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi ImGui::SameLine(); - m_imgui->disabled_begin(is_worker_running); + m_imgui->disabled_begin(is_worker_running || ! is_result_ready); if (m_imgui->button(_L("Apply"))) { apply_simplify(); } else if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && is_worker_running) @@ -312,19 +319,8 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi } m_imgui->end(); - // refresh view when needed - // lm_FIXME - /*if (m_need_reload) { - m_need_reload = false; - bool close_on_end = (m_state.status == State::close_on_end); - request_rerender(); - // set m_state.status must be before close() !!! - m_state.status = State::settings; - if (close_on_end) after_apply(); - else init_model(m_volume->mesh().its); - // Fix warning icon in object list - wxGetApp().obj_list()->update_item_error_icon(m_obj_index, -1); - }*/ + if (start_process) + process(); } @@ -336,7 +332,6 @@ void GLGizmoSimplify::close() { void GLGizmoSimplify::stop_worker_thread_request() { - std::lock_guard lk(m_state_mutex); if (m_state.status == State::running) m_state.status = State::Status::cancelling; @@ -347,22 +342,25 @@ void GLGizmoSimplify::stop_worker_thread_request() // worker calls it through a CallAfter. void GLGizmoSimplify::worker_finished() { - if (! m_worker.joinable()) { - // Somebody already joined. - // Nobody cares about the result apparently. - assert(! m_state.result); - return; + { + std::lock_guard lk(m_state_mutex); + if (m_state.status == State::running) { + // Someone started the worker again, before this callback + // was called. Do nothing. + return; + } } - m_worker.join(); + if (m_worker.joinable()) + m_worker.join(); if (GLGizmoBase::m_state == Off) return; if (m_state.result) init_model(*m_state.result); - if (m_state.config != m_configuration) { + if (m_state.config != m_configuration || m_state.mv != m_volume) { // Settings were changed, restart the worker immediately. process(); } - request_rerender(); + request_rerender(true); } void GLGizmoSimplify::process() @@ -375,7 +373,7 @@ void GLGizmoSimplify::process() bool is_worker_running = false; { std::lock_guard lk(m_state_mutex); - configs_match = m_state.config == m_configuration; + configs_match = (m_volume == m_state.mv && m_state.config == m_configuration); result_valid = bool(m_state.result); is_worker_running = m_state.status == State::running; } @@ -385,17 +383,24 @@ void GLGizmoSimplify::process() return; } - if (is_worker_running && m_state.config != m_configuration) { + if (is_worker_running && ! configs_match) { // Worker is running with outdated config. Stop it. It will // restart itself when cancellation is done. stop_worker_thread_request(); return; } - assert(! is_worker_running && ! m_worker.joinable()); + if (m_worker.joinable()) { + // This can happen when process() is called after previous worker terminated, + // but before the worker_finished callback was called. In this case, just join the thread, + // the callback will check this and do nothing. + m_worker.join(); + } // Copy configuration that will be used. m_state.config = m_configuration; + m_state.mv = m_volume; + m_state.status = State::running; // Create a copy of current mesh to pass to the worker thread. // Using unique_ptr instead of pass-by-value to avoid an extra @@ -473,6 +478,8 @@ void GLGizmoSimplify::apply_simplify() { // fix hollowing, sla support points, modifiers, ... plater->changed_mesh(object_idx); + // Fix warning icon in object list + wxGetApp().obj_list()->update_item_error_icon(object_idx, -1); close(); } @@ -517,9 +524,9 @@ void GLGizmoSimplify::create_gui_cfg() { m_gui_cfg = cfg; } -void GLGizmoSimplify::request_rerender() { +void GLGizmoSimplify::request_rerender(bool force) { int64_t now = m_parent.timestamp_now(); - if (now > m_last_rerender_timestamp + 250) { // 250 ms + if (force || now > m_last_rerender_timestamp + 250) { // 250 ms set_dirty(); m_parent.schedule_extra_frame(0); m_last_rerender_timestamp = now; @@ -557,6 +564,10 @@ void GLGizmoSimplify::on_render() if (volume_idxs.empty() || volume_idxs.size() != 1) return; const GLVolume *selected_volume = selection.get_volume(*volume_idxs.begin()); + // Check that the GLVolume still belongs to the ModelObject we work on. + if (m_volume != get_model_volume(selection, wxGetApp().model())) + return; + const Transform3d trafo_matrix = selected_volume->world_matrix(); glsafe(::glPushMatrix()); glsafe(::glMultMatrixd(trafo_matrix.data())); diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp index 71907917b9..e6367ee275 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.hpp @@ -49,7 +49,7 @@ private: void worker_finished(); void create_gui_cfg(); - void request_rerender(); + void request_rerender(bool force = false); void init_model(const indexed_triangle_set& its); void set_center_position(); @@ -96,6 +96,7 @@ private: Status status = idle; int progress = 0; // percent of done work Configuration config; // Configuration we started with. + const ModelVolume* mv = nullptr; std::unique_ptr result; }; From d72fba2a6a6af8876c2afa2424745ee7f6f0977e Mon Sep 17 00:00:00 2001 From: Filip Sykala Date: Mon, 8 Nov 2021 15:09:52 +0100 Subject: [PATCH 10/11] translation of progress --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index e817fe9607..6c9e1b9fa3 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -313,12 +313,11 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi if (is_worker_running) { // apply or preview ImGui::SameLine(m_gui_cfg->bottom_left_width); // draw progress bar - char buf[32]; - sprintf(buf, L("Process %d / 100"), progress); - ImGui::ProgressBar(progress / 100., ImVec2(m_gui_cfg->input_width, 0.f), buf); + std::string progress_text = GUI::format(_L("Process %1% / 100"), std::to_string(progress)); + ImVec2 progress_size(m_gui_cfg->input_width, 0.f); + ImGui::ProgressBar(progress / 100., progress_size, progress_text.c_str()); } m_imgui->end(); - if (start_process) process(); } From 934ed0bbae99b845cc5b4458eead1ac7895b2a56 Mon Sep 17 00:00:00 2001 From: Filip Sykala Date: Mon, 8 Nov 2021 16:54:00 +0100 Subject: [PATCH 11/11] Remove static flag and extend case when call after will be from unactive Gizmo. --- src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp | 31 +++++++++++++---------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp index 6c9e1b9fa3..b71443b576 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSimplify.cpp @@ -15,16 +15,22 @@ namespace Slic3r::GUI { -// Following flag and function allow to schedule a function call through CallAfter, -// but only run it when the gizmo is still alive. Scheduling a CallAfter from the -// background thread may trigger the code after the gizmo is destroyed. Having -// both the gizmo and the checking function static should solve it. -static bool s_simplify_gizmo_destructor_run = false; -void call_after_if_alive(std::function fn) +// Extend call after only when Simplify gizmo is still alive +static void call_after_if_active(std::function fn, GUI_App* app = &wxGetApp()) { - wxGetApp().CallAfter([fn]() { - if (! s_simplify_gizmo_destructor_run) - fn(); + // check application GUI + if (app == nullptr) return; + app->CallAfter([fn, app]() { + // app must exist because it call this + // if (app == nullptr) return; + const Plater *plater = app->plater(); + if (plater == nullptr) return; + const GLCanvas3D *canvas = plater->canvas3D(); + if (canvas == nullptr) return; + const GLGizmosManager &mng = canvas->get_gizmos_manager(); + // check if simplify is still activ gizmo + if (mng.get_current_type() != GLGizmosManager::Simplify) return; + fn(); }); } @@ -63,8 +69,7 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent, {} GLGizmoSimplify::~GLGizmoSimplify() -{ - s_simplify_gizmo_destructor_run = true; +{ stop_worker_thread_request(); if (m_worker.joinable()) m_worker.join(); @@ -420,7 +425,7 @@ void GLGizmoSimplify::process() std::function statusfn = [this](int percent) { std::lock_guard lk(m_state_mutex); m_state.progress = percent; - call_after_if_alive([this]() { request_rerender(); }); + call_after_if_active([this]() { request_rerender(); }); }; // Initialize. @@ -453,7 +458,7 @@ void GLGizmoSimplify::process() } // Update UI. Use CallAfter so the function is run on UI thread. - call_after_if_alive([this]() { worker_finished(); }); + call_after_if_active([this]() { worker_finished(); }); }, std::move(its)); }