Skip to content

Commit

Permalink
Implemented generic mechanism for executing tasks on UI thread synchr…
Browse files Browse the repository at this point in the history 
…onously

from the background slicing thread, that supports cancellation.
The generic mechanism is used for generating thumbnails into G-code and
Fixes Fix deadlock when canceling the slicing while gcode is creating thumbnails #6476
Thanks @supermerill for pointing out the issue.
  • Loading branch information
@bubnikv
bubnikv committed May 4, 2021
1 parent 5f5d0df commit 1aef86f
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 46 deletions.
3 changes: 1 addition & 2 deletions src/libslic3r/GCode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -887,8 +887,7 @@ namespace DoExport {
if (thumbnail_cb != nullptr)
{
const size_t max_row_length = 78;
ThumbnailsList thumbnails;
thumbnail_cb(thumbnails, sizes, true, true, true, true);
ThumbnailsList thumbnails = thumbnail_cb(ThumbnailsParams{ sizes, true, true, true, true });
for (const ThumbnailData& data : thumbnails)
{
if (data.is_valid())
Expand Down
14 changes: 12 additions & 2 deletions src/libslic3r/GCode/ThumbnailData.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,18 @@ struct ThumbnailData
bool is_valid() const;
};

typedef std::vector<ThumbnailData> ThumbnailsList;
typedef std::function<void(ThumbnailsList & thumbnails, const Vec2ds & sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background)> ThumbnailsGeneratorCallback;
using ThumbnailsList = std::vector<ThumbnailData>;

struct ThumbnailsParams
{
const Vec2ds sizes;
bool printable_only;
bool parts_only;
bool show_bed;
bool transparent_background;
};

typedef std::function<ThumbnailsList(const ThumbnailsParams&)> ThumbnailsGeneratorCallback;

} // namespace Slic3r

Expand Down
110 changes: 83 additions & 27 deletions src/slic3r/GUI/BackgroundSlicingProcess.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ void BackgroundSlicingProcess::process_fff()
// Passing the timestamp
evt.SetInt((int)(m_fff_print->step_state_with_timestamp(PrintStep::psSlicingFinished).timestamp));
wxQueueEvent(GUI::wxGetApp().mainframe->m_plater, evt.Clone());
m_fff_print->export_gcode(m_temp_output_path, m_gcode_result, m_thumbnail_cb);
m_fff_print->export_gcode(m_temp_output_path, m_gcode_result, [this](const ThumbnailsParams& params) { return this->render_thumbnails(params); });
if (this->set_step_started(bspsGCodeFinalize)) {
if (! m_export_path.empty()) {
wxQueueEvent(GUI::wxGetApp().mainframe->m_plater, new wxCommandEvent(m_event_export_began_id));
Expand Down Expand Up @@ -221,21 +221,14 @@ void BackgroundSlicingProcess::process_sla()

const std::string export_path = m_sla_print->print_statistics().finalize_output_path(m_export_path);

Zipper zipper(export_path);
m_sla_archive.export_print(zipper, *m_sla_print);

if (m_thumbnail_cb != nullptr)
{
ThumbnailsList thumbnails;
m_thumbnail_cb(thumbnails, current_print()->full_print_config().option<ConfigOptionPoints>("thumbnails")->values, true, true, true, true);
// m_thumbnail_cb(thumbnails, current_print()->full_print_config().option<ConfigOptionPoints>("thumbnails")->values, true, false, true, true); // renders also supports and pad
for (const ThumbnailData& data : thumbnails)
{
if (data.is_valid())
write_thumbnail(zipper, data);
}
}
ThumbnailsList thumbnails = this->render_thumbnails(
ThumbnailsParams{current_print()->full_print_config().option<ConfigOptionPoints>("thumbnails")->values, true, true, true, true});

Zipper zipper(export_path);
m_sla_archive.export_print(zipper, *m_sla_print); // true, false, true, true); // renders also supports and pad
for (const ThumbnailData& data : thumbnails)
if (data.is_valid())
write_thumbnail(zipper, data);
zipper.finalize();

m_print->set_status(100, (boost::format(_utf8(L("Masked SLA file exported to %1%"))) % export_path).str());
Expand Down Expand Up @@ -362,6 +355,7 @@ bool BackgroundSlicingProcess::start()
return true;
}

// To be called on the UI thread.
bool BackgroundSlicingProcess::stop()
{
// m_print->state_mutex() shall NOT be held. Unfortunately there is no interface to test for it.
Expand All @@ -372,6 +366,8 @@ bool BackgroundSlicingProcess::stop()
}
// assert(this->running());
if (m_state == STATE_STARTED || m_state == STATE_RUNNING) {
// Cancel any task planned by the background thread on UI thread.
cancel_ui_task(m_ui_task);
m_print->cancel();
// Wait until the background processing stops by being canceled.
m_condition.wait(lck, [this](){ return m_state == STATE_CANCELED; });
Expand All @@ -396,7 +392,7 @@ bool BackgroundSlicingProcess::reset()
return stopped;
}

// To be called by Print::apply() through the Print::m_cancel_callback to stop the background
// To be called by Print::apply() on the UI thread through the Print::m_cancel_callback to stop the background
// processing before changing any data of running or finalized milestones.
// This function shall not trigger any UI update through the wxWidgets event.
void BackgroundSlicingProcess::stop_internal()
Expand All @@ -408,6 +404,8 @@ void BackgroundSlicingProcess::stop_internal()
std::unique_lock<std::mutex> lck(m_mutex);
assert(m_state == STATE_STARTED || m_state == STATE_RUNNING || m_state == STATE_FINISHED || m_state == STATE_CANCELED);
if (m_state == STATE_STARTED || m_state == STATE_RUNNING) {
// Cancel any task planned by the background thread on UI thread.
cancel_ui_task(m_ui_task);
// At this point of time the worker thread may be blocking on m_print->state_mutex().
// Set the print state to canceled before unlocking the state_mutex(), so when the worker thread wakes up,
// it throws the CanceledException().
Expand All @@ -424,6 +422,60 @@ void BackgroundSlicingProcess::stop_internal()
m_print->set_cancel_callback([](){});
}

// Execute task from background thread on the UI thread. Returns true if processed, false if cancelled.
bool BackgroundSlicingProcess::execute_ui_task(std::function<void()> task)
{
bool running = false;
if (m_mutex.try_lock()) {
// Cancellation is either not in process, or already canceled and waiting for us to finish.
// There must be no UI task planned.
assert(! m_ui_task);
if (! m_print->canceled()) {
running = true;
m_ui_task = std::make_shared<UITask>();
}
m_mutex.unlock();
} else {
// Cancellation is in process.
}

bool result = false;
if (running) {
std::shared_ptr<UITask> ctx = m_ui_task;
GUI::wxGetApp().mainframe->m_plater->CallAfter([task, ctx]() {
// Running on the UI thread, thus ctx->state does not need to be guarded with mutex against ::cancel_ui_task().
assert(ctx->state == UITask::Planned || ctx->state == UITask::Canceled);
if (ctx->state == UITask::Planned) {
task();
std::unique_lock<std::mutex> lck(ctx->mutex);
ctx->state = UITask::Finished;
}
// Wake up the worker thread from the UI thread.
ctx->condition.notify_all();
});

{
std::unique_lock<std::mutex> lock(ctx->mutex);
ctx->condition.wait(lock, [&ctx]{ return ctx->state == UITask::Finished || ctx->state == UITask::Canceled; });
}
result = ctx->state == UITask::Finished;
m_ui_task.reset();
}

return result;
}

// To be called on the UI thread from ::stop() and ::stop_internal().
void BackgroundSlicingProcess::cancel_ui_task(std::shared_ptr<UITask> task)
{
if (task) {
std::unique_lock<std::mutex> lck(task->mutex);
task->state = UITask::Canceled;
lck.unlock();
task->condition.notify_all();
}
}

bool BackgroundSlicingProcess::empty() const
{
assert(m_print != nullptr);
Expand Down Expand Up @@ -546,19 +598,14 @@ void BackgroundSlicingProcess::prepare_upload()
} else {
m_upload_job.upload_data.upload_path = m_sla_print->print_statistics().finalize_output_path(m_upload_job.upload_data.upload_path.string());

ThumbnailsList thumbnails = this->render_thumbnails(
ThumbnailsParams{current_print()->full_print_config().option<ConfigOptionPoints>("thumbnails")->values, true, true, true, true});
// true, false, true, true); // renders also supports and pad
Zipper zipper{source_path.string()};
m_sla_archive.export_print(zipper, *m_sla_print, m_upload_job.upload_data.upload_path.string());
if (m_thumbnail_cb != nullptr)
{
ThumbnailsList thumbnails;
m_thumbnail_cb(thumbnails, current_print()->full_print_config().option<ConfigOptionPoints>("thumbnails")->values, true, true, true, true);
// m_thumbnail_cb(thumbnails, current_print()->full_print_config().option<ConfigOptionPoints>("thumbnails")->values, true, false, true, true); // renders also supports and pad
for (const ThumbnailData& data : thumbnails)
{
if (data.is_valid())
write_thumbnail(zipper, data);
}
}
for (const ThumbnailData& data : thumbnails)
if (data.is_valid())
write_thumbnail(zipper, data);
zipper.finalize();
}

Expand All @@ -569,4 +616,13 @@ void BackgroundSlicingProcess::prepare_upload()
GUI::wxGetApp().printhost_job_queue().enqueue(std::move(m_upload_job));
}

// Executed by the background thread, to start a task on the UI thread.
ThumbnailsList BackgroundSlicingProcess::render_thumbnails(const ThumbnailsParams &params)
{
ThumbnailsList thumbnails;
if (m_thumbnail_cb)
this->execute_ui_task([this, &params, &thumbnails](){ thumbnails = this->m_thumbnail_cb(params); });
return thumbnails;
}

}; // namespace Slic3r
23 changes: 23 additions & 0 deletions src/slic3r/GUI/BackgroundSlicingProcess.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,23 @@ class BackgroundSlicingProcess
std::condition_variable m_condition;
State m_state = STATE_INITIAL;

