Keep Layer Pauses on Their Own Layer in the Preview

The G-code processor remembers where a pause, color change or custom
G-code vertex sits in the move list, so it can move that vertex to the
first extrusion after it. A pause flushes the time planner first, which
inserts actual speed moves ahead of the vertex, so the remembered index
pointed at an earlier move: the previous layer's last extrusion was
moved instead, a real move was erased, and the time blocks queued after
the vertex were left one move off.

Shift the remembered index by the moves each flush inserts before it,
and shift the queued blocks after the erase. The Preview numbers layers
from the move order, so the stray move showed up as extra layers and
listed pauses on the wrong layer.

build_release_macos.sh

@@ -47,7 +47,7 @@ while getopts ":dpa:snt:xbc:i:j:Tuh" opt; do
     u )
         export BUILD_TARGET="universal"
         ;;
-    h ) echo "Usage: ./build_release_macos.sh [-d]"
+    h ) echo "Usage: ./build_release_macos.sh [-d] [-- CMAKE_ARGS...]"
         echo "   -d: Build deps only"
         echo "   -a: Set ARCHITECTURE (arm64 or x86_64 or universal)"
         echo "   -s: Build slicer only"
@@ -60,13 +60,33 @@ while getopts ":dpa:snt:xbc:i:j:Tuh" opt; do
         echo "   -i: Add a prefix to ignore during CMake dependency discovery (repeatable), defaults to /opt/local:/usr/local:/opt/homebrew"
         echo "   -j: Set the number of parallel build jobs (CMAKE_BUILD_PARALLEL_LEVEL)"
         echo "   -T: Build and run tests (set ORCA_TESTS_BUILD_ONLY=1 to build without running)"
+        echo "   -- CMAKE_ARGS...: pass everything after '--' to the cmake configure step of the deps (-d) and slicer (-s) builds,"
+        echo "       e.g. -DDESTDIR=<dir> for the deps, -DCMAKE_PREFIX_PATH=<dir>/usr/local -DSLIC3R_PCH=OFF for the slicer, --fresh."
+        echo "       They come last on the cmake command line. Use absolute paths: cmake runs in the build directory."
         exit 0
         ;;
+    : )
+        echo "Option -${OPTARG} requires an argument, aborting."
+        exit 1
+        ;;
     * )
+        echo "Unknown option -${OPTARG}, aborting. CMake arguments must follow '--'."
+        exit 1
         ;;
   esac
 done
 
+# getopts stops at '--'; whatever follows is handed to cmake unchanged.
+if [[ ${OPTIND} -le $# ]] ; then
+    separator_index=$((OPTIND-1))
+    if [[ "${!separator_index}" != "--" ]] ; then
+        echo "Unexpected argument '${!OPTIND}': cmake arguments must follow '--', aborting."
+        exit 1
+    fi
+fi
+shift $((OPTIND-1))
+EXTRA_CMAKE_ARGS=("$@")
+
 # Set defaults
 
 if [ -z "$ARCH" ]; then
@@ -211,7 +231,8 @@ function build_deps() {
                         -DCMAKE_OSX_DEPLOYMENT_TARGET="${OSX_DEPLOYMENT_TARGET}" \
                         -DCMAKE_IGNORE_PREFIX_PATH="${CMAKE_IGNORE_PREFIX_PATH}" \
                         "${CMAKE_CCACHE_ARGS[@]}" \
-                        ${CMAKE_POLICY_COMPAT}
+                        ${CMAKE_POLICY_COMPAT} \
+                        "${EXTRA_CMAKE_ARGS[@]}"
                 fi
                 cmake --build . --config "$BUILD_CONFIG" --target deps
             )
@@ -314,7 +335,8 @@ function build_slicer() {
                     -DCMAKE_OSX_DEPLOYMENT_TARGET="${OSX_DEPLOYMENT_TARGET}" \
                     -DCMAKE_IGNORE_PREFIX_PATH="${CMAKE_IGNORE_PREFIX_PATH}" \
                     "${CMAKE_CCACHE_ARGS[@]}" \
-                    ${CMAKE_POLICY_COMPAT}
+                    ${CMAKE_POLICY_COMPAT} \
+                    "${EXTRA_CMAKE_ARGS[@]}"
             fi
             cmake --build . --config "$BUILD_CONFIG" --target "$SLICER_BUILD_TARGET"
         )

src/libslic3r/GCode/GCodeProcessor.cpp

@@ -5234,7 +5234,15 @@ void GCodeProcessor::process_G1(const std::array<std::optional<double>, 4>& axes
 
         if (origin == G1DiscretizationOrigin::G1)
             m_extruded_last_z = m_end_position[Z];
-        m_options_z_corrector.update(m_height);
+        // The queued blocks of the moves after the erased one follow them down the list.
+        if (const std::optional<size_t> erased = m_options_z_corrector.update(m_height)) {
+            for (TimeMachine& machine : m_time_processor.machines) {
+                for (TimeBlock& block : machine.blocks) {
+                    if (block.move_id > *erased)
+                        --block.move_id;
+                }
+            }
+        }
 
         if (m_forced_width > 0.0f)
             m_width = m_forced_width;
@@ -7958,6 +7966,7 @@ void GCodeProcessor::calculate_time(GCodeProcessorResult& result, size_t keep_la
                                 it->second : block.move_id + (unsigned int)inserted_actual_speed_moves_count;
         }
     }
+    m_options_z_corrector.on_moves_inserted(m_actual_speed_id_map);
 }
 
 void GCodeProcessor::simulate_st_synchronize(float additional_time, EMoveType target_move_type)

src/libslic3r/GCode/GCodeProcessor.hpp

@@ -18,6 +18,7 @@
 #include <cstddef>
 #include <cassert>
 #include <cstdint>
+#include <algorithm>
 #include <array>
 #include <unordered_map>
 #include <utility>
@@ -1076,18 +1077,34 @@ inline constexpr float DEFAULT_FILAMENT_DENSITY = 1.245f;
                 m_custom_gcode_per_print_z_id = m_result.custom_gcode_per_print_z.size() - 1;
             }
 
