GCodeWriter: extract MachineKinematics, delete BeltGCodeWriter

BeltGCodeWriter subclassed GCodeWriter and overrode seven methods, five of them
by copying the base body and changing the transform. The base writer already
carried an axis remap and already branched at each of its seven
coordinate-emission decisions; the subclass did the same branching with a
different transform, and the two copies had begun to drift.

Replace the inheritance with a strategy object owned by GCodeWriter:

  CartesianKinematics  to_machine = the existing apply_axis_remap; today's base
                       behaviour, moved rather than changed.
  BeltKinematics       to_machine = MachineFrameTransform o axis_remap o
                       BeltBackTransform, plus a world_coordinates variant for
                       the PA calibration generators.

New: src/libslic3r/GCode/MachineKinematics.{hpp,cpp}, GCode/BeltKinematics.{hpp,cpp}
Deleted: src/libslic3r/BeltGCodeWriter.{hpp,cpp} (341 lines)

Points worth a reviewer's attention:

  * The predicate is must_emit_all_axes(), not couples_axes(). The base returns
    true for any non-identity remap, including pure permutations that do not
    physically couple axes, so the question is "must every axis word be
    emitted", not a statement about kinematics.
  * Every per-site word-omission branch is preserved. The base deliberately
    emits X/Y only, or Z only, or drops Z when its quantised value is unchanged.
    The strategy changes which transform applies, never whether words are
    omitted.
  * set_kinematics() replays the configured remap and build volume onto a newly
    installed strategy, because BeltGCode::init_belt_writer runs before
    GCode.cpp calls set_axis_remap/set_build_volume_max.
  * uses_pointwise_travel_speed() preserves a pre-existing divergence rather
    than introducing one: the base travel_to_xyz emits the raw configured travel
    speed in its final branch, ignoring the first-layer value computed at the
    top, whereas the belt path used the first-layer-aware value throughout. Both
    are kept. Unifying them changes feedrates and belongs in its own change.
  * The [BELT-DEBUG] block is deleted; it rate-limited itself with a
    function-local static thread_local in the hot emission path, and this is the
    commit that would otherwise have moved it into shared code.

This commit is intended to preserve existing export output. That is reviewed by
construction -- each emission site keeps its own omission branch and each policy
divergence is preserved -- and is NOT verified against a G-code diff corpus.
Building that corpus is the outstanding work here.

Two API-equivalence exceptions, neither reachable by any caller today:

  * Belt kinematics with no plane pointer installed, m_is_first_layer true,
    initial and normal travel speeds differing, travel_to_xyz() reaching its
    final branch: the old belt writer selected the initial-layer speed, the new
    writer selects the normal travel speed. The pending-lift and XY-only
    branches keep their previous selection.
  * Belt kinematics installed without set_force_normal_lift(true) and a
    non-normal lift requested: the old belt writer forced a normal lift, the new
    writer can take the slope branch.

The PA-pattern generator reaches the writer through explicit travel_to_z() /
travel_to_xy(), not travel_to_xyz() or the lazy/eager lift paths, and normal
belt export installs both the plane and the forced-normal-lift policy, so
neither exception changes output produced today. They are recorded because a
future caller could reach them.

tests/fff_print/test_gcodewriter.cpp was also not compiling before this branch:
it called writer.to_machine_coords(), a method that existed only on
BeltGCodeWriter. It never surfaced because the build targets OrcaSlicer, not
all, and BUILD_TESTS defaults to OFF, so that translation unit was outside every
compile path. Fixed here; the existing 30-degree coordinate assertions are kept
verbatim as the best available regression net.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011jgzj1sf53KMLPweZ8yeUQ
This commit is contained in:
harrierpigeonandClaude Opus 5 committed 2026-09-08 23:57:55 -05:00
1 parent c96945490b
commit e695da66df
23 files changed
+398 -406

No files matched your search