// For executing tasks from the background thread on UI thread synchronously (waiting for result) using wxWidgets CallAfter().
// When the background proces is canceled, the UITask has to be invalidated as well, so that it will not be
// executed on the UI thread referencing invalid data.
struct UITask {
enum State {
Planned,
Finished,
Canceled,
};
State state = Planned;
std::mutex mutex;
std::condition_variable condition;
};
// Only one UI task may be planned by the background thread to be executed on the UI thread, as the background
// thread is blocking until the UI thread calculation finishes.
std::shared_ptr<UITask> m_ui_task;

PrintState<BackgroundSlicingProcessStep, bspsCount> m_step_state;
mutable tbb::mutex m_step_state_mutex;
bool set_step_started(BackgroundSlicingProcessStep step);
Expand All @@ -222,6 +239,12 @@ class BackgroundSlicingProcess
// If the background processing stop was requested, throw CanceledException.
void throw_if_canceled() const { if (m_print->canceled()) throw CanceledException(); }
void prepare_upload();
// To be executed at the background thread.
ThumbnailsList render_thumbnails(const ThumbnailsParams &params);
// Execute task from background thread on the UI thread synchronously. Returns true if processed, false if cancelled before executing the task.
bool execute_ui_task(std::function<void()> task);
// To be called from inside m_mutex to cancel a planned UI task.
static void cancel_ui_task(std::shared_ptr<BackgroundSlicingProcess::UITask> task);

