From 7353d8c6e96b934550aa0c743c26b5cf3bd8b98e Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Wed, 9 Dec 2020 09:19:46 +0100 Subject: [PATCH] Rework of Add random function for gcode macros. #5219 after merge: The thread local storage variables for the random number generator were replaced by an external context variable. Thread local storage is a scarce resource. --- src/libslic3r/GCode.cpp | 2 +- src/libslic3r/GCode.hpp | 2 ++ src/libslic3r/PlaceholderParser.cpp | 53 ++++++++++++++++++----------- src/libslic3r/PlaceholderParser.hpp | 14 ++++++-- 4 files changed, 48 insertions(+), 23 deletions(-) diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index 60ef1ed5f..7deb38349 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -1340,7 +1340,7 @@ void GCode::_do_export(Print& print, FILE* file, ThumbnailsGeneratorCallback thu std::string GCode::placeholder_parser_process(const std::string &name, const std::string &templ, unsigned int current_extruder_id, const DynamicConfig *config_override) { try { - return m_placeholder_parser.process(templ, current_extruder_id, config_override); + return m_placeholder_parser.process(templ, current_extruder_id, config_override, &m_placeholder_parser_context); } catch (std::runtime_error &err) { // Collect the names of failed template substitutions for error reporting. auto it = m_placeholder_parser_failed_templates.find(name); diff --git a/src/libslic3r/GCode.hpp b/src/libslic3r/GCode.hpp index 5efff5386..458eae80a 100644 --- a/src/libslic3r/GCode.hpp +++ b/src/libslic3r/GCode.hpp @@ -300,6 +300,8 @@ private: FullPrintConfig m_config; GCodeWriter m_writer; PlaceholderParser m_placeholder_parser; + // For random number generator etc. + PlaceholderParser::ContextData m_placeholder_parser_context; // Collection of templates, on which the placeholder substitution failed. std::map m_placeholder_parser_failed_templates; OozePrevention m_ooze_prevention; diff --git a/src/libslic3r/PlaceholderParser.cpp b/src/libslic3r/PlaceholderParser.cpp index 4ad00927a..6f2b7f3ea 100644 --- a/src/libslic3r/PlaceholderParser.cpp +++ b/src/libslic3r/PlaceholderParser.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #ifdef _MSC_VER #include // provides **_environ #else @@ -497,35 +496,26 @@ namespace client static void leq (expr &lhs, expr &rhs) { compare_op(lhs, rhs, '>', true ); } static void geq (expr &lhs, expr &rhs) { compare_op(lhs, rhs, '<', true ); } - // Random number generators - static int random_int(int min, int max) { - thread_local static std::mt19937 engine(std::random_device{}()); - thread_local static std::uniform_int_distribution dist; - return dist(engine, decltype(dist)::param_type{ min, max }); - } - static double random_double(double min, double max) { - thread_local static std::mt19937 engine(std::random_device{}()); - thread_local static std::uniform_real_distribution dist; - return dist(engine, decltype(dist)::param_type{ min, max }); + static void throw_if_not_numeric(const expr ¶m) + { + const char *err_msg = "Not a numeric type."; + param.throw_if_not_numeric(err_msg); } enum Function2ParamsType { FUNCTION_MIN, FUNCTION_MAX, - FUNCTION_RANDOM, }; // Store the result into param1. static void function_2params(expr ¶m1, expr ¶m2, Function2ParamsType fun) { - const char *err_msg = "Not a numeric type."; - param1.throw_if_not_numeric(err_msg); - param2.throw_if_not_numeric(err_msg); + throw_if_not_numeric(param1); + throw_if_not_numeric(param2); if (param1.type == TYPE_DOUBLE || param2.type == TYPE_DOUBLE) { double d = 0.; switch (fun) { case FUNCTION_MIN: d = std::min(param1.as_d(), param2.as_d()); break; case FUNCTION_MAX: d = std::max(param1.as_d(), param2.as_d()); break; - case FUNCTION_RANDOM: d = random_double(param1.as_d(), param2.as_d()); break; default: param1.throw_exception("Internal error: invalid function"); } param1.data.d = d; @@ -535,7 +525,6 @@ namespace client switch (fun) { case FUNCTION_MIN: i = std::min(param1.as_i(), param2.as_i()); break; case FUNCTION_MAX: i = std::max(param1.as_i(), param2.as_i()); break; - case FUNCTION_RANDOM: i = random_int(param1.as_i(), param2.as_i()); break; default: param1.throw_exception("Internal error: invalid function"); } param1.data.i = i; @@ -545,7 +534,20 @@ namespace client // Store the result into param1. static void min(expr ¶m1, expr ¶m2) { function_2params(param1, param2, FUNCTION_MIN); } static void max(expr ¶m1, expr ¶m2) { function_2params(param1, param2, FUNCTION_MAX); } - static void random(expr ¶m1, expr ¶m2) { function_2params(param1, param2, FUNCTION_RANDOM); } + + // Store the result into param1. + static void random(expr ¶m1, expr ¶m2, std::mt19937 &rng) + { + throw_if_not_numeric(param1); + throw_if_not_numeric(param2); + if (param1.type == TYPE_DOUBLE || param2.type == TYPE_DOUBLE) { + param1.data.d = std::uniform_real_distribution<>(param1.as_d(), param2.as_d())(rng); + param1.type = TYPE_DOUBLE; + } else { + param1.data.i = std::uniform_int_distribution<>(param1.as_i(), param2.as_i())(rng); + param1.type = TYPE_INT; + } + } static void regex_op(expr &lhs, boost::iterator_range &rhs, char op) { @@ -641,6 +643,7 @@ namespace client const DynamicConfig *config = nullptr; const DynamicConfig *config_override = nullptr; size_t current_extruder_id = 0; + PlaceholderParser::ContextData *context_data = nullptr; // If false, the macro_processor will evaluate a full macro. // If true, the macro processor will evaluate just a boolean condition using the full expressive power of the macro processor. bool just_boolean_expression = false; @@ -841,6 +844,15 @@ namespace client output = expr_index.i(); } + template + static void random(const MyContext *ctx, expr ¶m1, expr ¶m2) + { + if (ctx->context_data == nullptr) + ctx->throw_exception("Random number generator not available in this context.", + boost::iterator_range(param1.it_range.begin(), param2.it_range.end())); + expr::random(param1, param2, ctx->context_data->rng); + } + template static void throw_exception(const std::string &msg, const boost::iterator_range &it_range) { @@ -1194,7 +1206,7 @@ namespace client | (kw["max"] > '(' > conditional_expression(_r1) [_val = _1] > ',' > conditional_expression(_r1) > ')') [ px::bind(&expr::max, _val, _2) ] | (kw["random"] > '(' > conditional_expression(_r1) [_val = _1] > ',' > conditional_expression(_r1) > ')') - [ px::bind(&expr::random, _val, _2) ] + [ px::bind(&MyContext::random, _r1, _val, _2) ] | (kw["int"] > '(' > unary_expression(_r1) > ')') [ px::bind(&FactorActions::to_int, _1, _val) ] | (strict_double > iter_pos) [ px::bind(&FactorActions::double_, _1, _2, _val) ] | (int_ > iter_pos) [ px::bind(&FactorActions::int_, _1, _2, _val) ] @@ -1332,13 +1344,14 @@ static std::string process_macro(const std::string &templ, client::MyContext &co return output; } -std::string PlaceholderParser::process(const std::string &templ, unsigned int current_extruder_id, const DynamicConfig *config_override) const +std::string PlaceholderParser::process(const std::string &templ, unsigned int current_extruder_id, const DynamicConfig *config_override, ContextData *context_data) const { client::MyContext context; context.external_config = this->external_config(); context.config = &this->config(); context.config_override = config_override; context.current_extruder_id = current_extruder_id; + context.context_data = context_data; return process_macro(templ, context); } diff --git a/src/libslic3r/PlaceholderParser.hpp b/src/libslic3r/PlaceholderParser.hpp index 40a7d5b53..d19e35d41 100644 --- a/src/libslic3r/PlaceholderParser.hpp +++ b/src/libslic3r/PlaceholderParser.hpp @@ -3,6 +3,7 @@ #include "libslic3r.h" #include +#include #include #include #include "PrintConfig.hpp" @@ -11,7 +12,16 @@ namespace Slic3r { class PlaceholderParser { -public: +public: + // Context to be shared during multiple executions of the PlaceholderParser. + // The context is kept external to the PlaceholderParser, so that the same PlaceholderParser + // may be called safely from multiple threads. + // In the future, the context may hold variables created and modified by the PlaceholderParser + // and shared between the PlaceholderParser::process() invocations. + struct ContextData { + std::mt19937 rng; + }; + PlaceholderParser(const DynamicConfig *external_config = nullptr); // Return a list of keys, which should be changed in m_config from rhs. @@ -41,7 +51,7 @@ public: // Fill in the template using a macro processing language. // Throws Slic3r::PlaceholderParserError on syntax or runtime error. - std::string process(const std::string &templ, unsigned int current_extruder_id = 0, const DynamicConfig *config_override = nullptr) const; + std::string process(const std::string &templ, unsigned int current_extruder_id, const DynamicConfig *config_override, ContextData *context = nullptr) const; // Evaluate a boolean expression using the full expressive power of the PlaceholderParser boolean expression syntax. // Throws Slic3r::PlaceholderParserError on syntax or runtime error.