From b2128cc330c1ffc70f1aadf4825e29d13d3654d8 Mon Sep 17 00:00:00 2001 From: Clifford Garwood Date: Tue, 21 Apr 2026 22:49:08 -0400 Subject: [PATCH] refactor(imex): route three inline tool-state parsers through parse_imex_active_tools PartPlate::calc_imex_zones, GCodeViewer::render, and Plater::collect_imex_warnings each hand-rolled their own "phys:P/C/M" tokenizer with subtly different error handling. Replace the three inline loops with parse_imex_active_tools + imex_primary_tool_for_mode so the Primary/Copy/Mirror classification agrees across zones, the G-code viewer legend, and slice warnings. No behavior change: the shared helpers preserve the 1=Primary / 2=Copy / 3=Mirror encoding already consumed downstream and continue to accept the legacy bare-index form as Primary. Co-Authored-By: Claude Opus 4.7 --- src/slic3r/GUI/GCodeViewer.cpp | 38 ++++++++++++++-------------------- src/slic3r/GUI/PartPlate.cpp | 36 ++++++++++++-------------------- src/slic3r/GUI/Plater.cpp | 15 ++++---------- 3 files changed, 32 insertions(+), 57 deletions(-) diff --git a/src/slic3r/GUI/GCodeViewer.cpp b/src/slic3r/GUI/GCodeViewer.cpp index 3a53ef6e33..4401dcd7c9 100644 --- a/src/slic3r/GUI/GCodeViewer.cpp +++ b/src/slic3r/GUI/GCodeViewer.cpp @@ -10,6 +10,7 @@ #include "libslic3r/Utils.hpp" #include "libslic3r/LocalesUtils.hpp" #include "libslic3r/PresetBundle.hpp" +#include "libslic3r/IMEXHelpers.hpp" //BBS: add convex hull logic for toolpath check #include "libslic3r/Geometry/ConvexHull.hpp" @@ -1558,35 +1559,26 @@ void GCodeViewer::render(int canvas_width, int canvas_height, int right_margin) imex_box_wx = wx_opt ? (float)wx_opt->value : 30.0f; imex_box_wy = wy_opt ? (float)wy_opt->value : 30.0f; - // Parse "idx:P/C/M" format — matches PartPlate::calc_imex_zones() exactly. - // Primary = state 1 (P), Copy = 2 (C), Mirror = 3 (M). - // Inactive tools (state 0) are not included in the stored string. + // Parse "idx:P/C/M" via shared helpers — matches PartPlate::calc_imex_zones. + // Secondary state is 2=Copy / 3=Mirror to match the legacy encoding used + // downstream for marker color selection. int pri_tool = -1; std::vector sec_tool_ids; std::map sec_tool_states; // tool_id -> 2=Copy, 3=Mirror if (mode_names_opt && active_tools_opt) { for (size_t i = 0; i < mode_names_opt->values.size(); ++i) { if (i < active_tools_opt->values.size() && mode_names_opt->values[i] == mode) { - std::istringstream ss(active_tools_opt->values[i]); - std::string tok; - while (std::getline(ss, tok, ',')) { - tok.erase(std::remove_if(tok.begin(), tok.end(), ::isspace), tok.end()); - if (tok.empty()) continue; - try { - auto colon = tok.find(':'); - int idx = std::stoi(tok.substr(0, colon != std::string::npos ? colon : tok.size())); - int state = 1; // default: primary - if (colon != std::string::npos) { - char role = std::toupper((unsigned char)tok[colon + 1]); - if (role == 'C') state = 2; - else if (role == 'M') state = 3; - } - if (state == 1) pri_tool = idx; - else { - sec_tool_ids.push_back(idx); - sec_tool_states[idx] = state; - } - } catch (...) {} + const std::string& entry = active_tools_opt->values[i]; + pri_tool = imex_primary_tool_for_mode(entry); + for (const auto& [phys_idx, role] : parse_imex_active_tools(entry)) { + if (phys_idx < 0 || phys_idx == pri_tool) continue; + if (role == ImexRole::Mirror) { + sec_tool_ids.push_back(phys_idx); + sec_tool_states[phys_idx] = 3; + } else if (role == ImexRole::Copy) { + sec_tool_ids.push_back(phys_idx); + sec_tool_states[phys_idx] = 2; + } } break; } diff --git a/src/slic3r/GUI/PartPlate.cpp b/src/slic3r/GUI/PartPlate.cpp index 09bc91249c..5e1617dd05 100644 --- a/src/slic3r/GUI/PartPlate.cpp +++ b/src/slic3r/GUI/PartPlate.cpp @@ -621,31 +621,21 @@ void PartPlate::calc_imex_zones() } } - // Parse "phys_idx:P/C/M" format → map (1=Primary, 2=Copy, 3=Mirror) - // Backwards compat: plain "idx" → Primary - // Mode strings use physical T-indices directly (set on the IMEX config tab). - // Filament routing is a per-plate concern handled separately via imex_head_filament_map. + // Parse "phys_idx:P/C/M" format → map (1=Primary, 2=Copy, 3=Mirror). + // imex_primary_tool_for_mode handles the Primary slot (bare legacy token → Primary); + // parse_imex_active_tools fills in the Copy/Mirror secondaries. Mode strings use + // physical T-indices directly; filament routing is separate (imex_head_filament_map). std::map tool_states; { - std::istringstream ss(active_tools_str); - std::string token; - while (std::getline(ss, token, ',')) { - token.erase(std::remove_if(token.begin(), token.end(), ::isspace), token.end()); - if (token.empty()) continue; - try { - auto colon = token.find(':'); - int phys_idx, state = 1; - if (colon != std::string::npos) { - phys_idx = std::stoi(token.substr(0, colon)); - char role = std::toupper((unsigned char)token[colon + 1]); - if (role == 'C') state = 2; - else if (role == 'M') state = 3; - } else { - phys_idx = std::stoi(token); - } - if (phys_idx >= 0 && phys_idx < n_rows * n_cols) - tool_states[phys_idx] = state; - } catch (...) {} + const int primary = imex_primary_tool_for_mode(active_tools_str); + if (primary >= 0 && primary < n_rows * n_cols) + tool_states[primary] = 1; + for (const auto& [phys_idx, role] : parse_imex_active_tools(active_tools_str)) { + if (phys_idx < 0 || phys_idx >= n_rows * n_cols) continue; + if (phys_idx == primary) continue; + if (role == ImexRole::Mirror) tool_states[phys_idx] = 3; + else if (role == ImexRole::Copy) tool_states[phys_idx] = 2; + // Extra ImexRole::Primary entries beyond the first are ignored. } } diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index b29b32ba77..01f925b62e 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -9964,17 +9964,10 @@ static std::vector collect_imex_warnings(PartPlate* plate) if (i >= tools_opt->values.size() || mode_names_opt->values[i] != mode) continue; const std::string& entry = tools_opt->values[i]; primary_tool = imex_primary_tool_for_mode(entry); - std::istringstream ss(entry); - std::string token; - while (std::getline(ss, token, ',')) { - token.erase(std::remove_if(token.begin(), token.end(), ::isspace), token.end()); - if (token.empty()) continue; - // std::stoi parses the leading integer and ignores any ":P/:C/:M" suffix. - try { - int idx = std::stoi(token); - if (idx >= 0 && (max_tool == 0 || (size_t)idx < max_tool)) - active_tools.push_back(idx); - } catch (...) {} + for (const auto& [phys_idx, role] : parse_imex_active_tools(entry)) { + (void)role; + if (phys_idx >= 0 && (max_tool == 0 || (size_t)phys_idx < max_tool)) + active_tools.push_back(phys_idx); } break; }