From 3b445905d1d971356500fc05d0e20a5991962fcc Mon Sep 17 00:00:00 2001 From: SoftFever Date: Fri, 7 Aug 2026 11:31:43 +0800 Subject: [PATCH] Fix out-of-bounds reads in the multi-extruder flush-volume matrix flush_volumes_matrix stores one (filaments x filaments) block per nozzle, but set_extruder, WipeTower2::extract_wipe_volumes and the PresetBundle rebuild indexed each block with filament_colour.size() as the row stride. When the stored matrix does not match that assumption (a legacy project, or a just-switched printer -- e.g. a single 2x2 block while nozzle_diameter has 2 entries) the index runs past the sliced block and reads out of bounds. The read is undefined, so its value depends on the C++ std-lib/allocator layout: identical across Linux architectures but different on macOS/Windows. That surfaced as the "Toolchange temperature commands are unchanged when the wipe tower wait is off" regression failing on Linux CI while passing on macOS/Windows, and crashing hardened (-O0, _GLIBCXX_ASSERTIONS) builds. - Centralize the block-dimension derivation in get_flush_volumes_matrix_dims (sqrt(size / nozzles) with a filaments^2 * nozzles == size check and a single-block fallback) and use it wherever the matrix is sliced/indexed; bounds-guard the reads as defense in depth. It weighs both options that claim to say how many blocks are stored, since either can be stale: flush_multiplier, written with the matrix in the project config, and nozzle_diameter, which changes the moment a printer is selected. - PresetBundle::update_multi_material_filament_presets: rebuild the matrix on a nozzle-count-only change too (the old per-block gate missed those), and seed a brand-new nozzle from the first nozzle's tuned block. - is_flush_config_modified: stride the stored matrix by its own dimension rather than the current filament count, and bound the nozzle loop by the printer's extruder count, which CalcFlushingVolumes indexes as well. - is_flushing_matrix_error: the same derivation, which additionally divided by zero on an empty flush_multiplier. - update_slice_warnings: guard nozzle_hrc_lists, which is sized by the nullable nozzle_type option and can be shorter than the extruder count. With the read fixed the emitted trace is deterministic across builds and platforms, so regenerate the golden from it and stop comparing the rounded "time: s" preheat comment (the tolerant lead time already carries that timing). Add unit tests for the flush-matrix rebuild and dimension logic. --- src/libslic3r/GCode.cpp | 21 ++- src/libslic3r/GCode/GCodeProcessor.cpp | 5 +- src/libslic3r/GCode/WipeTower2.cpp | 26 ++-- src/libslic3r/PresetBundle.cpp | 31 ++-- src/libslic3r/PrintConfig.hpp | 27 ++++ src/slic3r/GUI/GLCanvas3D.cpp | 19 ++- src/slic3r/GUI/WipeTowerDialog.cpp | 42 +++--- .../wipe_tower_temperature_trace_main.txt | 107 +++++++------- tests/fff_print/test_multifilament.cpp | 13 +- tests/libslic3r/CMakeLists.txt | 1 + .../test_preset_bundle_flush_matrix.cpp | 132 ++++++++++++++++++ 11 files changed, 308 insertions(+), 116 deletions(-) create mode 100644 tests/libslic3r/test_preset_bundle_flush_matrix.cpp diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index c20405781b..13e97698fa 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -9087,12 +9087,21 @@ std::string GCode::set_extruder(unsigned int new_filament_id, double print_z, bo int old_filament_id = -1; int old_extruder_id = -1; if (m_writer.filament() != nullptr || m_start_gcode_filament != -1) { - std::vector flush_matrix(cast(get_flush_volumes_matrix(m_config.flush_volumes_matrix.values, new_extruder_id, m_config.nozzle_diameter.values.size()))); - const unsigned int number_of_extruders = (unsigned int) (m_config.filament_colour.values.size()); // if is multi_extruder only use the fist extruder matrix + // The row stride of the per-nozzle flush matrix block is its validated dimension, + // NOT filament_colour.size() (see get_flush_volumes_matrix_dims). + const std::vector& raw_flush_matrix = m_config.flush_volumes_matrix.values; + const FlushVolumesMatrixDims flush_dims = get_flush_volumes_matrix_dims(raw_flush_matrix.size(), + m_config.flush_multiplier.values.size(), m_config.nozzle_diameter.values.size()); + std::vector flush_matrix(cast(get_flush_volumes_matrix(raw_flush_matrix, new_extruder_id, flush_dims.nozzle_nums))); + // Guard the read in case the stored matrix is smaller than the filament ids in play. + auto flush_volume_at = [&](size_t old_id, size_t new_id) -> float { + const size_t idx = old_id * flush_dims.filament_nums + new_id; + return idx < flush_matrix.size() ? flush_matrix[idx] : 0.f; + }; if (m_writer.filament() != nullptr) - assert(m_writer.filament()->id() < number_of_extruders); + assert(m_writer.filament()->id() < m_config.filament_colour.values.size()); else - assert(m_start_gcode_filament < number_of_extruders); + assert(m_start_gcode_filament < (int) m_config.filament_colour.values.size()); old_filament_id = m_writer.filament() != nullptr ? m_writer.filament()->id() : m_start_gcode_filament; old_extruder_id = m_writer.filament() != nullptr ? m_writer.filament()->extruder_id() : get_extruder_id(m_start_gcode_filament); @@ -9112,12 +9121,12 @@ std::string GCode::set_extruder(unsigned int new_filament_id, double print_z, bo if (old_filament_id_in_new_extruder == -1) wipe_volume = 0; else { - wipe_volume = flush_matrix[old_filament_id_in_new_extruder * number_of_extruders + new_filament_id]; + wipe_volume = flush_volume_at(old_filament_id_in_new_extruder, new_filament_id); wipe_volume *= m_config.flush_multiplier.get_at(new_extruder_id); } } else { - wipe_volume = flush_matrix[old_filament_id * number_of_extruders + new_filament_id]; + wipe_volume = flush_volume_at(old_filament_id, new_filament_id); wipe_volume *= m_config.flush_multiplier.get_at(new_extruder_id); // if is multi_extruder only use the fist extruder matrix } wipe_volume = std::max(0.f, wipe_volume-grab_purge_volume); diff --git a/src/libslic3r/GCode/GCodeProcessor.cpp b/src/libslic3r/GCode/GCodeProcessor.cpp index cebfe486cb..50bcc05997 100644 --- a/src/libslic3r/GCode/GCodeProcessor.cpp +++ b/src/libslic3r/GCode/GCodeProcessor.cpp @@ -7479,7 +7479,10 @@ void GCodeProcessor::update_slice_warnings() filament_hrc = m_result.required_nozzle_HRC[used_filaments[idx]]; int filament_extruder_id = m_filament_maps[used_filaments[idx]]; - int extruder_hrc = nozzle_hrc_lists[filament_extruder_id]; + // nozzle_type is a nullable option that can be shorter than the extruder count; + // out-of-range extruders get hrc 0, which skips the check below. + int extruder_hrc = (filament_extruder_id >= 0 && filament_extruder_id < (int) nozzle_hrc_lists.size()) + ? nozzle_hrc_lists[filament_extruder_id] : 0; BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(": Check HRC: filament:%1%, hrc=%2%, extruder:%3%, hrc:%4%") % used_filaments[idx] % filament_hrc % filament_extruder_id % extruder_hrc; diff --git a/src/libslic3r/GCode/WipeTower2.cpp b/src/libslic3r/GCode/WipeTower2.cpp index ee0f9c375a..a7a5a49047 100644 --- a/src/libslic3r/GCode/WipeTower2.cpp +++ b/src/libslic3r/GCode/WipeTower2.cpp @@ -2134,21 +2134,17 @@ std::pair WipeTower2::get_wipe_tower_cone_base(double width, dou // DynamicPrintConfig directly instead of materializing a full PrintConfig per call. std::vector> WipeTower2::extract_wipe_volumes(const ConfigBase& config) { - // flush_volumes_matrix holds one filaments x filaments block per nozzle (written by - // PresetBundle::update_multi_material_filament_presets), so the filament count is - // sqrt(size / nozzles). One tower serves every nozzle and the filament to nozzle assignment is - // only decided later by ToolOrdering, so fold the blocks with std::max: the depth reserved here - // has to cover the worst nozzle. With a single nozzle the fold has one term. - const std::vector &raw_matrix = config.option("flush_volumes_matrix")->values; - const auto *nozzle_diameter = config.option("nozzle_diameter"); - size_t nozzle_nums = (nozzle_diameter == nullptr || nozzle_diameter->values.empty()) ? 1 : nozzle_diameter->values.size(); - unsigned int number_of_extruders = (unsigned int)(sqrt(raw_matrix.size() / nozzle_nums) + EPSILON); - if (size_t(number_of_extruders) * number_of_extruders * nozzle_nums != raw_matrix.size()) { - // Saved for a different nozzle count (older project, or the printer was just switched): - // fall back to reading the whole option as one block, as this did before. - nozzle_nums = 1; - number_of_extruders = (unsigned int)(sqrt(raw_matrix.size()) + EPSILON); - } + // One tower serves every nozzle and the filament to nozzle assignment is only decided later + // by ToolOrdering, so fold the per-nozzle blocks with std::max: the depth reserved here has + // to cover the worst nozzle. With a single nozzle the fold has one term. + const std::vector &raw_matrix = config.option("flush_volumes_matrix")->values; + const auto *nozzle_diameter = config.option("nozzle_diameter"); + const auto *flush_multiplier = config.option("flush_multiplier"); + const FlushVolumesMatrixDims dims = get_flush_volumes_matrix_dims(raw_matrix.size(), + flush_multiplier == nullptr ? 0 : flush_multiplier->values.size(), + nozzle_diameter == nullptr ? 0 : nozzle_diameter->values.size()); + const size_t nozzle_nums = dims.nozzle_nums; + const unsigned int number_of_extruders = dims.filament_nums; // The values shall only be used when SEMM is enabled. The purging for other printers // is determined by filament_minimal_purge_on_wipe_tower. diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index 01cbc43bc2..ea3f4f7b5e 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -5370,15 +5370,23 @@ void PresetBundle::update_multi_material_filament_presets(size_t to_delete_filam // Now verify if flush_volumes_matrix has proper size (it is used to deduce number of extruders in wipe tower generator): std::vector old_matrix = this->project_config.option("flush_volumes_matrix")->values; - size_t old_nozzle_nums = this->project_config.option("flush_multiplier")->values.size(); - size_t old_number_of_filaments = size_t(sqrt(old_matrix.size() / old_nozzle_nums) + EPSILON); + // Infer the stored layout from the matrix itself rather than trusting flush_multiplier's + // length (stale in older projects), falling back to a single block when they disagree. + const FlushVolumesMatrixDims old_dims = get_flush_volumes_matrix_dims(old_matrix.size(), + this->project_config.option("flush_multiplier")->values.size()); + size_t old_nozzle_nums = old_dims.nozzle_nums; + size_t old_number_of_filaments = old_dims.filament_nums; size_t nozzle_nums = get_printer_extruder_count(); - if (old_nozzle_nums != nozzle_nums) { + { std::vector& f_multiplier = this->project_config.option("flush_multiplier")->values; - f_multiplier.resize(nozzle_nums, 1.f); + if (f_multiplier.size() != nozzle_nums) + f_multiplier.resize(nozzle_nums, 1.f); } - if ( (num_filaments * num_filaments) != size_t(old_matrix.size() / old_nozzle_nums) ) { + // Rebuild whenever the stored size breaks the (filaments x filaments) block-per-nozzle + // invariant, including a nozzle-count-only change (the old per-block gate missed those, + // leaving a matrix short or over by whole blocks). + if (old_matrix.size() != num_filaments * num_filaments * nozzle_nums) { // First verify if purging volumes presets for each extruder matches number of extruders std::vector& filaments = this->project_config.option("flush_volumes_vector")->values; while (filaments.size() < 2* num_filaments) { @@ -5399,14 +5407,11 @@ void PresetBundle::update_multi_material_filament_presets(size_t to_delete_filam unsigned int old_i = i >= to_delete_filament_id ? i + 1 : i; unsigned int old_j = j >= to_delete_filament_id ? j + 1 : j; for (size_t nozzle_id = 0; nozzle_id < nozzle_nums; ++nozzle_id) { - // Orca: only copy from old_matrix when the old layout actually has data - // for this nozzle slot; otherwise initialize from the per-filament - // flush volumes the same way the (i,j) out-of-range branch does. - if (nozzle_id < old_nozzle_nums) { - new_matrix[i * num_filaments + j + new_matrix_size * nozzle_id] = old_matrix[old_i * old_number_of_filaments + old_j + old_matrix_size * nozzle_id]; - } else { - new_matrix[i * num_filaments + j + new_matrix_size * nozzle_id] = (i == j ? 0. : filaments[2 * i] + filaments[2 * j + 1]); - } + // Orca: a nozzle with stored data keeps its own block; a brand-new nozzle + // replicates the first nozzle's tuned values (the same grow policy as + // Plater::update_flush_volume_matrix) rather than resetting to defaults. + size_t src_nozzle_id = nozzle_id < old_nozzle_nums ? nozzle_id : 0; + new_matrix[i * num_filaments + j + new_matrix_size * nozzle_id] = old_matrix[old_i * old_number_of_filaments + old_j + old_matrix_size * src_nozzle_id]; } } else { for (size_t nozzle_id = 0; nozzle_id < nozzle_nums; ++nozzle_id) { diff --git a/src/libslic3r/PrintConfig.hpp b/src/libslic3r/PrintConfig.hpp index 6029d5bd88..c62abdfd6c 100644 --- a/src/libslic3r/PrintConfig.hpp +++ b/src/libslic3r/PrintConfig.hpp @@ -2367,6 +2367,33 @@ private: static uint64_t s_last_timestamp; }; +// A flush_volumes_matrix option holds one (filament_nums x filament_nums) block per nozzle +// (see PresetBundle::update_multi_material_filament_presets). Validate that partitioning +// against the stored size; when it does not match (older project, or the printer was just +// switched) fall back to treating the whole option as a single block — slicing or indexing +// with a mismatched row stride would read out of bounds. +struct FlushVolumesMatrixDims +{ + size_t nozzle_nums; // number of per-nozzle blocks actually stored + unsigned int filament_nums; // dimension (row stride) of each block +}; +// Two options claim to say how many blocks are stored and either can be stale: flush_multiplier +// lives in the project config and is written with the matrix, while nozzle_diameter comes from the +// printer preset and changes the moment a printer is selected. Pass both where both are at hand, +// preferred one first; the first candidate that squares up with the stored size wins. +inline FlushVolumesMatrixDims get_flush_volumes_matrix_dims(size_t matrix_size, size_t nozzle_nums, size_t alt_nozzle_nums = 0) +{ + for (size_t candidate : { nozzle_nums, alt_nozzle_nums, size_t(1) }) { + if (candidate == 0) + continue; + const unsigned int filament_nums = (unsigned int) (std::sqrt(double(matrix_size) / candidate) + EPSILON); + if (size_t(filament_nums) * filament_nums * candidate == matrix_size) + return { candidate, filament_nums }; + } + // Nothing partitions cleanly: read the whole option as one block, as this did before. + return { 1, (unsigned int) (std::sqrt(double(matrix_size)) + EPSILON) }; +} + // const std::vector &fv_matrix: origin matrix from json // size_t extruder_id: -1 means single-nozzle for old file, 0 means the 1st extruder, 1 means the 2nd extruder template diff --git a/src/slic3r/GUI/GLCanvas3D.cpp b/src/slic3r/GUI/GLCanvas3D.cpp index 27fe44a867..eaf4f171a5 100644 --- a/src/slic3r/GUI/GLCanvas3D.cpp +++ b/src/slic3r/GUI/GLCanvas3D.cpp @@ -10746,14 +10746,19 @@ bool GLCanvas3D::is_flushing_matrix_error() { if (multiplier == 0) return true; } - int matrix_len = config_matrix.size() / config_multiplier.size(); - int row_len = std::sqrt(matrix_len); - for (int i = 0; i < config_matrix.size(); i++) + // Orca: derive the block layout from the stored matrix instead of dividing by the multiplier + // count outright. A stale multiplier gives the wrong row stride, so cells are tested against + // the wrong diagonal, and an empty one divides by zero. + const FlushVolumesMatrixDims dims = get_flush_volumes_matrix_dims(config_matrix.size(), config_multiplier.size(), + wxGetApp().preset_bundle->get_printer_extruder_count()); + if (dims.filament_nums == 0) + return false; + const size_t block_len = size_t(dims.filament_nums) * dims.filament_nums; + for (size_t i = 0; i < config_matrix.size(); i++) { - int relative_id = i % matrix_len; - int row_id = relative_id / row_len; - int col_id = relative_id % row_len; - if (row_id != col_id && config_matrix[i] == 0) return true; + const size_t relative_id = i % block_len; + if (relative_id / dims.filament_nums != relative_id % dims.filament_nums && config_matrix[i] == 0) + return true; } return false; } diff --git a/src/slic3r/GUI/WipeTowerDialog.cpp b/src/slic3r/GUI/WipeTowerDialog.cpp index abf8baf086..9c103636a9 100644 --- a/src/slic3r/GUI/WipeTowerDialog.cpp +++ b/src/slic3r/GUI/WipeTowerDialog.cpp @@ -205,27 +205,29 @@ bool is_flush_config_modified() const std::vector &config_matrix = (project_config.option("flush_volumes_matrix"))->values; const std::vector &config_multiplier = (project_config.option("flush_multiplier"))->values; - bool has_modify = false; - for (int i = 0; i < config_multiplier.size(); i++) { - if (config_multiplier[i] != 1) { - has_modify = true; - break; - } - std::vector> default_matrix = WipingDialog::CalcFlushingVolumes(i); - int len = default_matrix.size(); - for (int m = 0; m < len; m++) { - for (int n = 0; n < len; n++) { - int idx = i * len * len + m * len + n; - if (config_matrix[idx] != default_matrix[m][n] * config_multiplier[i]) { - has_modify = true; - break; - } - } - if (has_modify) break; - } - if (has_modify) break; + for (double multiplier : config_multiplier) + if (multiplier != 1) + return true; + + // Orca: take the row stride and the block count from the stored matrix, not from the current + // filament count. The two disagree until update_multi_material_filament_presets rebuilds the + // matrix (a project saved with fewer filaments, or a printer just switched), and indexing a + // stale matrix with the current stride reads past the option. Clamp to the printer's extruder + // count as well, since CalcFlushingVolumes indexes per-extruder options with the same id. + const size_t extruder_count = wxGetApp().preset_bundle->get_printer_extruder_count(); + const FlushVolumesMatrixDims dims = get_flush_volumes_matrix_dims(config_matrix.size(), config_multiplier.size(), extruder_count); + const size_t nozzle_nums = std::min(dims.nozzle_nums, extruder_count); + for (size_t i = 0; i < nozzle_nums; i++) { + std::vector> default_matrix = WipingDialog::CalcFlushingVolumes(int(i)); + if (default_matrix.size() != dims.filament_nums) + return false; // Stored matrix predates the current filament count; it is about to be rebuilt. + // Every multiplier is 1 here, so the stored block has to equal the defaults outright. + for (size_t m = 0; m < dims.filament_nums; m++) + for (size_t n = 0; n < dims.filament_nums; n++) + if (config_matrix[(i * dims.filament_nums + m) * dims.filament_nums + n] != default_matrix[m][n]) + return true; } - return has_modify; + return false; } void open_flushing_dialog(wxEvtHandler *parent, const wxEvent &event) diff --git a/tests/data/wipe_tower_temperature_trace_main.txt b/tests/data/wipe_tower_temperature_trace_main.txt index b7453bdc93..bb98f8242b 100644 --- a/tests/data/wipe_tower_temperature_trace_main.txt +++ b/tests/data/wipe_tower_temperature_trace_main.txt @@ -1,5 +1,6 @@ # Temperature and tool-change commands of a wait_for_temp_on_wipe_tower-off slice, -# captured from the main branch at a10d9e77cf. Regeneration is described +# captured from main (18ca06ec6b) with the set_extruder flush-volume out-of-bounds fix applied. +# Regeneration is described # at the test that reads this file: "Toolchange temperature commands are unchanged # when the wipe tower wait is off" in tests/fff_print/test_multifilament.cpp. M104 S215 T0 ; set nozzle temperature @@ -30,55 +31,7 @@ M104 S200 T0 ; set nozzle temperature ;cooldown T1 ; change extruder M109 S240 T1 ; set nozzle temperature and wait for it to be reached ; CP TOOLCHANGE END -M104 S240 T0 ; preheat T0 time: 31s lead 30.9s -; CP TOOLCHANGE START -M104 S200 T1 ; set nozzle temperature ;cooldown -T0 ; change extruder -M109 S240 T0 ; set nozzle temperature and wait for it to be reached -; CP TOOLCHANGE END -M104 S240 T1 ; preheat T1 time: 30s lead 30.2s -; CP TOOLCHANGE START -M104 S200 T0 ; set nozzle temperature ;cooldown -T1 ; change extruder -M109 S240 T1 ; set nozzle temperature and wait for it to be reached -; CP TOOLCHANGE END -M104 S240 T0 ; preheat T0 time: 30s lead 30.2s -; CP TOOLCHANGE START -M104 S200 T1 ; set nozzle temperature ;cooldown -T0 ; change extruder -M109 S240 T0 ; set nozzle temperature and wait for it to be reached -; CP TOOLCHANGE END -M104 S240 T1 ; preheat T1 time: 30s lead 30.2s -; CP TOOLCHANGE START -M104 S200 T0 ; set nozzle temperature ;cooldown -T1 ; change extruder -M109 S240 T1 ; set nozzle temperature and wait for it to be reached -; CP TOOLCHANGE END -M104 S240 T0 ; preheat T0 time: 30s lead 30.2s -; CP TOOLCHANGE START -M104 S200 T1 ; set nozzle temperature ;cooldown -T0 ; change extruder -M109 S240 T0 ; set nozzle temperature and wait for it to be reached -; CP TOOLCHANGE END -M104 S240 T1 ; preheat T1 time: 30s lead 30.2s -; CP TOOLCHANGE START -M104 S200 T0 ; set nozzle temperature ;cooldown -T1 ; change extruder -M109 S240 T1 ; set nozzle temperature and wait for it to be reached -; CP TOOLCHANGE END -M104 S240 T0 ; preheat T0 time: 30s lead 30.2s -; CP TOOLCHANGE START -M104 S200 T1 ; set nozzle temperature ;cooldown -T0 ; change extruder -M109 S240 T0 ; set nozzle temperature and wait for it to be reached -; CP TOOLCHANGE END -M104 S240 T1 ; preheat T1 time: 31s lead 30.7s -; CP TOOLCHANGE START -M104 S200 T0 ; set nozzle temperature ;cooldown -T1 ; change extruder -M109 S240 T1 ; set nozzle temperature and wait for it to be reached -; CP TOOLCHANGE END -M104 S240 T0 ; preheat T0 time: 31s lead 30.6s +M104 S240 T0 ; preheat T0 time: 30s lead 30.3s ; CP TOOLCHANGE START M104 S200 T1 ; set nozzle temperature ;cooldown T0 ; change extruder @@ -90,13 +43,61 @@ M104 S200 T0 ; set nozzle temperature ;cooldown T1 ; change extruder M109 S240 T1 ; set nozzle temperature and wait for it to be reached ; CP TOOLCHANGE END -M104 S240 T0 ; preheat T0 time: 31s lead 30.6s +M104 S240 T0 ; preheat T0 time: 30s lead 30.3s ; CP TOOLCHANGE START M104 S200 T1 ; set nozzle temperature ;cooldown T0 ; change extruder M109 S240 T0 ; set nozzle temperature and wait for it to be reached ; CP TOOLCHANGE END -M104 S240 T1 ; preheat T1 time: 30s lead 30.0s +M104 S240 T1 ; preheat T1 time: 30s lead 30.3s +; CP TOOLCHANGE START +M104 S200 T0 ; set nozzle temperature ;cooldown +T1 ; change extruder +M109 S240 T1 ; set nozzle temperature and wait for it to be reached +; CP TOOLCHANGE END +M104 S240 T0 ; preheat T0 time: 30s lead 30.3s +; CP TOOLCHANGE START +M104 S200 T1 ; set nozzle temperature ;cooldown +T0 ; change extruder +M109 S240 T0 ; set nozzle temperature and wait for it to be reached +; CP TOOLCHANGE END +M104 S240 T1 ; preheat T1 time: 30s lead 30.3s +; CP TOOLCHANGE START +M104 S200 T0 ; set nozzle temperature ;cooldown +T1 ; change extruder +M109 S240 T1 ; set nozzle temperature and wait for it to be reached +; CP TOOLCHANGE END +M104 S240 T0 ; preheat T0 time: 30s lead 30.3s +; CP TOOLCHANGE START +M104 S200 T1 ; set nozzle temperature ;cooldown +T0 ; change extruder +M109 S240 T0 ; set nozzle temperature and wait for it to be reached +; CP TOOLCHANGE END +M104 S240 T1 ; preheat T1 time: 30s lead 30.3s +; CP TOOLCHANGE START +M104 S200 T0 ; set nozzle temperature ;cooldown +T1 ; change extruder +M109 S240 T1 ; set nozzle temperature and wait for it to be reached +; CP TOOLCHANGE END +M104 S240 T0 ; preheat T0 time: 30s lead 30.1s +; CP TOOLCHANGE START +M104 S200 T1 ; set nozzle temperature ;cooldown +T0 ; change extruder +M109 S240 T0 ; set nozzle temperature and wait for it to be reached +; CP TOOLCHANGE END +M104 S240 T1 ; preheat T1 time: 31s lead 30.6s +; CP TOOLCHANGE START +M104 S200 T0 ; set nozzle temperature ;cooldown +T1 ; change extruder +M109 S240 T1 ; set nozzle temperature and wait for it to be reached +; CP TOOLCHANGE END +M104 S240 T0 ; preheat T0 time: 30s lead 30.3s +; CP TOOLCHANGE START +M104 S200 T1 ; set nozzle temperature ;cooldown +T0 ; change extruder +M109 S240 T0 ; set nozzle temperature and wait for it to be reached +; CP TOOLCHANGE END +M104 S240 T1 ; preheat T1 time: 31s lead 30.5s ; CP TOOLCHANGE START M104 S200 T0 ; set nozzle temperature ;cooldown T1 ; change extruder @@ -132,7 +133,7 @@ M104 S200 T1 ; set nozzle temperature ;cooldown T0 ; change extruder M109 S240 T0 ; set nozzle temperature and wait for it to be reached ; CP TOOLCHANGE END -M104 S240 T1 ; preheat T1 time: 31s lead 30.7s +M104 S240 T1 ; preheat T1 time: 31s lead 30.6s ; CP TOOLCHANGE START M104 S200 T0 ; set nozzle temperature ;cooldown T1 ; change extruder diff --git a/tests/fff_print/test_multifilament.cpp b/tests/fff_print/test_multifilament.cpp index 081c0fa2ba..31d9bcb423 100644 --- a/tests/fff_print/test_multifilament.cpp +++ b/tests/fff_print/test_multifilament.cpp @@ -175,6 +175,17 @@ static std::pair> split_lead(const std::strin return { entry.substr(0, tab), std::stod(tail.substr(tail.find(' ') + 1)) }; } +// The "time: s" a preheat comment carries is round()'d from the same estimate the lead measures, +// so it sits on a rounding boundary and flips (e.g. 30<->31) across platforms and build optimisation +// levels; drop it from the command text so only the tolerant lead below carries that timing. +static std::string strip_preheat_time(std::string command) +{ + const size_t pos = command.find(" time: "); + if (pos != std::string::npos) + command.erase(pos); + return command; +} + // Same command, and a lead time within half a second. The lead is an estimate summed over every // move before it, so it drifts slightly with unrelated changes to travel or tower geometry; half a // second is far below the tens of seconds a preheat leaving its backtrace position would shift it. @@ -182,7 +193,7 @@ static bool trace_entries_match(const std::string& a, const std::string& b) { const auto x = split_lead(a); const auto y = split_lead(b); - if (x.first != y.first) + if (strip_preheat_time(x.first) != strip_preheat_time(y.first)) return false; if (x.second.has_value() != y.second.has_value()) return false; diff --git a/tests/libslic3r/CMakeLists.txt b/tests/libslic3r/CMakeLists.txt index 1ad299473c..564e8c82a1 100644 --- a/tests/libslic3r/CMakeLists.txt +++ b/tests/libslic3r/CMakeLists.txt @@ -14,6 +14,7 @@ add_executable(${_TEST_NAME}_tests test_config.cpp test_config_variant_expansion.cpp test_toolordering_nozzle_group.cpp + test_preset_bundle_flush_matrix.cpp test_preset_bundle_loading.cpp test_preset_setting_id.cpp test_preset_diff.cpp diff --git a/tests/libslic3r/test_preset_bundle_flush_matrix.cpp b/tests/libslic3r/test_preset_bundle_flush_matrix.cpp new file mode 100644 index 0000000000..72620bd693 --- /dev/null +++ b/tests/libslic3r/test_preset_bundle_flush_matrix.cpp @@ -0,0 +1,132 @@ +#include + +#include "libslic3r/PresetBundle.hpp" + +using namespace Slic3r; + +namespace { + +// Put the bundle in a known multi-material state: the edited printer's nozzle count, the number +// of selected filaments, and the stored flush matrix/multiplier the scenario starts from. +// flush_volumes_vector is fixed at 140 per filament, so a matrix cell seeded from it (rather +// than preserved) is exactly 280 (= from-filament 140 + to-filament 140). +void setup_flush_state(PresetBundle &bundle, size_t nozzle_count, size_t filament_count, + std::vector matrix, std::vector multiplier) +{ + bundle.printers.get_edited_preset().config.option("nozzle_diameter", true)->values + .assign(nozzle_count, 0.4); + bundle.filament_presets.assign(filament_count, + bundle.filament_presets.empty() ? bundle.filaments.default_preset().name : bundle.filament_presets.front()); + bundle.project_config.option("flush_volumes_matrix")->values = std::move(matrix); + bundle.project_config.option("flush_multiplier")->values = std::move(multiplier); + bundle.project_config.option("flush_volumes_vector")->values + .assign(2 * filament_count, 140.); +} + +const std::vector &flush_matrix(const PresetBundle &bundle) +{ + return bundle.project_config.option("flush_volumes_matrix")->values; +} + +const std::vector &flush_multiplier(const PresetBundle &bundle) +{ + return bundle.project_config.option("flush_multiplier")->values; +} + +} // namespace + +// The matrix must hold one (filaments x filaments) block per nozzle. Growing the printer from +// one to two nozzles with the filament count unchanged has to add a block, preserving the +// existing nozzle's values and replicating them to the new nozzle (the same policy as +// Plater::update_flush_volume_matrix), so tuned flush volumes survive a printer switch. +TEST_CASE("Adding an extruder replicates the tuned flush matrix block to the new nozzle", "[Preset][FlushMatrix]") +{ + PresetBundle bundle; + setup_flush_state(bundle, 2, 2, /*matrix=*/{ 0., 100., 200., 0. }, /*multiplier=*/{ 1. }); + + bundle.update_multi_material_filament_presets(); + + CHECK(flush_matrix(bundle) == std::vector{ 0., 100., 200., 0., // nozzle 0: preserved + 0., 100., 200., 0. }); // nozzle 1: replicated + CHECK(flush_multiplier(bundle).size() == 2); +} + +// When a filament and a nozzle are added at once, known filament pairs keep (and replicate) +// their tuned values while pairs involving the new filament are seeded from flush_volumes_vector. +TEST_CASE("Adding a filament and an extruder together seeds only the new filament's pairs", "[Preset][FlushMatrix]") +{ + PresetBundle bundle; + setup_flush_state(bundle, 2, 3, /*matrix=*/{ 0., 100., 200., 0. }, /*multiplier=*/{ 1. }); + + bundle.update_multi_material_filament_presets(); + + const std::vector block { 0., 100., 280., + 200., 0., 280., + 280., 280., 0. }; + std::vector expected(block); + expected.insert(expected.end(), block.begin(), block.end()); + CHECK(flush_matrix(bundle) == expected); +} + +TEST_CASE("Removing an extruder keeps only the remaining nozzle's flush matrix block", "[Preset][FlushMatrix]") +{ + PresetBundle bundle; + setup_flush_state(bundle, 1, 2, /*matrix=*/{ 0., 101., 201., 0., // nozzle 0 + 0., 303., 403., 0. }, // nozzle 1 + /*multiplier=*/{ 1., 1. }); + + bundle.update_multi_material_filament_presets(); + + CHECK(flush_matrix(bundle) == std::vector{ 0., 101., 201., 0. }); + CHECK(flush_multiplier(bundle).size() == 1); +} + +// A stale flush_multiplier length must not be trusted as the block count: a single 2x2 block +// with a 2-entry multiplier describes one nozzle's worth of data, and "repairing" it with the +// multiplier's layout would scramble the stored values. +TEST_CASE("A flush matrix whose multiplier lies about the block count survives unscrambled", "[Preset][FlushMatrix]") +{ + PresetBundle bundle; + setup_flush_state(bundle, 1, 2, /*matrix=*/{ 0., 100., 200., 0. }, /*multiplier=*/{ 1., 1. }); + + bundle.update_multi_material_filament_presets(); + + CHECK(flush_matrix(bundle) == std::vector{ 0., 100., 200., 0. }); + CHECK(flush_multiplier(bundle).size() == 1); +} + +TEST_CASE("Flush matrix dimensions come from the block count that squares up with the stored size", "[Preset][FlushMatrix]") +{ + // Two 3x3 blocks: only a block count of 2 partitions 18 into squares. + CHECK(get_flush_volumes_matrix_dims(18, 2).nozzle_nums == 2); + CHECK(get_flush_volumes_matrix_dims(18, 2).filament_nums == 3); + + // One 2x2 block with a nozzle count that outgrew it, the case a printer switch leaves behind. + // Reading it as two blocks would halve the row stride and index past the block. + CHECK(get_flush_volumes_matrix_dims(4, 2).nozzle_nums == 1); + CHECK(get_flush_volumes_matrix_dims(4, 2).filament_nums == 2); + + // A count of zero says "unknown", not "zero blocks". + CHECK(get_flush_volumes_matrix_dims(4, 0).nozzle_nums == 1); + CHECK(get_flush_volumes_matrix_dims(4, 0).filament_nums == 2); + + // Nothing partitions 12 into equal squares: fall back to the whole option as one block, so a + // caller slices everything it has rather than a wrongly-sized window into it. + CHECK(get_flush_volumes_matrix_dims(12, 5).nozzle_nums == 1); +} + +// Both flush_multiplier and nozzle_diameter claim to say how many blocks are stored and either can +// be stale, so callers pass both. The alternate rescues a wrong preferred hint, and where both fit +// the stored size the preferred one settles the ambiguity. +TEST_CASE("The alternate block count is used only when the preferred one does not fit", "[Preset][FlushMatrix]") +{ + // Preferred 3 does not divide 18 into squares, alternate 2 does. + CHECK(get_flush_volumes_matrix_dims(18, 3, 2).nozzle_nums == 2); + CHECK(get_flush_volumes_matrix_dims(18, 3, 2).filament_nums == 3); + + // 16 is four 2x2 blocks or one 4x4; the preferred hint decides. + CHECK(get_flush_volumes_matrix_dims(16, 4, 1).nozzle_nums == 4); + CHECK(get_flush_volumes_matrix_dims(16, 4, 1).filament_nums == 2); + CHECK(get_flush_volumes_matrix_dims(16, 1, 4).nozzle_nums == 1); + CHECK(get_flush_volumes_matrix_dims(16, 1, 4).filament_nums == 4); +}