-            void update(float height) {
-                if (!m_move_id.has_value() || !m_custom_gcode_per_print_z_id.has_value())
+            // The time pass inserts actual speed moves in front of the moves of its blocks, and the stored
+            // vertex has no block of its own, so it shifts with every insertion made before it.
+            void on_moves_inserted(const std::vector<std::pair<unsigned int, unsigned int>>& id_map) {
+                if (!m_move_id.has_value())
                     return;
+                auto it = std::lower_bound(id_map.begin(), id_map.end(), *m_move_id,
+                                           [](const std::pair<unsigned int, unsigned int>& entry, size_t id) { return entry.first < id; });
+                if (it != id_map.begin()) {
+                    --it;
+                    *m_move_id += it->second - it->first;
+                }
+            }
 
-                const Vec3f position = m_result.moves.back().position;
+            // Returns the index of the move erased from the list, if any.
+            std::optional<size_t> update(float height) {
+                if (!m_move_id.has_value() || !m_custom_gcode_per_print_z_id.has_value())
+                    return std::nullopt;
 
-                GCodeProcessorResult::MoveVertex& move = m_result.moves.emplace_back(m_result.moves[*m_move_id]);
+                const Vec3f  position = m_result.moves.back().position;
+                const size_t erased   = *m_move_id;
+
+                GCodeProcessorResult::MoveVertex& move = m_result.moves.emplace_back(m_result.moves[erased]);
                 move.position = position;
                 move.height = height;
-                m_result.moves.erase(m_result.moves.begin() + *m_move_id);
+                m_result.moves.erase(m_result.moves.begin() + erased);
                 m_result.custom_gcode_per_print_z[*m_custom_gcode_per_print_z_id].print_z = position.z();
                 reset();
+                return erased;
             }
 
             void reset() {

tests/fff_print/test_gcodeprocessor.cpp

@@ -282,6 +282,48 @@ TEST_CASE("A seam takes the actual speed of the move it follows", "[GCodeProcess
     REQUIRE(seams > 0);
 }
 
+TEST_CASE("A pause keeps the moves around it in order and on their own layers", "[GCodeProcessor][Regression]")
+{
+    // libvgcode numbers layers from the order of the moves, so the moves around a pause must stay in layer and
+    // line order. The pause vertex itself moves to the first extrusion after it and keeps its own line.
+    std::ostringstream gcode;
+    gcode << "M83\nG90\n";
+    for (int layer = 1; layer <= 3; ++layer) {
+        gcode << "; CHANGE_LAYER\n; LAYER_HEIGHT: 0.2\n";
+        if (layer == 2)
+            gcode << "G1 E-0.8 F3000\n; PAUSE_PRINTING\nM601\n";
+        gcode << "G1 X10 Y10 F12000\nG1 Z" << 0.2 * layer << "\n";
+        if (layer == 2)
+            gcode << "G1 E0.8 F3000\n";
+        gcode << "; FEATURE: Outer wall\n"
+              << "G1 X50 Y10 E2 F3000\nG1 X50 Y50 E2\nG1 X10 Y50 E2\nG1 X10 Y10 E2\n";
+    }
+    GCodeProcessorResult result;
+    process_gcode(gcode.str(), result);
+    const auto &moves = result.moves;
+
+    REQUIRE(result.custom_gcode_per_print_z.size() == 1);
+    CHECK_THAT(result.custom_gcode_per_print_z.front().print_z, Catch::Matchers::WithinAbs(0.4, 1e-4));
+    REQUIRE(std::count_if(moves.begin(), moves.end(), [](const auto &move) { return move.internal_only; }) > 0);
+
+    const auto pause = std::find_if(moves.begin(), moves.end(), [](const auto &move) { return move.type == EMoveType::Pause_Print; });
+    REQUIRE(pause != moves.end());
+    CHECK(pause->layer_id == 1);
+    CHECK_THAT(pause->position.z(), Catch::Matchers::WithinAbs(0.4f, 1e-4f));
+
+    const GCodeProcessorResult::MoveVertex *previous = nullptr;
+    for (const auto &move : moves) {
+        if (move.type == EMoveType::Pause_Print)
+            continue;
+        if (previous != nullptr) {
+            CAPTURE(previous->gcode_id, move.gcode_id, static_cast<int>(move.type));
+            CHECK(move.layer_id >= previous->layer_id);
+            CHECK(move.gcode_id >= previous->gcode_id);
+        }
+        previous = &move;
+    }
+}
+
 TEST_CASE("Line ends of the exported G-code mark every newline in the file", "[GCodeProcessor]")
 {
     struct Case