juli/OrcaSlicer
1
1
Fork 0
mirror of https://github.com/OrcaSlicer/OrcaSlicer.git synced 2026-10-07 07:41:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

GCodeWriter: fix two machine-mapping bugs the extraction preserved

Browse Source 
Both change emitted G-code, which is why they were kept out of the extraction
commit. Both are wrong only where the machine mapping is non-identity, which is
the definition of each bug.

1. Suppress lifts commanded through an unknown position.

_travel_to_z() emits full XYZ whenever the mapping must emit every axis, because
the mapping can make machine Z depend on logical X/Y, and it builds that point
from m_pos. At print start, and after any custom G-code that invalidates
position, m_pos.xy is the uninitialised origin; mapping (0, 0, z) through a
non-identity remap produces a real but wrong machine point -- for a reverse
mapping, build_vol_max, i.e. the far corner of the bed. The subsequent full-XYZ
move corrects the position, but the lift has already commanded a rapid across
the whole bed at travel speed.

Belt kinematics already guarded this; the Cartesian path did not. The guard is
now applied at all three lift sites through must_skip_lift_now(), not just the
one the extraction covered: travel_to_xyz()'s pending-lift branch,
lazy_lift(spiral_vase=true), and eager_lift(). The latter two also needed the
state fix -- both recorded m_lifted = target_lift regardless, so suppressing
only the emission would leave a later unlift() descending from a height that was
never commanded.

2. Never emit a G2/G3 arc a mapping cannot represent.

extrude_arc_to_xy() emitted G2/G3 with logical X/Y and I/J and never consulted
the mapping. There is no general fix by transforming the arc: a permutation
moves it out of the XY plane that I/J describes, a negation reverses handedness,
and the belt shear maps a circle to an ellipse that G2/G3 cannot express at all.

So supports_arc_moves() gates generation through the existing
GCode::should_disable_arc_fitting() hook, and BeltGCode's special-case override
is deleted -- belt now gets the same behaviour from the general rule instead of
its own exception.

supports_arc_moves() is m_remap_x == 0 && m_remap_y == 1, not !has_axis_remap():
an arc emits only X/Y/I/J, so a mapping that merely negates or reverses Z leaves
every emitted word untouched and keeps its arcs.

The fallback for an unrepresentable arc tessellates it into linear segments at a
0.005mm chord tolerance rather than substituting a single chord, and splits dE
proportionally across the segments. The capability check is hoisted above every
extrusion mutation: an earlier form ran it after filament()->extrude(dE) and so
extruded 2*dE on the fallback path.

Known limits of that fallback, since it is worth stating rather than discovering:
emitted relative E is conserved only to per-segment rounding (a radius-5
semicircle with dE=1.5 emits 1.50012 across 36 segments); the 0.005mm bound is a
logical-frame bound, about 0.00855mm in machine space under a 45-degree belt
shear; unequal endpoint radii and non-finite inputs are unchecked. Ordinary
export takes the original polyline when the mapping rejects arcs, so this path
is a fallback rather than the normal route.

Known gap, not claimed fixed: classic wipe towers have their own
enable_arc_fitting and their own G2/G3 emitter in GCode/WipeTower.cpp, which
should_disable_arc_fitting() does not govern. Belt printers are barred from
classic wipe towers; a remapped Cartesian printer is not.

