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() {

src/slic3r/plugin/PluginAuditManager.cpp

@@ -183,52 +183,47 @@ static const char* category_name(AuditEventCategory category)
 // Path safety
 // ---------------------------------------------------------------------------
 
-namespace {
-
-// Canonicalize a path for containment comparison.  weakly_canonical resolves symlinks but does
-// NOT require the path to exist — it canonicalizes the prefix that exists and appends the
-// non-existing tail lexically.
-boost::filesystem::path canonical_for_comparison(const boost::filesystem::path& path)
+bool is_inside_allowed_root(const boost::filesystem::path& candidate, const boost::filesystem::path& allowed_root)
 {
     namespace fs = boost::filesystem;
 
     boost::system::error_code ec;
-    fs::path canon = fs::weakly_canonical(path, ec);
+
+    // Canonicalize both paths.  weakly_canonical resolves symlinks but does
+    // NOT require the path to exist — it canonicalizes the prefix that exists
+    // and appends the non-existing tail lexically.
+    fs::path canon_candidate = fs::weakly_canonical(candidate, ec);
     if (ec) {
         // Fall back to lexically_normal + absolute
-        canon = fs::absolute(path, ec).lexically_normal();
+        canon_candidate = fs::absolute(candidate, ec).lexically_normal();
         if (ec)
-            canon = path;
+            canon_candidate = candidate;
     }
-    return canon;
-}
 
-bool same_path_component(const boost::filesystem::path& lhs, const boost::filesystem::path& rhs)
-{
+    fs::path canon_root = fs::weakly_canonical(allowed_root, ec);
+    if (ec) {
+        canon_root = fs::absolute(allowed_root, ec).lexically_normal();
+        if (ec)
+            canon_root = allowed_root;
+    }
+
+    // Component-wise comparison: the root must be a prefix of candidate,
+    // and the next component must not be ".." or missing.
+    auto cand_it  = canon_candidate.begin();
+    auto cand_end = canon_candidate.end();
+    auto root_it  = canon_root.begin();
+    auto root_end = canon_root.end();
+
+    const auto same_component = [](const fs::path& lhs, const fs::path& rhs) {
 #ifdef _WIN32
-    return boost::algorithm::iequals(lhs.native(), rhs.native());
+        return boost::algorithm::iequals(lhs.native(), rhs.native());
 #else
-    return lhs == rhs;
+        return lhs == rhs;
 #endif
-}
-
-// Component-wise containment: the whole of canonical_root must be a prefix of
-// canonical_candidate, and the remaining components must not traverse upward. Both arguments are
-// expected to come from canonical_for_comparison(). On a match, suffix receives the candidate's
-// components below the root (empty when the two paths are equal).
-bool suffix_inside_root(const boost::filesystem::path& canonical_candidate,
-                        const boost::filesystem::path& canonical_root,
-                        boost::filesystem::path&       suffix)
-{
-    suffix.clear();
-
-    auto cand_it  = canonical_candidate.begin();
-    auto cand_end = canonical_candidate.end();
-    auto root_it  = canonical_root.begin();
-    auto root_end = canonical_root.end();
+    };
 
     // Consume matching components
-    while (root_it != root_end && cand_it != cand_end && same_path_component(*root_it, *cand_it)) {
+    while (root_it != root_end && cand_it != cand_end && same_component(*root_it, *cand_it)) {
         ++root_it;
         ++cand_it;
     }
@@ -243,19 +238,9 @@ bool suffix_inside_root(const boost::filesystem::path& canonical_candidate,
             return false;
     }
 
-    for (; cand_it != cand_end; ++cand_it)
-        suffix /= *cand_it;
     return true;
 }
 
-} // namespace
-
-bool is_inside_allowed_root(const boost::filesystem::path& candidate, const boost::filesystem::path& allowed_root)
-{
-    boost::filesystem::path suffix;
-    return suffix_inside_root(canonical_for_comparison(candidate), canonical_for_comparison(allowed_root), suffix);
-}
-
 // ---------------------------------------------------------------------------
 // ScopedPluginAuditContext
 // ---------------------------------------------------------------------------
@@ -425,8 +410,7 @@ std::vector<std::string> PluginAuditManager::default_denied_path_keywords()
     // it happens to live inside an otherwise-allowed root (e.g. the bundled TLS client cert at
     // resources_dir()/cert/..., which would become reachable the moment resources_dir() is
     // granted as a read-only allowed root). Match as whole path components, not substrings, so
-    // imports such as numpy/__config__.py and stdlib configparser.py remain usable. Inside
-    // data_dir() they match below that root only; see is_denied_path_keyword().
+    // imports such as numpy/__config__.py and stdlib configparser.py remain usable.
     return {"secret", "secrets", "cert", "certs", "certificate", "certificates", "conf", "config"};
 }
 
@@ -445,29 +429,19 @@ bool PluginAuditManager::is_denied_path_keyword(const boost::filesystem::path& c
 {
     namespace fs = boost::filesystem;
 
-    const fs::path canon = canonical_for_comparison(candidate);
-
-    // The app's data folder can itself sit below a component named like one of the keywords
-    // without being one: Flatpak keeps it under <home>/.var/app/<app-id>/config/OrcaSlicer, and
-    // the sandbox's home layout is not ours to change.  A keyword above data_dir() therefore says
-    // nothing about what a plugin may reach inside it, so inside data_dir() the keywords apply to
-    // the suffix below data_dir() only.  Everywhere else -- resources_dir() with the bundled
-    // certificates, a plugin's own folders, arbitrary files -- the whole path is still checked,
-    // and so is a keyword under data_dir() itself.
-    fs::path canonical_data_dir;
-    if (!data_dir().empty())
-        canonical_data_dir = canonical_for_comparison(data_dir());
-
-    fs::path scope = canon;
-    fs::path suffix;
-    if (!canonical_data_dir.empty() && suffix_inside_root(canon, canonical_data_dir, suffix))
-        scope = suffix;
+    boost::system::error_code ec;
+    fs::path canon = fs::weakly_canonical(candidate, ec);
+    if (ec) {
+        canon = fs::absolute(candidate, ec).lexically_normal();
+        if (ec)
+            canon = candidate;
+    }
 
     std::lock_guard<std::mutex> lock(m_mutex);
     if (m_denied_path_keywords.empty())
         return false;
 
-    for (const auto& component : scope) {
+    for (const auto& component : canon) {
         std::string name = component.string();
         if (name.empty())
             continue;
@@ -1321,12 +1295,10 @@ void PluginAuditManager::install_hook()
     for (const auto& name : default_denied_filenames())
         add_denied_filename(name);
 
-    // Categorical denies on top of the exact-name list above: a path a plugin reaches must not
-    // have a "secret", "cert"(ificate), or "conf"(ig) component below data_dir() -- or anywhere
-    // in the path outside data_dir(), whose own location may legitimately contain one (Flatpak
-    // keeps it under .../config/OrcaSlicer). This holds regardless of which allowed root the
-    // path happens to sit inside. default_denied_path_keywords() is the single source of this
-    // list; the tests seed from it too.
+    // Categorical denies on top of the exact-name list above: no path a plugin can reach may
+    // contain a "secret", "cert"(ificate), or "conf"(ig) path component, regardless of which
+    // allowed root it happens to sit inside. default_denied_path_keywords() is the single
+    // source of this list; the tests seed from it too.
     for (const auto& keyword : default_denied_path_keywords())
         add_denied_path_keyword(keyword);
 

src/slic3r/plugin/PluginAuditManager.hpp

@@ -102,13 +102,12 @@ public:
     bool is_denied_filename(const boost::filesystem::path& candidate) const;
 
     // --- denied-path-keyword registry ---
-    // Keywords that categorically deny a path when a component matches one case-insensitively --
+    // Keywords that categorically deny a path if ANY component matches one case-insensitively --
     // e.g. a "secrets" subfolder, a "certificates" folder, a "conf"/"config" directory, or a
     // .conf/.ini file anywhere the plugin can otherwise reach, including inside an allowed root.
-    // The components above the app's data_dir() are not considered (see below). This is broader
-    // than the exact-name is_denied_filename registry, but it is not a substring match:
-    // importable modules such as numpy/__config__.py, configparser.py, sysconfig.py, or user
-    // folders such as "Conference" and "Concert" are unrelated names and must stay promptable.
+    // This is broader than the exact-name is_denied_filename registry, but it is not a substring
+    // match: importable modules such as numpy/__config__.py, configparser.py, sysconfig.py, or
+    // user folders such as "Conference" and "Concert" are unrelated names and must stay promptable.
     void add_denied_path_keyword(const std::string& keyword);
 
     // The list install_hook() seeds into the keyword registry. Exposed so tests seed the exact
@@ -116,10 +115,7 @@ public:
     static std::vector<std::string> default_denied_path_keywords();
 
     // True when any component of candidate's (canonicalized) path matches a registered keyword,
-    // case-insensitively. Inside data_dir(), only the components below data_dir() are checked:
-    // the app's own data folder may itself sit under a component named like a keyword (Flatpak
-    // keeps it under .../config/OrcaSlicer), which must not deny everything inside it. A
-    // registered "conf" keyword also blocks .conf/.ini file components.
+    // case-insensitively. A registered "conf" keyword also blocks .conf/.ini file components.
     bool is_denied_path_keyword(const boost::filesystem::path& candidate) const;
 
     // is_denied_filename(candidate) || is_denied_path_keyword(candidate). Convenience for

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

tests/slic3rutils/test_plugin_audit.cpp

@@ -288,67 +288,6 @@ TEST_CASE("Plugin audit denies secret/certificate/config-like paths by keyword",
     }
 }
 
-TEST_CASE("Plugin audit checks denied keywords below data_dir, not in the directories above it", "[PluginAudit]")
-{
-    // Flatpak keeps the app's data folder under <home>/.var/app/<app-id>/config/OrcaSlicer, so
-    // data_dir() itself has a "config" component. That ancestor describes the sandbox's layout,
-    // not what the folder holds, and must not hard-deny every path inside it.
-    ScopedDataDir data_dir_guard("plugin-audit-data-dir-keyword");
-    const fs::path flatpak_data_dir = data_dir_guard.dir / "config" / "OrcaSlicer";
-    set_data_dir(flatpak_data_dir.string());
-
-    seed_denied_names();
-    seed_denied_keywords();
-
-    PluginAuditManager& mgr = PluginAuditManager::instance();
-    // Reproduce install_hook()'s grant: data_dir() is a global allowed root.
-    mgr.add_global_allowed_root(data_dir());
-
-    ScopedPluginAuditContext ctx("test_plugin", "");
-
-    SECTION("the root and the paths below it that carry no keyword are allowed")
-    {
-        CHECK_FALSE(mgr.is_denied_path_keyword(flatpak_data_dir / "orca_plugins" / "state.json"));
-        CHECK(mgr.check_open(data_dir(), "r").allowed);
-        CHECK(mgr.check_open((flatpak_data_dir / "orca_plugins" / "test_plugin" / "state.json").string(), "r").allowed);
-        CHECK(mgr.check_open((flatpak_data_dir / "orca_plugins" / "test_plugin" / "state.json").string(), "w").allowed);
-        CHECK(mgr.check_open((flatpak_data_dir / "system" / "profiles" / "vendor.json").string(), "r").allowed);
-    }
-
-    SECTION("a 'secrets' component below data_dir() is still denied")
-    {
-        AuditDecision decision = mgr.check_open((flatpak_data_dir / "secrets" / "token.json").string(), "r");
-        CHECK_FALSE(decision.allowed);
-        CHECK(decision.reason == "denied path keyword");
-    }
-
-    SECTION("a 'config' component below data_dir() is still denied")
-    {
-        AuditDecision decision = mgr.check_open((flatpak_data_dir / "config" / "settings.json").string(), "r");
-        CHECK_FALSE(decision.allowed);
-        CHECK(decision.reason == "denied path keyword");
-    }
-
-    SECTION("the app config below data_dir() is still denied by name")
-    {
-        AuditDecision decision = mgr.check_open((flatpak_data_dir / (SLIC3R_APP_KEY ".conf")).string(), "r");
-        CHECK_FALSE(decision.allowed);
-        CHECK(decision.reason == "denied filename");
-    }
-
-    SECTION("an unrelated resources root keeps the full-path keyword check")
-    {
-        ScopedResourcesDir resources_dir_guard("plugin-audit-data-dir-keyword-resources");
-        mgr.add_global_allowed_root(resources_dir(), /*allow_write=*/false);
-
-        CHECK(mgr.check_open((fs::path(resources_dir()) / "profiles" / "vendor.json").string(), "r").allowed);
-
-        AuditDecision decision = mgr.check_open((fs::path(resources_dir()) / "cert" / "slicer_base64.cer").string(), "r");
-        CHECK_FALSE(decision.allowed);
-        CHECK(decision.reason == "denied path keyword");
-    }
-}
-
 TEST_CASE("Plugin audit is_denied_path combines the filename and keyword registries", "[audit]")
 {
     seed_denied_names();