// wxWidgets command ID to be sent to the plater to inform that the slicing is finished, and the G-code export will continue.
int m_event_slicing_completed_id = 0;
Expand Down
22 changes: 7 additions & 15 deletions src/slic3r/GUI/Plater.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1684,7 +1684,7 @@ struct Plater::priv
bool can_split(bool to_objects) const;

void generate_thumbnail(ThumbnailData& data, unsigned int w, unsigned int h, bool printable_only, bool parts_only, bool show_bed, bool transparent_background);
void generate_thumbnails(ThumbnailsList& thumbnails, const Vec2ds& sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background);
ThumbnailsList generate_thumbnails(const ThumbnailsParams& params);

void bring_instance_forward() const;

Expand Down Expand Up @@ -1760,15 +1760,7 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame)
background_process.set_fff_print(&fff_print);
background_process.set_sla_print(&sla_print);
background_process.set_gcode_result(&gcode_result);
background_process.set_thumbnail_cb([this](ThumbnailsList& thumbnails, const Vec2ds& sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background)
{
std::packaged_task<void(ThumbnailsList&, const Vec2ds&, bool, bool, bool, bool)> task([this](ThumbnailsList& thumbnails, const Vec2ds& sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background) {
generate_thumbnails(thumbnails, sizes, printable_only, parts_only, show_bed, transparent_background);
});
std::future<void> result = task.get_future();
wxTheApp->CallAfter([&]() { task(thumbnails, sizes, printable_only, parts_only, show_bed, transparent_background); });
result.wait();
});
background_process.set_thumbnail_cb([this](const ThumbnailsParams& params) { return this->generate_thumbnails(params); });
background_process.set_slicing_completed_event(EVT_SLICING_COMPLETED);
background_process.set_finished_event(EVT_PROCESS_COMPLETED);
background_process.set_export_began_event(EVT_EXPORT_BEGAN);
Expand Down Expand Up @@ -3812,17 +3804,17 @@ void Plater::priv::generate_thumbnail(ThumbnailData& data, unsigned int w, unsig
view3D->get_canvas3d()->render_thumbnail(data, w, h, printable_only, parts_only, show_bed, transparent_background);
}

void Plater::priv::generate_thumbnails(ThumbnailsList& thumbnails, const Vec2ds& sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background)
ThumbnailsList Plater::priv::generate_thumbnails(const ThumbnailsParams& params)
{
thumbnails.clear();
for (const Vec2d& size : sizes)
{
ThumbnailsList thumbnails;
for (const Vec2d& size : params.sizes) {
thumbnails.push_back(ThumbnailData());
Point isize(size); // round to ints
generate_thumbnail(thumbnails.back(), isize.x(), isize.y(), printable_only, parts_only, show_bed, transparent_background);
generate_thumbnail(thumbnails.back(), isize.x(), isize.y(), params.printable_only, params.parts_only, params.show_bed, params.transparent_background);
if (!thumbnails.back().is_valid())
thumbnails.pop_back();
}
return thumbnails;
}

wxString Plater::priv::get_project_filename(const wxString& extension) const
Expand Down

0 comments on commit 1aef86f

Please sign in to comment.