+55 -5
View File
@@ -9,8 +9,12 @@
#include "Polygon.hpp"
#include "PrintConfig.hpp"
#include "GCode/CoolingBuffer.hpp"
#include "GCode/MachineKinematics.hpp"
#include <memory>
namespace Slic3r {
class FirstLayerPlane;
class GCodeWriter {
public:
virtual ~GCodeWriter() = default;
@@ -28,7 +32,8 @@ public:
m_lifted(0),
m_to_lift(0),
m_to_lift_type(LiftType::NormalLift),
m_current_speed(3600), m_is_first_layer(true)
m_current_speed(3600), m_is_first_layer(true),
m_kinematics(std::make_unique<CartesianKinematics>())
{}
Extruder* filament(size_t extruder_id) { assert(extruder_id < m_curr_filament_extruder.size()); return m_curr_filament_extruder[extruder_id]; }
const Extruder* filament(size_t extruder_id) const { assert(extruder_id < m_curr_filament_extruder.size()); return m_curr_filament_extruder[extruder_id]; }
@@ -142,10 +147,28 @@ public:
void set_build_volume_max(const Vec3d &max);
bool has_axis_remap() const;
// Install the machine frame mapping. Any axis remap / build volume already
// configured is carried over, so install order does not matter.
void set_kinematics(std::unique_ptr<MachineKinematics> kinematics);
const MachineKinematics& kinematics() const { return *m_kinematics; }
// First-layer plane evaluator. When set to an active plane, travel speed
// selection consults the plane per destination point instead of the
// layer-coarse m_is_first_layer flag. Borrowed pointer; lifetime is owned
// by GCode, which constructs the plane after the writer exists -- so this is
// deliberately a setter and not a constructor argument.
void set_first_layer_plane(const FirstLayerPlane *plane, double first_layer_height_mm)
{ m_first_layer_plane = plane; m_first_layer_thickness_mm = first_layer_height_mm; }
// Force every lift to a plain vertical lift. Spiral and slope lifts compute
// their slope in the logical frame and do not account for a machine mapping
// that couples axes.
void set_force_normal_lift(bool force) { m_force_normal_lift = force; }
// Returns whether this flavor supports separate print and travel acceleration.
static bool supports_separate_travel_acceleration(GCodeFlavor flavor);
protected:
// Position/lift/offset state — accessible to subclasses (e.g. BeltGCodeWriter)
// Position/lift/offset state.
Vec3d m_pos = Vec3d::Zero();
double m_x_offset{ 0 };
double m_y_offset{ 0 };
@@ -158,17 +181,44 @@ protected:
virtual std::string _travel_to_z(double z, const std::string &comment);
// Axis remap state — accessible to subclasses.
// Whether a destination gets first-layer treatment. With an active plane
// evaluator, distance from the plane decides; otherwise the layer-coarse
// m_is_first_layer flag does.
bool point_on_first_layer(const Vec3d &point_logical) const;
// True when travel speed is selected per destination point rather than per
// layer. Set for writers that install a first-layer plane. The historical
// path emits the raw configured travel speed in the final branch of
// travel_to_xyz(), ignoring the first-layer selection computed at the top of
// that function; a plane-driven writer uses the first-layer-aware value
// throughout. Both are preserved exactly -- unifying them would change
// emitted feedrates and belongs in its own commit.
bool uses_pointwise_travel_speed() const { return m_first_layer_plane != nullptr; }
// Borrowed; null = inactive.
const FirstLayerPlane *m_first_layer_plane = nullptr;
double m_first_layer_thickness_mm = 0.;
bool m_force_normal_lift = false;
// The machine frame mapping. Owns the axis-remap state that used to live
// here as m_remap_* / m_build_vol_max; the setters above forward to it.
// Never null: a CartesianKinematics at the identity remap reproduces the
// historical behaviour exactly.
std::unique_ptr<MachineKinematics> m_kinematics;
// Last configured remap / build volume, replayed onto a newly installed
// kinematics so set_kinematics() and the setters are order-independent.
int m_remap_x = 0; // RemapAxis: 0=+X, 1=+Y, 2=+Z, 3=-X, etc.
int m_remap_y = 1;
int m_remap_z = 2;
Vec3d m_build_vol_max = Vec3d::Zero();
// Apply axis remap to a point. Returns pos unchanged if remap is identity.
// Apply the machine frame mapping to a point. Returns pos unchanged when the
// mapping is the identity.
Vec3d apply_axis_remap(const Vec3d &pos) const;
// Motion uses the global/base process variant until a filament becomes active.
// Protected so BeltGCodeWriter indexes the per-extruder speed options (travel_speed,
// Protected so subclasses index the per-extruder speed options (travel_speed,
// travel_speed_z, initial_layer_travel_speed) exactly as the base writer does.
size_t m_cached_extruder_idx;