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
This commit is contained in:
Lukas Matena 2021-11-01 09:36:44 +01:00
parent 9ad54ab4db
commit 0bfa81be56
2 changed files with 51 additions and 36 deletions

View file

@ -15,6 +15,19 @@
namespace Slic3r::GUI { 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<void()> fn)
{
wxGetApp().CallAfter([fn]() {
if (! s_simplify_gizmo_destructor_run)
fn();
});
}
static ModelVolume* get_model_volume(const Selection& selection, Model& model) static ModelVolume* get_model_volume(const Selection& selection, Model& model)
{ {
const Selection::IndicesList& idxs = selection.get_volume_idxs(); const Selection::IndicesList& idxs = selection.get_volume_idxs();
@ -49,8 +62,12 @@ GLGizmoSimplify::GLGizmoSimplify(GLCanvas3D & parent,
, tr_decimate_ratio(_u8L("Decimate ratio")) , tr_decimate_ratio(_u8L("Decimate ratio"))
{} {}
GLGizmoSimplify::~GLGizmoSimplify() { GLGizmoSimplify::~GLGizmoSimplify()
stop_worker_thread(true); {
s_simplify_gizmo_destructor_run = true;
stop_worker_thread_request();
if (m_worker.joinable())
m_worker.join();
m_glmodel.reset(); m_glmodel.reset();
} }
@ -58,7 +75,7 @@ bool GLGizmoSimplify::on_esc_key_down() {
return false; return false;
/*if (!m_is_worker_running) /*if (!m_is_worker_running)
return false; return false;
stop_worker_thread(false); stop_worker_thread_request();
return true;*/ 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 Selection &selection = m_parent.get_selection();
const ModelVolume *act_volume = get_model_volume(selection, wxGetApp().plater()->model()); const ModelVolume *act_volume = get_model_volume(selection, wxGetApp().plater()->model());
if (act_volume == nullptr) { if (act_volume == nullptr) {
stop_worker_thread(false); stop_worker_thread_request();
close(); close();
return; return;
} }
bool is_cancelling = false; bool is_cancelling = false;
bool is_worker_running = false;
int progress = 0; int progress = 0;
{ {
std::lock_guard lk(m_state_mutex); std::lock_guard lk(m_state_mutex);
is_cancelling = m_state.status == State::cancelling; is_cancelling = m_state.status == State::cancelling;
is_worker_running = m_state.status == State::running;
progress = m_state.progress; progress = m_state.progress;
} }
// Check selection of new volume // Check selection of new volume
// Do not reselect object when processing // 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); bool change_window_position = (m_volume == nullptr);
// select different model // select different model
@ -276,15 +295,15 @@ void GLGizmoSimplify::on_render_input_window(float x, float y, float bottom_limi
ImGui::SameLine(); ImGui::SameLine();
m_imgui->disabled_begin(m_is_worker_running); m_imgui->disabled_begin(is_worker_running);
if (m_imgui->button(_L("Apply"))) { if (m_imgui->button(_L("Apply"))) {
apply_simplify(); 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()); ImGui::SetTooltip("%s", _u8L("Can't apply when proccess preview.").c_str());
m_imgui->disabled_end(); // state !settings m_imgui->disabled_end(); // state !settings
// draw progress bar // 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); ImGui::SameLine(m_gui_cfg->bottom_left_width);
// draw progress bar // draw progress bar
char buf[32]; char buf[32];
@ -315,17 +334,12 @@ void GLGizmoSimplify::close() {
gizmos_mgr.open_gizmo(GLGizmosManager::EType::Simplify); 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); std::lock_guard lk(m_state_mutex);
if (m_state.status == State::running) if (m_state.status == State::running)
m_state.status = State::Status::cancelling; m_state.status = State::Status::cancelling;
}
if (wait && m_worker.joinable()) {
m_worker.join();
m_is_worker_running = false;
}
} }
@ -333,9 +347,8 @@ void GLGizmoSimplify::stop_worker_thread(bool wait)
// worker calls it through a CallAfter. // worker calls it through a CallAfter.
void GLGizmoSimplify::worker_finished() void GLGizmoSimplify::worker_finished()
{ {
m_is_worker_running = false;
if (! m_worker.joinable()) { if (! m_worker.joinable()) {
// Probably stop_worker_thread waited after cancel. // Somebody already joined.
// Nobody cares about the result apparently. // Nobody cares about the result apparently.
assert(! m_state.result); assert(! m_state.result);
return; return;
@ -359,25 +372,27 @@ void GLGizmoSimplify::process()
bool configs_match = false; bool configs_match = false;
bool result_valid = false; bool result_valid = false;
bool is_worker_running = false;
{ {
std::lock_guard lk(m_state_mutex); std::lock_guard lk(m_state_mutex);
configs_match = m_state.config == m_configuration; configs_match = m_state.config == m_configuration;
result_valid = bool(m_state.result); 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. // Either finished or waiting for result already. Nothing to do.
return; 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 // Worker is running with outdated config. Stop it. It will
// restart itself when cancellation is done. // restart itself when cancellation is done.
stop_worker_thread(false); stop_worker_thread_request();
return; return;
} }
assert(! m_is_worker_running && ! m_worker.joinable()); assert(! is_worker_running && ! m_worker.joinable());
// Copy configuration that will be used. // Copy configuration that will be used.
m_state.config = m_configuration; m_state.config = m_configuration;
@ -397,9 +412,11 @@ void GLGizmoSimplify::process()
}; };
// Called by worker thread, updates progress bar. // Called by worker thread, updates progress bar.
// Using CallAfter so the rerequest function is run in UI thread.
std::function<void(int)> statusfn = [this](int percent) { std::function<void(int)> statusfn = [this](int percent) {
std::lock_guard lk(m_state_mutex); std::lock_guard lk(m_state_mutex);
m_state.progress = percent; m_state.progress = percent;
call_after_if_alive([this]() { request_rerender(); });
}; };
// Initialize. // Initialize.
@ -432,14 +449,11 @@ void GLGizmoSimplify::process()
} }
// Update UI. Use CallAfter so the function is run on UI thread. // 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)); }, std::move(its));
m_is_worker_running = true;
} }
void GLGizmoSimplify::apply_simplify() { void GLGizmoSimplify::apply_simplify() {
assert(! m_is_worker_running);
const Selection& selection = m_parent.get_selection(); const Selection& selection = m_parent.get_selection();
int object_idx = selection.get_object_idx(); int object_idx = selection.get_object_idx();
@ -473,7 +487,7 @@ void GLGizmoSimplify::on_set_state()
if (GLGizmoBase::m_state == GLGizmoBase::Off) { if (GLGizmoBase::m_state == GLGizmoBase::Off) {
m_parent.toggle_model_objects_visibility(true); 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_volume = nullptr; // invalidate selected model
m_glmodel.reset(); m_glmodel.reset();
} else if (GLGizmoBase::m_state == GLGizmoBase::On) { } else if (GLGizmoBase::m_state == GLGizmoBase::On) {
@ -504,10 +518,12 @@ void GLGizmoSimplify::create_gui_cfg() {
} }
void GLGizmoSimplify::request_rerender() { 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(); set_dirty();
m_parent.schedule_extra_frame(0); m_parent.schedule_extra_frame(0);
//}); m_last_rerender_timestamp = now;
}
} }
void GLGizmoSimplify::set_center_position() { void GLGizmoSimplify::set_center_position() {

View file

@ -45,7 +45,7 @@ private:
void close(); void close();
void process(); void process();
void stop_worker_thread(bool wait); void stop_worker_thread_request();
void worker_finished(); void worker_finished();
void create_gui_cfg(); void create_gui_cfg();
@ -81,6 +81,9 @@ private:
GLModel m_glmodel; GLModel m_glmodel;
size_t m_triangle_count; // triangle count of the model currently shown 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<int64_t>::min();
// Following struct is accessed by both UI and worker thread. // Following struct is accessed by both UI and worker thread.
// Accesses protected by a mutex. // Accesses protected by a mutex.
struct State { struct State {
@ -100,10 +103,6 @@ private:
std::mutex m_state_mutex; // guards m_state std::mutex m_state_mutex; // guards m_state
State m_state; // accessed by both threads 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. // This configs holds GUI layout size given by translated texts.
// etc. When language changes, GUI is recreated and this class constructed again, // etc. When language changes, GUI is recreated and this class constructed again,