From d643b10ac4495e81192136dbe69f55a80949ce23 Mon Sep 17 00:00:00 2001 From: Kris Austin Date: Sun, 13 Sep 2026 15:46:03 -0500 Subject: [PATCH] build: expand PrintConfig.hpp option lists twice per class instead of five times (#15658) --- src/libslic3r/PrintConfig.hpp | 84 +++++++++++++++++---------------- tests/libslic3r/test_config.cpp | 55 +++++++++++++++++++++ 2 files changed, 98 insertions(+), 41 deletions(-) diff --git a/src/libslic3r/PrintConfig.hpp b/src/libslic3r/PrintConfig.hpp index f7cbe8b2e5..18e66adb34 100644 --- a/src/libslic3r/PrintConfig.hpp +++ b/src/libslic3r/PrintConfig.hpp @@ -1011,41 +1011,46 @@ public: \ { PrintConfigDef::handle_legacy(opt_key, value); } #define PRINT_CONFIG_CLASS_ELEMENT_DEFINITION(r, data, elem) BOOST_PP_TUPLE_ELEM(0, elem) BOOST_PP_TUPLE_ELEM(1, elem); -#define PRINT_CONFIG_CLASS_ELEMENT_INITIALIZATION2(KEY) cache.opt_add(BOOST_PP_STRINGIZE(KEY), base_ptr, this->KEY); -#define PRINT_CONFIG_CLASS_ELEMENT_INITIALIZATION(r, data, elem) PRINT_CONFIG_CLASS_ELEMENT_INITIALIZATION2(BOOST_PP_TUPLE_ELEM(1, elem)) -#define PRINT_CONFIG_CLASS_ELEMENT_HASH(r, data, elem) boost::hash_combine(seed, BOOST_PP_TUPLE_ELEM(1, elem).hash()); -#define PRINT_CONFIG_CLASS_ELEMENT_EQUAL(r, data, elem) if (! (BOOST_PP_TUPLE_ELEM(1, elem) == rhs.BOOST_PP_TUPLE_ELEM(1, elem))) return false; -#define PRINT_CONFIG_CLASS_ELEMENT_LOWER(r, data, elem) \ - if (BOOST_PP_TUPLE_ELEM(1, elem) < rhs.BOOST_PP_TUPLE_ELEM(1, elem)) return true; \ - if (! (BOOST_PP_TUPLE_ELEM(1, elem) == rhs.BOOST_PP_TUPLE_ELEM(1, elem))) return false; +#define PRINT_CONFIG_CLASS_ELEMENT_VISIT(r, data, elem) if (! f(BOOST_PP_STRINGIZE(BOOST_PP_TUPLE_ELEM(1, elem)), this->BOOST_PP_TUPLE_ELEM(1, elem), rhs.BOOST_PP_TUPLE_ELEM(1, elem))) return; +// Each option list is expanded into the members and again into for_each_option_pair(), which calls +// f(key, this->option, rhs.option) in declaration order and stops when f returns false. hash(), +// operator==, operator< and initialize() iterate the options through that visitor. +#define PRINT_CONFIG_CLASS_COMMON_BODY(CLASS_NAME) \ + size_t hash() const throw() \ + { \ + size_t seed = 0; \ + this->for_each_option_pair(*this, [&seed](const char*, const auto &a, const auto&) { boost::hash_combine(seed, a.hash()); return true; }); \ + return seed; \ + } \ + bool operator==(const CLASS_NAME &rhs) const throw() \ + { \ + bool eq = true; \ + this->for_each_option_pair(rhs, [&eq](const char*, const auto &a, const auto &b) { eq = (a == b); return eq; }); \ + return eq; \ + } \ + bool operator!=(const CLASS_NAME &rhs) const throw() { return ! (*this == rhs); } \ + bool operator<(const CLASS_NAME &rhs) const throw() \ + { \ + int c = 0; \ + this->for_each_option_pair(rhs, [&c](const char*, const auto &a, const auto &b) { if (a < b) c = -1; else if (! (a == b)) c = 1; return c == 0; }); \ + return c < 0; \ + } \ +protected: \ + void initialize(StaticCacheBase &cache, const char *base_ptr) \ + { \ + this->for_each_option_pair(*this, [&cache, base_ptr](const char *key, const auto &a, const auto&) { cache.opt_add(key, base_ptr, a); return true; }); \ + } #define PRINT_CONFIG_CLASS_DEFINE(CLASS_NAME, PARAMETER_DEFINITION_SEQ) \ class CLASS_NAME : public StaticPrintConfig { \ STATIC_PRINT_CONFIG_CACHE(CLASS_NAME) \ public: \ BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_DEFINITION, _, PARAMETER_DEFINITION_SEQ) \ - size_t hash() const throw() \ + template void for_each_option_pair(const CLASS_NAME &rhs, F &&f) const \ { \ - size_t seed = 0; \ - BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_HASH, _, PARAMETER_DEFINITION_SEQ) \ - return seed; \ - } \ - bool operator==(const CLASS_NAME &rhs) const throw() \ - { \ - BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_EQUAL, _, PARAMETER_DEFINITION_SEQ) \ - return true; \ - } \ - bool operator!=(const CLASS_NAME &rhs) const throw() { return ! (*this == rhs); } \ - bool operator<(const CLASS_NAME &rhs) const throw() \ - { \ - BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_LOWER, _, PARAMETER_DEFINITION_SEQ) \ - return false; \ - } \ -protected: \ - void initialize(StaticCacheBase &cache, const char *base_ptr) \ - { \ - BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_INITIALIZATION, _, PARAMETER_DEFINITION_SEQ) \ + BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_VISIT, _, PARAMETER_DEFINITION_SEQ) \ } \ + PRINT_CONFIG_CLASS_COMMON_BODY(CLASS_NAME) \ }; #define PRINT_CONFIG_CLASS_DERIVED_CLASS_LIST_ITEM(r, data, i, elem) BOOST_PP_COMMA_IF(i) public elem @@ -1059,43 +1064,43 @@ protected: \ if (! (*static_cast(this) == static_cast(rhs))) return false; // Generic version, with or without new parameters. Don't use this directly. -#define PRINT_CONFIG_CLASS_DERIVED_DEFINE1(CLASS_NAME, CLASSES_PARENTS_TUPLE, PARAMETER_DEFINITION, PARAMETER_REGISTRATION, PARAMETER_HASHES, PARAMETER_EQUALS) \ +#define PRINT_CONFIG_CLASS_DERIVED_DEFINE1(CLASS_NAME, CLASSES_PARENTS_TUPLE, PARAMETER_DEFINITION, PARAMETER_VISIT) \ class CLASS_NAME : PRINT_CONFIG_CLASS_DERIVED_CLASS_LIST(CLASSES_PARENTS_TUPLE) { \ STATIC_PRINT_CONFIG_CACHE_DERIVED(CLASS_NAME) \ CLASS_NAME() : PRINT_CONFIG_CLASS_DERIVED_INITIALIZER(CLASSES_PARENTS_TUPLE, 0) { assert(s_cache_##CLASS_NAME.initialized()); *this = s_cache_##CLASS_NAME.defaults(); } \ public: \ PARAMETER_DEFINITION \ + template void for_each_option_pair(const CLASS_NAME &rhs, F &&f) const { PARAMETER_VISIT } \ size_t hash() const throw() \ { \ size_t seed = 0; \ BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_DERIVED_HASH, _, BOOST_PP_TUPLE_TO_SEQ(CLASSES_PARENTS_TUPLE)) \ - PARAMETER_HASHES \ + this->for_each_option_pair(*this, [&seed](const char*, const auto &a, const auto&) { boost::hash_combine(seed, a.hash()); return true; }); \ return seed; \ } \ bool operator==(const CLASS_NAME &rhs) const throw() \ { \ BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_DERIVED_EQUAL, _, BOOST_PP_TUPLE_TO_SEQ(CLASSES_PARENTS_TUPLE)) \ - PARAMETER_EQUALS \ - return true; \ + bool eq = true; \ + this->for_each_option_pair(rhs, [&eq](const char*, const auto &a, const auto &b) { eq = (a == b); return eq; }); \ + return eq; \ } \ bool operator!=(const CLASS_NAME &rhs) const throw() { return ! (*this == rhs); } \ protected: \ CLASS_NAME(int) : PRINT_CONFIG_CLASS_DERIVED_INITIALIZER(CLASSES_PARENTS_TUPLE, 1) {} \ void initialize(StaticCacheBase &cache, const char* base_ptr) { \ PRINT_CONFIG_CLASS_DERIVED_INITCACHE(CLASSES_PARENTS_TUPLE) \ - PARAMETER_REGISTRATION \ + this->for_each_option_pair(*this, [&cache, base_ptr](const char *key, const auto &a, const auto&) { cache.opt_add(key, base_ptr, a); return true; }); \ } \ }; // Variant without adding new parameters. #define PRINT_CONFIG_CLASS_DERIVED_DEFINE0(CLASS_NAME, CLASSES_PARENTS_TUPLE) \ - PRINT_CONFIG_CLASS_DERIVED_DEFINE1(CLASS_NAME, CLASSES_PARENTS_TUPLE, BOOST_PP_EMPTY(), BOOST_PP_EMPTY(), BOOST_PP_EMPTY(), BOOST_PP_EMPTY()) + PRINT_CONFIG_CLASS_DERIVED_DEFINE1(CLASS_NAME, CLASSES_PARENTS_TUPLE, BOOST_PP_EMPTY(), BOOST_PP_EMPTY()) // Variant with adding new parameters. #define PRINT_CONFIG_CLASS_DERIVED_DEFINE(CLASS_NAME, CLASSES_PARENTS_TUPLE, PARAMETER_DEFINITION_SEQ) \ PRINT_CONFIG_CLASS_DERIVED_DEFINE1(CLASS_NAME, CLASSES_PARENTS_TUPLE, \ BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_DEFINITION, _, PARAMETER_DEFINITION_SEQ), \ - BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_INITIALIZATION, _, PARAMETER_DEFINITION_SEQ), \ - BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_HASH, _, PARAMETER_DEFINITION_SEQ), \ - BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_EQUAL, _, PARAMETER_DEFINITION_SEQ)) + BOOST_PP_SEQ_FOR_EACH(PRINT_CONFIG_CLASS_ELEMENT_VISIT, _, PARAMETER_DEFINITION_SEQ)) // This object is mapped to Perl as Slic3r::Config::PrintObject. PRINT_CONFIG_CLASS_DEFINE( @@ -2148,11 +2153,8 @@ PRINT_CONFIG_CLASS_DERIVED_DEFINE0( #undef STATIC_PRINT_CONFIG_CACHE_BASE #undef STATIC_PRINT_CONFIG_CACHE_DERIVED #undef PRINT_CONFIG_CLASS_ELEMENT_DEFINITION -#undef PRINT_CONFIG_CLASS_ELEMENT_EQUAL -#undef PRINT_CONFIG_CLASS_ELEMENT_LOWER -#undef PRINT_CONFIG_CLASS_ELEMENT_HASH -#undef PRINT_CONFIG_CLASS_ELEMENT_INITIALIZATION -#undef PRINT_CONFIG_CLASS_ELEMENT_INITIALIZATION2 +#undef PRINT_CONFIG_CLASS_ELEMENT_VISIT +#undef PRINT_CONFIG_CLASS_COMMON_BODY #undef PRINT_CONFIG_CLASS_DEFINE #undef PRINT_CONFIG_CLASS_DERIVED_CLASS_LIST #undef PRINT_CONFIG_CLASS_DERIVED_CLASS_LIST_ITEM diff --git a/tests/libslic3r/test_config.cpp b/tests/libslic3r/test_config.cpp index 9a70ecbaeb..3813e2df3f 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -1161,3 +1161,58 @@ TEST_CASE("min_object_distance yields no floor when an FFF config lacks the opti CHECK_THAT(min_object_distance(c), Catch::Matchers::WithinAbs(12., 1e-9)); } } + +TEST_CASE("Static print configs compare, order and hash by their option values", "[Config]") +{ + // PrintObjectConfig comes from PRINT_CONFIG_CLASS_DEFINE; PrintConfig combines MachineEnvelopeConfig + // and GCodeConfig through PRINT_CONFIG_CLASS_DERIVED_DEFINE. Both generate hash(), operator==, + // operator< and the option registration from the same option list. The hash inequalities use fixed + // inputs, so they are deterministic; they check that hash() covers the changed option. + SECTION("default-constructed configs are equal and find their options by key") + { + PrintObjectConfig a, b; + REQUIRE(a == b); + REQUIRE(a.hash() == b.hash()); + REQUIRE_FALSE(a < b); + REQUIRE_FALSE(b < a); + REQUIRE(a.optptr("layer_height") == &a.layer_height); + REQUIRE(a.optptr("brim_object_gap") == &a.brim_object_gap); + } + + SECTION("one differing option makes the configs unequal and orders them") + { + PrintObjectConfig a, b; + b.layer_height.value = a.layer_height.value + 0.05; + REQUIRE(a != b); + REQUIRE(a.hash() != b.hash()); + REQUIRE(a < b); + REQUIRE_FALSE(b < a); + } + + SECTION("ordering is decided by the first option in declaration order that differs") + { + PrintObjectConfig a, b; + a.brim_object_gap.value = b.brim_object_gap.value + 1.0; // declared first + a.layer_height.value = b.layer_height.value - 0.05; // declared later, points the other way + REQUIRE(b < a); + REQUIRE_FALSE(a < b); + } + + SECTION("a derived config sees differences in its parents and in its own options") + { + PrintConfig a, b; + REQUIRE(a == b); + REQUIRE(a.hash() == b.hash()); + + b.gcode_flavor.value = b.gcode_flavor.value == gcfMarlinLegacy ? gcfKlipper : gcfMarlinLegacy; // GCodeConfig parent + REQUIRE(a != b); + REQUIRE(a.hash() != b.hash()); + + PrintConfig c, d; + d.skirt_distance.value = c.skirt_distance.value + 1.0; // PrintConfig's own list + REQUIRE(c != d); + REQUIRE(c.hash() != d.hash()); + REQUIRE(c.optptr("skirt_distance") == &c.skirt_distance); + REQUIRE(c.optptr("gcode_flavor") == &c.gcode_flavor); + } +}