From a56f7d60e58f05624ccb45573e4d6344c71a9692 Mon Sep 17 00:00:00 2001 From: bubnikv Date: Sun, 3 Feb 2019 10:41:14 +0100 Subject: [PATCH] Fixed an issue, where the output G-code file name was not always updated from the current Model/ModelObjects. Fixed a possible race condition in updating Print::m_placeholder_parser with the proposed filename / filename base. Improved documentation (source code comments). --- src/libslic3r/GCode.cpp | 1 + src/libslic3r/GCode/CoolingBuffer.cpp | 6 +- src/libslic3r/Model.cpp | 17 +++++- src/libslic3r/Model.hpp | 4 +- src/libslic3r/PlaceholderParser.hpp | 3 +- src/libslic3r/Print.cpp | 8 +-- src/libslic3r/PrintBase.cpp | 16 +++--- src/libslic3r/PrintBase.hpp | 2 +- src/libslic3r/SLAPrint.cpp | 2 - src/slic3r.cpp | 5 +- src/slic3r/GUI/Plater.cpp | 83 +++++++++++++-------------- src/slic3r/GUI/Plater.hpp | 3 +- 12 files changed, 80 insertions(+), 70 deletions(-) diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index ec9392ec4..4d3ad00dd 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -717,6 +717,7 @@ void GCode::_do_export(Print &print, FILE *file) // Prepare the helper object for replacing placeholders in custom G-code and output filename. m_placeholder_parser = print.placeholder_parser(); m_placeholder_parser.update_timestamp(); + print.update_object_placeholders(m_placeholder_parser.config_writable()); // Get optimal tool ordering to minimize tool switches of a multi-exruder print. // For a print by objects, find the 1st printing object. diff --git a/src/libslic3r/GCode/CoolingBuffer.cpp b/src/libslic3r/GCode/CoolingBuffer.cpp index 09d211994..552fbf88c 100644 --- a/src/libslic3r/GCode/CoolingBuffer.cpp +++ b/src/libslic3r/GCode/CoolingBuffer.cpp @@ -252,8 +252,10 @@ float new_feedrate_to_reach_time_stretch( for (size_t i = 0; i < (*it)->n_lines_adjustable; ++i) { const CoolingLine &line = (*it)->lines[i]; if (line.feedrate > min_feedrate && line.feedrate < new_feedrate) - // Some of the line segments taken into account in the calculation of nomin / denom are now slower than new_feedrate. - // Re-run the calculation with a new min_feedrate limit. + // Some of the line segments taken into account in the calculation of nomin / denom are now slower than new_feedrate, + // which makes the new_feedrate lower than it should be. + // Re-run the calculation with a new min_feedrate limit, so that the segments with current feedrate lower than new_feedrate + // are not taken into account. goto not_finished_yet; } goto finished; diff --git a/src/libslic3r/Model.cpp b/src/libslic3r/Model.cpp index 968a3e234..08eb8df81 100644 --- a/src/libslic3r/Model.cpp +++ b/src/libslic3r/Model.cpp @@ -547,13 +547,26 @@ void Model::reset_auto_extruder_id() s_auto_extruder_id = 1; } -std::string Model::propose_export_file_name() const +// Propose a filename including path derived from the ModelObject's input path. +// If object's name is filled in, use the object name, otherwise use the input name. +std::string Model::propose_export_file_name_and_path() const { std::string input_file; for (const ModelObject *model_object : this->objects) for (ModelInstance *model_instance : model_object->instances) if (model_instance->is_printable()) { - input_file = model_object->name.empty() ? model_object->input_file : model_object->name; + input_file = model_object->input_file; + if (! model_object->name.empty()) { + if (input_file.empty()) + // model_object->input_file was empty, just use model_object->name + input_file = model_object->name; + else { + // Replace file name in input_file with model_object->name, but keep the path and file extension. + input_file = (boost::filesystem::path(model_object->name).parent_path().empty()) ? + (boost::filesystem::path(input_file).parent_path() / model_object->name).make_preferred().string() : + model_object->name; + } + } if (! input_file.empty()) goto end; // Other instances will produce the same name, skip them. diff --git a/src/libslic3r/Model.hpp b/src/libslic3r/Model.hpp index ba109246a..732cacaf0 100644 --- a/src/libslic3r/Model.hpp +++ b/src/libslic3r/Model.hpp @@ -607,8 +607,8 @@ public: static std::string get_auto_extruder_id_as_string(unsigned int max_extruders); static void reset_auto_extruder_id(); - // Propose an output file name based on the first printable object's name. - std::string propose_export_file_name() const; + // Propose an output file name & path based on the first printable object's name and source input file's path. + std::string propose_export_file_name_and_path() const; private: MODELBASE_DERIVED_PRIVATE_COPY_MOVE(Model) diff --git a/src/libslic3r/PlaceholderParser.hpp b/src/libslic3r/PlaceholderParser.hpp index b5ed56fa1..22c790e6b 100644 --- a/src/libslic3r/PlaceholderParser.hpp +++ b/src/libslic3r/PlaceholderParser.hpp @@ -32,7 +32,8 @@ public: void set(const std::string &key, double value) { this->set(key, new ConfigOptionFloat(value)); } void set(const std::string &key, const std::vector &values) { this->set(key, new ConfigOptionStrings(values)); } void set(const std::string &key, ConfigOption *opt) { m_config.set_key_value(key, opt); } - const DynamicConfig& config() const { return m_config; } + DynamicConfig& config_writable() { return m_config; } + const DynamicConfig& config() const { return m_config; } const ConfigOption* option(const std::string &key) const { return m_config.option(key); } // Fill in the template using a macro processing language. diff --git a/src/libslic3r/Print.cpp b/src/libslic3r/Print.cpp index 42fda92fe..bc692ca90 100644 --- a/src/libslic3r/Print.cpp +++ b/src/libslic3r/Print.cpp @@ -421,8 +421,6 @@ void Print::add_model_object(ModelObject* model_object, int idx) src_normalized.normalize(); object->config_apply(src_normalized, true); } - - this->update_object_placeholders(); } bool Print::apply_config(DynamicPrintConfig config) @@ -1096,9 +1094,6 @@ Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &co } } - //FIXME there may be a race condition with the G-code export running at the background thread. - this->update_object_placeholders(); - #ifdef _DEBUG check_model_ids_equal(m_model, model); #endif /* _DEBUG */ @@ -1855,6 +1850,9 @@ int Print::get_extruder(const ExtrusionEntityCollection& fill, const PrintRegion std::max(region.config().perimeter_extruder.value - 1, 0); } +// Generate a recommended G-code output file name based on the format template, default extension, and template parameters +// (timestamps, object placeholders derived from the model, current placeholder prameters and print statistics. +// Use the final print statistics if available, or just keep the print statistics placeholders if not available yet (before G-code is finalized). std::string Print::output_filename() const { // Set the placeholders for the data know first after the G-code export is finished. diff --git a/src/libslic3r/PrintBase.cpp b/src/libslic3r/PrintBase.cpp index 48e991b8d..3fe9a2b4d 100644 --- a/src/libslic3r/PrintBase.cpp +++ b/src/libslic3r/PrintBase.cpp @@ -15,7 +15,7 @@ namespace Slic3r size_t PrintStateBase::g_last_timestamp = 0; // Update "scale", "input_filename", "input_filename_base" placeholders from the current m_objects. -void PrintBase::update_object_placeholders() +void PrintBase::update_object_placeholders(DynamicConfig &config) const { // get the first input file name std::string input_file; @@ -33,27 +33,29 @@ void PrintBase::update_object_placeholders() "% y:" + boost::lexical_cast(printable->get_scaling_factor(Y) * 100) + "% z:" + boost::lexical_cast(printable->get_scaling_factor(Z) * 100) + "%"); if (input_file.empty()) - input_file = model_object->input_file; + input_file = model_object->name.empty() ? model_object->input_file : model_object->name; } } - PlaceholderParser &pp = m_placeholder_parser; - pp.set("scale", v_scale); + config.set_key_value("year", new ConfigOptionStrings(v_scale)); if (! input_file.empty()) { // get basename with and without suffix const std::string input_basename = boost::filesystem::path(input_file).filename().string(); - pp.set("input_filename", input_basename); + config.set_key_value("input_filename", new ConfigOptionString(input_basename)); const std::string input_basename_base = input_basename.substr(0, input_basename.find_last_of(".")); - pp.set("input_filename_base", input_basename_base); + config.set_key_value("input_filename_base", new ConfigOptionString(input_basename_base)); } } +// Generate an output file name based on the format template, default extension, and template parameters +// (timestamps, object placeholders derived from the model, current placeholder prameters, print statistics - config_override) std::string PrintBase::output_filename(const std::string &format, const std::string &default_ext, const DynamicConfig *config_override) const { DynamicConfig cfg; if (config_override != nullptr) cfg = *config_override; PlaceholderParser::update_timestamp(cfg); + this->update_object_placeholders(cfg); try { boost::filesystem::path filename = this->placeholder_parser().process(format, 0, &cfg); if (filename.extension().empty()) @@ -69,7 +71,7 @@ std::string PrintBase::output_filepath(const std::string &path) const // if we were supplied no path, generate an automatic one based on our first object's input file if (path.empty()) // get the first input file name - return (boost::filesystem::path(m_model.propose_export_file_name()).parent_path() / this->output_filename()).make_preferred().string(); + return (boost::filesystem::path(m_model.propose_export_file_name_and_path()).parent_path() / this->output_filename()).make_preferred().string(); // if we were supplied a directory, use it and append our automatically generated filename boost::filesystem::path p(path); diff --git a/src/libslic3r/PrintBase.hpp b/src/libslic3r/PrintBase.hpp index 1a61921d6..84d04d26f 100644 --- a/src/libslic3r/PrintBase.hpp +++ b/src/libslic3r/PrintBase.hpp @@ -309,7 +309,7 @@ protected: // To be called by this->output_filename() with the format string pulled from the configuration layer. std::string output_filename(const std::string &format, const std::string &default_ext, const DynamicConfig *config_override = nullptr) const; // Update "scale", "input_filename", "input_filename_base" placeholders from the current printable ModelObjects. - void update_object_placeholders(); + void update_object_placeholders(DynamicConfig &config) const; Model m_model; diff --git a/src/libslic3r/SLAPrint.cpp b/src/libslic3r/SLAPrint.cpp index dc55b196b..3cc689558 100644 --- a/src/libslic3r/SLAPrint.cpp +++ b/src/libslic3r/SLAPrint.cpp @@ -387,8 +387,6 @@ SLAPrint::ApplyStatus SLAPrint::apply(const Model &model, const DynamicPrintConf update_apply_status(false); } - this->update_object_placeholders(); - #ifdef _DEBUG check_model_ids_equal(m_model, model); #endif /* _DEBUG */ diff --git a/src/slic3r.cpp b/src/slic3r.cpp index 0b7dada70..62b56f7ff 100644 --- a/src/slic3r.cpp +++ b/src/slic3r.cpp @@ -252,10 +252,6 @@ int main(int argc, char **argv) model.arrange_objects(fff_print.config().min_object_distance()); model.center_instances_around_point(cli_config.print_center); } - if (outfile.empty()) { - outfile = model.propose_export_file_name(); - outfile += (printer_technology == ptFFF) ? ".gcode" : ".zip"; - } if (printer_technology == ptFFF) { for (auto* mo : model.objects) fff_print.auto_assign_extruders(mo); @@ -265,6 +261,7 @@ int main(int argc, char **argv) std::string err = print->validate(); if (err.empty()) { if (printer_technology == ptFFF) { + // The outfile is processed by a PlaceholderParser. fff_print.export_gcode(outfile, nullptr); } else { assert(printer_technology == ptSLA); diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index edc0fecd6..2fe032f9e 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -1028,7 +1028,8 @@ struct Plater::priv unsigned int update_background_process(bool force_validation = false); // Restart background processing thread based on a bitmask of UpdateBackgroundProcessReturnState. bool restart_background_process(unsigned int state); - void update_restart_background_process(bool force_scene_update, bool force_preview_update); + // returns bit mask of UpdateBackgroundProcessReturnState + unsigned int update_restart_background_process(bool force_scene_update, bool force_preview_update); void export_gcode(fs::path output_path, PrintHostJob upload_job); void reload_from_disk(); void fix_through_netfabb(const int obj_idx, const int vol_idx = -1); @@ -1575,7 +1576,7 @@ std::unique_ptr Plater::priv::get_export_file(GUI::FileType // Update printbility state of each of the ModelInstances. this->update_print_volume_state(); // Find the file name of the first printable object. - fs::path output_file = this->model.propose_export_file_name(); + fs::path output_file = this->model.propose_export_file_name_and_path(); switch (file_type) { case FT_STL: output_file.replace_extension("stl"); break; @@ -2045,7 +2046,7 @@ void Plater::priv::export_gcode(fs::path output_path, PrintHostJob upload_job) this->restart_background_process(priv::UPDATE_BACKGROUND_PROCESS_FORCE_EXPORT); } -void Plater::priv::update_restart_background_process(bool force_update_scene, bool force_update_preview) +unsigned int Plater::priv::update_restart_background_process(bool force_update_scene, bool force_update_preview) { // bitmask of UpdateBackgroundProcessReturnState unsigned int state = this->update_background_process(false); @@ -2055,6 +2056,7 @@ void Plater::priv::update_restart_background_process(bool force_update_scene, bo if (force_update_preview) this->preview->reload_print(); this->restart_background_process(state); + return state; } void Plater::priv::update_fff_scene() @@ -2846,51 +2848,43 @@ void Plater::cut(size_t obj_idx, size_t instance_idx, coordf_t z, bool keep_uppe p->load_model_objects(new_objects); } -void Plater::export_gcode(fs::path output_path) +void Plater::export_gcode() { if (p->model.objects.empty()) return; - // select output file - if (output_path.empty()) { - // XXX: take output path from CLI opts? Ancient Slic3r versions used to do that... - - // If possible, remove accents from accented latin characters. - // This function is useful for generating file names to be processed by legacy firmwares. - fs::path default_output_file; - try { - default_output_file = this->p->background_process.current_print()->output_filepath(output_path.string()); - } catch (const std::exception &ex) { - show_error(this, ex.what()); - return; - } - default_output_file = fs::path(Slic3r::fold_utf8_to_ascii(default_output_file.string())); - auto start_dir = wxGetApp().app_config->get_last_output_dir(default_output_file.parent_path().string()); - - wxFileDialog dlg(this, (printer_technology() == ptFFF) ? _(L("Save G-code file as:")) : _(L("Save Zip file as:")), - start_dir, - from_path(default_output_file.filename()), - GUI::file_wildcards((printer_technology() == ptFFF) ? FT_GCODE : FT_PNGZIP, default_output_file.extension().string()), - wxFD_SAVE | wxFD_OVERWRITE_PROMPT - ); - - if (dlg.ShowModal() == wxID_OK) { - fs::path path = into_path(dlg.GetPath()); - wxGetApp().app_config->update_last_output_dir(path.parent_path().string()); - output_path = std::move(path); - } - } else { - try { - output_path = this->p->background_process.current_print()->output_filepath(output_path.string()); - } catch (const std::exception &ex) { - show_error(this, ex.what()); - return; - } + // If possible, remove accents from accented latin characters. + // This function is useful for generating file names to be processed by legacy firmwares. + fs::path default_output_file; + try { + // Update the background processing, so that the placeholder parser will get the correct values for the ouput file template. + // Also if there is something wrong with the current configuration, a pop-up dialog will be shown and the export will not be performed. + unsigned int state = this->p->update_restart_background_process(false, false); + if (state & priv::UPDATE_BACKGROUND_PROCESS_INVALID) + return; + default_output_file = this->p->background_process.current_print()->output_filepath(""); + } catch (const std::exception &ex) { + show_error(this, ex.what()); + return; } + default_output_file = fs::path(Slic3r::fold_utf8_to_ascii(default_output_file.string())); + auto start_dir = wxGetApp().app_config->get_last_output_dir(default_output_file.parent_path().string()); - if (! output_path.empty()) { + wxFileDialog dlg(this, (printer_technology() == ptFFF) ? _(L("Save G-code file as:")) : _(L("Save Zip file as:")), + start_dir, + from_path(default_output_file.filename()), + GUI::file_wildcards((printer_technology() == ptFFF) ? FT_GCODE : FT_PNGZIP, default_output_file.extension().string()), + wxFD_SAVE | wxFD_OVERWRITE_PROMPT + ); + + fs::path output_path; + if (dlg.ShowModal() == wxID_OK) { + fs::path path = into_path(dlg.GetPath()); + wxGetApp().app_config->update_last_output_dir(path.parent_path().string()); + output_path = std::move(path); + } + if (! output_path.empty()) p->export_gcode(std::move(output_path), PrintHostJob()); - } } void Plater::export_stl(bool selection_only) @@ -2991,7 +2985,12 @@ void Plater::send_gcode() // Obtain default output path fs::path default_output_file; try { - default_output_file = this->p->background_process.current_print()->output_filepath(""); + // Update the background processing, so that the placeholder parser will get the correct values for the ouput file template. + // Also if there is something wrong with the current configuration, a pop-up dialog will be shown and the export will not be performed. + unsigned int state = this->p->update_restart_background_process(false, false); + if (state & priv::UPDATE_BACKGROUND_PROCESS_INVALID) + return; + default_output_file = this->p->background_process.current_print()->output_filepath(""); } catch (const std::exception &ex) { show_error(this, ex.what()); return; diff --git a/src/slic3r/GUI/Plater.hpp b/src/slic3r/GUI/Plater.hpp index 09b7348d5..e3601b65c 100644 --- a/src/slic3r/GUI/Plater.hpp +++ b/src/slic3r/GUI/Plater.hpp @@ -140,8 +140,7 @@ public: void cut(size_t obj_idx, size_t instance_idx, coordf_t z, bool keep_upper = true, bool keep_lower = true, bool rotate_lower = false); - // Note: empty path means "use the default" - void export_gcode(boost::filesystem::path output_path = boost::filesystem::path()); + void export_gcode(); void export_stl(bool selection_only = false); void export_amf(); void export_3mf(const boost::filesystem::path& output_path = boost::filesystem::path());