From c83f164ed543136e3e5aa2380fb66b04eaa3aa70 Mon Sep 17 00:00:00 2001 From: jalapnopuzzle <8386278+jalapenopuzzle@users.noreply.github.com> Date: Sun, 26 Jan 2025 13:18:47 +1100 Subject: [PATCH] #13420 GCodeWriter separate XYZ_EPSILON consistent with XYZF_EXPORT_DIGITS and add unit tests #13420 test_gcodewriter.cpp add tests for XYZF_EXPORT_DIGITS Check that movement GCodes are emitted / not emitted according to XYZF_EXPORT_DIGITS. The new tests demonstrate that XYZF_EXPORT_DIGITS is NOT respected. #13420 GCodeWriter separate XYZ_EPSILON consistent with XYZF_EXPORT_DIGITS The value of EPSILON=1e-4 (0.0001) in libslicer3r.h is inconsistent with XYZF_EXPORT_DIGITS=3 (0.001). This change addresses the inconsistency by introducing XYZ_EPSILON which is computed from XYZ_EXPORT_DIGITS, and changing all relevant GCodeWriter functions which used EPSILON to use XYZ_EPSILON instead. --- src/libslic3r/GCode/GCodeWriter.cpp | 6 +- src/libslic3r/GCode/GCodeWriter.hpp | 3 + tests/fff_print/test_gcodewriter.cpp | 241 +++++++++++++++++++++++++++ 3 files changed, 247 insertions(+), 3 deletions(-) diff --git a/src/libslic3r/GCode/GCodeWriter.cpp b/src/libslic3r/GCode/GCodeWriter.cpp index 4aa4389641..3b51c4daab 100644 --- a/src/libslic3r/GCode/GCodeWriter.cpp +++ b/src/libslic3r/GCode/GCodeWriter.cpp @@ -329,9 +329,9 @@ std::string GCodeWriter::travel_to_xy_G2G3IJ(const Vec2d &point, const Vec2d &ij std::string GCodeWriter::travel_to_xyz(const Vec3d &to, const std::string_view comment) { - if (std::abs(to.x() - m_pos.x()) < EPSILON && std::abs(to.y() - m_pos.y()) < EPSILON) { + if (std::abs(to.x() - m_pos.x()) < GCodeFormatter::XYZ_EPSILON && std::abs(to.y() - m_pos.y()) < GCodeFormatter::XYZ_EPSILON) { return this->travel_to_z(to.z(), comment); - } else if (std::abs(to.z() - m_pos.z()) < EPSILON) { + } else if (std::abs(to.z() - m_pos.z()) < GCodeFormatter::XYZ_EPSILON) { return this->travel_to_xy(to.head<2>(), comment); } else { std::string result{this->get_travel_to_xyz_gcode(to, comment)}; @@ -365,7 +365,7 @@ std::string GCodeWriter::get_travel_to_xyz_gcode(const Vec3d &to, const std::str std::string GCodeWriter::travel_to_z(double z, const std::string_view comment) { - if (std::abs(m_pos.z() - z) < EPSILON) { + if (std::abs(m_pos.z() - z) < GCodeFormatter::XYZ_EPSILON) { return ""; } else { m_pos.z() = z; diff --git a/src/libslic3r/GCode/GCodeWriter.hpp b/src/libslic3r/GCode/GCodeWriter.hpp index 47aedab680..48988d3d67 100644 --- a/src/libslic3r/GCode/GCodeWriter.hpp +++ b/src/libslic3r/GCode/GCodeWriter.hpp @@ -176,6 +176,9 @@ public: static constexpr const std::array pow_10 { 1., 10., 100., 1000., 10000., 100000., 1000000., 10000000., 100000000., 1000000000.}; static constexpr const std::array pow_10_inv{1./1., 1./10., 1./100., 1./1000., 1./10000., 1./100000., 1./1000000., 1./10000000., 1./100000000., 1./1000000000.}; + // Compute XYZ_EPSILON based on XYZF_EXPORT_DIGITS + static constexpr double XYZ_EPSILON = pow_10_inv[XYZF_EXPORT_DIGITS]; + // Quantize doubles to a resolution of the G-code. static double quantize(double v, size_t ndigits) { return std::round(v * pow_10[ndigits]) * pow_10_inv[ndigits]; } static double quantize_xyzf(double v) { return quantize(v, XYZF_EXPORT_DIGITS); } diff --git a/tests/fff_print/test_gcodewriter.cpp b/tests/fff_print/test_gcodewriter.cpp index bba09ea911..bc85c103fd 100644 --- a/tests/fff_print/test_gcodewriter.cpp +++ b/tests/fff_print/test_gcodewriter.cpp @@ -34,3 +34,244 @@ SCENARIO("set_speed emits values with fixed-point output.", "[GCodeWriter]") { } } } + +TEST_CASE("GCodeWriter emits G1 code correctly according to XYZF_EXPORT_DIGITS", "[GCodeWriter]") { + GCodeWriter writer; + + SECTION("Check quantize") { + CHECK(GCodeFormatter::quantize(1.0,0) == 1.); + CHECK(GCodeFormatter::quantize(0.0,0) == 0.); + CHECK(GCodeFormatter::quantize(0.1,0) == 0); + + CHECK(GCodeFormatter::quantize(1.0,1) == 1.); + CHECK(GCodeFormatter::quantize(0.0,1) == 0.); + CHECK(GCodeFormatter::quantize(0.1,1) == Approx(0.1)); + CHECK(GCodeFormatter::quantize(0.01,1) == 0.); + + CHECK(GCodeFormatter::quantize(1.0,2) == 1.); + CHECK(GCodeFormatter::quantize(0.0,2) == 0.); + CHECK(GCodeFormatter::quantize(0.1,2) == Approx(0.1)); + CHECK(GCodeFormatter::quantize(0.01,2) == Approx(0.01)); + CHECK(GCodeFormatter::quantize(0.001,2) == 0.); + + CHECK(GCodeFormatter::quantize(1.0,3) == 1.); + CHECK(GCodeFormatter::quantize(0.0,3) == 0.); + CHECK(GCodeFormatter::quantize(0.1,3) == Approx(0.1)); + CHECK(GCodeFormatter::quantize(0.01,3) == Approx(0.01)); + CHECK(GCodeFormatter::quantize(0.001,3) == Approx(0.001)); + CHECK(GCodeFormatter::quantize(0.0001,3) == 0.); + + CHECK(GCodeFormatter::quantize(1.0,4) == 1.); + CHECK(GCodeFormatter::quantize(0.0,4) == 0.); + CHECK(GCodeFormatter::quantize(0.1,4) == Approx(0.1)); + CHECK(GCodeFormatter::quantize(0.01,4) == Approx(0.01)); + CHECK(GCodeFormatter::quantize(0.001,4) == Approx(0.001)); + CHECK(GCodeFormatter::quantize(0.0001,4) == Approx(0.0001)); + CHECK(GCodeFormatter::quantize(0.00001,4) == 0.); + + CHECK(GCodeFormatter::quantize(1.0,5) == 1.); + CHECK(GCodeFormatter::quantize(0.0,5) == 0.); + CHECK(GCodeFormatter::quantize(0.1,5) == Approx(0.1)); + CHECK(GCodeFormatter::quantize(0.01,5) == Approx(0.01)); + CHECK(GCodeFormatter::quantize(0.001,5) == Approx(0.001)); + CHECK(GCodeFormatter::quantize(0.0001,5) == Approx(0.0001)); + CHECK(GCodeFormatter::quantize(0.00001,5) == Approx(0.00001)); + CHECK(GCodeFormatter::quantize(0.000001,5) == 0.); + + CHECK(GCodeFormatter::quantize(1.0,6) == 1.); + CHECK(GCodeFormatter::quantize(0.0,6) == 0.); + CHECK(GCodeFormatter::quantize(0.1,6) == Approx(0.1)); + CHECK(GCodeFormatter::quantize(0.01,6) == Approx(0.01)); + CHECK(GCodeFormatter::quantize(0.001,6) == Approx(0.001)); + CHECK(GCodeFormatter::quantize(0.0001,6) == Approx(0.0001)); + CHECK(GCodeFormatter::quantize(0.00001,6) == Approx(0.00001)); + CHECK(GCodeFormatter::quantize(0.000001,6) == Approx(0.000001)); + CHECK(GCodeFormatter::quantize(0.0000001,6) == 0.); + } + + SECTION("Check pow_10") { + // IEEE 754 floating point numbers can represent these numbers EXACTLY. + CHECK(GCodeFormatter::pow_10[0] == 1.); + CHECK(GCodeFormatter::pow_10[1] == 10.); + CHECK(GCodeFormatter::pow_10[2] == 100.); + CHECK(GCodeFormatter::pow_10[3] == 1000.); + CHECK(GCodeFormatter::pow_10[4] == 10000.); + CHECK(GCodeFormatter::pow_10[5] == 100000.); + CHECK(GCodeFormatter::pow_10[6] == 1000000.); + CHECK(GCodeFormatter::pow_10[7] == 10000000.); + CHECK(GCodeFormatter::pow_10[8] == 100000000.); + CHECK(GCodeFormatter::pow_10[9] == 1000000000.); + } + + SECTION("Check pow_10_inv") { + // IEEE 754 floating point numbers can NOT represent these numbers exactly. + CHECK(GCodeFormatter::pow_10_inv[0] == 1.); + CHECK(GCodeFormatter::pow_10_inv[1] == 0.1); + CHECK(GCodeFormatter::pow_10_inv[2] == 0.01); + CHECK(GCodeFormatter::pow_10_inv[3] == 0.001); + CHECK(GCodeFormatter::pow_10_inv[4] == 0.0001); + CHECK(GCodeFormatter::pow_10_inv[5] == 0.00001); + CHECK(GCodeFormatter::pow_10_inv[6] == 0.000001); + CHECK(GCodeFormatter::pow_10_inv[7] == 0.0000001); + CHECK(GCodeFormatter::pow_10_inv[8] == 0.00000001); + CHECK(GCodeFormatter::pow_10_inv[9] == 0.000000001); + } + + SECTION("travel_to_z Emit G1 code for very significant movement") { + double z1 = 10.0; + std::string result1{ writer.travel_to_z(z1) }; + CHECK(result1 == "G1 Z10 F7800\n"); + + double z2 = z1 * 2; + std::string result2{ writer.travel_to_z(z2) }; + CHECK(result2 == "G1 Z20 F7800\n"); + } + + SECTION("travel_to_z Emit G1 code for significant movement") { + double z1 = 10.0; + std::string result1{ writer.travel_to_z(z1) }; + CHECK(result1 == "G1 Z10 F7800\n"); + + // This should test with XYZ_EPSILON exactly, + // but IEEE 754 floating point numbers cannot pass the test. + double z2 = z1 + GCodeFormatter::XYZ_EPSILON * 1.001; + std::string result2{ writer.travel_to_z(z2) }; + + std::ostringstream oss; + oss << "G1 Z" + << GCodeFormatter::quantize_xyzf(z2) + << " F7800\n"; + + CHECK(result2 == oss.str()); + } + + SECTION("travel_to_z Do not emit G1 code for insignificant movement") { + double z1 = 10.0; + std::string result1{ writer.travel_to_z(z1) }; + CHECK(result1 == "G1 Z10 F7800\n"); + + // Movement smaller than XYZ_EPSILON + double z2 = z1 + (GCodeFormatter::XYZ_EPSILON * 0.999); + std::string result2{ writer.travel_to_z(z2) }; + CHECK(result2 == ""); + + double z3 = z1 + (GCodeFormatter::XYZ_EPSILON * 0.1); + std::string result3{ writer.travel_to_z(z3) }; + CHECK(result3 == ""); + } + + SECTION("travel_to_xyz Emit G1 code for very significant movement") { + Vec3d v1{10.0, 10.0, 10.0}; + std::string result1{ writer.travel_to_xyz(v1) }; + CHECK(result1 == "G1 X10 Y10 Z10 F7800\n"); + + Vec3d v2 = v1 * 2; + std::string result2{ writer.travel_to_xyz(v2) }; + CHECK(result2 == "G1 X20 Y20 Z20 F7800\n"); + } + + SECTION("travel_to_xyz Emit G1 code for significant XYZ movement") { + Vec3d v1{10.0, 10.0, 10.0}; + std::string result1{ writer.travel_to_xyz(v1) }; + CHECK(result1 == "G1 X10 Y10 Z10 F7800\n"); + + Vec3d v2 = v1; + // This should test with XYZ_EPSILON exactly, + // but IEEE 754 floating point numbers cannot pass the test. + v2.array() += GCodeFormatter::XYZ_EPSILON * 1.001; + std::string result2{ writer.travel_to_xyz(v2) }; + + std::ostringstream oss; + oss << "G1 X" + << GCodeFormatter::quantize_xyzf(v2.x()) + << " Y" + << GCodeFormatter::quantize_xyzf(v2.y()) + << " Z" + << GCodeFormatter::quantize_xyzf(v2.z()) + << " F7800\n"; + + CHECK(result2 == oss.str()); + } + + SECTION("travel_to_xyz Emit G1 code for significant X movement") { + Vec3d v1{10.0, 10.0, 10.0}; + std::string result1{ writer.travel_to_xyz(v1) }; + CHECK(result1 == "G1 X10 Y10 Z10 F7800\n"); + + Vec3d v2 = v1; + // This should test with XYZ_EPSILON exactly, + // but IEEE 754 floating point numbers cannot pass the test. + v2.x() += GCodeFormatter::XYZ_EPSILON * 1.001; + std::string result2{ writer.travel_to_xyz(v2) }; + + std::ostringstream oss; + // Only X needs to be emitted in this case, + // but this is how the code currently works. + oss << "G1 X" + << GCodeFormatter::quantize_xyzf(v2.x()) + << " Y" + << GCodeFormatter::quantize_xyzf(v2.y()) + << " F7800\n"; + + CHECK(result2 == oss.str()); + } + + SECTION("travel_to_xyz Emit G1 code for significant Y movement") { + Vec3d v1{10.0, 10.0, 10.0}; + std::string result1{ writer.travel_to_xyz(v1) }; + CHECK(result1 == "G1 X10 Y10 Z10 F7800\n"); + + Vec3d v2 = v1; + // This should test with XYZ_EPSILON exactly, + // but IEEE 754 floating point numbers cannot pass the test. + v2.y() += GCodeFormatter::XYZ_EPSILON * 1.001; + std::string result2{ writer.travel_to_xyz(v2) }; + + std::ostringstream oss; + // Only Y needs to be emitted in this case, + // but this is how the code currently works. + oss << "G1 X" + << GCodeFormatter::quantize_xyzf(v2.x()) + << " Y" + << GCodeFormatter::quantize_xyzf(v2.y()) + << " F7800\n"; + + CHECK(result2 == oss.str()); + } + + SECTION("travel_to_xyz Emit G1 code for significant Z movement") { + Vec3d v1{10.0, 10.0, 10.0}; + std::string result1{ writer.travel_to_xyz(v1) }; + CHECK(result1 == "G1 X10 Y10 Z10 F7800\n"); + + Vec3d v2 = v1; + // This should test with XYZ_EPSILON exactly, + // but IEEE 754 floating point numbers cannot pass the test. + v2.z() += GCodeFormatter::XYZ_EPSILON * 1.001; + std::string result2{ writer.travel_to_xyz(v2) }; + + std::ostringstream oss; + oss << "G1 Z" + << GCodeFormatter::quantize_xyzf(v2.z()) + << " F7800\n"; + + CHECK(result2 == oss.str()); + } + + SECTION("travel_to_xyz Do not emit G1 code for insignificant movement") { + Vec3d v1{10.0, 10.0, 10.0}; + std::string result1{ writer.travel_to_xyz(v1) }; + CHECK(result1 == "G1 X10 Y10 Z10 F7800\n"); + + // Movement smaller than XYZ_EPSILON + Vec3d v2 = v1; + v2.array() += GCodeFormatter::XYZ_EPSILON * 0.999; + std::string result2{ writer.travel_to_xyz(v2) }; + CHECK(result2 == ""); + + Vec3d v3 = v1; + v3.array() += GCodeFormatter::XYZ_EPSILON * 0.1; + std::string result3{ writer.travel_to_xyz(v3) }; + CHECK(result3 == ""); + } +} \ No newline at end of file