Tests in tests/fff_print/test_gcodewriter.cpp: reverse-X remap with unknown and
with known position plus an identity control; eager_lift emitting nothing and
recording nothing; the arc-capability matrix including the Z-only cases; and the
tessellated fallback. E accounting is asserted through used_filament() rather
than E(), which resets per line in relative-E mode.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011jgzj1sf53KMLPweZ8yeUQ
This commit is contained in:
harrierpigeon
2026-09-08 23:58:19 -05:00
co-authored by Claude Opus 5
parent e695da66df
commit 4d2c3a0af4
7 changed files with 404 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libslic3r/GCodeWriter.cpp
+71 -4
View File
@@ -25,6 +25,15 @@ namespace Slic3r {
bool GCodeWriter::full_gcode_comment = true;
// A lift emitted through _travel_to_z() re-emits the stored logical X/Y under a
// mapping that must emit every axis. While the position is unknown that X/Y is
// the uninitialised origin, which maps to a real but wrong machine point, so the
// lift has to be skipped rather than commanded.
bool GCodeWriter::must_skip_lift_now() const
{
return m_kinematics->suppress_lift_at_unknown_position() && ! this->is_current_position_clear();
}
bool GCodeWriter::point_on_first_layer(const Vec3d &point_logical) const
{
if (m_first_layer_plane && m_first_layer_plane->is_active())
@@ -837,6 +846,10 @@ std::string GCodeWriter::lazy_lift(LiftType lift_type, bool spiral_vase)
// BBS
if (m_lifted == 0 && m_to_lift == 0 && target_lift > 0) {
if (spiral_vase) {
if (this->must_skip_lift_now())
// Record no lift, so a later unlift() does not descend from a
// height that was never commanded.
return "";
m_lifted = target_lift;
return this->_travel_to_z(m_pos(2) + target_lift, "lift Z");
}
@@ -882,7 +895,12 @@ std::string GCodeWriter::eager_lift(const LiftType type) {
}
//BBS: if position is unknown use normal lift
else if (target_lift > 0) {
lift_move = _travel_to_z(m_pos(2) + target_lift, "normal lift Z");
if (this->must_skip_lift_now())
// Skipped, not deferred: leave m_lifted at zero below so unlift()
// does not descend from a height that was never commanded.
target_lift = 0.;
else
lift_move = _travel_to_z(m_pos(2) + target_lift, "normal lift Z");
}
m_lifted = target_lift;
m_to_lift = 0;
@@ -966,9 +984,7 @@ std::string GCodeWriter::travel_to_xyz(const Vec3d &point, const std::string &co
w0.emit_comment(GCodeWriter::full_gcode_comment, comment);
slop_move = w0.string();
}
else if (m_to_lift_type == LiftType::NormalLift &&
(! m_kinematics->suppress_lift_at_unknown_position() ||
this->is_current_position_clear())) {
else if (m_to_lift_type == LiftType::NormalLift && ! this->must_skip_lift_now()) {
// Only lift in place when the current position is known, for a mapping
// that makes _travel_to_z re-emit logical X/Y: at print start (and after
// custom gcode) m_pos.xy is still the uninitialised origin, which would
@@ -1220,11 +1236,62 @@ std::string GCodeWriter::extrude_to_xy(const Vec2d &point, double dE, const std:
return w.string();
}
// Approximate an arc with linear extrusions, for machine mappings that cannot
// express a G2/G3 (see extrude_arc_to_xy). center_offset is I/J: the centre
// relative to the CURRENT position, which is why this must run before m_pos is
// updated.
std::string GCodeWriter::extrude_arc_as_polyline(const Vec2d &point, const Vec2d &center_offset,
double dE, const bool is_ccw,
const std::string &comment, bool force_no_extrusion)
{
const Vec2d start = Vec2d(m_pos.x(), m_pos.y());
const Vec2d centre = start + center_offset;
const double r = (start - centre).norm();
if (r < EPSILON)
// Degenerate: no arc to speak of, so a single move is exact.
return this->extrude_to_xy(point, dE, comment, force_no_extrusion);
double a0 = std::atan2(start.y() - centre.y(), start.x() - centre.x());
double a1 = std::atan2(point.y() - centre.y(), point.x() - centre.x());
double sweep = a1 - a0;
if (is_ccw) { while (sweep <= 0.) sweep += 2. * PI; }
else { while (sweep >= 0.) sweep -= 2. * PI; }
// Segment count from a chord-deviation bound: r*(1-cos(dtheta/2)) <= tol.
const double tol = 0.005; // mm
const double dmax = (tol >= r) ? PI : 2. * std::acos(1. - tol / r);
const int n = std::max(2, int(std::ceil(std::abs(sweep) / std::max(dmax, EPSILON))));
std::string out;
for (int i = 1; i <= n; ++ i) {
const double a = a0 + sweep * (double(i) / double(n));
const Vec2d p = (i == n) ? point
: Vec2d(centre.x() + r * std::cos(a), centre.y() + r * std::sin(a));
out += this->extrude_to_xy(p, dE / double(n), i == n ? comment : std::string(), force_no_extrusion);
}
return out;
}
//BBS: generate G2 or G3 extrude which moves by arc
//point is end point which means X and Y axis
//center_offset is I and J axis
std::string GCodeWriter::extrude_arc_to_xy(const Vec2d& point, const Vec2d& center_offset, double dE, const bool is_ccw, const std::string& comment, bool force_no_extrusion)
{
// Arcs emit only X/Y/I/J, so a mapping that moves logical X or Y cannot be
// expressed as a G2/G3. GCode::should_disable_arc_fitting() normally stops
// arcs being generated at all for such a mapping, but this is public API, so
// define the behaviour rather than asserting.
//
// This check MUST precede every state mutation below: falling through to
// extrude_to_xy() after filament()->extrude(dE) would advance E twice.
//
// A single chord is not a safe substitute either -- a semicircle would become
// its diameter and a full circle a stationary blob -- so approximate the arc
// with linear segments bounded by a chord tolerance, splitting dE between
// them in proportion to arc length.
if (! m_kinematics->supports_arc_moves())
return this->extrude_arc_as_polyline(point, center_offset, dE, is_ccw, comment, force_no_extrusion);
m_pos(0) = point(0);
m_pos(1) = point(1);
if (!force_no_extrusion)