diff --git a/src/libslic3r/IMEXHelpers.hpp b/src/libslic3r/IMEXHelpers.hpp index a60c006a72..c7725ff2b1 100644 --- a/src/libslic3r/IMEXHelpers.hpp +++ b/src/libslic3r/IMEXHelpers.hpp @@ -375,17 +375,30 @@ void imex_cfg_report_unregistered(const char* fn, const std::string& key); template T imex_cfg_enum(const ConfigBase& cfg, const std::string& key) { - // dynamic_cast on BOTH halves here, unlike imex_cfg_int/_float. Every ConfigOptionEnum - // reports coEnum, so the type() comparison that option() (Config.hpp:2643) and the - // int/float accessors rely on cannot tell one enum type from another: it would accept a - // ConfigOptionEnum and static_cast it to this T. ConfigOptionEnum and - // ConfigOptionEnum are unrelated siblings (both derive from ConfigOptionSingle), so a - // dynamic_cast rejects the mismatch outright. The ConfigOptionPercent : ConfigOptionFloat - // inheritance that rules dynamic_cast out for imex_cfg_float has no analogue for enums. - if (const ConfigOption* opt = cfg.option(key)) + // Matched by type, not by the coEnum tag. Every ConfigOptionEnum reports coEnum, so the + // type() comparison that option() (Config.hpp:2656) and the int/float accessors rely on + // cannot tell one enum type from another: it would accept a ConfigOptionEnum and + // static_cast it to this T. ConfigOptionEnum and ConfigOptionEnum are unrelated + // siblings (both derive from ConfigOptionSingle), so a dynamic_cast rejects the mismatch. + // + // A coEnum value has a second representation, ConfigOptionEnumGeneric, which derives from + // ConfigOptionInt rather than ConfigOptionSingle - the enum analogue of the + // ConfigOptionPercent : ConfigOptionFloat inheritance that rules dynamic_cast out for + // imex_cfg_float. It is what a config not seeded from the static PrinterConfig holds, so it + // is read here too, keyed on the value map it carries: only T's own map yields a T. + const ConfigOptionDef* def = print_config_def.get(key); + if (const ConfigOption* opt = cfg.option(key)) { if (const auto* e = dynamic_cast*>(opt)) return e->value; - if (const ConfigOptionDef* def = print_config_def.get(key)) + // The map the generic option was built from identifies the enum it holds: T's own map + // means T's own enumerators, so the stored int is a T. A key whose definition names a + // different enum carries a different map and falls through to the default below, + // exactly as the typed cast above would reject it. + if (const auto* g = dynamic_cast(opt)) + if (g->keys_map == &ConfigOptionEnum::get_enum_values()) + return static_cast(g->value); + } + if (def) if (const ConfigOption* dv = def->default_value.get()) if (const auto* e = dynamic_cast*>(dv)) return e->value; diff --git a/src/libslic3r/Print.cpp b/src/libslic3r/Print.cpp index 4ec36e0850..864c31b7f3 100644 --- a/src/libslic3r/Print.cpp +++ b/src/libslic3r/Print.cpp @@ -441,6 +441,12 @@ bool Print::invalidate_state_by_config_options(const ConfigOptionResolver & /* n } else if (opt_key == "z_hop_types") { osteps.emplace_back(posDetectOverhangsForLift); + } + // The bed-zone colour scheme and the advisory margin bands are drawn from the printer + // config but never reach the slice: compute_imex_zone_layout keeps the margin out of + // primary_zone_box, which is the only part of the layout update_imex_slice_offset reads. + // imex_tool_layout does move that box, so it is left to the fallback below. + else if (opt_key == "imex_viz_theme" || opt_key == "imex_carriage_margin") { } else { // for legacy, if we can't handle this option let's invalidate all steps //FIXME invalidate all steps of all objects as well? @@ -3907,9 +3913,8 @@ Points Print::first_layer_wipe_tower_corners(bool check_wipe_tower_existance) co // firmware to fan copies out — parts in the wrong place, no diagnostic. // // Inputs, all reachable without a GUI: -// * m_full_print_config — the printer preset's IMEX keys. It is the full config rather -// than m_config because `imex_tool_layout` and `imex_carriage_margin` are printer-preset -// options with no home in the static PrintConfig, so m_config does not carry them. +// * m_full_print_config — the printer preset's IMEX keys, read from the full config so this +// sees the same values the preset holds whichever representation carries them. // * the plate's `imex_parallel_mode`, read off the object config. That is the same source // GCode.cpp and validate() resolve the active mode from (the plate's own config is // merged into the full config before apply() by BackgroundSlicingProcess in the GUI and diff --git a/src/libslic3r/PrintConfig.hpp b/src/libslic3r/PrintConfig.hpp index e463f1f1a9..3794c4cc39 100644 --- a/src/libslic3r/PrintConfig.hpp +++ b/src/libslic3r/PrintConfig.hpp @@ -1660,6 +1660,9 @@ PRINT_CONFIG_CLASS_DEFINE( ((ConfigOptionInt, imex_tools_per_gantry)) ((ConfigOptionFloat, imex_nozzle_clearance_x)) ((ConfigOptionFloat, imex_nozzle_clearance_y)) + ((ConfigOptionFloat, imex_carriage_margin)) + ((ConfigOptionEnum, imex_tool_layout)) + ((ConfigOptionEnum, imex_viz_theme)) ((ConfigOptionStrings, imex_mode_names)) ((ConfigOptionStrings, imex_mode_active_tools)) ((ConfigOptionStrings, imex_mode_gcodes)) diff --git a/src/slic3r/GUI/PartPlate.cpp b/src/slic3r/GUI/PartPlate.cpp index 5e30c14382..5d002a1fd8 100644 --- a/src/slic3r/GUI/PartPlate.cpp +++ b/src/slic3r/GUI/PartPlate.cpp @@ -1294,13 +1294,11 @@ void PartPlate::render_imex_zones(bool force_default_color) const IMEXTheme* theme = &k_standard; if (wxGetApp().preset_bundle) { const DynamicPrintConfig& pcfg = wxGetApp().preset_bundle->printers.get_edited_preset().config; - if (auto* t = pcfg.option>("imex_viz_theme")) { - switch (t->value) { - case ImexVizTheme::Deuteranopia: theme = &k_deuteranopia; break; - case ImexVizTheme::Tritanopia: theme = &k_tritanopia; break; - case ImexVizTheme::HighContrast: theme = &k_high_contrast; break; - default: break; // Standard - } + switch (imex_cfg_enum(pcfg, "imex_viz_theme")) { + case ImexVizTheme::Deuteranopia: theme = &k_deuteranopia; break; + case ImexVizTheme::Tritanopia: theme = &k_tritanopia; break; + case ImexVizTheme::HighContrast: theme = &k_high_contrast; break; + default: break; // Standard } } diff --git a/tests/libslic3r/test_imex_helpers.cpp b/tests/libslic3r/test_imex_helpers.cpp index 710de4956a..0c4fb5dbf7 100644 --- a/tests/libslic3r/test_imex_helpers.cpp +++ b/tests/libslic3r/test_imex_helpers.cpp @@ -1554,3 +1554,25 @@ TEST_CASE("resolve_filament_for_head answers in nozzle index space, not filament // Only a head with no routing at all yields -1, so "-1 means safe to index" is false. REQUIRE(resolve_filament_for_head({}, pem, 9) == -1); } + +// A coEnum value reaches a reader in either of two representations: the typed option a config +// seeded from the static PrinterConfig carries, and the ConfigOptionEnumGeneric that a config +// built from the definitions alone - a project's settings, the CLI's - creates. Both have to +// read back as the value that was stored, or a setting silently reverts to its default while +// the file on disk still holds what the user chose. +TEST_CASE("Enum printer keys read back the stored value, not their default", "[IMEX]") +{ + DynamicPrintConfig cfg = DynamicPrintConfig::full_print_config(); + + cfg.set_deserialize_strict("imex_tool_layout", "rear-left"); + CHECK(int(imex_cfg_enum(cfg, "imex_tool_layout")) == int(ImexToolLayout::RearLeft)); + + cfg.set_deserialize_strict("imex_viz_theme", "deuteranopia"); + CHECK(int(imex_cfg_enum(cfg, "imex_viz_theme")) == int(ImexVizTheme::Deuteranopia)); + + // A config that never saw the static defaults - a 3mf's project_settings.config, and the + // CLI's - holds the generic form of the same option, and has to read back the same. + DynamicPrintConfig bare; + bare.set_deserialize_strict("imex_tool_layout", "rear-right"); + CHECK(int(imex_cfg_enum(bare, "imex_tool_layout")) == int(ImexToolLayout::RearRight)); +} diff --git a/tests/libslic3r/test_imex_zones.cpp b/tests/libslic3r/test_imex_zones.cpp index 4946afc27a..6f600b8731 100644 --- a/tests/libslic3r/test_imex_zones.cpp +++ b/tests/libslic3r/test_imex_zones.cpp @@ -38,7 +38,10 @@ DynamicPrintConfig make_cfg(const PrinterCfg& p) cfg.set_key_value("is_imex", new ConfigOptionBool(p.is_imex)); cfg.set_key_value("imex_gantry_count", new ConfigOptionInt(p.gantry_count)); cfg.set_key_value("imex_tools_per_gantry", new ConfigOptionInt(p.tools_per_gantry)); - cfg.set_key_value("imex_tool_layout", new ConfigOptionEnum(p.tool_layout)); + // Deserialized from its stored string, not constructed typed: that is the generic form a + // preset, a project's settings and the CLI all carry, and the form a reader can reject + // while the typed one it never sees would have answered correctly. + cfg.set_deserialize_strict("imex_tool_layout", ConfigOptionEnum(p.tool_layout).serialize()); // Slot 0 is the reserved Primary mode, which never carries tools; kMode is slot 1. cfg.set_key_value("imex_mode_names", new ConfigOptionStrings(std::vector{ kImexPrimaryMode, kMode })); diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index 6ec6ba9b1e..d780d72dfa 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -5,6 +5,7 @@ #include #include "libslic3r/PresetBundle.hpp" +#include "libslic3r/IMEXHelpers.hpp" #include "libslic3r/AppConfig.hpp" #include "libslic3r/Model.hpp" #include "libslic3r/TriangleMesh.hpp" @@ -5482,3 +5483,24 @@ TEST_CASE("Config import confines zip entries, preset names and bundle ids to th CHECK_FALSE(any_filename_contains(temp_dir.path(), "bundle-escape")); } } + +TEST_CASE("A saved printer preset reloads the tool layout it was saved with", "[Preset][Bundle][IMEX]") +{ + ScopedTemporaryDir temp_dir; + PresetBundle bundle; + PresetsConfigSubstitutions substitutions; + + DynamicPrintConfig config(bundle.printers.default_preset().config); + config.set_deserialize_strict("imex_tool_layout", "rear-left"); + config.option(BBL_JSON_KEY_INHERITS, true)->value = ""; + const fs::path file = temp_dir.path() / PRESET_PRINTER_NAME / "ImexPrinter.json"; + fs::create_directories(file.parent_path()); + config.save_to_json(file.string(), "ImexPrinter", "User", "1.0.0"); + + bundle.printers.load_presets(temp_dir.path().string(), PRESET_PRINTER_NAME, substitutions, + ForwardCompatibilitySubstitutionRule::Disable); + + const Preset *loaded = bundle.printers.find_preset("ImexPrinter"); + REQUIRE(loaded != nullptr); + CHECK(int(imex_cfg_enum(loaded->config, "imex_tool_layout")) == int(ImexToolLayout::RearLeft)); +}