From b53ff75cf460d0bc5404729c1d02dddf3a6224d5 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Tue, 30 Nov 2021 16:02:49 +0100 Subject: [PATCH 01/13] Rework UI jobs to make them more understandable and flexible. --- src/slic3r/CMakeLists.txt | 8 +- src/slic3r/GUI/GUI_Init.cpp | 2 + src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp | 6 +- src/slic3r/GUI/Gizmos/GLGizmoRotate.hpp | 1 - src/slic3r/GUI/Jobs/ArrangeJob.cpp | 64 +++---- src/slic3r/GUI/Jobs/ArrangeJob.hpp | 30 ++-- src/slic3r/GUI/Jobs/BoostThreadWorker.cpp | 137 +++++++++++++++ src/slic3r/GUI/Jobs/BoostThreadWorker.hpp | 132 +++++++++++++++ src/slic3r/GUI/Jobs/BusyCursorJob.hpp | 48 ++++++ src/slic3r/GUI/Jobs/FillBedJob.cpp | 32 ++-- src/slic3r/GUI/Jobs/FillBedJob.hpp | 21 +-- src/slic3r/GUI/Jobs/Job.cpp | 158 ------------------ src/slic3r/GUI/Jobs/Job.hpp | 138 ++++----------- .../Jobs/NotificationProgressIndicator.cpp | 6 +- .../Jobs/NotificationProgressIndicator.hpp | 2 +- src/slic3r/GUI/Jobs/PlaterJob.cpp | 17 -- src/slic3r/GUI/Jobs/PlaterJob.hpp | 60 ++++++- src/slic3r/GUI/Jobs/RotoptimizeJob.cpp | 33 ++-- src/slic3r/GUI/Jobs/RotoptimizeJob.hpp | 24 ++- src/slic3r/GUI/Jobs/SLAImportDialog.hpp | 114 +++++++++++++ src/slic3r/GUI/Jobs/SLAImportJob.cpp | 149 ++++------------- src/slic3r/GUI/Jobs/SLAImportJob.hpp | 31 +++- src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp | 93 +++++++++++ src/slic3r/GUI/Jobs/Worker.hpp | 92 ++++++++++ src/slic3r/GUI/MainFrame.cpp | 4 +- src/slic3r/GUI/Plater.cpp | 107 ++++-------- src/slic3r/GUI/Plater.hpp | 39 ++++- 27 files changed, 956 insertions(+), 592 deletions(-) create mode 100644 src/slic3r/GUI/Jobs/BoostThreadWorker.cpp create mode 100644 src/slic3r/GUI/Jobs/BoostThreadWorker.hpp create mode 100644 src/slic3r/GUI/Jobs/BusyCursorJob.hpp delete mode 100644 src/slic3r/GUI/Jobs/Job.cpp delete mode 100644 src/slic3r/GUI/Jobs/PlaterJob.cpp create mode 100644 src/slic3r/GUI/Jobs/SLAImportDialog.hpp create mode 100644 src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp create mode 100644 src/slic3r/GUI/Jobs/Worker.hpp diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index 211a2c2e7a..f8a0124ced 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -169,9 +169,11 @@ set(SLIC3R_GUI_SOURCES GUI/PrintHostDialogs.cpp GUI/PrintHostDialogs.hpp GUI/Jobs/Job.hpp - GUI/Jobs/Job.cpp + GUI/Jobs/Worker.hpp + GUI/Jobs/BoostThreadWorker.hpp + GUI/Jobs/BoostThreadWorker.cpp + GUI/Jobs/BusyCursorJob.hpp GUI/Jobs/PlaterJob.hpp - GUI/Jobs/PlaterJob.cpp GUI/Jobs/ArrangeJob.hpp GUI/Jobs/ArrangeJob.cpp GUI/Jobs/RotoptimizeJob.hpp @@ -183,6 +185,8 @@ set(SLIC3R_GUI_SOURCES GUI/Jobs/ProgressIndicator.hpp GUI/Jobs/NotificationProgressIndicator.hpp GUI/Jobs/NotificationProgressIndicator.cpp + GUI/Jobs/ThreadSafeQueue.hpp + GUI/Jobs/SLAImportDialog.hpp GUI/ProgressStatusBar.hpp GUI/ProgressStatusBar.cpp GUI/Mouse3DController.cpp diff --git a/src/slic3r/GUI/GUI_Init.cpp b/src/slic3r/GUI/GUI_Init.cpp index 92223a767d..000199f93a 100644 --- a/src/slic3r/GUI/GUI_Init.cpp +++ b/src/slic3r/GUI/GUI_Init.cpp @@ -9,6 +9,8 @@ #include "slic3r/GUI/format.hpp" #include "slic3r/GUI/MainFrame.hpp" #include "slic3r/GUI/Plater.hpp" +#include "slic3r/GUI/I18N.hpp" + // To show a message box if GUI initialization ends up with an exception thrown. #include diff --git a/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp b/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp index a234a19ff9..bb70150c6f 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp @@ -11,6 +11,7 @@ #include "libslic3r/PresetBundle.hpp" #include "slic3r/GUI/Jobs/RotoptimizeJob.hpp" +#include "slic3r/GUI/Jobs/PlaterJob.hpp" namespace Slic3r { namespace GUI { @@ -540,11 +541,12 @@ GLGizmoRotate3D::RotoptimzeWindow::RotoptimzeWindow(ImGuiWrapper * imgui, ImVec2 button_sz = {btn_txt_sz.x + padding.x, btn_txt_sz.y + padding.y}; ImGui::SetCursorPosX(padding.x + sz.x - button_sz.x); - if (wxGetApp().plater()->is_any_job_running()) + if (!wxGetApp().plater()->get_ui_job_worker().is_idle()) imgui->disabled_begin(true); if ( imgui->button(btn_txt) ) { - wxGetApp().plater()->optimize_rotation(); + auto p = wxGetApp().plater(); + replace_job(*p); } imgui->disabled_end(); diff --git a/src/slic3r/GUI/Gizmos/GLGizmoRotate.hpp b/src/slic3r/GUI/Gizmos/GLGizmoRotate.hpp index af1ecf548e..448efd98a0 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoRotate.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoRotate.hpp @@ -2,7 +2,6 @@ #define slic3r_GLGizmoRotate_hpp_ #include "GLGizmoBase.hpp" -#include "../Jobs/RotoptimizeJob.hpp" namespace Slic3r { namespace GUI { diff --git a/src/slic3r/GUI/Jobs/ArrangeJob.cpp b/src/slic3r/GUI/Jobs/ArrangeJob.cpp index 2771f9d271..500fc3a10d 100644 --- a/src/slic3r/GUI/Jobs/ArrangeJob.cpp +++ b/src/slic3r/GUI/Jobs/ArrangeJob.cpp @@ -162,49 +162,42 @@ void ArrangeJob::prepare() wxGetKeyState(WXK_SHIFT) ? prepare_selected() : prepare_all(); } -void ArrangeJob::on_exception(const std::exception_ptr &eptr) +void ArrangeJob::process(Ctl &ctl) { - try { - if (eptr) - std::rethrow_exception(eptr); - } catch (libnest2d::GeometryException &) { - show_error(m_plater, _(L("Could not arrange model objects! " - "Some geometries may be invalid."))); - } catch (std::exception &) { - PlaterJob::on_exception(eptr); - } -} + ctl.call_on_main_thread([this]{ prepare(); }).wait(); -void ArrangeJob::process() -{ - static const auto arrangestr = _(L("Arranging")); + static const auto arrangestr = _u8L("Arranging"); arrangement::ArrangeParams params = get_arrange_params(m_plater); - auto count = unsigned(m_selected.size() + m_unprintable.size()); + auto count = unsigned(m_selected.size() + m_unprintable.size()); Points bedpts = get_bed_shape(*m_plater->config()); - - params.stopcondition = [this]() { return was_canceled(); }; - - params.progressind = [this, count](unsigned st) { + + params.stopcondition = [&ctl]() { return ctl.was_canceled(); }; + + params.progressind = [this, count, &ctl](unsigned st) { st += m_unprintable.size(); - if (st > 0) update_status(int(count - st), arrangestr); + if (st > 0) ctl.update_status(int(count - st) * 100 / status_range(), arrangestr); }; + ctl.update_status(0, arrangestr); + arrangement::arrange(m_selected, m_unselected, bedpts, params); - params.progressind = [this, count](unsigned st) { - if (st > 0) update_status(int(count - st), arrangestr); + params.progressind = [this, count, &ctl](unsigned st) { + if (st > 0) ctl.update_status(int(count - st) * 100 / status_range(), arrangestr); }; arrangement::arrange(m_unprintable, {}, bedpts, params); // finalize just here. - update_status(int(count), - was_canceled() ? _(L("Arranging canceled.")) - : _(L("Arranging done."))); + ctl.update_status(int(count) * 100 / status_range(), ctl.was_canceled() ? + _u8L("Arranging canceled.") : + _u8L("Arranging done.")); } +ArrangeJob::ArrangeJob() : m_plater{wxGetApp().plater()} { } + static std::string concat_strings(const std::set &strings, const std::string &delim = "\n") { @@ -215,10 +208,21 @@ static std::string concat_strings(const std::set &strings, }); } -void ArrangeJob::finalize() { - // Ignore the arrange result if aborted. - if (was_canceled()) return; - +void ArrangeJob::finalize(bool canceled, std::exception_ptr &eptr) { + try { + if (eptr) + std::rethrow_exception(eptr); + } catch (libnest2d::GeometryException &) { + show_error(m_plater, _(L("Could not arrange model objects! " + "Some geometries may be invalid."))); + eptr = nullptr; + } catch(...) { + eptr = std::current_exception(); + } + + if (canceled || eptr) + return; + // Unprintable items go to the last virtual bed int beds = 0; @@ -250,8 +254,6 @@ void ArrangeJob::finalize() { _L("Arrangement ignored the following objects which can't fit into a single bed:\n%s"), concat_strings(names, "\n"))); } - - Job::finalize(); } std::optional diff --git a/src/slic3r/GUI/Jobs/ArrangeJob.hpp b/src/slic3r/GUI/Jobs/ArrangeJob.hpp index a5ecc0c834..106cc57ddf 100644 --- a/src/slic3r/GUI/Jobs/ArrangeJob.hpp +++ b/src/slic3r/GUI/Jobs/ArrangeJob.hpp @@ -1,7 +1,9 @@ #ifndef ARRANGEJOB_HPP #define ARRANGEJOB_HPP -#include "PlaterJob.hpp" +#include + +#include "Job.hpp" #include "libslic3r/Arrange.hpp" namespace Slic3r { @@ -10,13 +12,16 @@ class ModelInstance; namespace GUI { -class ArrangeJob : public PlaterJob +class Plater; + +class ArrangeJob : public Job { using ArrangePolygon = arrangement::ArrangePolygon; using ArrangePolygons = arrangement::ArrangePolygons; ArrangePolygons m_selected, m_unselected, m_unprintable; std::vector m_unarranged; + Plater *m_plater; // clear m_selected and m_unselected, reserve space for next usage void clear_input(); @@ -30,25 +35,20 @@ class ArrangeJob : public PlaterJob ArrangePolygon get_arrange_poly_(ModelInstance *mi); -protected: - - void prepare() override; - - void on_exception(const std::exception_ptr &) override; - - void process() override; - public: - ArrangeJob(std::shared_ptr pri, Plater *plater) - : PlaterJob{std::move(pri), plater} - {} - int status_range() const override + void prepare(); + + void process(Ctl &ctl) override; + + ArrangeJob(); + + int status_range() const { return int(m_selected.size() + m_unprintable.size()); } - void finalize() override; + void finalize(bool canceled, std::exception_ptr &e) override; }; std::optional get_wipe_tower_arrangepoly(const Plater &); diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp new file mode 100644 index 0000000000..3d0cdb678f --- /dev/null +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp @@ -0,0 +1,137 @@ +#include + +#include "BoostThreadWorker.hpp" + +namespace Slic3r { namespace GUI { + +void BoostThreadWorker::WorkerMessage::deliver(BoostThreadWorker &runner) +{ + switch(MsgType(m_data.index())) { + case Empty: break; + case Status: { + StatusInfo info = std::get(m_data); + if (runner.get_pri()) { + runner.get_pri()->set_progress(info.status); + runner.get_pri()->set_status_text(info.msg.c_str()); + } + break; + } + case Finalize: { + JobEntry& entry = std::get(m_data); + entry.job->finalize(entry.canceled, entry.eptr); + + // Unhandled exceptions are rethrown without mercy. + if (entry.eptr) + std::rethrow_exception(entry.eptr); + + break; + } + case MainThreadCall: { + MainThreadCallData &calldata = std::get(m_data); + calldata.fn(); + calldata.barrier.set_value(); + + break; + } + } +} + +void BoostThreadWorker::run() +{ + bool stop = false; + while(!stop) { + m_input_queue.consume_one_blk([this, &stop](JobEntry &e) { + if (!e.job) + stop = true; + else { + m_canceled.store(false); + m_running.store(true); + + try { + e.job->process(*this); + } catch (...) { + e.eptr = std::current_exception(); + } + + e.canceled = m_canceled.load(); + m_output_queue.push(std::move(e)); // finalization message + m_running.store(false); + } + }); + }; +} + +void BoostThreadWorker::update_status(int st, const std::string &msg) +{ + m_output_queue.push(st, msg); +} + +std::future BoostThreadWorker::call_on_main_thread(std::function fn) +{ + MainThreadCallData cbdata{std::move(fn), {}}; + std::future future = cbdata.barrier.get_future(); + + m_output_queue.push(std::move(cbdata)); + + return future; +} + +BoostThreadWorker::BoostThreadWorker(std::shared_ptr pri, + boost::thread::attributes &attribs, + const char * name) + : m_progress(std::move(pri)), m_name{name} +{ + if (m_progress) + m_progress->set_cancel_callback([this](){ cancel(); }); + + m_thread = create_thread(attribs, [this] { this->run(); }); + + std::string nm{name}; + if (!nm.empty()) set_thread_name(m_thread, name); +} + +constexpr int ABORT_WAIT_MAX_MS = 10000; + +BoostThreadWorker::~BoostThreadWorker() +{ + replace_job(*this, nullptr); + + try { + join(ABORT_WAIT_MAX_MS); + } catch(...) { + BOOST_LOG_TRIVIAL(error) + << "Could not join worker thread '" << m_name << "'"; + } +} + +bool BoostThreadWorker::join(int timeout_ms) +{ + if (!m_thread.joinable()) + return true; + + if (timeout_ms <= 0) { + m_thread.join(); + } + else if (m_thread.try_join_for(boost::chrono::milliseconds(timeout_ms))) { + return true; + } + else + return false; + + return true; +} + +void BoostThreadWorker::process_events() +{ + while (m_output_queue.consume_one([this](WorkerMessage &msg) { + msg.deliver(*this); + })); +} + +bool BoostThreadWorker::start_next(std::unique_ptr job) +{ + m_input_queue.push(JobEntry{std::move(job)}); + return true; +} + +}} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp new file mode 100644 index 0000000000..aeacf39878 --- /dev/null +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp @@ -0,0 +1,132 @@ +#ifndef BOOSTTHREADWORKER_HPP +#define BOOSTTHREADWORKER_HPP + +#include + +#include "Worker.hpp" + +#include + +#include +#include + +#include "ThreadSafeQueue.hpp" + +namespace Slic3r { namespace GUI { + +// An implementation of the Worker interface which uses the boost::thread +// API and two thread safe message queues to communicate with the main thread +// back and forth. The queue from the main thread to the worker thread holds the +// job entries that will be performed on the worker. The other queue holds messages +// from the worker to the main thread. These messages include status updates, +// finishing operation and arbitrary functiors that need to be performed +// on the main thread during the jobs execution, like displaying intermediate +// results. +class BoostThreadWorker : public Worker, private Job::Ctl +{ + struct JobEntry // Goes into worker and also out of worker as a finalize msg + { + std::unique_ptr job; + bool canceled = false; + std::exception_ptr eptr = nullptr; + }; + + // A message data for status updates. Only goes from worker to main thread. + struct StatusInfo { int status; std::string msg; }; + + // An arbitrary callback to be called on the main thread. Only from worker + // to main thread. + struct MainThreadCallData + { + std::function fn; + std::promise barrier; + }; + + class WorkerMessage + { + enum MsgType { Empty, Status, Finalize, MainThreadCall }; + std::variant m_data; + + public: + WorkerMessage() = default; + WorkerMessage(int s, std::string txt) + : m_data{StatusInfo{s, std::move(txt)}} + {} + WorkerMessage(JobEntry &&entry) : m_data{std::move(entry)} {} + WorkerMessage(MainThreadCallData fn) : m_data{std::move(fn)} {} + + void deliver(BoostThreadWorker &runner); + }; + + using JobQueue = ThreadSafeQueueSPSC; + using MessageQueue = ThreadSafeQueueSPSC; + + boost::thread m_thread; + std::atomic m_running{false}, m_canceled{false}; + std::shared_ptr m_progress; + JobQueue m_input_queue; // from main thread to worker + MessageQueue m_output_queue; // form worker to main thread + std::string m_name; + + void run(); + + bool join(int timeout_ms = 0); + +protected: + // Implement Job::Ctl interface: + + void update_status(int st, const std::string &msg = "") override; + + bool was_canceled() const override { return m_canceled.load(); } + + std::future call_on_main_thread(std::function fn) override; + +public: + explicit BoostThreadWorker(std::shared_ptr pri, + boost::thread::attributes & attr, + const char * name = ""); + + explicit BoostThreadWorker(std::shared_ptr pri, + boost::thread::attributes && attr, + const char * name = "") + : BoostThreadWorker{std::move(pri), attr, name} + {} + + explicit BoostThreadWorker(std::shared_ptr pri, + const char * name = "") + : BoostThreadWorker{std::move(pri), {}, name} + {} + + ~BoostThreadWorker(); + + BoostThreadWorker(const BoostThreadWorker &) = delete; + BoostThreadWorker(BoostThreadWorker &&) = delete; + BoostThreadWorker &operator=(const BoostThreadWorker &) = delete; + BoostThreadWorker &operator=(BoostThreadWorker &&) = delete; + + bool start_next(std::unique_ptr job) override; + + bool is_idle() const override + { + // The assumption is that jobs can only be queued from a single main + // thread from which this method is also called. And the output + // messages are also processed only in this calling thread. In that + // case, if the input queue is empty, it will remain so during this + // function call. If the worker thread is also not running and the + // output queue is already processed, we can safely say that the + // worker is dormant. + return m_input_queue.empty() && !m_running.load() && m_output_queue.empty(); + } + + void cancel() override { m_canceled.store(true); } + void cancel_all() override { m_input_queue.clear(); cancel(); } + + ProgressIndicator * get_pri() { return m_progress.get(); } + const ProgressIndicator * get_pri() const { return m_progress.get(); } + + void process_events() override; +}; + +}} // namespace Slic3r::GUI + +#endif // BOOSTTHREADWORKER_HPP diff --git a/src/slic3r/GUI/Jobs/BusyCursorJob.hpp b/src/slic3r/GUI/Jobs/BusyCursorJob.hpp new file mode 100644 index 0000000000..530213b1d5 --- /dev/null +++ b/src/slic3r/GUI/Jobs/BusyCursorJob.hpp @@ -0,0 +1,48 @@ +#ifndef BUSYCURSORJOB_HPP +#define BUSYCURSORJOB_HPP + +#include "Job.hpp" + +#include + +namespace Slic3r { namespace GUI { + +struct CursorSetterRAII +{ + Job::Ctl &ctl; + CursorSetterRAII(Job::Ctl &c) : ctl{c} + { + ctl.call_on_main_thread([] { wxBeginBusyCursor(); }); + } + ~CursorSetterRAII() + { + ctl.call_on_main_thread([] { wxEndBusyCursor(); }); + } +}; + +template +class BusyCursored: public Job { + JobSubclass m_job; + +public: + template + BusyCursored(Args &&...args) : m_job{std::forward(args)...} + {} + + void process(Ctl &ctl) override + { + CursorSetterRAII cursor_setter{ctl}; + m_job.process(ctl); + } + + void finalize(bool canceled, std::exception_ptr &eptr) override + { + m_job.finalize(canceled, eptr); + } +}; + + +} +} + +#endif // BUSYCURSORJOB_HPP diff --git a/src/slic3r/GUI/Jobs/FillBedJob.cpp b/src/slic3r/GUI/Jobs/FillBedJob.cpp index 870f31f2fc..979db77cba 100644 --- a/src/slic3r/GUI/Jobs/FillBedJob.cpp +++ b/src/slic3r/GUI/Jobs/FillBedJob.cpp @@ -3,6 +3,7 @@ #include "libslic3r/Model.hpp" #include "libslic3r/ClipperUtils.hpp" +#include "slic3r/GUI/GUI_App.hpp" #include "slic3r/GUI/Plater.hpp" #include "slic3r/GUI/GLCanvas3D.hpp" #include "slic3r/GUI/GUI_ObjectList.hpp" @@ -102,8 +103,10 @@ void FillBedJob::prepare() p.translation(X) -= p.bed_idx * stride; } -void FillBedJob::process() +void FillBedJob::process(Ctl &ctl) { + ctl.call_on_main_thread([this]{ prepare(); }).wait(); + if (m_object_idx == -1 || m_selected.empty()) return; const GLCanvas3D::ArrangeSettings &settings = @@ -114,13 +117,17 @@ void FillBedJob::process() params.min_obj_distance = scaled(settings.distance); bool do_stop = false; - params.stopcondition = [this, &do_stop]() { - return was_canceled() || do_stop; + params.stopcondition = [&ctl, &do_stop]() { + return ctl.was_canceled() || do_stop; }; - params.progressind = [this](unsigned st) { + auto statustxt = _u8L("Filling bed"); + + ctl.update_status(0, statustxt); + + params.progressind = [this, &ctl, &statustxt](unsigned st) { if (st > 0) - update_status(int(m_status_range - st), _(L("Filling bed"))); + ctl.update_status(int(m_status_range - st) * 100 / status_range(), statustxt); }; params.on_packed = [&do_stop] (const ArrangePolygon &ap) { @@ -130,15 +137,18 @@ void FillBedJob::process() arrangement::arrange(m_selected, m_unselected, m_bedpts, params); // finalize just here. - update_status(m_status_range, was_canceled() ? - _(L("Bed filling canceled.")) : - _(L("Bed filling done."))); + ctl.update_status(100, ctl.was_canceled() ? + _u8L("Bed filling canceled.") : + _u8L("Bed filling done.")); } -void FillBedJob::finalize() +FillBedJob::FillBedJob() : m_plater{wxGetApp().plater()} {} + +void FillBedJob::finalize(bool canceled, std::exception_ptr &eptr) { // Ignore the arrange result if aborted. - if (was_canceled()) return; + if (canceled || eptr) + return; if (m_object_idx == -1) return; @@ -167,8 +177,6 @@ void FillBedJob::finalize() m_plater->sidebar() .obj_list()->increase_object_instances(m_object_idx, size_t(added_cnt)); } - - Job::finalize(); } }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/FillBedJob.hpp b/src/slic3r/GUI/Jobs/FillBedJob.hpp index bf407656d1..b1417bbbd4 100644 --- a/src/slic3r/GUI/Jobs/FillBedJob.hpp +++ b/src/slic3r/GUI/Jobs/FillBedJob.hpp @@ -7,9 +7,9 @@ namespace Slic3r { namespace GUI { class Plater; -class FillBedJob : public PlaterJob +class FillBedJob : public Job { - int m_object_idx = -1; + int m_object_idx = -1; using ArrangePolygon = arrangement::ArrangePolygon; using ArrangePolygons = arrangement::ArrangePolygons; @@ -20,23 +20,20 @@ class FillBedJob : public PlaterJob Points m_bedpts; int m_status_range = 0; - -protected: - - void prepare() override; - void process() override; + Plater *m_plater; public: - FillBedJob(std::shared_ptr pri, Plater *plater) - : PlaterJob{std::move(pri), plater} - {} + void prepare(); + void process(Ctl &ctl) override; - int status_range() const override + FillBedJob(); + + int status_range() const /*override*/ { return m_status_range; } - void finalize() override; + void finalize(bool canceled, std::exception_ptr &e) override; }; }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/Job.cpp b/src/slic3r/GUI/Jobs/Job.cpp deleted file mode 100644 index 9d0d4bc80c..0000000000 --- a/src/slic3r/GUI/Jobs/Job.cpp +++ /dev/null @@ -1,158 +0,0 @@ -#include -#include - -#include "Job.hpp" -#include -#include - -namespace Slic3r { - -void GUI::Job::run(std::exception_ptr &eptr) -{ - m_running.store(true); - try { - process(); - } catch (...) { - eptr = std::current_exception(); - } - - m_running.store(false); - - // ensure to call the last status to finalize the job - update_status(status_range(), ""); -} - -void GUI::Job::update_status(int st, const wxString &msg) -{ - auto evt = new wxThreadEvent(wxEVT_THREAD, m_thread_evt_id); - evt->SetInt(st); - evt->SetString(msg); - wxQueueEvent(this, evt); -} - -GUI::Job::Job(std::shared_ptr pri) - : m_progress(std::move(pri)) -{ - m_thread_evt_id = wxNewId(); - - Bind(wxEVT_THREAD, [this](const wxThreadEvent &evt) { - if (m_finalizing) return; - - auto msg = evt.GetString(); - if (!msg.empty() && !m_worker_error) - m_progress->set_status_text(msg.ToUTF8().data()); - - if (m_finalized) return; - - m_progress->set_progress(evt.GetInt()); - if (evt.GetInt() == status_range() || m_worker_error) { - // set back the original range and cancel callback - m_progress->set_range(m_range); - // Make sure progress indicators get the last value of their range - // to make sure they close, fade out, whathever - m_progress->set_progress(m_range); - m_progress->set_cancel_callback(); - wxEndBusyCursor(); - - if (m_worker_error) { - m_finalized = true; - m_progress->set_status_text(""); - m_progress->set_progress(m_range); - on_exception(m_worker_error); - } - else { - // This is an RAII solution to remember that finalization is - // running. The run method calls update_status(status_range(), "") - // at the end, which queues up a call to this handler in all cases. - // If process also calls update_status with maxed out status arg - // it will call this handler twice. It is not a problem unless - // yield is called inside the finilize() method, which would - // jump out of finalize and call this handler again. - struct Finalizing { - bool &flag; - Finalizing (bool &f): flag(f) { flag = true; } - ~Finalizing() { flag = false; } - } fin(m_finalizing); - - finalize(); - } - - // dont do finalization again for the same process - m_finalized = true; - } - }, m_thread_evt_id); -} - -void GUI::Job::start() -{ // Start the job. No effect if the job is already running - if (!m_running.load()) { - prepare(); - - // Save the current status indicatior range and push the new one - m_range = m_progress->get_range(); - m_progress->set_range(status_range()); - - // init cancellation flag and set the cancel callback - m_canceled.store(false); - m_progress->set_cancel_callback( - [this]() { m_canceled.store(true); }); - - m_finalized = false; - m_finalizing = false; - - // Changing cursor to busy - wxBeginBusyCursor(); - - try { // Execute the job - m_worker_error = nullptr; - m_thread = create_thread([this] { this->run(m_worker_error); }); - } catch (std::exception &) { - update_status(status_range(), - _(L("ERROR: not enough resources to " - "execute a new job."))); - } - - // The state changes will be undone when the process hits the - // last status value, in the status update handler (see ctor) - } -} - -bool GUI::Job::join(int timeout_ms) -{ - if (!m_thread.joinable()) return true; - - if (timeout_ms <= 0) - m_thread.join(); - else if (!m_thread.try_join_for(boost::chrono::milliseconds(timeout_ms))) - return false; - - return true; -} - -void GUI::ExclusiveJobGroup::start(size_t jid) { - assert(jid < m_jobs.size()); - stop_all(); - m_jobs[jid]->start(); -} - -void GUI::ExclusiveJobGroup::join_all(int wait_ms) -{ - std::vector aborted(m_jobs.size(), false); - - for (size_t jid = 0; jid < m_jobs.size(); ++jid) - aborted[jid] = m_jobs[jid]->join(wait_ms); - - if (!std::all_of(aborted.begin(), aborted.end(), [](bool t) { return t; })) - BOOST_LOG_TRIVIAL(error) << "Could not abort a job!"; -} - -bool GUI::ExclusiveJobGroup::is_any_running() const -{ - return std::any_of(m_jobs.begin(), m_jobs.end(), - [](const std::unique_ptr &j) { - return j->is_running(); - }); -} - -} - diff --git a/src/slic3r/GUI/Jobs/Job.hpp b/src/slic3r/GUI/Jobs/Job.hpp index 8243ce9430..824c0b8303 100644 --- a/src/slic3r/GUI/Jobs/Job.hpp +++ b/src/slic3r/GUI/Jobs/Job.hpp @@ -3,119 +3,53 @@ #include #include +#include #include "libslic3r/libslic3r.h" - -#include - #include "ProgressIndicator.hpp" -#include - -#include - namespace Slic3r { namespace GUI { -// A class to handle UI jobs like arranging and optimizing rotation. -// These are not instant jobs, the user has to be informed about their -// state in the status progress indicator. On the other hand they are -// separated from the background slicing process. Ideally, these jobs should -// run when the background process is not running. -// -// TODO: A mechanism would be useful for blocking the plater interactions: -// objects would be frozen for the user. In case of arrange, an animation -// could be shown, or with the optimize orientations, partial results -// could be displayed. -class Job : public wxEvtHandler -{ - int m_range = 100; - int m_thread_evt_id = wxID_ANY; - boost::thread m_thread; - std::atomic m_running{false}, m_canceled{false}; - bool m_finalized = false, m_finalizing = false; - std::shared_ptr m_progress; - std::exception_ptr m_worker_error = nullptr; - - void run(std::exception_ptr &); - -protected: - // status range for a particular job - virtual int status_range() const { return 100; } - - // status update, to be used from the work thread (process() method) - void update_status(int st, const wxString &msg = ""); +// A class representing a job that is to be run in the background, not blocking +// the main thread. Running it is up to a Worker object (see Worker interface) +class Job { +public: - bool was_canceled() const { return m_canceled.load(); } + // A controller interface that informs the job about cancellation and + // makes it possible for the job to advertise its status. + class Ctl { + public: + virtual ~Ctl() = default; - // Launched just before start(), a job can use it to prepare internals - virtual void prepare() {} + // status update, to be used from the work thread (process() method) + virtual void update_status(int st, const std::string &msg = "") = 0; - // The method where the actual work of the job should be defined. - virtual void process() = 0; - - // Launched when the job is finished. It refreshes the 3Dscene by def. - virtual void finalize() { m_finalized = true; } + // Returns true if the job was asked to cancel itself. + virtual bool was_canceled() const = 0; + // Execute a functor on the main thread. Note that the exact time of + // execution is hard to determine. This can be used to make modifications + // on the UI, like displaying some intermediate results or modify the + // cursor. + // This function returns a std::future object which enables the + // caller to optionally wait for the main thread to finish the function call. + virtual std::future call_on_main_thread(std::function fn) = 0; + }; + + virtual ~Job() = default; + + // The method where the actual work of the job should be defined. This is + // run on the worker thread. + virtual void process(Ctl &ctl) = 0; + + // Launched when the job is finished on the UI thread. + // If the job was cancelled, the first parameter will have a true value. // Exceptions occuring in process() are redirected from the worker thread - // into the main (UI) thread. This method is called from the main thread and - // can be overriden to handle these exceptions. - virtual void on_exception(const std::exception_ptr &eptr) - { - if (eptr) std::rethrow_exception(eptr); - } - -public: - Job(std::shared_ptr pri); - - bool is_finalized() const { return m_finalized; } - - Job(const Job &) = delete; - Job(Job &&) = delete; - Job &operator=(const Job &) = delete; - Job &operator=(Job &&) = delete; - - void start(); - - // To wait for the running job and join the threads. False is - // returned if the timeout has been reached and the job is still - // running. Call cancel() before this fn if you want to explicitly - // end the job. - bool join(int timeout_ms = 0); - - bool is_running() const { return m_running.load(); } - void cancel() { m_canceled.store(true); } -}; - -// Jobs defined inside the group class will be managed so that only one can -// run at a time. Also, the background process will be stopped if a job is -// started. -class ExclusiveJobGroup -{ - static const int ABORT_WAIT_MAX_MS = 10000; - - std::vector> m_jobs; - -protected: - virtual void before_start() {} - -public: - virtual ~ExclusiveJobGroup() = default; - - size_t add_job(std::unique_ptr &&job) - { - m_jobs.emplace_back(std::move(job)); - return m_jobs.size() - 1; - } - - void start(size_t jid); - - void cancel_all() { for (auto& j : m_jobs) j->cancel(); } - - void join_all(int wait_ms = 0); - - void stop_all() { cancel_all(); join_all(ABORT_WAIT_MAX_MS); } - - bool is_any_running() const; + // into the main (UI) thread. This method receives the exception and can + // handle it properly. Assign nullptr to this second argument before + // function return to prevent further action. Leaving it with a non-null + // value will result in rethrowing by the worker. + virtual void finalize(bool /*canceled*/, std::exception_ptr &) {} }; }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/NotificationProgressIndicator.cpp b/src/slic3r/GUI/Jobs/NotificationProgressIndicator.cpp index cb71705687..f398f73339 100644 --- a/src/slic3r/GUI/Jobs/NotificationProgressIndicator.cpp +++ b/src/slic3r/GUI/Jobs/NotificationProgressIndicator.cpp @@ -12,11 +12,15 @@ void NotificationProgressIndicator::set_range(int range) void NotificationProgressIndicator::set_cancel_callback(CancelFn fn) { - m_nm->progress_indicator_set_cancel_callback(std::move(fn)); + m_cancelfn = std::move(fn); + m_nm->progress_indicator_set_cancel_callback(m_cancelfn); } void NotificationProgressIndicator::set_progress(int pr) { + if (!pr) + set_cancel_callback(m_cancelfn); + m_nm->progress_indicator_set_progress(pr); } diff --git a/src/slic3r/GUI/Jobs/NotificationProgressIndicator.hpp b/src/slic3r/GUI/Jobs/NotificationProgressIndicator.hpp index 6b03af69df..b31cb7f7cd 100644 --- a/src/slic3r/GUI/Jobs/NotificationProgressIndicator.hpp +++ b/src/slic3r/GUI/Jobs/NotificationProgressIndicator.hpp @@ -9,7 +9,7 @@ class NotificationManager; class NotificationProgressIndicator: public ProgressIndicator { NotificationManager *m_nm = nullptr; - + CancelFn m_cancelfn; public: explicit NotificationProgressIndicator(NotificationManager *nm); diff --git a/src/slic3r/GUI/Jobs/PlaterJob.cpp b/src/slic3r/GUI/Jobs/PlaterJob.cpp deleted file mode 100644 index 4af205d41b..0000000000 --- a/src/slic3r/GUI/Jobs/PlaterJob.cpp +++ /dev/null @@ -1,17 +0,0 @@ -#include "PlaterJob.hpp" -#include "slic3r/GUI/GUI.hpp" -#include "slic3r/GUI/Plater.hpp" - -namespace Slic3r { namespace GUI { - -void PlaterJob::on_exception(const std::exception_ptr &eptr) -{ - try { - if (eptr) - std::rethrow_exception(eptr); - } catch (std::exception &e) { - show_error(m_plater, _(L("An unexpected error occured")) + ": "+ e.what()); - } -} - -}} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/PlaterJob.hpp b/src/slic3r/GUI/Jobs/PlaterJob.hpp index fcf0a54b80..8c8d2e3787 100644 --- a/src/slic3r/GUI/Jobs/PlaterJob.hpp +++ b/src/slic3r/GUI/Jobs/PlaterJob.hpp @@ -1,24 +1,72 @@ #ifndef PLATERJOB_HPP #define PLATERJOB_HPP -#include "Job.hpp" +#include "BusyCursorJob.hpp" + +#include "slic3r/GUI/GUI.hpp" +#include "slic3r/GUI/GUI_App.hpp" +#include "slic3r/GUI/I18N.hpp" +#include "slic3r/GUI/Plater.hpp" +#include "slic3r/GUI/GLCanvas3D.hpp" namespace Slic3r { namespace GUI { class Plater; +template class PlaterJob : public Job { -protected: + JobSubclass m_job; Plater *m_plater; - void on_exception(const std::exception_ptr &) override; - public: - PlaterJob(std::shared_ptr pri, Plater *plater): - Job{std::move(pri)}, m_plater{plater} {} + void process(Ctl &c) override + { + CursorSetterRAII busycursor{c}; + m_job.process(c); + } + + void finalize(bool canceled, std::exception_ptr &eptr) override + { + m_job.finalize(canceled, eptr); + + if (eptr) try { + std::rethrow_exception(eptr); + } catch (std::exception &e) { + show_error(m_plater, _L("An unexpected error occured: ") + e.what()); + eptr = nullptr; + } + } + + template + PlaterJob(Args &&...args) + : m_job(std::forward(args)...), m_plater{wxGetApp().plater()} + { + // TODO: decide if disabling slice button during UI job is what we want. +// if (m_plater) +// m_plater->sidebar().enable_buttons(false); + } + + ~PlaterJob() override + { + // TODO: decide if disabling slice button during UI job is what we want. + + // Reload scene ensures that the slice button gets properly + // enabled or disabled after the job finishes, depending on the + // state of slicing. This might be an overkill but works for now. +// if (m_plater) +// m_plater->canvas3D()->reload_scene(false); + } }; +template +void replace_job(Plater& p, Args && ...args) +{ + replace_job(p.get_ui_job_worker(), + std::make_unique>( + std::forward(args)...)); +} + }} // namespace Slic3r::GUI #endif // PLATERJOB_HPP diff --git a/src/slic3r/GUI/Jobs/RotoptimizeJob.cpp b/src/slic3r/GUI/Jobs/RotoptimizeJob.cpp index 95821a674d..e88d24fcdc 100644 --- a/src/slic3r/GUI/Jobs/RotoptimizeJob.cpp +++ b/src/slic3r/GUI/Jobs/RotoptimizeJob.cpp @@ -12,6 +12,8 @@ #include "slic3r/GUI/GUI_App.hpp" #include "libslic3r/AppConfig.hpp" +#include + namespace Slic3r { namespace GUI { void RotoptimizeJob::prepare() @@ -45,20 +47,23 @@ void RotoptimizeJob::prepare() } } -void RotoptimizeJob::process() +void RotoptimizeJob::process(Ctl &ctl) { int prev_status = 0; + auto statustxt = _u8L("Searching for optimal orientation"); + ctl.update_status(0, statustxt); + auto params = sla::RotOptimizeParams{} .accuracy(m_accuracy) .print_config(&m_default_print_cfg) - .statucb([this, &prev_status](int s) + .statucb([this, &prev_status, &ctl, &statustxt](int s) { if (s > 0 && s < 100) - update_status(prev_status + s / m_selected_object_ids.size(), - _(L("Searching for optimal orientation"))); + ctl.update_status(prev_status + s / m_selected_object_ids.size(), + statustxt); - return !was_canceled(); + return !ctl.was_canceled(); }); @@ -71,16 +76,20 @@ void RotoptimizeJob::process() prev_status += 100 / m_selected_object_ids.size(); - if (was_canceled()) break; + if (ctl.was_canceled()) break; } - update_status(100, was_canceled() ? _(L("Orientation search canceled.")) : - _(L("Orientation found."))); + ctl.update_status(100, ctl.was_canceled() ? + _u8L("Orientation search canceled.") : + _u8L("Orientation found.")); } -void RotoptimizeJob::finalize() +RotoptimizeJob::RotoptimizeJob() : m_plater{wxGetApp().plater()} { prepare(); } + +void RotoptimizeJob::finalize(bool canceled, std::exception_ptr &eptr) { - if (was_canceled()) return; + if (canceled || eptr) + return; for (const ObjRot &objrot : m_selected_object_ids) { ModelObject *o = m_plater->model().objects[size_t(objrot.idx)]; @@ -111,10 +120,8 @@ void RotoptimizeJob::finalize() // m_plater->find_new_position(o->instances); } - if (!was_canceled()) + if (!canceled) m_plater->update(); - - Job::finalize(); } }} diff --git a/src/slic3r/GUI/Jobs/RotoptimizeJob.hpp b/src/slic3r/GUI/Jobs/RotoptimizeJob.hpp index cdb367f23a..71a28deb7c 100644 --- a/src/slic3r/GUI/Jobs/RotoptimizeJob.hpp +++ b/src/slic3r/GUI/Jobs/RotoptimizeJob.hpp @@ -1,16 +1,17 @@ #ifndef ROTOPTIMIZEJOB_HPP #define ROTOPTIMIZEJOB_HPP -#include "PlaterJob.hpp" +#include "Job.hpp" #include "libslic3r/SLA/Rotfinder.hpp" #include "libslic3r/PrintConfig.hpp" +#include "slic3r/GUI/I18N.hpp" -namespace Slic3r { +namespace Slic3r { namespace GUI { -namespace GUI { +class Plater; -class RotoptimizeJob : public PlaterJob +class RotoptimizeJob : public Job { using FindFn = std::function; @@ -44,19 +45,16 @@ class RotoptimizeJob : public PlaterJob }; std::vector m_selected_object_ids; - -protected: - - void prepare() override; - void process() override; + Plater *m_plater; public: - RotoptimizeJob(std::shared_ptr pri, Plater *plater) - : PlaterJob{std::move(pri), plater} - {} + void prepare(); + void process(Ctl &ctl) override; - void finalize() override; + RotoptimizeJob(); + + void finalize(bool canceled, std::exception_ptr &) override; static constexpr size_t get_methods_count() { return std::size(Methods); } diff --git a/src/slic3r/GUI/Jobs/SLAImportDialog.hpp b/src/slic3r/GUI/Jobs/SLAImportDialog.hpp new file mode 100644 index 0000000000..7dbecff2ae --- /dev/null +++ b/src/slic3r/GUI/Jobs/SLAImportDialog.hpp @@ -0,0 +1,114 @@ +#ifndef SLAIMPORTDIALOG_HPP +#define SLAIMPORTDIALOG_HPP + +#include "SLAImportJob.hpp" + +#include +#include +#include +#include +#include + +#include "libslic3r/AppConfig.hpp" +#include "slic3r/GUI/I18N.hpp" + +#include "slic3r/GUI/GUI.hpp" +#include "slic3r/GUI/GUI_App.hpp" +#include "slic3r/GUI/Plater.hpp" + +//#include "libslic3r/Model.hpp" +//#include "libslic3r/PresetBundle.hpp" + +namespace Slic3r { namespace GUI { + +class SLAImportDialog: public wxDialog, public SLAImportJobView { + wxFilePickerCtrl *m_filepicker; + wxComboBox *m_import_dropdown, *m_quality_dropdown; + +public: + SLAImportDialog(Plater *plater) + : wxDialog{plater, wxID_ANY, "Import SLA archive"} + { + auto szvert = new wxBoxSizer{wxVERTICAL}; + auto szfilepck = new wxBoxSizer{wxHORIZONTAL}; + + m_filepicker = new wxFilePickerCtrl(this, wxID_ANY, + from_u8(wxGetApp().app_config->get_last_dir()), _(L("Choose SLA archive:")), + "SL1 / SL1S archive files (*.sl1, *.sl1s, *.zip)|*.sl1;*.SL1;*.sl1s;*.SL1S;*.zip;*.ZIP", + wxDefaultPosition, wxDefaultSize, wxFLP_DEFAULT_STYLE | wxFD_OPEN | wxFD_FILE_MUST_EXIST); + + szfilepck->Add(new wxStaticText(this, wxID_ANY, _L("Import file") + ": "), 0, wxALIGN_CENTER); + szfilepck->Add(m_filepicker, 1); + szvert->Add(szfilepck, 0, wxALL | wxEXPAND, 5); + + auto szchoices = new wxBoxSizer{wxHORIZONTAL}; + + static const std::vector inp_choices = { + _(L("Import model and profile")), + _(L("Import profile only")), + _(L("Import model only")) + }; + + m_import_dropdown = new wxComboBox( + this, wxID_ANY, inp_choices[0], wxDefaultPosition, wxDefaultSize, + inp_choices.size(), inp_choices.data(), wxCB_READONLY | wxCB_DROPDOWN); + + szchoices->Add(m_import_dropdown); + szchoices->Add(new wxStaticText(this, wxID_ANY, _L("Quality") + ": "), 0, wxALIGN_CENTER | wxALL, 5); + + static const std::vector qual_choices = { + _(L("Accurate")), + _(L("Balanced")), + _(L("Quick")) + }; + + m_quality_dropdown = new wxComboBox( + this, wxID_ANY, qual_choices[0], wxDefaultPosition, wxDefaultSize, + qual_choices.size(), qual_choices.data(), wxCB_READONLY | wxCB_DROPDOWN); + szchoices->Add(m_quality_dropdown); + + m_import_dropdown->Bind(wxEVT_COMBOBOX, [this](wxCommandEvent &) { + if (get_selection() == Sel::profileOnly) + m_quality_dropdown->Disable(); + else m_quality_dropdown->Enable(); + }); + + szvert->Add(szchoices, 0, wxALL, 5); + szvert->AddStretchSpacer(1); + auto szbtn = new wxBoxSizer(wxHORIZONTAL); + szbtn->Add(new wxButton{this, wxID_CANCEL}); + szbtn->Add(new wxButton{this, wxID_OK}); + szvert->Add(szbtn, 0, wxALIGN_RIGHT | wxALL, 5); + + SetSizerAndFit(szvert); + } + + Sel get_selection() const override + { + int sel = m_import_dropdown->GetSelection(); + return Sel(std::min(int(Sel::modelOnly), std::max(0, sel))); + } + + Vec2i get_marchsq_windowsize() const override + { + enum { Accurate, Balanced, Fast}; + + switch(m_quality_dropdown->GetSelection()) + { + case Fast: return {8, 8}; + case Balanced: return {4, 4}; + default: + case Accurate: + return {2, 2}; + } + } + + std::string get_path() const override + { + return m_filepicker->GetPath().ToUTF8().data(); + } +}; + +}} // namespace Slic3r::GUI + +#endif // SLAIMPORTDIALOG_HPP diff --git a/src/slic3r/GUI/Jobs/SLAImportJob.cpp b/src/slic3r/GUI/Jobs/SLAImportJob.cpp index 0d42cec2d3..1bb8cdf6c6 100644 --- a/src/slic3r/GUI/Jobs/SLAImportJob.cpp +++ b/src/slic3r/GUI/Jobs/SLAImportJob.cpp @@ -3,7 +3,6 @@ #include "libslic3r/Format/SL1.hpp" #include "slic3r/GUI/GUI.hpp" -#include "slic3r/GUI/GUI_App.hpp" #include "slic3r/GUI/Plater.hpp" #include "slic3r/GUI/GUI_ObjectList.hpp" #include "slic3r/GUI/NotificationManager.hpp" @@ -11,104 +10,10 @@ #include "libslic3r/Model.hpp" #include "libslic3r/PresetBundle.hpp" -#include -#include -#include #include -#include namespace Slic3r { namespace GUI { -enum class Sel { modelAndProfile, profileOnly, modelOnly}; - -class ImportDlg: public wxDialog { - wxFilePickerCtrl *m_filepicker; - wxComboBox *m_import_dropdown, *m_quality_dropdown; - -public: - ImportDlg(Plater *plater) - : wxDialog{plater, wxID_ANY, "Import SLA archive"} - { - auto szvert = new wxBoxSizer{wxVERTICAL}; - auto szfilepck = new wxBoxSizer{wxHORIZONTAL}; - - m_filepicker = new wxFilePickerCtrl(this, wxID_ANY, - from_u8(wxGetApp().app_config->get_last_dir()), _(L("Choose SLA archive:")), - "SL1 / SL1S archive files (*.sl1, *.sl1s, *.zip)|*.sl1;*.SL1;*.sl1s;*.SL1S;*.zip;*.ZIP", - wxDefaultPosition, wxDefaultSize, wxFLP_DEFAULT_STYLE | wxFD_OPEN | wxFD_FILE_MUST_EXIST); - - szfilepck->Add(new wxStaticText(this, wxID_ANY, _L("Import file") + ": "), 0, wxALIGN_CENTER); - szfilepck->Add(m_filepicker, 1); - szvert->Add(szfilepck, 0, wxALL | wxEXPAND, 5); - - auto szchoices = new wxBoxSizer{wxHORIZONTAL}; - - static const std::vector inp_choices = { - _(L("Import model and profile")), - _(L("Import profile only")), - _(L("Import model only")) - }; - - m_import_dropdown = new wxComboBox( - this, wxID_ANY, inp_choices[0], wxDefaultPosition, wxDefaultSize, - inp_choices.size(), inp_choices.data(), wxCB_READONLY | wxCB_DROPDOWN); - - szchoices->Add(m_import_dropdown); - szchoices->Add(new wxStaticText(this, wxID_ANY, _L("Quality") + ": "), 0, wxALIGN_CENTER | wxALL, 5); - - static const std::vector qual_choices = { - _(L("Accurate")), - _(L("Balanced")), - _(L("Quick")) - }; - - m_quality_dropdown = new wxComboBox( - this, wxID_ANY, qual_choices[0], wxDefaultPosition, wxDefaultSize, - qual_choices.size(), qual_choices.data(), wxCB_READONLY | wxCB_DROPDOWN); - szchoices->Add(m_quality_dropdown); - - m_import_dropdown->Bind(wxEVT_COMBOBOX, [this](wxCommandEvent &) { - if (get_selection() == Sel::profileOnly) - m_quality_dropdown->Disable(); - else m_quality_dropdown->Enable(); - }); - - szvert->Add(szchoices, 0, wxALL, 5); - szvert->AddStretchSpacer(1); - auto szbtn = new wxBoxSizer(wxHORIZONTAL); - szbtn->Add(new wxButton{this, wxID_CANCEL}); - szbtn->Add(new wxButton{this, wxID_OK}); - szvert->Add(szbtn, 0, wxALIGN_RIGHT | wxALL, 5); - - SetSizerAndFit(szvert); - } - - Sel get_selection() const - { - int sel = m_import_dropdown->GetSelection(); - return Sel(std::min(int(Sel::modelOnly), std::max(0, sel))); - } - - Vec2i get_marchsq_windowsize() const - { - enum { Accurate, Balanced, Fast}; - - switch(m_quality_dropdown->GetSelection()) - { - case Fast: return {8, 8}; - case Balanced: return {4, 4}; - default: - case Accurate: - return {2, 2}; - } - } - - wxString get_path() const - { - return m_filepicker->GetPath(); - } -}; - class SLAImportJob::priv { public: Plater *plater; @@ -122,23 +27,28 @@ public: std::string err; ConfigSubstitutions config_substitutions; - ImportDlg import_dlg; + const SLAImportJobView * import_dlg; - priv(Plater *plt) : plater{plt}, import_dlg{plt} {} + priv(Plater *plt, const SLAImportJobView *view) : plater{plt}, import_dlg{view} {} }; -SLAImportJob::SLAImportJob(std::shared_ptr pri, Plater *plater) - : PlaterJob{std::move(pri), plater}, p{std::make_unique(plater)} -{} +SLAImportJob::SLAImportJob(const SLAImportJobView *view) + : p{std::make_unique(wxGetApp().plater(), view)} +{ + prepare(); +} SLAImportJob::~SLAImportJob() = default; -void SLAImportJob::process() +void SLAImportJob::process(Ctl &ctl) { - auto progr = [this](int s) { + auto statustxt = _u8L("Importing SLA archive"); + ctl.update_status(0, statustxt); + + auto progr = [&ctl, &statustxt](int s) { if (s < 100) - update_status(int(s), _(L("Importing SLA archive"))); - return !was_canceled(); + ctl.update_status(int(s), statustxt); + return !ctl.was_canceled(); }; if (p->path.empty()) return; @@ -161,15 +71,15 @@ void SLAImportJob::process() p->err = ex.what(); } - update_status(100, was_canceled() ? _(L("Importing canceled.")) : - _(L("Importing done."))); + ctl.update_status(100, ctl.was_canceled() ? _u8L("Importing canceled.") : + _u8L("Importing done.")); } void SLAImportJob::reset() { p->sel = Sel::modelAndProfile; p->mesh = {}; - p->profile = m_plater->sla_print().full_print_config(); + p->profile = p->plater->sla_print().full_print_config(); p->win = {2, 2}; p->path.Clear(); } @@ -178,22 +88,19 @@ void SLAImportJob::prepare() { reset(); - if (p->import_dlg.ShowModal() == wxID_OK) { - auto path = p->import_dlg.get_path(); - auto nm = wxFileName(path); - p->path = !nm.Exists(wxFILE_EXISTS_REGULAR) ? "" : nm.GetFullPath(); - p->sel = p->import_dlg.get_selection(); - p->win = p->import_dlg.get_marchsq_windowsize(); - p->config_substitutions.clear(); - } else { - p->path = ""; - } + auto path = p->import_dlg->get_path(); + auto nm = wxFileName(path); + p->path = !nm.Exists(wxFILE_EXISTS_REGULAR) ? "" : nm.GetFullPath(); + p->sel = p->import_dlg->get_selection(); + p->win = p->import_dlg->get_marchsq_windowsize(); + p->config_substitutions.clear(); } -void SLAImportJob::finalize() +void SLAImportJob::finalize(bool canceled, std::exception_ptr &eptr) { // Ignore the arrange result if aborted. - if (was_canceled()) return; + if (canceled || eptr) + return; if (!p->err.empty()) { show_error(p->plater, p->err); @@ -204,7 +111,7 @@ void SLAImportJob::finalize() std::string name = wxFileName(p->path).GetName().ToUTF8().data(); if (p->profile.empty()) { - m_plater->get_notification_manager()->push_notification( + p->plater->get_notification_manager()->push_notification( NotificationType::CustomNotification, NotificationManager::NotificationLevel::WarningNotificationLevel, _L("The imported SLA archive did not contain any presets. " @@ -213,7 +120,7 @@ void SLAImportJob::finalize() if (p->sel != Sel::modelOnly) { if (p->profile.empty()) - p->profile = m_plater->sla_print().full_print_config(); + p->profile = p->plater->sla_print().full_print_config(); const ModelObjectPtrs& objects = p->plater->model().objects; for (auto object : objects) diff --git a/src/slic3r/GUI/Jobs/SLAImportJob.hpp b/src/slic3r/GUI/Jobs/SLAImportJob.hpp index c2ca10ef69..b2aea8bf89 100644 --- a/src/slic3r/GUI/Jobs/SLAImportJob.hpp +++ b/src/slic3r/GUI/Jobs/SLAImportJob.hpp @@ -1,22 +1,37 @@ #ifndef SLAIMPORTJOB_HPP #define SLAIMPORTJOB_HPP -#include "PlaterJob.hpp" +#include "Job.hpp" + +#include "libslic3r/Point.hpp" namespace Slic3r { namespace GUI { -class SLAImportJob : public PlaterJob { +class SLAImportJobView { +public: + enum Sel { modelAndProfile, profileOnly, modelOnly}; + + virtual ~SLAImportJobView() = default; + + virtual Sel get_selection() const = 0; + virtual Vec2i get_marchsq_windowsize() const = 0; + virtual std::string get_path() const = 0; +}; + +class Plater; + +class SLAImportJob : public Job { class priv; std::unique_ptr p; - -protected: - void prepare() override; - void process() override; - void finalize() override; + using Sel = SLAImportJobView::Sel; public: - SLAImportJob(std::shared_ptr pri, Plater *plater); + void prepare(); + void process(Ctl &ctl) override; + void finalize(bool canceled, std::exception_ptr &) override; + + SLAImportJob(const SLAImportJobView *); ~SLAImportJob(); void reset(); diff --git a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp new file mode 100644 index 0000000000..f01698cb2f --- /dev/null +++ b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp @@ -0,0 +1,93 @@ +#ifndef THREADSAFEQUEUE_HPP +#define THREADSAFEQUEUE_HPP + +#include +#include +#include +#include + +namespace Slic3r { namespace GUI { + +// A thread safe queue for one producer and one consumer. Use consume_one_blk +// to block on an empty queue. +template class Container = std::deque, + class... ContainerArgs> +class ThreadSafeQueueSPSC +{ + std::queue> m_queue; + mutable std::mutex m_mutex; + std::condition_variable m_cond_var; +public: + + // Consume one element, block if the queue is empty. + template void consume_one_blk(Fn &&fn) + { + static_assert(!std::is_reference_v, ""); + static_assert(std::is_default_constructible_v, ""); + static_assert(std::is_move_assignable_v || std::is_copy_assignable_v, ""); + + T el; + { + std::unique_lock lk{m_mutex}; + m_cond_var.wait(lk, [this]{ return !m_queue.empty(); }); + + if constexpr (std::is_move_assignable_v) + el = std::move(m_queue.front()); + else + el = m_queue.front(); + + m_queue.pop(); + } + + fn(el); + } + + // Consume one element, return true if consumed, false if queue was empty. + template bool consume_one(Fn &&fn) + { + T el; + { + std::unique_lock lk{m_mutex}; + if (!m_queue.empty()) { + el = std::move(m_queue.front()); + m_queue.pop(); + } else + return false; + } + + fn(el); + + return true; + } + + // Push element into the queue. + template void push(TArgs&&...el) + { + std::lock_guard lk{m_mutex}; + m_queue.emplace(std::forward(el)...); + m_cond_var.notify_one(); + } + + bool empty() const + { + std::lock_guard lk{m_mutex}; + return m_queue.empty(); + } + + size_t size() const + { + std::lock_guard lk{m_mutex}; + return m_queue.size(); + } + + void clear() + { + std::lock_guard lk{m_mutex}; + while (!m_queue.empty()) m_queue.pop(); + } +}; + +}} // namespace Slic3r::GUI + +#endif // THREADSAFEQUEUE_HPP diff --git a/src/slic3r/GUI/Jobs/Worker.hpp b/src/slic3r/GUI/Jobs/Worker.hpp new file mode 100644 index 0000000000..459349d5b5 --- /dev/null +++ b/src/slic3r/GUI/Jobs/Worker.hpp @@ -0,0 +1,92 @@ +#ifndef PRUSALSICER_WORKER_HPP +#define PRUSALSICER_WORKER_HPP + +#include + +#include "Job.hpp" + +namespace Slic3r { namespace GUI { + +// An interface of a worker that runs jobs on a dedicated worker thread, one +// after the other. It is assumed that every method of this class is called +// from the same main thread. +class Worker { +public: + // Queue up a new job after the current one. This call does not block. + // Returns false if the job gets discarded. + virtual bool start_next(std::unique_ptr job) = 0; + + // Returns true if no job is running and no job message is left to be processed. + // This means that nothing is left to finalize or take care of in the main thread. + virtual bool is_idle() const = 0; + + // Ask the current job gracefully to cancel. This call is not blocking and + // the job may or may not cancel eventually, depending on its + // implementation. Note that it is not trivial to kill a thread forcefully + // and we don't need that. + virtual void cancel() = 0; + + // This method will delete the queued jobs and cancel the current one. + virtual void cancel_all() = 0; + + // Needs to be called continuously to process events (like status update or + // finalizing of jobs) in the UI thread. This can be done e.g. in a wxIdle + // handler. + virtual void process_events() = 0; + + // The destructor shall properly close the worker thread. + virtual ~Worker() = default; +}; + +template constexpr bool IsProcessFn = std::is_invocable_v; +template constexpr bool IsFinishFn = std::is_invocable_v; + +// Helper function to use the worker with arbitrary functors. +template>, + class = std::enable_if_t> > +bool queue_job(Worker &w, ProcessFn fn, FinishFn finishfn) +{ + struct LambdaJob: Job { + ProcessFn fn; + FinishFn finishfn; + + LambdaJob(ProcessFn pfn, FinishFn ffn) + : fn{std::move(pfn)}, finishfn{std::move(ffn)} + {} + + void process(Ctl &ctl) override { fn(ctl); } + void finalize(bool canceled, std::exception_ptr &eptr) override + { + finishfn(canceled, eptr); + } + }; + + auto j = std::make_unique(std::move(fn), std::move(finishfn)); + return w.start_next(std::move(j)); +} + +template>> +bool queue_job(Worker &w, ProcessFn fn) +{ + return queue_job(w, std::move(fn), [](bool, std::exception_ptr &) {}); +} + +inline bool queue_job(Worker &w, std::unique_ptr j) +{ + return w.start_next(std::move(j)); +} + +// Replace the current job queue with a new job. This cancels all jobs and +// will not wait. The new job will begin after the queue cancels properly. +// Note that this can be called from the UI thread and will not block it if the +// jobs take longer to cancel. +template bool replace_job(Worker &w, Args&& ...args) +{ + w.cancel_all(); + return queue_job(w, std::forward(args)...); +} + +}} // namespace Slic3r::GUI + +#endif // WORKER_HPP diff --git a/src/slic3r/GUI/MainFrame.cpp b/src/slic3r/GUI/MainFrame.cpp index a88509527b..c61c0e6d24 100644 --- a/src/slic3r/GUI/MainFrame.cpp +++ b/src/slic3r/GUI/MainFrame.cpp @@ -574,7 +574,7 @@ void MainFrame::shutdown() #endif // _WIN32 if (m_plater != nullptr) { - m_plater->stop_jobs(); + m_plater->get_ui_job_worker().cancel_all(); // Unbinding of wxWidgets event handling in canvases needs to be done here because on MAC, // when closing the application using Command+Q, a mouse event is triggered after this lambda is completed, @@ -1211,7 +1211,7 @@ void MainFrame::init_menubar_as_editor() append_menu_item(import_menu, wxID_ANY, _L("Import SL1 / SL1S Archive") + dots, _L("Load an SL1 / Sl1S archive"), [this](wxCommandEvent&) { if (m_plater) m_plater->import_sl1_archive(); }, "import_plater", nullptr, - [this](){return m_plater != nullptr && !m_plater->is_any_job_running(); }, this); + [this](){return m_plater != nullptr && m_plater->get_ui_job_worker().is_idle(); }, this); import_menu->AppendSeparator(); append_menu_item(import_menu, wxID_ANY, _L("Import &Config") + dots + "\tCtrl+L", _L("Load exported configuration file"), diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 461eb02e27..c4d1f31c28 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -73,7 +73,10 @@ #include "Jobs/FillBedJob.hpp" #include "Jobs/RotoptimizeJob.hpp" #include "Jobs/SLAImportJob.hpp" +#include "Jobs/SLAImportDialog.hpp" #include "Jobs/NotificationProgressIndicator.hpp" +#include "Jobs/PlaterJob.hpp" +#include "Jobs/BoostThreadWorker.hpp" #include "BackgroundSlicingProcess.hpp" #include "PrintHostDialogs.hpp" #include "ConfigWizard.hpp" @@ -1638,54 +1641,12 @@ struct Plater::priv BackgroundSlicingProcess background_process; bool suppressed_backround_processing_update { false }; - // Jobs defined inside the group class will be managed so that only one can - // run at a time. Also, the background process will be stopped if a job is - // started. It is up the the plater to ensure that the background slicing - // can't be restarted while a ui job is still running. - class Jobs: public ExclusiveJobGroup - { - priv *m; - size_t m_arrange_id, m_fill_bed_id, m_rotoptimize_id, m_sla_import_id; - std::shared_ptr m_pri; - - void before_start() override { m->background_process.stop(); } - - public: - Jobs(priv *_m) : - m(_m), - m_pri{std::make_shared(m->notification_manager.get())} - { - m_arrange_id = add_job(std::make_unique(m_pri, m->q)); - m_fill_bed_id = add_job(std::make_unique(m_pri, m->q)); - m_rotoptimize_id = add_job(std::make_unique(m_pri, m->q)); - m_sla_import_id = add_job(std::make_unique(m_pri, m->q)); - } - - void arrange() - { - m->take_snapshot(_L("Arrange")); - start(m_arrange_id); - } - - void fill_bed() - { - m->take_snapshot(_L("Fill bed")); - start(m_fill_bed_id); - } - - void optimize_rotation() - { - m->take_snapshot(_L("Optimize Rotation")); - start(m_rotoptimize_id); - } - - void import_sla_arch() - { - m->take_snapshot(_L("Import SLA archive")); - start(m_sla_import_id); - } - - } m_ui_jobs; + // TODO: A mechanism would be useful for blocking the plater interactions: + // objects would be frozen for the user. In case of arrange, an animation + // could be shown, or with the optimize orientations, partial results + // could be displayed. + BoostThreadWorker m_worker; + SLAImportDialog * m_sla_import_dlg; bool delayed_scene_refresh; std::string delayed_error_message; @@ -1990,7 +1951,8 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame) })) , sidebar(new Sidebar(q)) , notification_manager(std::make_unique(q)) - , m_ui_jobs(this) + , m_worker{std::make_unique(notification_manager.get()), "ui_worker"} + , m_sla_import_dlg{new SLAImportDialog{q}} , delayed_scene_refresh(false) , view_toolbar(GLToolbar::Radio, "View") , collapse_toolbar(GLToolbar::Normal, "Collapse") @@ -2219,6 +2181,10 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame) bool is_collapsed = wxGetApp().app_config->get("collapsed_sidebar") == "1"; sidebar->collapse(is_collapsed); } + + // Ensure that messages from the worker thread to the UI thread are processed + // continuously. + main_frame->Bind(wxEVT_IDLE, [this](const wxIdleEvent &){ m_worker.process_events(); }); } Plater::priv::~priv() @@ -2926,7 +2892,7 @@ void Plater::priv::remove(size_t obj_idx) if (view3D->is_layers_editing_enabled()) view3D->enable_layers_editing(false); - m_ui_jobs.cancel_all(); + m_worker.cancel_all(); model.delete_object(obj_idx); update(); // Delete object from Sidebar list. Do it after update, so that the GLScene selection is updated with the modified model. @@ -2941,7 +2907,7 @@ void Plater::priv::delete_object_from_model(size_t obj_idx) if (! model.objects[obj_idx]->name.empty()) snapshot_label += ": " + wxString::FromUTF8(model.objects[obj_idx]->name.c_str()); Plater::TakeSnapshot snapshot(q, snapshot_label); - m_ui_jobs.cancel_all(); + m_worker.cancel_all(); model.delete_object(obj_idx); update(); object_list_changed(); @@ -2959,7 +2925,7 @@ void Plater::priv::delete_all_objects_from_model() view3D->get_canvas3d()->reset_sequential_print_clearance(); - m_ui_jobs.cancel_all(); + m_worker.cancel_all(); // Stop and reset the Print content. background_process.reset(); @@ -2991,7 +2957,7 @@ void Plater::priv::reset() view3D->get_canvas3d()->reset_sequential_print_clearance(); - m_ui_jobs.cancel_all(); + m_worker.cancel_all(); // Stop and reset the Print content. this->background_process.reset(); @@ -3292,7 +3258,7 @@ unsigned int Plater::priv::update_background_process(bool force_validation, bool // Restart background processing thread based on a bitmask of UpdateBackgroundProcessReturnState. bool Plater::priv::restart_background_process(unsigned int state) { - if (m_ui_jobs.is_any_running()) { + if (!m_worker.is_idle()) { // Avoid a race condition return false; } @@ -3920,7 +3886,7 @@ void Plater::priv::on_select_preset(wxCommandEvent &evt) void Plater::priv::on_slicing_update(SlicingStatusEvent &evt) { if (evt.status.percent >= -1) { - if (m_ui_jobs.is_any_running()) { + if (!m_worker.is_idle()) { // Avoid a race condition return; } @@ -4609,7 +4575,7 @@ bool Plater::priv::can_simplify() const bool Plater::priv::can_increase_instances() const { - if (m_ui_jobs.is_any_running() + if (!m_worker.is_idle() || q->canvas3D()->get_gizmos_manager().is_in_editing_mode()) return false; @@ -4619,7 +4585,7 @@ bool Plater::priv::can_increase_instances() const bool Plater::priv::can_decrease_instances() const { - if (m_ui_jobs.is_any_running() + if (!m_worker.is_idle() || q->canvas3D()->get_gizmos_manager().is_in_editing_mode()) return false; @@ -4639,7 +4605,7 @@ bool Plater::priv::can_split_to_volumes() const bool Plater::priv::can_arrange() const { - return !model.objects.empty() && !m_ui_jobs.is_any_running(); + return !model.objects.empty() && m_worker.is_idle(); } bool Plater::priv::can_layers_editing() const @@ -5093,8 +5059,9 @@ void Plater::add_model(bool imperial_units/* = false*/) void Plater::import_sl1_archive() { - if (!p->m_ui_jobs.is_any_running()) - p->m_ui_jobs.import_sla_arch(); + if (get_ui_job_worker().is_idle() && p->m_sla_import_dlg->ShowModal() == wxID_OK) { + replace_job(*this, p->m_sla_import_dlg); + } } void Plater::extract_config_from_project() @@ -5376,12 +5343,9 @@ bool Plater::load_files(const wxArrayString& filenames) void Plater::update() { p->update(); } -void Plater::stop_jobs() { p->m_ui_jobs.stop_all(); } +Worker &Plater::get_ui_job_worker() { return p->m_worker; } -bool Plater::is_any_job_running() const -{ - return p->m_ui_jobs.is_any_running(); -} +const Worker &Plater::get_ui_job_worker() const { return p->m_worker; } void Plater::update_ui_from_settings() { p->update_ui_from_settings(); } @@ -5422,7 +5386,7 @@ void Plater::remove_selected() return; Plater::TakeSnapshot snapshot(this, _L("Delete Selected Objects")); - p->m_ui_jobs.cancel_all(); + get_ui_job_worker().cancel_all(); p->view3D->delete_selected(); } @@ -5531,8 +5495,8 @@ void Plater::set_number_of_copies(/*size_t num*/) void Plater::fill_bed_with_instances() { - if (!p->m_ui_jobs.is_any_running()) - p->m_ui_jobs.fill_bed(); + if (get_ui_job_worker().is_idle()) + replace_job(*this); } bool Plater::is_selection_empty() const @@ -5934,7 +5898,7 @@ void Plater::reslice() return; // Stop arrange and (or) optimize rotation tasks. - this->stop_jobs(); + this->get_ui_job_worker().cancel_all(); if (printer_technology() == ptSLA) { for (auto& object : model().objects) @@ -6398,8 +6362,8 @@ GLCanvas3D* Plater::get_current_canvas3D() void Plater::arrange() { - if (!p->m_ui_jobs.is_any_running()) - p->m_ui_jobs.arrange(); + if (get_ui_job_worker().is_idle()) + replace_job(*this); } void Plater::set_current_canvas_as_dirty() @@ -6570,7 +6534,6 @@ void Plater::suppress_background_process(const bool stop_background_process) void Plater::mirror(Axis axis) { p->mirror(axis); } void Plater::split_object() { p->split_object(); } void Plater::split_volume() { p->split_volume(); } -void Plater::optimize_rotation() { if (!p->m_ui_jobs.is_any_running()) p->m_ui_jobs.optimize_rotation(); } void Plater::update_menus() { p->menus.update(); } void Plater::show_action_buttons(const bool ready_to_slice) const { p->show_action_buttons(ready_to_slice); } diff --git a/src/slic3r/GUI/Plater.hpp b/src/slic3r/GUI/Plater.hpp index d3eead4ede..b0c5a354d5 100644 --- a/src/slic3r/GUI/Plater.hpp +++ b/src/slic3r/GUI/Plater.hpp @@ -14,6 +14,7 @@ #include "libslic3r/BoundingBox.hpp" #include "libslic3r/GCode/GCodeProcessor.hpp" #include "Jobs/Job.hpp" +#include "Jobs/Worker.hpp" #include "Search.hpp" class wxButton; @@ -177,8 +178,41 @@ public: const wxString& get_last_loaded_gcode() const { return m_last_loaded_gcode; } void update(); - void stop_jobs(); - bool is_any_job_running() const; + + // Get the worker handling the UI jobs (arrange, fill bed, etc...) + // Here is an example of starting up an ad-hoc job: + // queue_job( + // get_ui_job_worker(), + // [](Job::Ctl &ctl) { + // // Executed in the worker thread + // + // CursorSetterRAII cursor_setter{ctl}; + // std::string msg = "Running"; + // + // ctl.update_status(0, msg); + // for (int i = 0; i < 100; i++) { + // usleep(100000); + // if (ctl.was_canceled()) break; + // ctl.update_status(i + 1, msg); + // } + // ctl.update_status(100, msg); + // }, + // [](bool, std::exception_ptr &e) { + // // Executed in UI thread after the work is done + // + // try { + // if (e) std::rethrow_exception(e); + // } catch (std::exception &e) { + // BOOST_LOG_TRIVIAL(error) << e.what(); + // } + // e = nullptr; + // }); + // This would result in quick run of the progress indicator notification + // from 0 to 100. Use replace_job() instead of queue_job() to cancel all + // pending jobs. + Worker& get_ui_job_worker(); + const Worker & get_ui_job_worker() const; + void select_view(const std::string& direction); void select_view_3D(const std::string& name); @@ -302,7 +336,6 @@ public: void mirror(Axis axis); void split_object(); void split_volume(); - void optimize_rotation(); bool can_delete() const; bool can_delete_all() const; From 2b25c6fab2be2c21ab8ba7bdb681053326eabd19 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Wed, 1 Dec 2021 09:45:26 +0100 Subject: [PATCH 02/13] Clarify doc comment for replace_job --- src/slic3r/GUI/Jobs/Worker.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/slic3r/GUI/Jobs/Worker.hpp b/src/slic3r/GUI/Jobs/Worker.hpp index 459349d5b5..43f3dacc5b 100644 --- a/src/slic3r/GUI/Jobs/Worker.hpp +++ b/src/slic3r/GUI/Jobs/Worker.hpp @@ -77,10 +77,11 @@ inline bool queue_job(Worker &w, std::unique_ptr j) return w.start_next(std::move(j)); } -// Replace the current job queue with a new job. This cancels all jobs and +// Replace the current job queue with a new job. The signature is the same +// as for queue_job(). This cancels all jobs and // will not wait. The new job will begin after the queue cancels properly. -// Note that this can be called from the UI thread and will not block it if the -// jobs take longer to cancel. +// Note that this can be called from the UI thread and will not block it if +// the jobs take longer to cancel. template bool replace_job(Worker &w, Args&& ...args) { w.cancel_all(); From a802bdc764d4319af2aaaaca3d9adaa7c2aa9dd4 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Wed, 1 Dec 2021 10:02:01 +0100 Subject: [PATCH 03/13] Prevent accidental stopping of BoostThreadWorker before destruction --- src/slic3r/GUI/Jobs/BoostThreadWorker.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp index 3d0cdb678f..21455e103a 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp @@ -94,9 +94,9 @@ constexpr int ABORT_WAIT_MAX_MS = 10000; BoostThreadWorker::~BoostThreadWorker() { - replace_job(*this, nullptr); - try { + cancel_all(); + m_input_queue.push(JobEntry{nullptr}); join(ABORT_WAIT_MAX_MS); } catch(...) { BOOST_LOG_TRIVIAL(error) @@ -130,8 +130,10 @@ void BoostThreadWorker::process_events() bool BoostThreadWorker::start_next(std::unique_ptr job) { - m_input_queue.push(JobEntry{std::move(job)}); - return true; + if (job) + m_input_queue.push(JobEntry{std::move(job)}); + + return bool{job}; } }} // namespace Slic3r::GUI From 3be7d5f0dc864a54ba646d643f397c6e67790dfd Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Wed, 1 Dec 2021 11:09:10 +0100 Subject: [PATCH 04/13] Make a PlaterWorker to handle PlaterJobs --- src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp | 4 +- src/slic3r/GUI/Jobs/PlaterJob.hpp | 102 +++++++++++++----------- src/slic3r/GUI/Plater.cpp | 23 +++--- 3 files changed, 72 insertions(+), 57 deletions(-) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp b/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp index bb70150c6f..999c81b5ef 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp @@ -545,8 +545,8 @@ GLGizmoRotate3D::RotoptimzeWindow::RotoptimzeWindow(ImGuiWrapper * imgui, imgui->disabled_begin(true); if ( imgui->button(btn_txt) ) { - auto p = wxGetApp().plater(); - replace_job(*p); + replace_job(wxGetApp().plater()->get_ui_job_worker(), + std::make_unique()); } imgui->disabled_end(); diff --git a/src/slic3r/GUI/Jobs/PlaterJob.hpp b/src/slic3r/GUI/Jobs/PlaterJob.hpp index 8c8d2e3787..c20075f0e7 100644 --- a/src/slic3r/GUI/Jobs/PlaterJob.hpp +++ b/src/slic3r/GUI/Jobs/PlaterJob.hpp @@ -13,60 +13,70 @@ namespace Slic3r { namespace GUI { class Plater; -template -class PlaterJob : public Job { - JobSubclass m_job; - Plater *m_plater; +template +class PlaterWorker: public Worker { + WorkerSubclass m_w; + + class PlaterJob : public Job { + std::unique_ptr m_job; + Plater *m_plater; + + public: + void process(Ctl &c) override + { + CursorSetterRAII busycursor{c}; + m_job->process(c); + } + + void finalize(bool canceled, std::exception_ptr &eptr) override + { + m_job->finalize(canceled, eptr); + + if (eptr) try { + std::rethrow_exception(eptr); + } catch (std::exception &e) { + show_error(m_plater, _L("An unexpected error occured: ") + e.what()); + eptr = nullptr; + } + } + + PlaterJob(std::unique_ptr j) + : m_job{std::move(j)}, m_plater{wxGetApp().plater()} + { + // TODO: decide if disabling slice button during UI job is what we want. + // if (m_plater) + // m_plater->sidebar().enable_buttons(false); + } + + ~PlaterJob() override + { + // TODO: decide if disabling slice button during UI job is what we want. + + // Reload scene ensures that the slice button gets properly + // enabled or disabled after the job finishes, depending on the + // state of slicing. This might be an overkill but works for now. + // if (m_plater) + // m_plater->canvas3D()->reload_scene(false); + } + }; public: - void process(Ctl &c) override + template + PlaterWorker(WorkerArgs &&...args) : m_w{std::forward(args)...} {} + + // Always package the job argument into a PlaterJob + bool start_next(std::unique_ptr job) override { - CursorSetterRAII busycursor{c}; - m_job.process(c); + return m_w.start_next(std::make_unique(std::move(job))); } - void finalize(bool canceled, std::exception_ptr &eptr) override - { - m_job.finalize(canceled, eptr); - - if (eptr) try { - std::rethrow_exception(eptr); - } catch (std::exception &e) { - show_error(m_plater, _L("An unexpected error occured: ") + e.what()); - eptr = nullptr; - } - } - - template - PlaterJob(Args &&...args) - : m_job(std::forward(args)...), m_plater{wxGetApp().plater()} - { - // TODO: decide if disabling slice button during UI job is what we want. -// if (m_plater) -// m_plater->sidebar().enable_buttons(false); - } - - ~PlaterJob() override - { - // TODO: decide if disabling slice button during UI job is what we want. - - // Reload scene ensures that the slice button gets properly - // enabled or disabled after the job finishes, depending on the - // state of slicing. This might be an overkill but works for now. -// if (m_plater) -// m_plater->canvas3D()->reload_scene(false); - } + bool is_idle() const override { return m_w.is_idle(); } + void cancel() override { m_w.cancel(); } + void cancel_all() override { m_w.cancel_all(); } + void process_events() override { m_w.process_events(); } }; -template -void replace_job(Plater& p, Args && ...args) -{ - replace_job(p.get_ui_job_worker(), - std::make_unique>( - std::forward(args)...)); -} - }} // namespace Slic3r::GUI #endif // PLATERJOB_HPP diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index c4d1f31c28..5a77d00008 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -1645,8 +1645,8 @@ struct Plater::priv // objects would be frozen for the user. In case of arrange, an animation // could be shown, or with the optimize orientations, partial results // could be displayed. - BoostThreadWorker m_worker; - SLAImportDialog * m_sla_import_dlg; + PlaterWorker m_worker; + SLAImportDialog * m_sla_import_dlg; bool delayed_scene_refresh; std::string delayed_error_message; @@ -2184,7 +2184,9 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame) // Ensure that messages from the worker thread to the UI thread are processed // continuously. - main_frame->Bind(wxEVT_IDLE, [this](const wxIdleEvent &){ m_worker.process_events(); }); + main_frame->Bind(wxEVT_IDLE, [this](const wxIdleEvent &) { + m_worker.process_events(); + }); } Plater::priv::~priv() @@ -5059,8 +5061,9 @@ void Plater::add_model(bool imperial_units/* = false*/) void Plater::import_sl1_archive() { - if (get_ui_job_worker().is_idle() && p->m_sla_import_dlg->ShowModal() == wxID_OK) { - replace_job(*this, p->m_sla_import_dlg); + auto &w = get_ui_job_worker(); + if (w.is_idle() && p->m_sla_import_dlg->ShowModal() == wxID_OK) { + replace_job(w, std::make_unique(p->m_sla_import_dlg)); } } @@ -5495,8 +5498,9 @@ void Plater::set_number_of_copies(/*size_t num*/) void Plater::fill_bed_with_instances() { - if (get_ui_job_worker().is_idle()) - replace_job(*this); + auto &w = get_ui_job_worker(); + if (w.is_idle()) + replace_job(w, std::make_unique()); } bool Plater::is_selection_empty() const @@ -6362,8 +6366,9 @@ GLCanvas3D* Plater::get_current_canvas3D() void Plater::arrange() { - if (get_ui_job_worker().is_idle()) - replace_job(*this); + auto &w = get_ui_job_worker(); + if (w.is_idle()) + replace_job(w, std::make_unique()); } void Plater::set_current_canvas_as_dirty() From 17f4b1bea317a0cf59762ce830cc6439c18058c1 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Wed, 1 Dec 2021 12:47:14 +0100 Subject: [PATCH 05/13] Avoid issue with invisible status indication. --- src/slic3r/CMakeLists.txt | 2 +- src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp | 1 - src/slic3r/GUI/Jobs/ArrangeJob.cpp | 7 +-- src/slic3r/GUI/Jobs/BoostThreadWorker.cpp | 8 ++-- src/slic3r/GUI/Jobs/BoostThreadWorker.hpp | 4 +- src/slic3r/GUI/Jobs/FillBedJob.cpp | 8 ++-- .../Jobs/{PlaterJob.hpp => PlaterWorker.hpp} | 48 +++++++++++++++---- src/slic3r/GUI/Plater.cpp | 10 +--- 8 files changed, 57 insertions(+), 31 deletions(-) rename src/slic3r/GUI/Jobs/{PlaterJob.hpp => PlaterWorker.hpp} (59%) diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index f8a0124ced..34c0efd014 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -173,7 +173,7 @@ set(SLIC3R_GUI_SOURCES GUI/Jobs/BoostThreadWorker.hpp GUI/Jobs/BoostThreadWorker.cpp GUI/Jobs/BusyCursorJob.hpp - GUI/Jobs/PlaterJob.hpp + GUI/Jobs/PlaterWorker.hpp GUI/Jobs/ArrangeJob.hpp GUI/Jobs/ArrangeJob.cpp GUI/Jobs/RotoptimizeJob.hpp diff --git a/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp b/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp index 999c81b5ef..700b1c17e8 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoRotate.cpp @@ -11,7 +11,6 @@ #include "libslic3r/PresetBundle.hpp" #include "slic3r/GUI/Jobs/RotoptimizeJob.hpp" -#include "slic3r/GUI/Jobs/PlaterJob.hpp" namespace Slic3r { namespace GUI { diff --git a/src/slic3r/GUI/Jobs/ArrangeJob.cpp b/src/slic3r/GUI/Jobs/ArrangeJob.cpp index 500fc3a10d..3c7dad0a61 100644 --- a/src/slic3r/GUI/Jobs/ArrangeJob.cpp +++ b/src/slic3r/GUI/Jobs/ArrangeJob.cpp @@ -164,10 +164,11 @@ void ArrangeJob::prepare() void ArrangeJob::process(Ctl &ctl) { - ctl.call_on_main_thread([this]{ prepare(); }).wait(); - static const auto arrangestr = _u8L("Arranging"); + ctl.update_status(0, arrangestr); + ctl.call_on_main_thread([this]{ prepare(); }).wait();; + arrangement::ArrangeParams params = get_arrange_params(m_plater); auto count = unsigned(m_selected.size() + m_unprintable.size()); @@ -196,7 +197,7 @@ void ArrangeJob::process(Ctl &ctl) _u8L("Arranging done.")); } -ArrangeJob::ArrangeJob() : m_plater{wxGetApp().plater()} { } +ArrangeJob::ArrangeJob() : m_plater{wxGetApp().plater()} {} static std::string concat_strings(const std::set &strings, const std::string &delim = "\n") diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp index 21455e103a..40729a0e89 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp @@ -29,7 +29,7 @@ void BoostThreadWorker::WorkerMessage::deliver(BoostThreadWorker &runner) case MainThreadCall: { MainThreadCallData &calldata = std::get(m_data); calldata.fn(); - calldata.barrier.set_value(); + calldata.promise.set_value(); break; } @@ -69,7 +69,7 @@ void BoostThreadWorker::update_status(int st, const std::string &msg) std::future BoostThreadWorker::call_on_main_thread(std::function fn) { MainThreadCallData cbdata{std::move(fn), {}}; - std::future future = cbdata.barrier.get_future(); + std::future future = cbdata.promise.get_future(); m_output_queue.push(std::move(cbdata)); @@ -77,8 +77,8 @@ std::future BoostThreadWorker::call_on_main_thread(std::function } BoostThreadWorker::BoostThreadWorker(std::shared_ptr pri, - boost::thread::attributes &attribs, - const char * name) + boost::thread::attributes &attribs, + const char * name) : m_progress(std::move(pri)), m_name{name} { if (m_progress) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp index aeacf39878..31ca9b5910 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp @@ -39,7 +39,7 @@ class BoostThreadWorker : public Worker, private Job::Ctl struct MainThreadCallData { std::function fn; - std::promise barrier; + std::promise promise; }; class WorkerMessage @@ -55,6 +55,8 @@ class BoostThreadWorker : public Worker, private Job::Ctl WorkerMessage(JobEntry &&entry) : m_data{std::move(entry)} {} WorkerMessage(MainThreadCallData fn) : m_data{std::move(fn)} {} + int get_type () const { return m_data.index(); } + void deliver(BoostThreadWorker &runner); }; diff --git a/src/slic3r/GUI/Jobs/FillBedJob.cpp b/src/slic3r/GUI/Jobs/FillBedJob.cpp index 979db77cba..c7d69eb500 100644 --- a/src/slic3r/GUI/Jobs/FillBedJob.cpp +++ b/src/slic3r/GUI/Jobs/FillBedJob.cpp @@ -105,7 +105,9 @@ void FillBedJob::prepare() void FillBedJob::process(Ctl &ctl) { - ctl.call_on_main_thread([this]{ prepare(); }).wait(); + auto statustxt = _u8L("Filling bed"); + ctl.call_on_main_thread([this] { prepare(); }).wait(); + ctl.update_status(0, statustxt); if (m_object_idx == -1 || m_selected.empty()) return; @@ -121,10 +123,6 @@ void FillBedJob::process(Ctl &ctl) return ctl.was_canceled() || do_stop; }; - auto statustxt = _u8L("Filling bed"); - - ctl.update_status(0, statustxt); - params.progressind = [this, &ctl, &statustxt](unsigned st) { if (st > 0) ctl.update_status(int(m_status_range - st) * 100 / status_range(), statustxt); diff --git a/src/slic3r/GUI/Jobs/PlaterJob.hpp b/src/slic3r/GUI/Jobs/PlaterWorker.hpp similarity index 59% rename from src/slic3r/GUI/Jobs/PlaterJob.hpp rename to src/slic3r/GUI/Jobs/PlaterWorker.hpp index c20075f0e7..ee64718e65 100644 --- a/src/slic3r/GUI/Jobs/PlaterJob.hpp +++ b/src/slic3r/GUI/Jobs/PlaterWorker.hpp @@ -1,5 +1,5 @@ -#ifndef PLATERJOB_HPP -#define PLATERJOB_HPP +#ifndef PLATERWORKER_HPP +#define PLATERWORKER_HPP #include "BusyCursorJob.hpp" @@ -24,8 +24,32 @@ class PlaterWorker: public Worker { public: void process(Ctl &c) override { - CursorSetterRAII busycursor{c}; - m_job->process(c); + // Ensure that wxWidgets processing wakes up to handle outgoing + // messages in plater's wxIdle handler. Otherwise it might happen + // that the message will only be processed when an event like mouse + // move comes along which might be too late. + struct WakeUpCtl: Ctl { + Ctl &ctl; + WakeUpCtl(Ctl &c) : ctl{c} {} + + void update_status(int st, const std::string &msg = "") override + { + wxWakeUpIdle(); + ctl.update_status(st, msg); + } + + bool was_canceled() const override { return ctl.was_canceled(); } + + std::future call_on_main_thread(std::function fn) override + { + wxWakeUpIdle(); + return ctl.call_on_main_thread(std::move(fn)); + } + + } wctl{c}; + + CursorSetterRAII busycursor{wctl}; + m_job->process(wctl); } void finalize(bool canceled, std::exception_ptr &eptr) override @@ -60,10 +84,18 @@ class PlaterWorker: public Worker { } }; -public: - template - PlaterWorker(WorkerArgs &&...args) : m_w{std::forward(args)...} {} +public: + template + PlaterWorker(Plater *plater, WorkerArgs &&...args) + : m_w{std::forward(args)...} + { + // Ensure that messages from the worker thread to the UI thread are + // processed continuously. + plater->Bind(wxEVT_IDLE, [this](wxIdleEvent &) { + m_w.process_events(); + }); + } // Always package the job argument into a PlaterJob bool start_next(std::unique_ptr job) override @@ -74,7 +106,7 @@ public: bool is_idle() const override { return m_w.is_idle(); } void cancel() override { m_w.cancel(); } void cancel_all() override { m_w.cancel_all(); } - void process_events() override { m_w.process_events(); } + void process_events() override {} }; }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 5a77d00008..9804b19295 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -75,7 +75,7 @@ #include "Jobs/SLAImportJob.hpp" #include "Jobs/SLAImportDialog.hpp" #include "Jobs/NotificationProgressIndicator.hpp" -#include "Jobs/PlaterJob.hpp" +#include "Jobs/PlaterWorker.hpp" #include "Jobs/BoostThreadWorker.hpp" #include "BackgroundSlicingProcess.hpp" #include "PrintHostDialogs.hpp" @@ -1951,7 +1951,7 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame) })) , sidebar(new Sidebar(q)) , notification_manager(std::make_unique(q)) - , m_worker{std::make_unique(notification_manager.get()), "ui_worker"} + , m_worker{q, std::make_unique(notification_manager.get()), "ui_worker"} , m_sla_import_dlg{new SLAImportDialog{q}} , delayed_scene_refresh(false) , view_toolbar(GLToolbar::Radio, "View") @@ -2181,12 +2181,6 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame) bool is_collapsed = wxGetApp().app_config->get("collapsed_sidebar") == "1"; sidebar->collapse(is_collapsed); } - - // Ensure that messages from the worker thread to the UI thread are processed - // continuously. - main_frame->Bind(wxEVT_IDLE, [this](const wxIdleEvent &) { - m_worker.process_events(); - }); } Plater::priv::~priv() From 7352e1a01af65ef2dcf17c1bfc20b784c2cb0dfc Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Thu, 2 Dec 2021 15:06:35 +0100 Subject: [PATCH 06/13] Basic tests for BoostThreadWorker Separate job tests --- src/slic3r/GUI/Jobs/BoostThreadWorker.hpp | 2 - tests/slic3rutils/CMakeLists.txt | 1 + tests/slic3rutils/slic3r_jobs_tests.cpp | 134 ++++++++++++++++++++++ 3 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 tests/slic3rutils/slic3r_jobs_tests.cpp diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp index 31ca9b5910..f2da275c35 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp @@ -5,8 +5,6 @@ #include "Worker.hpp" -#include - #include #include diff --git a/tests/slic3rutils/CMakeLists.txt b/tests/slic3rutils/CMakeLists.txt index e1ac113aef..be1b645d70 100644 --- a/tests/slic3rutils/CMakeLists.txt +++ b/tests/slic3rutils/CMakeLists.txt @@ -1,6 +1,7 @@ get_filename_component(_TEST_NAME ${CMAKE_CURRENT_LIST_DIR} NAME) add_executable(${_TEST_NAME}_tests ${_TEST_NAME}_tests_main.cpp + slic3r_jobs_tests.cpp ) target_link_libraries(${_TEST_NAME}_tests test_common libslic3r_gui libslic3r) diff --git a/tests/slic3rutils/slic3r_jobs_tests.cpp b/tests/slic3rutils/slic3r_jobs_tests.cpp new file mode 100644 index 0000000000..1d4f165af1 --- /dev/null +++ b/tests/slic3rutils/slic3r_jobs_tests.cpp @@ -0,0 +1,134 @@ +#include "catch2/catch.hpp" + +#include "slic3r/GUI/Jobs/BoostThreadWorker.hpp" +#include "slic3r/GUI/Jobs/ProgressIndicator.hpp" + +struct Progress: Slic3r::ProgressIndicator { + int range = 100; + int pr = 0; + std::string statustxt; + void set_range(int r) override { range = r; } + void set_cancel_callback(CancelFn = CancelFn()) override {} + void set_progress(int p) override { pr = p; } + void set_status_text(const char *txt) override { statustxt = txt; } + int get_range() const override { return range; } +}; + +TEST_CASE("nullptr job should be ignored", "[Jobs]") { + Slic3r::GUI::BoostThreadWorker worker{std::make_unique()}; + worker.push(nullptr); + + REQUIRE(worker.is_idle()); +} + +TEST_CASE("State should not be idle while running a job", "[Jobs]") { + using namespace Slic3r; + using namespace Slic3r::GUI; + BoostThreadWorker worker{std::make_unique(), "worker_thread"}; + + queue_job(worker, [&worker](Job::Ctl &ctl) { + ctl.call_on_main_thread([&worker] { + REQUIRE(!worker.is_idle()); + }).wait(); + }); + + while (!worker.is_idle()) + worker.process_events(); + + REQUIRE(worker.is_idle()); +} + +TEST_CASE("Status messages should be received by the main thread during job execution", "[Jobs]") { + using namespace Slic3r; + using namespace Slic3r::GUI; + auto pri = std::make_shared(); + BoostThreadWorker worker{pri}; + + queue_job(worker, [](Job::Ctl &ctl){ + for (int s = 0; s <= 100; ++s) { + ctl.update_status(s, "Running"); + } + }); + + while (!worker.is_idle()) + worker.process_events(); + + REQUIRE(pri->pr == 100); + REQUIRE(pri->statustxt == "Running"); +} + +TEST_CASE("Cancellation should be recognized be the worker", "[Jobs]") { + using namespace Slic3r; + using namespace Slic3r::GUI; + + auto pri = std::make_shared(); + BoostThreadWorker worker{pri}; + + queue_job( + worker, + [](Job::Ctl &ctl) { + for (int s = 0; s <= 100; ++s) { + usleep(10000); + ctl.update_status(s, "Running"); + if (ctl.was_canceled()) break; + } + }, + [](bool cancelled, std::exception_ptr &) { // finalize + REQUIRE(cancelled == true); + }); + + usleep(1000); + worker.cancel(); + + while (!worker.is_idle()) + worker.process_events(); + + REQUIRE(pri->pr != 100); +} + +TEST_CASE("Cancel_all should remove all pending jobs", "[Jobs]") { + using namespace Slic3r; + using namespace Slic3r::GUI; + + auto pri = std::make_shared(); + BoostThreadWorker worker{pri}; + + std::array jobres = {false}; + + queue_job(worker, [&jobres](Job::Ctl &) { jobres[0] = true; usleep(1000); }); + queue_job(worker, [&jobres](Job::Ctl &) { jobres[1] = true; usleep(1000); }); + queue_job(worker, [&jobres](Job::Ctl &) { jobres[2] = true; usleep(1000); }); + queue_job(worker, [&jobres](Job::Ctl &) { jobres[3] = true; usleep(1000); }); + + usleep(500); + worker.cancel_all(); + + REQUIRE(jobres[0] == true); + REQUIRE(jobres[1] == false); + REQUIRE(jobres[2] == false); + REQUIRE(jobres[3] == false); +} + +TEST_CASE("Exception should be properly forwarded to finalize()", "[Jobs]") { + using namespace Slic3r; + using namespace Slic3r::GUI; + + auto pri = std::make_shared(); + BoostThreadWorker worker{pri}; + + queue_job( + worker, [](Job::Ctl &) { throw std::runtime_error("test"); }, + [](bool /*canceled*/, std::exception_ptr &eptr) { + REQUIRE(eptr != nullptr); + try { + std::rethrow_exception(eptr); + } catch (std::runtime_error &e) { + REQUIRE(std::string(e.what()) == "test"); + } + + eptr = nullptr; + }); + + while (!worker.is_idle()) + worker.process_events(); +} From 7e070d393ec536682bfa34fbd6bd7cf661353f8f Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Thu, 2 Dec 2021 14:55:52 +0100 Subject: [PATCH 07/13] Fix issue with non atomic transition to running state After popping a job from input queue --- src/slic3r/GUI/Jobs/BoostThreadWorker.cpp | 13 +++++++------ src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp | 12 ++++++++++-- tests/slic3rutils/slic3r_jobs_tests.cpp | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp index 40729a0e89..9e7e9c15ad 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp @@ -45,7 +45,6 @@ void BoostThreadWorker::run() stop = true; else { m_canceled.store(false); - m_running.store(true); try { e.job->process(*this); @@ -55,9 +54,9 @@ void BoostThreadWorker::run() e.canceled = m_canceled.load(); m_output_queue.push(std::move(e)); // finalization message - m_running.store(false); } - }); + m_running.store(false); + }, &m_running); }; } @@ -94,14 +93,16 @@ constexpr int ABORT_WAIT_MAX_MS = 10000; BoostThreadWorker::~BoostThreadWorker() { + bool joined = false; try { cancel_all(); m_input_queue.push(JobEntry{nullptr}); - join(ABORT_WAIT_MAX_MS); - } catch(...) { + joined = join(ABORT_WAIT_MAX_MS); + } catch(...) {} + + if (!joined) BOOST_LOG_TRIVIAL(error) << "Could not join worker thread '" << m_name << "'"; - } } bool BoostThreadWorker::join(int timeout_ms) diff --git a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp index f01698cb2f..3b1c4abeaf 100644 --- a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp +++ b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp @@ -5,6 +5,7 @@ #include #include #include +#include namespace Slic3r { namespace GUI { @@ -21,7 +22,7 @@ class ThreadSafeQueueSPSC public: // Consume one element, block if the queue is empty. - template void consume_one_blk(Fn &&fn) + template void consume_one_blk(Fn &&fn, std::atomic *pop_flag = nullptr) { static_assert(!std::is_reference_v, ""); static_assert(std::is_default_constructible_v, ""); @@ -38,6 +39,9 @@ public: el = m_queue.front(); m_queue.pop(); + + if (pop_flag) // The optional atomic is set before the lock us unlocked + pop_flag->store(true); } fn(el); @@ -50,7 +54,11 @@ public: { std::unique_lock lk{m_mutex}; if (!m_queue.empty()) { - el = std::move(m_queue.front()); + if constexpr (std::is_move_assignable_v) + el = std::move(m_queue.front()); + else + el = m_queue.front(); + m_queue.pop(); } else return false; diff --git a/tests/slic3rutils/slic3r_jobs_tests.cpp b/tests/slic3rutils/slic3r_jobs_tests.cpp index 1d4f165af1..9c75d79ac9 100644 --- a/tests/slic3rutils/slic3r_jobs_tests.cpp +++ b/tests/slic3rutils/slic3r_jobs_tests.cpp @@ -86,7 +86,7 @@ TEST_CASE("Cancellation should be recognized be the worker", "[Jobs]") { REQUIRE(pri->pr != 100); } -TEST_CASE("Cancel_all should remove all pending jobs", "[Jobs]") { +TEST_CASE("cancel_all should remove all pending jobs", "[Jobs]") { using namespace Slic3r; using namespace Slic3r::GUI; From 583c123c9703d95a4ae944c981554e7fda3a57d0 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Thu, 2 Dec 2021 15:09:59 +0100 Subject: [PATCH 08/13] Rename start_next() to push PlaterJob refinements --- src/slic3r/GUI/Jobs/BoostThreadWorker.cpp | 2 +- src/slic3r/GUI/Jobs/BoostThreadWorker.hpp | 2 +- src/slic3r/GUI/Jobs/PlaterWorker.hpp | 51 +++++++++++++++-------- src/slic3r/GUI/Jobs/Worker.hpp | 6 +-- 4 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp index 9e7e9c15ad..a6eee28acc 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp @@ -129,7 +129,7 @@ void BoostThreadWorker::process_events() })); } -bool BoostThreadWorker::start_next(std::unique_ptr job) +bool BoostThreadWorker::push(std::unique_ptr job) { if (job) m_input_queue.push(JobEntry{std::move(job)}); diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp index f2da275c35..1775a90bd6 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp @@ -104,7 +104,7 @@ public: BoostThreadWorker &operator=(const BoostThreadWorker &) = delete; BoostThreadWorker &operator=(BoostThreadWorker &&) = delete; - bool start_next(std::unique_ptr job) override; + bool push(std::unique_ptr job) override; bool is_idle() const override { diff --git a/src/slic3r/GUI/Jobs/PlaterWorker.hpp b/src/slic3r/GUI/Jobs/PlaterWorker.hpp index ee64718e65..e924cc6ca7 100644 --- a/src/slic3r/GUI/Jobs/PlaterWorker.hpp +++ b/src/slic3r/GUI/Jobs/PlaterWorker.hpp @@ -1,6 +1,9 @@ #ifndef PLATERWORKER_HPP #define PLATERWORKER_HPP +#include + +#include "Worker.hpp" #include "BusyCursorJob.hpp" #include "slic3r/GUI/GUI.hpp" @@ -16,9 +19,21 @@ class Plater; template class PlaterWorker: public Worker { WorkerSubclass m_w; + Plater *m_plater; - class PlaterJob : public Job { + struct JobHolder : Job { std::unique_ptr m_job; + void process(Ctl &ctl) override { m_job->process(ctl); }; + void finalize(bool canceled, std::exception_ptr &e) override + { + m_job->finalize(canceled, e); + } + JobHolder(std::unique_ptr &&j) : m_job{std::move(j)} {} + }; + + template + class PlaterJob : public Job { + JobSubclass m_job; Plater *m_plater; public: @@ -49,25 +64,27 @@ class PlaterWorker: public Worker { } wctl{c}; CursorSetterRAII busycursor{wctl}; - m_job->process(wctl); + m_job.process(wctl); } void finalize(bool canceled, std::exception_ptr &eptr) override { - m_job->finalize(canceled, eptr); + m_job.finalize(canceled, eptr); if (eptr) try { - std::rethrow_exception(eptr); - } catch (std::exception &e) { - show_error(m_plater, _L("An unexpected error occured: ") + e.what()); - eptr = nullptr; - } + std::rethrow_exception(eptr); + } catch (std::exception &e) { + show_error(m_plater, _L("An unexpected error occured: ") + e.what()); + eptr = nullptr; + } } - PlaterJob(std::unique_ptr j) - : m_job{std::move(j)}, m_plater{wxGetApp().plater()} + template + PlaterJob(Plater *p, Args&&...args) + : m_job{std::forward(args)...}, m_plater{p} { - // TODO: decide if disabling slice button during UI job is what we want. + // TODO: decide if disabling slice button during UI job is what we + // want. // if (m_plater) // m_plater->sidebar().enable_buttons(false); } @@ -84,29 +101,29 @@ class PlaterWorker: public Worker { } }; - public: + template PlaterWorker(Plater *plater, WorkerArgs &&...args) - : m_w{std::forward(args)...} + : m_w{std::forward(args)...}, m_plater{plater} { // Ensure that messages from the worker thread to the UI thread are // processed continuously. plater->Bind(wxEVT_IDLE, [this](wxIdleEvent &) { - m_w.process_events(); + process_events(); }); } // Always package the job argument into a PlaterJob - bool start_next(std::unique_ptr job) override + bool push(std::unique_ptr job) override { - return m_w.start_next(std::make_unique(std::move(job))); + return m_w.push(std::make_unique>(m_plater, std::move(job))); } bool is_idle() const override { return m_w.is_idle(); } void cancel() override { m_w.cancel(); } void cancel_all() override { m_w.cancel_all(); } - void process_events() override {} + void process_events() override { m_w.process_events(); } }; }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/Worker.hpp b/src/slic3r/GUI/Jobs/Worker.hpp index 43f3dacc5b..8d66feb5da 100644 --- a/src/slic3r/GUI/Jobs/Worker.hpp +++ b/src/slic3r/GUI/Jobs/Worker.hpp @@ -14,7 +14,7 @@ class Worker { public: // Queue up a new job after the current one. This call does not block. // Returns false if the job gets discarded. - virtual bool start_next(std::unique_ptr job) = 0; + virtual bool push(std::unique_ptr job) = 0; // Returns true if no job is running and no job message is left to be processed. // This means that nothing is left to finalize or take care of in the main thread. @@ -63,7 +63,7 @@ bool queue_job(Worker &w, ProcessFn fn, FinishFn finishfn) }; auto j = std::make_unique(std::move(fn), std::move(finishfn)); - return w.start_next(std::move(j)); + return w.push(std::move(j)); } template>> @@ -74,7 +74,7 @@ bool queue_job(Worker &w, ProcessFn fn) inline bool queue_job(Worker &w, std::unique_ptr j) { - return w.start_next(std::move(j)); + return w.push(std::move(j)); } // Replace the current job queue with a new job. The signature is the same From e367ef8011ffa419ab1decb035e5fb46f01c8ae1 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Thu, 2 Dec 2021 15:34:20 +0100 Subject: [PATCH 09/13] Fix job tests on Win, don't use usleep() --- tests/slic3rutils/slic3r_jobs_tests.cpp | 31 +++++++++++++++++++------ 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/slic3rutils/slic3r_jobs_tests.cpp b/tests/slic3rutils/slic3r_jobs_tests.cpp index 9c75d79ac9..56c2fab431 100644 --- a/tests/slic3rutils/slic3r_jobs_tests.cpp +++ b/tests/slic3rutils/slic3r_jobs_tests.cpp @@ -1,8 +1,13 @@ #include "catch2/catch.hpp" +#include +#include + #include "slic3r/GUI/Jobs/BoostThreadWorker.hpp" #include "slic3r/GUI/Jobs/ProgressIndicator.hpp" +//#include + struct Progress: Slic3r::ProgressIndicator { int range = 100; int pr = 0; @@ -68,7 +73,7 @@ TEST_CASE("Cancellation should be recognized be the worker", "[Jobs]") { worker, [](Job::Ctl &ctl) { for (int s = 0; s <= 100; ++s) { - usleep(10000); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); ctl.update_status(s, "Running"); if (ctl.was_canceled()) break; } @@ -77,7 +82,7 @@ TEST_CASE("Cancellation should be recognized be the worker", "[Jobs]") { REQUIRE(cancelled == true); }); - usleep(1000); + std::this_thread::sleep_for(std::chrono::milliseconds(1)); worker.cancel(); while (!worker.is_idle()) @@ -95,12 +100,24 @@ TEST_CASE("cancel_all should remove all pending jobs", "[Jobs]") { std::array jobres = {false}; - queue_job(worker, [&jobres](Job::Ctl &) { jobres[0] = true; usleep(1000); }); - queue_job(worker, [&jobres](Job::Ctl &) { jobres[1] = true; usleep(1000); }); - queue_job(worker, [&jobres](Job::Ctl &) { jobres[2] = true; usleep(1000); }); - queue_job(worker, [&jobres](Job::Ctl &) { jobres[3] = true; usleep(1000); }); + queue_job(worker, [&jobres](Job::Ctl &) { + jobres[0] = true; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + }); + queue_job(worker, [&jobres](Job::Ctl &) { + jobres[1] = true; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + }); + queue_job(worker, [&jobres](Job::Ctl &) { + jobres[2] = true; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + }); + queue_job(worker, [&jobres](Job::Ctl &) { + jobres[3] = true; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + }); - usleep(500); + std::this_thread::sleep_for(std::chrono::microseconds(500)); worker.cancel_all(); REQUIRE(jobres[0] == true); From 4d0088e72f029c1dcafec99a1306ef0fc601bda6 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Thu, 2 Dec 2021 15:55:57 +0100 Subject: [PATCH 10/13] Replace std::variant with boost::variant Unavailable on MacOS < 1.14 --- src/slic3r/GUI/Jobs/BoostThreadWorker.cpp | 8 ++++---- src/slic3r/GUI/Jobs/BoostThreadWorker.hpp | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp index a6eee28acc..8475f05373 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp @@ -6,10 +6,10 @@ namespace Slic3r { namespace GUI { void BoostThreadWorker::WorkerMessage::deliver(BoostThreadWorker &runner) { - switch(MsgType(m_data.index())) { + switch(MsgType(get_type())) { case Empty: break; case Status: { - StatusInfo info = std::get(m_data); + auto info = boost::get(m_data); if (runner.get_pri()) { runner.get_pri()->set_progress(info.status); runner.get_pri()->set_status_text(info.msg.c_str()); @@ -17,7 +17,7 @@ void BoostThreadWorker::WorkerMessage::deliver(BoostThreadWorker &runner) break; } case Finalize: { - JobEntry& entry = std::get(m_data); + auto& entry = boost::get(m_data); entry.job->finalize(entry.canceled, entry.eptr); // Unhandled exceptions are rethrown without mercy. @@ -27,7 +27,7 @@ void BoostThreadWorker::WorkerMessage::deliver(BoostThreadWorker &runner) break; } case MainThreadCall: { - MainThreadCallData &calldata = std::get(m_data); + auto &calldata = boost::get(m_data); calldata.fn(); calldata.promise.set_value(); diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp index 1775a90bd6..908ebbb052 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp @@ -1,7 +1,7 @@ #ifndef BOOSTTHREADWORKER_HPP #define BOOSTTHREADWORKER_HPP -#include +#include #include "Worker.hpp" @@ -40,10 +40,12 @@ class BoostThreadWorker : public Worker, private Job::Ctl std::promise promise; }; + struct EmptyMessage {}; + class WorkerMessage { enum MsgType { Empty, Status, Finalize, MainThreadCall }; - std::variant m_data; + boost::variant m_data; public: WorkerMessage() = default; @@ -53,7 +55,7 @@ class BoostThreadWorker : public Worker, private Job::Ctl WorkerMessage(JobEntry &&entry) : m_data{std::move(entry)} {} WorkerMessage(MainThreadCallData fn) : m_data{std::move(fn)} {} - int get_type () const { return m_data.index(); } + int get_type () const { return m_data.which(); } void deliver(BoostThreadWorker &runner); }; From 43f5e61b5f602e0bc33a3b4a1724123768871614 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Thu, 2 Dec 2021 23:05:55 +0100 Subject: [PATCH 11/13] Add possibility to wait for current job to stop. --- src/slic3r/GUI/Jobs/BoostThreadWorker.cpp | 73 ++++++++++++++++++----- src/slic3r/GUI/Jobs/BoostThreadWorker.hpp | 6 ++ src/slic3r/GUI/Jobs/PlaterWorker.hpp | 8 +++ src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp | 23 +++++-- src/slic3r/GUI/Jobs/Worker.hpp | 36 +++++++++-- src/slic3r/GUI/Plater.cpp | 11 +++- tests/slic3rutils/slic3r_jobs_tests.cpp | 13 ++-- 7 files changed, 134 insertions(+), 36 deletions(-) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp index 8475f05373..a4f22be559 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp @@ -39,24 +39,25 @@ void BoostThreadWorker::WorkerMessage::deliver(BoostThreadWorker &runner) void BoostThreadWorker::run() { bool stop = false; - while(!stop) { - m_input_queue.consume_one_blk([this, &stop](JobEntry &e) { - if (!e.job) - stop = true; - else { - m_canceled.store(false); + while (!stop) { + m_input_queue + .consume_one(BlockingWait{0, &m_running}, [this, &stop](JobEntry &e) { + if (!e.job) + stop = true; + else { + m_canceled.store(false); - try { - e.job->process(*this); - } catch (...) { - e.eptr = std::current_exception(); + try { + e.job->process(*this); + } catch (...) { + e.eptr = std::current_exception(); + } + + e.canceled = m_canceled.load(); + m_output_queue.push(std::move(e)); // finalization message } - - e.canceled = m_canceled.load(); - m_output_queue.push(std::move(e)); // finalization message - } - m_running.store(false); - }, &m_running); + m_running.store(false); + }); }; } @@ -96,6 +97,7 @@ BoostThreadWorker::~BoostThreadWorker() bool joined = false; try { cancel_all(); + wait_for_idle(ABORT_WAIT_MAX_MS); m_input_queue.push(JobEntry{nullptr}); joined = join(ABORT_WAIT_MAX_MS); } catch(...) {} @@ -129,6 +131,45 @@ void BoostThreadWorker::process_events() })); } +bool BoostThreadWorker::wait_for_current_job(unsigned timeout_ms) +{ + bool ret = true; + + if (!is_idle()) { + bool was_finish = false; + bool timeout_reached = false; + while (!timeout_reached && !was_finish) { + timeout_reached = + !m_output_queue.consume_one(BlockingWait{timeout_ms}, + [this, &was_finish]( + WorkerMessage &msg) { + msg.deliver(*this); + if (msg.get_type() == + WorkerMessage::Finalize) + was_finish = true; + }); + } + + ret = !timeout_reached; + } + + return ret; +} + +bool BoostThreadWorker::wait_for_idle(unsigned timeout_ms) +{ + bool timeout_reached = false; + while (!timeout_reached && !is_idle()) { + timeout_reached = !m_output_queue + .consume_one(BlockingWait{timeout_ms}, + [this](WorkerMessage &msg) { + msg.deliver(*this); + }); + } + + return !timeout_reached; +} + bool BoostThreadWorker::push(std::unique_ptr job) { if (job) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp index 908ebbb052..2fe39c0a7b 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp @@ -44,7 +44,10 @@ class BoostThreadWorker : public Worker, private Job::Ctl class WorkerMessage { + public: enum MsgType { Empty, Status, Finalize, MainThreadCall }; + + private: boost::variant m_data; public: @@ -127,6 +130,9 @@ public: const ProgressIndicator * get_pri() const { return m_progress.get(); } void process_events() override; + bool wait_for_current_job(unsigned timeout_ms = 0) override; + bool wait_for_idle(unsigned timeout_ms = 0) override; + }; }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/PlaterWorker.hpp b/src/slic3r/GUI/Jobs/PlaterWorker.hpp index e924cc6ca7..186506193f 100644 --- a/src/slic3r/GUI/Jobs/PlaterWorker.hpp +++ b/src/slic3r/GUI/Jobs/PlaterWorker.hpp @@ -124,6 +124,14 @@ public: void cancel() override { m_w.cancel(); } void cancel_all() override { m_w.cancel_all(); } void process_events() override { m_w.process_events(); } + bool wait_for_current_job(unsigned timeout_ms = 0) override + { + return m_w.wait_for_current_job(timeout_ms); + } + bool wait_for_idle(unsigned timeout_ms = 0) override + { + return m_w.wait_for_idle(timeout_ms); + } }; }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp index 3b1c4abeaf..46b4d87b6c 100644 --- a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp +++ b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp @@ -9,6 +9,12 @@ namespace Slic3r { namespace GUI { +struct BlockingWait +{ + unsigned timeout_ms = 0; + std::atomic *pop_flag = nullptr; +}; + // A thread safe queue for one producer and one consumer. Use consume_one_blk // to block on an empty queue. template void consume_one_blk(Fn &&fn, std::atomic *pop_flag = nullptr) + template bool consume_one(const BlockingWait &blkw, Fn &&fn) { static_assert(!std::is_reference_v, ""); static_assert(std::is_default_constructible_v, ""); @@ -31,7 +37,15 @@ public: T el; { std::unique_lock lk{m_mutex}; - m_cond_var.wait(lk, [this]{ return !m_queue.empty(); }); + + auto pred = [this]{ return !m_queue.empty(); }; + if (blkw.timeout_ms > 0) { + auto timeout = std::chrono::milliseconds(blkw.timeout_ms); + if (!m_cond_var.wait_for(lk, timeout, pred)) + return false; + } + else + m_cond_var.wait(lk, pred); if constexpr (std::is_move_assignable_v) el = std::move(m_queue.front()); @@ -40,11 +54,12 @@ public: m_queue.pop(); - if (pop_flag) // The optional atomic is set before the lock us unlocked - pop_flag->store(true); + if (blkw.pop_flag) // The optional atomic is set before the lock us unlocked + blkw.pop_flag->store(true); } fn(el); + return true; } // Consume one element, return true if consumed, false if queue was empty. diff --git a/src/slic3r/GUI/Jobs/Worker.hpp b/src/slic3r/GUI/Jobs/Worker.hpp index 8d66feb5da..f9476152e7 100644 --- a/src/slic3r/GUI/Jobs/Worker.hpp +++ b/src/slic3r/GUI/Jobs/Worker.hpp @@ -16,8 +16,9 @@ public: // Returns false if the job gets discarded. virtual bool push(std::unique_ptr job) = 0; - // Returns true if no job is running and no job message is left to be processed. - // This means that nothing is left to finalize or take care of in the main thread. + // Returns true if no job is running, the job queue is empty and no job + // message is left to be processed. This means that nothing is left to + // finalize or take care of in the main thread. virtual bool is_idle() const = 0; // Ask the current job gracefully to cancel. This call is not blocking and @@ -29,11 +30,21 @@ public: // This method will delete the queued jobs and cancel the current one. virtual void cancel_all() = 0; - // Needs to be called continuously to process events (like status update or - // finalizing of jobs) in the UI thread. This can be done e.g. in a wxIdle - // handler. + // Needs to be called continuously to process events (like status update + // or finalizing of jobs) in the main thread. This can be done e.g. in a + // wxIdle handler. virtual void process_events() = 0; + // Wait until the current job finishes. Timeout will only be considered + // if not zero. Returns false if timeout is reached but the job has not + // finished. + virtual bool wait_for_current_job(unsigned timeout_ms = 0) = 0; + + // Wait until the whole job queue finishes. Timeout will only be considered + // if not zero. Returns false only if timeout is reached but the worker has + // not reached the idle state. + virtual bool wait_for_idle(unsigned timeout_ms = 0) = 0; + // The destructor shall properly close the worker thread. virtual ~Worker() = default; }; @@ -88,6 +99,21 @@ template bool replace_job(Worker &w, Args&& ...args) return queue_job(w, std::forward(args)...); } +// Cancel the current job and wait for it to actually be stopped. +inline void stop_current_job(Worker &w, unsigned timeout_ms = 0) +{ + w.cancel(); + w.wait_for_current_job(timeout_ms); +} + +// Cancel all pending jobs including current one and wait until the worker +// becomes idle. +inline void stop_queue(Worker &w, unsigned timeout_ms = 0) +{ + w.cancel_all(); + w.wait_for_idle(timeout_ms); +} + }} // namespace Slic3r::GUI #endif // WORKER_HPP diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 9804b19295..47865dfb5f 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -5057,6 +5057,7 @@ void Plater::import_sl1_archive() { auto &w = get_ui_job_worker(); if (w.is_idle() && p->m_sla_import_dlg->ShowModal() == wxID_OK) { + p->take_snapshot(_L("Import SLA archive")); replace_job(w, std::make_unique(p->m_sla_import_dlg)); } } @@ -5493,8 +5494,10 @@ void Plater::set_number_of_copies(/*size_t num*/) void Plater::fill_bed_with_instances() { auto &w = get_ui_job_worker(); - if (w.is_idle()) + if (w.is_idle()) { + p->take_snapshot(_L("Fill bed")); replace_job(w, std::make_unique()); + } } bool Plater::is_selection_empty() const @@ -5896,7 +5899,7 @@ void Plater::reslice() return; // Stop arrange and (or) optimize rotation tasks. - this->get_ui_job_worker().cancel_all(); + stop_queue(this->get_ui_job_worker()); if (printer_technology() == ptSLA) { for (auto& object : model().objects) @@ -6361,8 +6364,10 @@ GLCanvas3D* Plater::get_current_canvas3D() void Plater::arrange() { auto &w = get_ui_job_worker(); - if (w.is_idle()) + if (w.is_idle()) { + p->take_snapshot(_L("Arrange")); replace_job(w, std::make_unique()); + } } void Plater::set_current_canvas_as_dirty() diff --git a/tests/slic3rutils/slic3r_jobs_tests.cpp b/tests/slic3rutils/slic3r_jobs_tests.cpp index 56c2fab431..39682ff98d 100644 --- a/tests/slic3rutils/slic3r_jobs_tests.cpp +++ b/tests/slic3rutils/slic3r_jobs_tests.cpp @@ -37,8 +37,7 @@ TEST_CASE("State should not be idle while running a job", "[Jobs]") { }).wait(); }); - while (!worker.is_idle()) - worker.process_events(); + worker.wait_for_idle(); REQUIRE(worker.is_idle()); } @@ -55,8 +54,7 @@ TEST_CASE("Status messages should be received by the main thread during job exec } }); - while (!worker.is_idle()) - worker.process_events(); + worker.wait_for_idle(); REQUIRE(pri->pr == 100); REQUIRE(pri->statustxt == "Running"); @@ -85,8 +83,7 @@ TEST_CASE("Cancellation should be recognized be the worker", "[Jobs]") { std::this_thread::sleep_for(std::chrono::milliseconds(1)); worker.cancel(); - while (!worker.is_idle()) - worker.process_events(); + worker.wait_for_current_job(); REQUIRE(pri->pr != 100); } @@ -146,6 +143,6 @@ TEST_CASE("Exception should be properly forwarded to finalize()", "[Jobs]") { eptr = nullptr; }); - while (!worker.is_idle()) - worker.process_events(); + worker.wait_for_idle(); + REQUIRE(worker.is_idle()); } From 0fbe7001403324986e01fb05823fc74b2a98b38a Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Fri, 3 Dec 2021 10:40:41 +0100 Subject: [PATCH 12/13] Add timeout for plater stopping the UI jobs. --- src/slic3r/GUI/Jobs/Worker.hpp | 8 ++++---- src/slic3r/GUI/Plater.cpp | 10 ++++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/slic3r/GUI/Jobs/Worker.hpp b/src/slic3r/GUI/Jobs/Worker.hpp index f9476152e7..0bc7bc0863 100644 --- a/src/slic3r/GUI/Jobs/Worker.hpp +++ b/src/slic3r/GUI/Jobs/Worker.hpp @@ -100,18 +100,18 @@ template bool replace_job(Worker &w, Args&& ...args) } // Cancel the current job and wait for it to actually be stopped. -inline void stop_current_job(Worker &w, unsigned timeout_ms = 0) +inline bool stop_current_job(Worker &w, unsigned timeout_ms = 0) { w.cancel(); - w.wait_for_current_job(timeout_ms); + return w.wait_for_current_job(timeout_ms); } // Cancel all pending jobs including current one and wait until the worker // becomes idle. -inline void stop_queue(Worker &w, unsigned timeout_ms = 0) +inline bool stop_queue(Worker &w, unsigned timeout_ms = 0) { w.cancel_all(); - w.wait_for_idle(timeout_ms); + return w.wait_for_idle(timeout_ms); } }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 47865dfb5f..7a93909715 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -5898,8 +5898,14 @@ void Plater::reslice() if (canvas3D()->get_gizmos_manager().is_in_editing_mode(true)) return; - // Stop arrange and (or) optimize rotation tasks. - stop_queue(this->get_ui_job_worker()); + // Stop the running (and queued) UI jobs and only proceed if they actually + // get stopped. + unsigned timeout_ms = 10000; + if (!stop_queue(this->get_ui_job_worker(), timeout_ms)) { + BOOST_LOG_TRIVIAL(error) << "Could not stop UI job within " + << timeout_ms << " milliseconds timeout!"; + return; + } if (printer_technology() == ptSLA) { for (auto& object : model().objects) From 3a1eee0f219a1cafebae37ea5107fa7af45397ab Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Fri, 3 Dec 2021 14:09:54 +0100 Subject: [PATCH 13/13] Clarify comments for thread safe queue Cleanup --- src/slic3r/GUI/Jobs/PlaterWorker.hpp | 24 ++++++------------------ src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp | 13 ++++++++++--- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/slic3r/GUI/Jobs/PlaterWorker.hpp b/src/slic3r/GUI/Jobs/PlaterWorker.hpp index 186506193f..5735902728 100644 --- a/src/slic3r/GUI/Jobs/PlaterWorker.hpp +++ b/src/slic3r/GUI/Jobs/PlaterWorker.hpp @@ -21,19 +21,8 @@ class PlaterWorker: public Worker { WorkerSubclass m_w; Plater *m_plater; - struct JobHolder : Job { - std::unique_ptr m_job; - void process(Ctl &ctl) override { m_job->process(ctl); }; - void finalize(bool canceled, std::exception_ptr &e) override - { - m_job->finalize(canceled, e); - } - JobHolder(std::unique_ptr &&j) : m_job{std::move(j)} {} - }; - - template class PlaterJob : public Job { - JobSubclass m_job; + std::unique_ptr m_job; Plater *m_plater; public: @@ -64,12 +53,12 @@ class PlaterWorker: public Worker { } wctl{c}; CursorSetterRAII busycursor{wctl}; - m_job.process(wctl); + m_job->process(wctl); } void finalize(bool canceled, std::exception_ptr &eptr) override { - m_job.finalize(canceled, eptr); + m_job->finalize(canceled, eptr); if (eptr) try { std::rethrow_exception(eptr); @@ -79,9 +68,8 @@ class PlaterWorker: public Worker { } } - template - PlaterJob(Plater *p, Args&&...args) - : m_job{std::forward(args)...}, m_plater{p} + PlaterJob(Plater *p, std::unique_ptr j) + : m_job{std::move(j)}, m_plater{p} { // TODO: decide if disabling slice button during UI job is what we // want. @@ -117,7 +105,7 @@ public: // Always package the job argument into a PlaterJob bool push(std::unique_ptr job) override { - return m_w.push(std::make_unique>(m_plater, std::move(job))); + return m_w.push(std::make_unique(m_plater, std::move(job))); } bool is_idle() const override { return m_w.is_idle(); } diff --git a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp index 46b4d87b6c..d400490137 100644 --- a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp +++ b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp @@ -9,14 +9,20 @@ namespace Slic3r { namespace GUI { +// Helper structure for overloads of ThreadSafeQueueSPSC::consume_one() +// to block if the queue is empty. struct BlockingWait { + // Timeout to wait for the arrival of new element into the queue. unsigned timeout_ms = 0; + + // An optional atomic flag to set true if an incoming element gets + // consumed. The flag will be atomically set to true when popping the + // front of the queue. std::atomic *pop_flag = nullptr; }; -// A thread safe queue for one producer and one consumer. Use consume_one_blk -// to block on an empty queue. +// A thread safe queue for one producer and one consumer. template class Container = std::deque, class... ContainerArgs> @@ -54,7 +60,8 @@ public: m_queue.pop(); - if (blkw.pop_flag) // The optional atomic is set before the lock us unlocked + if (blkw.pop_flag) + // The optional flag is set before the lock us unlocked. blkw.pop_flag->store(true); }