From f8180803da02141da24c8e15c045cc99c0f34aed Mon Sep 17 00:00:00 2001 From: remi durand Date: Thu, 10 Feb 2022 23:17:42 +0100 Subject: [PATCH] Fix custom var exists() function Fix regression on legacy placeholder syntax supermerill/SuperSlicer#2359 --- src/libslic3r/GCode.cpp | 1 + src/libslic3r/PlaceholderParser.cpp | 86 ++++++++++++++--------------- src/libslic3r/PlaceholderParser.hpp | 4 +- src/libslic3r/PrintRegion.cpp | 6 +- 4 files changed, 48 insertions(+), 49 deletions(-) diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index 0f3b6abd6..d212025e6 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -695,6 +695,7 @@ void GCode::do_export(Print* print, const char* path, GCodeProcessor::Result* re throw Slic3r::RuntimeError(std::string("G-code export to ") + path + " failed.\nCannot open the file for writing.\n"); try { + m_placeholder_parser.reset(); m_placeholder_parser_failed_templates.clear(); this->_do_export(*print, file, thumbnail_cb); fflush(file); diff --git a/src/libslic3r/PlaceholderParser.cpp b/src/libslic3r/PlaceholderParser.cpp index 64f924a02..41c491d19 100644 --- a/src/libslic3r/PlaceholderParser.cpp +++ b/src/libslic3r/PlaceholderParser.cpp @@ -730,7 +730,7 @@ namespace client if (vector_opt->is_extruder_size()) { if (raw_opt->type() == coFloats || raw_opt->type() == coInts || raw_opt->type() == coBools) - return vector_opt->getFloat(current_extruder_id); + return vector_opt->getFloat(int(current_extruder_id)); if (raw_opt->type() == coFloatsOrPercents) { const ConfigOptionFloatsOrPercents* opt_fl_per = static_cast(raw_opt); if (!opt_fl_per->values[current_extruder_id].percent) @@ -856,8 +856,9 @@ namespace client boost::iterator_range& opt_key, Iterator& end_pos, expr& out, - ConfigOption* default_val = nullptr) + std::unique_ptr&& default_val) { + bool has_default_value = default_val.get() != nullptr; t_config_option_key key = std::string(opt_key.begin(), opt_key.end()); const ConfigOption* opt = nullptr; if (ctx->config_override != nullptr) @@ -866,26 +867,18 @@ namespace client opt = ctx->config->option(key); if (opt == nullptr && ctx->external_config != nullptr) opt = ctx->external_config->option(key); - if (opt == nullptr) { - std::unique_ptr ppt; - if(default_val == nullptr) - ppt = std::unique_ptr(new ConfigOptionBool(false)); - else - ppt = std::unique_ptr(default_val); - // set flag to say "it's a var that isn't here, please ignore it" - ppt->flags |= ConfigOption::FCO_PLACEHOLDER_TEMP; - if (MyContext::checked_vars.find(key) != MyContext::checked_vars.end()) { - if (default_val != nullptr) { - // erase previous value - MyContext::checked_vars[key] = std::move(ppt); - } - } else { - // put the var - MyContext::checked_vars.emplace(std::move(key), std::move(ppt)); + if (opt == nullptr && (has_default_value || MyContext::checked_vars.find(key) == MyContext::checked_vars.end()) ) { + // set stub bool value only if a default() hasn't been called yet. + if (!has_default_value) { + default_val.reset(new ConfigOptionBool(false)); } + // set flag to say "it's a var that isn't here, please ignore it" + default_val->flags |= ConfigOption::FCO_PLACEHOLDER_TEMP; + MyContext::checked_vars[key] = std::move(default_val); } - //return - //out = expr(opt != nullptr, out.it_range.begin(), end_pos); + // return (wanted for exists() but not for default()) + if(!has_default_value) + out = expr(opt != nullptr, out.it_range.begin(), end_pos); } template @@ -952,7 +945,7 @@ namespace client case coInts: opt_def = print_config_def.get(opt_key); if (opt_def->is_vector_extruder) { - output.set_i((int)((ConfigOptionVectorBase*)opt.opt)->getFloat(ctx->current_extruder_id)); + output.set_i(int(((ConfigOptionVectorBase*)opt.opt)->getFloat(int(ctx->current_extruder_id)))); break; } else ctx->throw_exception("Unknown scalar variable type", opt.it_range); @@ -960,7 +953,7 @@ namespace client case coPercents: vector_opt = static_cast(opt.opt); if (vector_opt->is_extruder_size()) { - output.set_d(((ConfigOptionVectorBase*)opt.opt)->getFloat(ctx->current_extruder_id)); + output.set_d(((ConfigOptionVectorBase*)opt.opt)->getFloat(int(ctx->current_extruder_id))); break; } else ctx->throw_exception("Unknown scalar variable type", opt.it_range); @@ -1301,13 +1294,11 @@ namespace client "endif"; */ - // Legacy variable expansion of the original Slic3r, in the form of [scalar_variable] or [vector_variable_index]. - // note: should use 'identifier' instead of 'raw[lexeme[*((utf8char - char_('\\') - char_(']')) | ('\\' > char_))]]' - // but it's that way to allow to ignore the legacy_variable_expansion is disabled, witout outputting an exception + // Legacy variable expansion of the original Slic3r, in the form of [scalar_variable] or [vector_variable_index] or [vector_variable_[index_variable]]. legacy_variable_expansion = - ((raw[lexeme[*((utf8char - char_('\\') - char_(']')) | ('\\' > char_))]] >> &lit(']')) >> &lit(']')) + (identifier >> &lit(']')) [ px::bind(&MyContext::legacy_variable_expansion, _r1, _1, _val) ] - | ((raw[lexeme[*((utf8char - char_('\\') - char_(']')) | ('\\' > char_))]] >> &lit(']')) > lit('[') > (raw[lexeme[*((utf8char - char_('\\') - char_(']')) | ('\\' > char_))]] >> &lit(']')) > ']') + | (identifier > lit('[') > identifier > ']') [ px::bind(&MyContext::legacy_variable_expansion2, _r1, _1, _2, _val) ] ; legacy_variable_expansion.name("legacy_variable_expansion"); @@ -1392,17 +1383,17 @@ namespace client { out = value.unary_integer(out.it_range.begin()); } //function for default keyword static void default_bool_(bool &value, const MyContext* ctx, boost::iterator_range& opt_key, Iterator& end_pos, expr& out) - { MyContext::check_variable(ctx, opt_key, end_pos, out, new ConfigOptionBool(value)); } + { MyContext::check_variable(ctx, opt_key, end_pos, out, std::make_unique(value)); } static void default_int_(int &value, const MyContext* ctx, boost::iterator_range& opt_key, Iterator& end_pos, expr& out) - { MyContext::check_variable(ctx, opt_key, end_pos, out, new ConfigOptionInt(value)); } + { MyContext::check_variable(ctx, opt_key, end_pos, out, std::make_unique(value)); } static void default_double_(double &value, const MyContext* ctx, boost::iterator_range& opt_key, Iterator& end_pos, expr& out) - { MyContext::check_variable(ctx, opt_key, end_pos, out, new ConfigOptionFloat(value)); } + { MyContext::check_variable(ctx, opt_key, end_pos, out, std::make_unique(value)); } static void default_string_(boost::iterator_range& it_range, const MyContext* ctx, boost::iterator_range& opt_key, Iterator& end_pos, expr& out) - { MyContext::check_variable(ctx, opt_key, end_pos, out, new ConfigOptionString(std::string(it_range.begin() + 1, it_range.end() - 1))); } + { MyContext::check_variable(ctx, opt_key, end_pos, out, std::make_unique(std::string(it_range.begin() + 1, it_range.end() - 1))); } + static void exists_(const MyContext* ctx, boost::iterator_range& opt_key, Iterator& end_pos, expr& out) + { MyContext::check_variable(ctx, opt_key, end_pos, out, std::unique_ptr{nullptr}); } static void set_ignore_legacy_(bool& value) - { - MyContext::ignore_legacy = value; - } + { MyContext::ignore_legacy = value; } }; unary_expression = iter_pos[px::bind(&FactorActions::set_start_pos, _1, _val)] >> ( scalar_variable_reference(_r1) [ _val = _1 ] @@ -1417,7 +1408,7 @@ namespace client | (kw["random"] > '(' > conditional_expression(_r1) [_val = _1] > ',' > conditional_expression(_r1) > ')') [ px::bind(&MyContext::random, _r1, _val, _2) ] | (kw["int"] > '(' > unary_expression(_r1) > ')') [ px::bind(&FactorActions::to_int, _1, _val) ] - | (kw["exists"] > '(' > identifier > ')' > iter_pos) [ px::bind(&MyContext::check_variable, _r1, _1, _2, _val, nullptr) ] + | (kw["exists"] > '(' > identifier > ')' > iter_pos) [ px::bind(&FactorActions::exists_, _r1, _1, _2, _val) ] | (kw["default_double"] > '(' > identifier > ',' > strict_double > ')' > iter_pos) [px::bind(&FactorActions::default_double_, _2, _r1, _1, _3, _val)] | (kw["default_int"] > '(' > identifier > ',' > int_ > ')' > iter_pos) @@ -1593,7 +1584,7 @@ bool PlaceholderParser::evaluate_boolean_expression(const std::string &templ, co } -void PlaceholderParser::append_custom_variables(std::map> name2var_array, int nb_extruders) { +void PlaceholderParser::append_custom_variables(std::map> name2var_array, uint16_t nb_extruders) { bool is_array = nb_extruders > 0; if (!is_array) nb_extruders = 1; @@ -1606,7 +1597,7 @@ void PlaceholderParser::append_custom_variables(std::map& values = entry.second; //check if all values are empty bool is_not_string = false; - for (int extruder_id = 0; extruder_id < nb_extruders; ++extruder_id) { + for (uint16_t extruder_id = 0; extruder_id < nb_extruders; ++extruder_id) { if (!values[extruder_id].empty()) { is_not_string = true; break; @@ -1616,7 +1607,7 @@ void PlaceholderParser::append_custom_variables(std::map bool_values; if (!is_not_bool) { - for (int extruder_id = 0; extruder_id < nb_extruders; ++extruder_id) { + for (uint16_t extruder_id = 0; extruder_id < nb_extruders; ++extruder_id) { if (!values[extruder_id].empty()) { if (boost::algorithm::to_lower_copy(values[extruder_id]) == "true") { bool_values.push_back(true); @@ -1652,7 +1643,7 @@ void PlaceholderParser::append_custom_variables(std::map double_values; //SLIC3R_REGEX_NAMESPACE::regex("\\s*[+-]?([0-9]+\\.[0-9]*([Ee][+-]?[0-9]+)?|\\.[0-9]+([Ee][+-]?[0-9]+)?|[0-9]+[Ee][+-]?[0-9]+)"); if (!is_not_numeric) { - for (int extruder_id = 0; extruder_id < nb_extruders; ++extruder_id) { + for (uint16_t extruder_id = 0; extruder_id < nb_extruders; ++extruder_id) { if (!values[extruder_id].empty()) { try { double_values.push_back(boost::lexical_cast(values[extruder_id])); @@ -1717,8 +1708,14 @@ void PlaceholderParser::append_custom_variables(std::map> name2var_array; std::string raw_text = custom_variables.value; @@ -1726,7 +1723,7 @@ void PlaceholderParser::parse_custom_variables(const ConfigOptionString& custom_ std::vector lines; boost::algorithm::split(lines, raw_text, boost::is_any_of("\n")); for (const std::string& line : lines) { - int equal_pos = line.find_first_of('='); + size_t equal_pos = line.find_first_of('='); if (equal_pos != std::string::npos) { std::string name = line.substr(0, equal_pos); std::string value = line.substr(equal_pos + 1); @@ -1754,7 +1751,7 @@ void PlaceholderParser::parse_custom_variables(const ConfigOptionStrings& filame std::vector lines; boost::algorithm::split(lines, raw_text, boost::is_any_of("\n")); for (const std::string& line : lines) { - int equal_pos = line.find_first_of('='); + size_t equal_pos = line.find_first_of('='); if (equal_pos != std::string::npos) { std::string name = line.substr(0, equal_pos); std::string value = line.substr(equal_pos + 1); @@ -1768,8 +1765,7 @@ void PlaceholderParser::parse_custom_variables(const ConfigOptionStrings& filame } } } - append_custom_variables(name2var_array, filament_custom_variables.values.size()); + append_custom_variables(name2var_array, uint16_t(filament_custom_variables.values.size())); } - } diff --git a/src/libslic3r/PlaceholderParser.hpp b/src/libslic3r/PlaceholderParser.hpp index f0867443e..1fe5698f2 100644 --- a/src/libslic3r/PlaceholderParser.hpp +++ b/src/libslic3r/PlaceholderParser.hpp @@ -66,9 +66,11 @@ public: void parse_custom_variables(const ConfigOptionString& custom_variables); void parse_custom_variables(const ConfigOptionStrings& filament_custom_variables); + //remove custom vars and stored config + void reset(); private: - void append_custom_variables(std::map> name2var_array, int nb_extruders); + void append_custom_variables(std::map> name2var_array, uint16_t nb_extruders); // config has a higher priority than external_config when looking up a symbol. DynamicConfig m_config; diff --git a/src/libslic3r/PrintRegion.cpp b/src/libslic3r/PrintRegion.cpp index 1f5b6783f..72315a0e6 100644 --- a/src/libslic3r/PrintRegion.cpp +++ b/src/libslic3r/PrintRegion.cpp @@ -19,7 +19,7 @@ uint16_t PrintRegion::extruder(FlowRole role, const PrintObject& object) const extruder = object.config().support_material_interface_extruder; else throw Slic3r::InvalidArgument("Unknown role"); - return extruder; + return (uint16_t)extruder; } Flow PrintRegion::flow(FlowRole role, double layer_height, bool bridge, bool first_layer, double width, const PrintObject &object) const @@ -56,7 +56,7 @@ Flow PrintRegion::flow(FlowRole role, double layer_height, bool bridge, bool fir // Here this->extruder(role) - 1 may underflow to MAX_INT, but then the get_at() will follback to zero'th element, so everything is all right. double nozzle_diameter = m_print->config().nozzle_diameter.get_at(this->extruder(role, object) - 1); return Flow::new_from_config_width(role, config_width, (float)nozzle_diameter, (float)layer_height, - this->config().get_computed_value("filament_max_overlap", this->extruder(role, object) - 1), + (float)this->config().get_computed_value("filament_max_overlap", this->extruder(role, object) - 1), bridge ? (float)m_config.bridge_flow_ratio.get_abs_value(1) : 0.0f); } @@ -91,7 +91,7 @@ float PrintRegion::width(FlowRole role, bool first_layer, const PrintObject& ob double nozzle_diameter = m_print->config().nozzle_diameter.get_at(this->extruder(role, object) - 1); if (config_width->value <= 0.) { // If user left option to 0, calculate a sane default width. - return Flow::auto_extrusion_width(role, nozzle_diameter); + return float(Flow::auto_extrusion_width(role, nozzle_diameter)); } else { // If user set a manual value, use it. return float(config_width->get_abs_value(nozzle_diameter));