From 06583b935fde3c8480cbf51d7b707a4b3f475c00 Mon Sep 17 00:00:00 2001 From: highpower Date: Sun, 11 Oct 2026 20:55:16 -0700 Subject: [PATCH] Respect machine Z speed limits in generated travel Cap pure-Z and combined XYZ travel, retain physical Z through deferred layer changes, and preserve hop height above the new layer. Add writer and exported-G-code regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/libslic3r/GCode.cpp | 4 +- src/libslic3r/GCodeWriter.cpp | 62 +++++---- src/libslic3r/GCodeWriter.hpp | 3 + src/libslic3r/PrintConfig.cpp | 2 +- tests/fff_print/test_gcodewriter.cpp | 181 +++++++++++++++++++++++++++ tests/fff_print/test_printgcode.cpp | 32 +++++ 6 files changed, 259 insertions(+), 25 deletions(-) diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index c76591879be1..0a0617c3a052 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -7280,8 +7280,8 @@ std::string GCode::change_layer(coordf_t print_z) m_need_change_layer_lift_z = true; - m_nominal_z = z; - m_writer.get_position().z() = z; + // The writer must retain the actual Z until the deferred layer-change travel is emitted. + m_nominal_z = z; // forget last wiping path as wiping after raising Z is pointless // BBS. Dont forget wiping path to reduce stringing. diff --git a/src/libslic3r/GCodeWriter.cpp b/src/libslic3r/GCodeWriter.cpp index 6c23f8d0149e..8fe11d62b380 100644 --- a/src/libslic3r/GCodeWriter.cpp +++ b/src/libslic3r/GCodeWriter.cpp @@ -39,6 +39,7 @@ void GCodeWriter::apply_print_config(const PrintConfig &print_config) }; m_max_jerk_z = print_config.machine_max_jerk_z.values.front(); m_max_jerk_e = print_config.machine_max_jerk_e.values.front(); + m_max_speed_z = print_config.machine_max_speed_z.values.front(); } void GCodeWriter::set_extruders(std::vector extruder_ids) @@ -500,6 +501,28 @@ double GCodeWriter::active_travel_speed(bool first_layer_aware) const return speed; } +double GCodeWriter::z_travel_speed() const +{ + double speed = this->config.travel_speed_z.value; + if (speed == 0.) + speed = this->active_travel_speed(true); + return m_max_speed_z > 0. ? std::min(speed, m_max_speed_z) : speed; +} + +double GCodeWriter::xyz_travel_speed(const Vec3d &from, const Vec3d &to, double speed) const +{ + Vec3d delta; + for (int axis = 0; axis < 3; ++axis) + delta(axis) = GCodeFormatter::quantize_xyzf(to(axis)) - GCodeFormatter::quantize_xyzf(from(axis)); + if (m_max_speed_z > 0. && delta.z() != 0.) { + const double max_speed = m_max_speed_z * delta.norm() / std::abs(delta.z()); + // Round the feedrate down so formatting cannot raise the Z component above the limit. + const double feedrate_scale = 60. * GCodeFormatter::pow_10[GCodeFormatter::XYZF_EXPORT_DIGITS]; + speed = std::min(speed, std::floor(max_speed * feedrate_scale) / feedrate_scale); + } + return speed; +} + std::string GCodeWriter::travel_to_xy(const Vec2d &point, const std::string &comment) { m_pos(0) = point(0); @@ -520,28 +543,26 @@ std::string GCodeWriter::travel_to_xy(const Vec2d &point, const std::string &com std::string GCodeWriter::travel_to_xyz(const Vec3d &point, const std::string &comment, bool force_z) { - // FIXME: This function was not being used when travel_speed_z was separated (bd6badf). - // Calculation of feedrate was not updated accordingly. If you want to use - // this function, fix it first. - //std::terminate(); - /* If target Z is lower than current Z but higher than nominal Z we don't perform the Z move but we only move in the XY plane and adjust the nominal Z by reducing the lift amount that will be used for unlift. */ // BBS Vec3d dest_point = point; - auto travel_speed = this->active_travel_speed(true); + const bool only_z = to_2d(dest_point) == to_2d(m_pos); + auto travel_speed = only_z ? this->z_travel_speed() : this->active_travel_speed(true); //BBS: a z_hop need to be handle when travel if (std::abs(m_to_lift) > EPSILON) { assert(std::abs(m_lifted) < EPSILON); + // A deferred layer change lifts relative to the new layer, not the previous physical Z. + const double lifted_z = m_to_lift + (force_z ? point(2) : m_pos(2)); //BBS: don't need to do real lift if the current position is absolutely same with target. //This ususally happens when the last extrusion line is short and the end of wipe position //is same with the traget point by chance. if ((!this->is_current_position_clear() || m_pos != dest_point) && - m_to_lift + m_pos(2) > point(2)) { - m_lifted = m_to_lift + m_pos(2) - point(2); - dest_point(2) = m_to_lift + m_pos(2); + lifted_z > point(2)) { + m_lifted = lifted_z - point(2); + dest_point(2) = lifted_z; } m_to_lift = 0.; @@ -561,6 +582,7 @@ std::string GCodeWriter::travel_to_xyz(const Vec3d &point, const std::string &co Vec2d ij_offset = radius * delta_no_z.normalized(); ij_offset = { -ij_offset(1), ij_offset(0) }; slop_move = this->_spiral_travel_to_z(target(2), ij_offset, "spiral lift Z"); + source(2) = target(2); } //BBS: LazyLift else if (m_to_lift_type == LiftType::LazyLift && @@ -573,13 +595,15 @@ std::string GCodeWriter::travel_to_xyz(const Vec3d &point, const std::string &co Vec3d slope_top_point = Vec3d(temp(0), temp(1), delta(2)) + source; GCodeG1Formatter w0; w0.emit_xyz(slope_top_point); - w0.emit_f(travel_speed * 60.0); + w0.emit_f(this->xyz_travel_speed(source, slope_top_point, travel_speed) * 60.0); //BBS w0.emit_comment(GCodeWriter::full_gcode_comment, comment); slop_move = w0.string(); + source = slope_top_point; } else if (m_to_lift_type == LiftType::NormalLift) { slop_move = _travel_to_z(target.z(), "normal lift Z"); + source(2) = target(2); } } @@ -588,7 +612,7 @@ std::string GCodeWriter::travel_to_xyz(const Vec3d &point, const std::string &co GCodeG1Formatter w0; if (this->is_current_position_clear()) { w0.emit_xyz(target); - w0.emit_f(travel_speed * 60.0); + w0.emit_f(this->xyz_travel_speed(source, target, travel_speed) * 60.0); w0.emit_comment(GCodeWriter::full_gcode_comment, comment); xy_z_move = w0.string(); } @@ -634,7 +658,9 @@ std::string GCodeWriter::travel_to_xyz(const Vec3d &point, const std::string &co } else { GCodeG1Formatter w; w.emit_xyz(point_on_plate); - w.emit_f(this->active_travel_speed() * 60.0); + const Vec3d source = { m_pos(0) - m_x_offset, m_pos(1) - m_y_offset, m_pos(2) }; + const double speed = only_z ? this->z_travel_speed() : this->active_travel_speed(); + w.emit_f(this->xyz_travel_speed(source, point_on_plate, speed) * 60.0); w.emit_comment(GCodeWriter::full_gcode_comment, comment); out_string = w.string(); } @@ -667,13 +693,9 @@ std::string GCodeWriter::_travel_to_z(double z, const std::string &comment) { m_pos(2) = z; - double speed = this->config.travel_speed_z.value; - if (speed == 0.) - speed = this->active_travel_speed(true); - GCodeG1Formatter w; w.emit_z(z); - w.emit_f(speed * 60.0); + w.emit_f(this->z_travel_speed() * 60.0); //BBS w.emit_comment(GCodeWriter::full_gcode_comment, comment); return w.string(); @@ -683,16 +705,12 @@ std::string GCodeWriter::_spiral_travel_to_z(double z, const Vec2d &ij_offset, c { m_pos(2) = z; - double speed = this->config.travel_speed_z.value; - if (speed == 0.) - speed = this->active_travel_speed(true); - std::string output = "G17\n"; GCodeG2G3Formatter w(true); w.emit_z(z); w.emit_ij(ij_offset); w.emit_string(" P1 "); - w.emit_f(speed * 60.0); + w.emit_f(this->z_travel_speed() * 60.0); w.emit_comment(GCodeWriter::full_gcode_comment, comment); return output + w.string(); } diff --git a/src/libslic3r/GCodeWriter.hpp b/src/libslic3r/GCodeWriter.hpp index 9fcb4bf9af99..3495c4a526a6 100644 --- a/src/libslic3r/GCodeWriter.hpp +++ b/src/libslic3r/GCodeWriter.hpp @@ -143,6 +143,7 @@ class GCodeWriter { double m_max_jerk_z; double m_max_jerk_e; double m_max_junction_deviation; + double m_max_speed_z = 0.; unsigned int m_travel_acceleration; unsigned int m_travel_jerk; @@ -179,6 +180,8 @@ class GCodeWriter { Print }; + double z_travel_speed() const; + double xyz_travel_speed(const Vec3d &from, const Vec3d &to, double speed) const; std::string _travel_to_z(double z, const std::string &comment); std::string _spiral_travel_to_z(double z, const Vec2d &ij_offset, const std::string &comment); std::string _retract(double length, double restart_extra, const std::string &comment); diff --git a/src/libslic3r/PrintConfig.cpp b/src/libslic3r/PrintConfig.cpp index d41f4c0d8288..49cd9e2b5db0 100644 --- a/src/libslic3r/PrintConfig.cpp +++ b/src/libslic3r/PrintConfig.cpp @@ -6109,7 +6109,7 @@ void PrintConfigDef::init_fff_params() //def->label = L("Z travel"); //def->tooltip = L("Speed of vertical travel along z axis. " // "This is typically lower because build plate or gantry is hard to be moved. " - // "Zero means using travel speed directly in G-code, but will be limited by printer's ability when run G-code."); + // "Zero uses the active travel speed. Z travel is capped by a positive machine_max_speed_z."); def->sidetext = "mm/s"; // milimeters per second, don't need translation def->min = 0; def->mode = comDevelop; diff --git a/tests/fff_print/test_gcodewriter.cpp b/tests/fff_print/test_gcodewriter.cpp index eb9641c81788..b710df5bfbab 100644 --- a/tests/fff_print/test_gcodewriter.cpp +++ b/tests/fff_print/test_gcodewriter.cpp @@ -3,10 +3,191 @@ #include +#include "libslic3r/GCodeReader.hpp" #include "libslic3r/GCodeWriter.hpp" using namespace Slic3r; +TEST_CASE("Z travel respects the machine speed limit", "[GCodeWriter]") { + struct SpeedCase { + double travel_speed; + double travel_speed_z; + double max_speed_z; + std::string feedrate; + }; + const SpeedCase cases[] = { + {100., 0., 40., " F2400"}, + {30., 0., 40., " F1800"}, + {100., 70., 40., " F2400"}, + {100., 20., 40., " F1200"}, + {100., 0., 0., " F6000"}, + }; + + for (const SpeedCase &speed_case : cases) { + for (bool emit_limits : {false, true}) { + CAPTURE(speed_case.travel_speed, speed_case.travel_speed_z, speed_case.max_speed_z, emit_limits); + PrintConfig config = static_cast(FullPrintConfig::defaults()); + config.travel_speed.values = {speed_case.travel_speed}; + config.travel_speed_z.value = speed_case.travel_speed_z; + config.machine_max_speed_z.values = {speed_case.max_speed_z, 5.}; + config.emit_machine_limits_to_gcode.value = emit_limits; + config.z_hop.values = {1.5}; + config.retract_lift_above.values = {0.}; + config.retract_lift_below.values = {0.}; + + GCodeWriter writer; + writer.apply_print_config(config); + writer.set_is_first_layer(false); + writer.set_extruders({0}); + writer.set_extruder(0); + + REQUIRE(writer.travel_to_z(10.) == "G1 Z10" + speed_case.feedrate + "\n"); + REQUIRE(writer.travel_to_z(9.) == "G1 Z9" + speed_case.feedrate + "\n"); + + writer.set_current_position_clear(true); + writer.lift(); + const std::string hop = writer.travel_to_xyz(Vec3d(10., 0., 9.)); + REQUIRE(hop.find("G1 Z10.5" + speed_case.feedrate) != std::string::npos); + REQUIRE(hop.find("X10 Y0 Z10.5 F" + std::to_string(int(speed_case.travel_speed * 60.))) != std::string::npos); + REQUIRE(writer.unlift().find("G1 Z9" + speed_case.feedrate) == 0); + + writer.lift(); + REQUIRE(writer.travel_to_xyz(Vec3d(10., 0., 9.2)).find("G1 X10 Y0 Z10.5" + speed_case.feedrate) == 0); + REQUIRE(writer.unlift().find("G1 Z9.2" + speed_case.feedrate) == 0); + writer.travel_to_z(9.); + + REQUIRE(writer.lift(LiftType::NormalLift, true).find("G1 Z10.5" + speed_case.feedrate) == 0); + REQUIRE(writer.unlift().find("G1 Z9" + speed_case.feedrate) == 0); + + writer.lift(LiftType::SpiralLift); + const std::string spiral = writer.travel_to_xyz(Vec3d(20., 0., 9.)); + REQUIRE(spiral.find("G17\nG3") == 0); + REQUIRE(spiral.find(speed_case.feedrate) != std::string::npos); + REQUIRE(writer.unlift().find("G1 Z9" + speed_case.feedrate) == 0); + + REQUIRE(writer.travel_to_xyz(Vec3d(20., 0., 8.)) == "G1 X20 Y0 Z8" + speed_case.feedrate + "\n"); + writer.set_current_position_clear(false); + REQUIRE(writer.travel_to_xyz(Vec3d(30., 0., 7.)).find("G1 Z7" + speed_case.feedrate) != std::string::npos); + } + } +} + +TEST_CASE("Z travel preserves first-layer speed and refreshes the machine limit", "[GCodeWriter]") { + PrintConfig config = static_cast(FullPrintConfig::defaults()); + config.travel_speed.values = {100.}; + config.travel_speed_z.value = 0.; + config.machine_max_speed_z.values = {40.}; + config.initial_layer_travel_speed.values.front().value = 25.; + config.initial_layer_travel_speed.values.front().percent = true; + + GCodeWriter writer; + writer.apply_print_config(config); + REQUIRE(writer.travel_to_z(1.) == "G1 Z1 F1500\n"); + writer.set_is_first_layer(false); + REQUIRE(writer.travel_to_z(2.) == "G1 Z2 F2400\n"); + + config.machine_max_speed_z.values = {10.}; + writer.apply_print_config(config); + REQUIRE(writer.travel_to_z(3.) == "G1 Z3 F600\n"); + REQUIRE(writer.travel_to_xy(Vec2d(10., 0.)) == "G1 X10 Y0 F6000\n"); +} + +TEST_CASE("Combined XYZ travel limits its Z velocity without slowing shallow moves", "[GCodeWriter]") { + PrintConfig config = static_cast(FullPrintConfig::defaults()); + config.travel_speed.values = {100.}; + config.travel_speed_z.value = 0.; + config.machine_max_speed_z.values = {40.}; + GCodeWriter writer; + writer.apply_print_config(config); + writer.set_is_first_layer(false); + writer.set_current_position_clear(true); + + REQUIRE(writer.travel_to_xyz(Vec3d(10., 0., 10.)) == "G1 X10 Y0 Z10 F3394.112\n"); + REQUIRE(writer.travel_to_xyz(Vec3d(20., 0., 0.)) == "G1 X20 Y0 Z0 F3394.112\n"); + REQUIRE(writer.travel_to_xyz(Vec3d(120., 0., 1.)) == "G1 X120 Y0 Z1 F6000\n"); + REQUIRE(writer.travel_to_xyz(Vec3d(220., 0., 1.)) == "G1 X220 Y0 F6000\n"); + + config.machine_max_speed_z.values = {0.}; + writer.apply_print_config(config); + REQUIRE(writer.travel_to_xyz(Vec3d(230., 0., 11.)) == "G1 X230 Y0 Z11 F6000\n"); + + config.machine_max_speed_z.values = {40.}; + writer.apply_print_config(config); + writer.set_position(Vec3d(0.00049, 0., 0.00049)); + GCodeReader reader; + reader.parse_buffer(writer.travel_to_xyz(Vec3d(1.00049, 0., 1.00051)), + [](GCodeReader &reader, const GCodeReader::GCodeLine &line) { + const double dz = std::abs(line.dist_Z(reader)); + const double distance = std::hypot(line.dist_XY(reader), dz); + REQUIRE(line.f() / 60. * dz / distance <= 40. + 0.0001); + }); +} + +TEST_CASE("Each Z-hop segment respects the Z velocity limit", "[GCodeWriter]") { + for (LiftType type : {LiftType::NormalLift, LiftType::LazyLift, LiftType::SpiralLift}) { + for (double distance : {0.5, 10., 200.}) { + CAPTURE(type, distance); + PrintConfig config = static_cast(FullPrintConfig::defaults()); + config.travel_speed.values = {100.}; + config.travel_speed_z.value = 0.; + config.machine_max_speed_z.values = {40.}; + config.z_hop.values = {1.5}; + config.travel_slope.values = {45.}; + config.retract_lift_above.values = {0.}; + config.retract_lift_below.values = {0.}; + GCodeWriter writer; + writer.apply_print_config(config); + writer.set_is_first_layer(false); + writer.set_extruders({0}); + writer.set_extruder(0); + writer.set_current_position_clear(true); + std::string gcode = writer.travel_to_xyz(Vec3d(0., 0., 10.)); + writer.lift(type); + gcode += writer.travel_to_xyz(Vec3d(distance, distance, 10.)); + gcode += writer.unlift(); + + int z_moves = 0; + GCodeReader reader; + reader.parse_buffer(gcode, [&z_moves](GCodeReader &reader, const GCodeReader::GCodeLine &line) { + if (line.cmd_is("G1") && line.dist_Z(reader) != 0.) { + ++z_moves; + const double dz = std::abs(line.dist_Z(reader)); + const double distance = std::hypot(line.dist_XY(reader), dz); + REQUIRE(line.f() / 60. * dz / distance <= 40. + 0.0001); + } + }); + REQUIRE(z_moves >= 2); + if (type == LiftType::NormalLift || distance > 10.) + REQUIRE(gcode.find(" F6000") != std::string::npos); + } + } +} + +TEST_CASE("Deferred layer changes preserve the hop height and cap the physical Z move", "[GCodeWriter]") { + PrintConfig config = static_cast(FullPrintConfig::defaults()); + config.travel_speed.values = {100.}; + config.machine_max_speed_z.values = {40.}; + config.travel_speed_z.value = 0.; + config.z_hop.values = {0.4}; + config.retract_lift_above.values = {0.}; + config.retract_lift_below.values = {0.}; + GCodeWriter writer; + writer.apply_print_config(config); + writer.set_is_first_layer(false); + writer.set_extruders({0}); + writer.set_extruder(0); + writer.set_position(Vec3d(167.804, 167.985, 0.3)); + writer.set_current_position_clear(true); + REQUIRE(writer.travel_to_xyz(Vec3d(167.921, 167.804, 0.5), "", true) == + "G1 X167.921 Y167.804 Z.5 F3528.285\n"); + + writer.set_position(Vec3d(167.804, 167.985, 0.3)); + writer.lift(); + REQUIRE(writer.travel_to_xyz(Vec3d(167.921, 167.804, 0.5), "", true).find("G1 Z.9 F2400") == 0); + REQUIRE(std::abs(writer.get_zhop() - 0.4) < 0.000001); + REQUIRE(writer.unlift().find("G1 Z.5 F2400") == 0); +} + SCENARIO("lift() is not ignored after unlift() at normal values of Z", "[GCodeWriter]") { GIVEN("A config with z-hop and a single extruder.") { GCodeWriter writer; diff --git a/tests/fff_print/test_printgcode.cpp b/tests/fff_print/test_printgcode.cpp index b9ea949464a8..2e4b3faf2d4d 100644 --- a/tests/fff_print/test_printgcode.cpp +++ b/tests/fff_print/test_printgcode.cpp @@ -16,6 +16,38 @@ std::regex perimeters_regex("G1 X[-0-9.]* Y[-0-9.]* E[-0-9.]* ; perimeter"); std::regex infill_regex("G1 X[-0-9.]* Y[-0-9.]* E[-0-9.]* ; infill"); std::regex skirt_regex("G1 X[-0-9.]* Y[-0-9.]* E[-0-9.]* ; skirt"); +TEST_CASE("Exported layer-change travel respects the Z speed limit", "[PrintGCode][ZTravel]") { + Print print; + Model model; + init_print({TestMesh::cube_20x20x20}, print, model, { + {"layer_height", 0.2}, + {"initial_layer_print_height", 0.3}, + {"travel_speed", 100.}, + {"travel_speed_z", 0.}, + {"machine_max_speed_z", 0.1}, + {"z_hop", 0.}, + {"wipe", true}, + {"retract_when_changing_layer", false}, + {"enable_arc_fitting", false}, + {"machine_start_gcode", ""}, + {"machine_end_gcode", ""} + }); + const std::string output = gcode(print); + int z_travels = 0; + GCodeReader reader; + reader.apply_config(print.config()); + reader.parse_buffer(output, [&z_travels](GCodeReader &reader, const GCodeReader::GCodeLine &line) { + if (line.cmd_is("G1") && line.travel() && line.dist_Z(reader) != 0.) { + ++z_travels; + const double dz = std::abs(line.dist_Z(reader)); + const double distance = std::hypot(line.dist_XY(reader), dz); + CAPTURE(line.raw(), reader.z()); + REQUIRE(line.new_F(reader) / 60. * dz / distance <= 0.10001); + } + }); + REQUIRE(z_travels > 1); +} + SCENARIO( "PrintGCode basic functionality", "[PrintGCode]") { GIVEN("A default configuration and a print test object") { WHEN("the output is executed with no support material") {