Merge pull request #500 from xyz-tools/codex/497-inch-units

Fix inch-unit conversion for coordinates and extrusion

Author: remcoder

src/__tests__/arc-tessellator.ts

@@ -4,9 +4,9 @@ import { ArcTessellator, ArcMove, ArcPoint } from '../arc-tessellator';
 describe('ArcTessellator', () => {
   const tessellator = new ArcTessellator();
 
-  function tessellate(start: ArcPoint, move: ArcMove, units: 'mm' | 'in' = 'mm'): ArcPoint[] {
+  function tessellate(start: ArcPoint, move: ArcMove): ArcPoint[] {
     const points: ArcPoint[] = [];
-    tessellator.tessellate(start, move, (x, y, z) => points.push({ x, y, z }), units);
+    tessellator.tessellate(start, move, (x, y, z) => points.push({ x, y, z }));
     return points;
   }
 
@@ -162,14 +162,4 @@ describe('ArcTessellator', () => {
 
     expect(points).toEqual([{ x: 10, y: 0, z: 0 }]);
   });
-
-  test('emits more segments for the same arc in inches', () => {
-    const start = { x: 1, y: 0, z: 0 };
-    const move = { cw: false, x: 0, y: 1, i: -1, j: 0 };
-
-    const mm = tessellate(start, move, 'mm');
-    const inches = tessellate(start, move, 'in');
-
-    expect(inches.length).toBeGreaterThan(mm.length);
-  });
 });

src/__tests__/interpreter/commands/set-units.ts

@@ -20,3 +20,98 @@ test('G21 sets the units to millimeters', () => {
 
   expect(job.state.units).toEqual('mm');
 });
+
+import { Parser } from '../../../parser/gcode-parser';
+import { Interpreter } from '../../../interpreter';
+
+function run(gcode: string) {
+  return new Interpreter().execute(new Parser().parseGCode(gcode).commands);
+}
+
+test.each(['M82', 'M83'])('inch and mm straight moves have identical geometry and filament length in %s', (mode) => {
+  const inch = run(`G20\n${mode}\nG28\nG0 X1 Y1 Z1\nG1 X2 E1\nG1 Y2 Z2 E${mode === 'M82' ? 2 : 1}`);
+  const mm = run(
+    `G21\n${mode}\nG28\nG0 X25.4 Y25.4 Z25.4\nG1 X50.8 E25.4\nG1 Y50.8 Z50.8 E${mode === 'M82' ? 50.8 : 25.4}`
+  );
+
+  expect(inch.paths.map((path) => ({ type: path.type, vertices: path.vertices }))).toEqual(
+    mm.paths.map((path) => ({ type: path.type, vertices: path.vertices }))
+  );
+  expect(inch.boundingBox).toEqual(mm.boundingBox);
+  expect(inch.stats.extrusionDistance).toBeCloseTo(mm.stats.extrusionDistance);
+  expect(inch.stats.extrusionDistance).toBeCloseTo(50.8);
+});
+
+test.each(['M82', 'M83'])('inch E-only retract and prime preserve geometry and subsequent extrusion in %s', (mode) => {
+  const start = `G20\n${mode}\nG28\nG1 X1 Y1 Z1 E2`;
+  const baseline = run(start);
+  const job = run(start);
+  const execute = (gcode: string) => new Interpreter().execute(new Parser().parseGCode(gcode).commands, job);
+
+  for (const [command, expectedE] of [
+    [mode === 'M82' ? 'G1 E1' : 'G1 E-1', 25.4],
+    [mode === 'M82' ? 'G1 E2' : 'G1 E1', 50.8]
+  ] as const) {
+    execute(command);
+    expect(job.state.e).toBeCloseTo(expectedE);
+    expect([job.state.x, job.state.y, job.state.z]).toEqual([25.4, 25.4, 25.4]);
+    expect(job.paths.map((path) => path.vertices)).toEqual(baseline.paths.map((path) => path.vertices));
+    expect(job.boundingBox).toEqual(baseline.boundingBox);
+    expect(job.stats.extrusionDistance).toBeCloseTo(50.8);
+  }
+
+  const nextMove = `G1 X2 E${mode === 'M82' ? 3 : 1}`;
+  execute(nextMove);
+  const uninterrupted = run(`${start}\n${nextMove}`);
+  expect(job.paths.map((path) => ({ type: path.type, vertices: path.vertices }))).toEqual(
+    uninterrupted.paths.map((path) => ({ type: path.type, vertices: path.vertices }))
+  );
+  expect(job.boundingBox).toEqual(uninterrupted.boundingBox);
+  expect(job.state.e).toBeCloseTo(76.2);
+  expect(job.stats.extrusionDistance).toBeCloseTo(76.2);
+});
+
+test.each(['M82', 'M83'])('normalizes linear moves, E-only moves and G92 in %s', (mode) => {
+  const job = run(`G20\n${mode}\nG28\nG1 X1 Y2 Z3 E1\nG1 E-1\nG92 X2 Y3 Z4 E2\nG1 X3 Y4 Z5 E3`);
+  expect(job.state.x).toBeCloseTo(50.8);
+  expect(job.state.y).toBeCloseTo(76.2);
+  expect(job.state.z).toBeCloseTo(101.6);
+  expect(job.state.e).toBeCloseTo(mode === 'M82' ? 76.2 : 127);
+});
+
+test('unit switches preserve stored coordinates and omitted axes', () => {
+  const job = run('G20\nG28\nG1 X1 Y2 Z3 E1\nG21\nG1 X50.8 E50.8\nG92 X0 E0\nG20\nG1 X1 E1');
+  expect(job.state.x).toBeCloseTo(76.2);
+  expect(job.state.y).toBeCloseTo(50.8);
+  expect(job.state.z).toBeCloseTo(76.2);
+  expect(job.stats.extrusionDistance).toBeCloseTo(76.2);
+  expect(job.state.e).toBeCloseTo(25.4);
+});
+
+test.each(['G2 X2 Y0 Z1 I1 J0 E2', 'G3 X2 Y0 Z1 R1 E2'])('inch and mm arcs have identical geometry: %s', (arc) => {
+  const inch = run(`G20\nG28\n${arc}`);
+  const mmArc = arc.replace(/([XYZIJRE])(-?\d+)/g, (_, word, value) => `${word}${Number(value) * 25.4}`);
+  const mm = run(`G21\nG28\n${mmArc}`);
+  expect(inch.paths.map((path) => path.vertices)).toEqual(mm.paths.map((path) => path.vertices));
+  expect(inch.boundingBox).toEqual(mm.boundingBox);
+  expect(inch.stats.extrusionDistance).toBeCloseTo(50.8);
+});
+
+test.each(['G31', 'G38.2', 'G38.3', 'G38.4', 'G38.5'])('normalizes %s targets and preserves Z0 contact', (probe) => {
+  const job = run(`G20\nG28\nG1 Z1\n${probe} X2 Y3 Z-1`);
+  expect(job.state.x).toBeCloseTo(50.8);
+  expect(job.state.y).toBeCloseTo(76.2);
+  expect(job.state.z).toBe(0);
+  const unknown = run(`G20\n${probe} Z-1`);
+  expect(unknown.state.z).toBeCloseTo(-25.4);
+});
+
+test('slicer dimensions remain millimeters in inch mode', () => {
+  const parser = new Parser();
+  const parsed = parser.parseGCode('; generated by PrusaSlicer\nG20\n;WIDTH:0.45\n;HEIGHT:0.2\nG28\nG1 X1 E1');
+  const job = new Job();
+  job.metadata = parser.metadata;
+  new Interpreter().execute(parsed.commands, job);
+  expect(job.state.extrusionWidth).toBe(0.45);
+  expect(job.state.lineHeight).toBe(0.2);
+});

src/arc-tessellator.ts

@@ -1,5 +1,3 @@
-import { Units, MM_PER_INCH } from './units';
-
 /** A point along a tessellated arc, in absolute G-code coordinates */
 export interface ArcPoint {
   x: number;
@@ -68,17 +66,15 @@ export class ArcTessellator {
   }
   /**
    * Converts an arc move into the points to draw, ending on the arc's endpoint
-   * @param start - Absolute position at the start of the arc
-   * @param move - Arc parameters from the G2/G3 command
+   * @param start - Absolute position in millimeters at the start of the arc
+   * @param move - Arc parameters normalized to millimeters
    * @param emit - Called once per point, in order, endpoint last. A callback
    * instead of a returned array so arc-heavy files do not allocate a throwaway
    * point object per segment.
-   * @param units - Current units; the chord tolerance is defined in
-   * millimeters, so inch-based arcs are tessellated proportionally finer
    * @returns The arc's exact endpoint (also the last point emitted). Emits at
    * least the endpoint, even for degenerate arcs.
    */
-  tessellate(start: ArcPoint, move: ArcMove, emit: EmitPoint, units: Units = 'mm'): ArcPoint {
+  tessellate(start: ArcPoint, move: ArcMove, emit: EmitPoint): ArcPoint {
     const { cw } = move;
     let { i, j, r } = move;
     // Omitted words are defaults, not "unset": G-code reads a missing I/J as a zero
@@ -150,8 +146,7 @@ export class ArcTessellator {
     // step would satisfy it and only the MAX_SEGMENT_ANGLE cap matters. A
     // non-finite radius flows through as NaN or a 0 step, making totalSegments
     // non-finite; the guard below the z handling skips the loop for those.
-    const radiusMm = units == 'in' ? arcRadius * MM_PER_INCH : arcRadius;
-    const maxStep = 2 * Math.acos(Math.max(1 - this.chordTolerance / radiusMm, -1));
+    const maxStep = 2 * Math.acos(Math.max(1 - this.chordTolerance / arcRadius, -1));
     const step = Math.min(maxStep, MAX_SEGMENT_ANGLE);
 
     let totalSegments = totalArc / step;

src/interpreter/commands/arc-move.ts

@@ -1,3 +1,4 @@
+import { toMillimeters } from '../../units';
 import { PathType } from '../../path';
 import { ArcTessellator, ArcTessellatorOptions } from '../../arc-tessellator';
 import type { CommandHandler } from '../../interpreter';
@@ -15,15 +16,19 @@ import type { CommandHandler } from '../../interpreter';
 export const makeArcMove = (options: ArcTessellatorOptions = {}): CommandHandler => {
   const arcTessellator = new ArcTessellator(options);
   return (command, job) => {
-    const { e, i, j, r } = command.params;
     const { state } = job;
+    const { units } = state;
+    const e = toMillimeters(command.params.e, units);
+    const i = toMillimeters(command.params.i, units);
+    const j = toMillimeters(command.params.j, units);
+    const r = toMillimeters(command.params.r, units);
     // The endpoint arrives in logical coordinates; translate it into physical
     // space up front so the tessellator's derived values agree with `from`.
     // I/J/R are relative distances and need no shift.
     const { positionShift } = state;
-    const x = command.params.x === undefined ? undefined : command.params.x + positionShift.x;
-    const y = command.params.y === undefined ? undefined : command.params.y + positionShift.y;
-    const z = command.params.z === undefined ? undefined : command.params.z + positionShift.z;
+    const x = command.params.x === undefined ? undefined : toMillimeters(command.params.x, units)! + positionShift.x;
+    const y = command.params.y === undefined ? undefined : toMillimeters(command.params.y, units)! + positionShift.y;
+    const z = command.params.z === undefined ? undefined : toMillimeters(command.params.z, units)! + positionShift.z;
     // Starting position for the arc, with any un-homed axis assumed at the origin.
     const from = job.resolvePosition();
 
@@ -45,17 +50,12 @@ export const makeArcMove = (options: ArcTessellatorOptions = {}): CommandHandler
     // The tessellator runs on the resolved position and emits every point,
     // ending with the exact endpoint -- which equals resolvePosition() after
     // the state update below, so no separate endpoint emission is needed.
-    arcTessellator.tessellate(
-      from,
-      { cw, x, y, z, i, j, r },
-      (px, py, pz) => {
-        currentPath.addPoint(px, py, pz);
-        if (pathType === PathType.Extrusion) {
-          job.boundingBox.update(px, py, pz);
-        }
-      },
-      state.units
-    );
+    arcTessellator.tessellate(from, { cw, x, y, z, i, j, r }, (px, py, pz) => {
+      currentPath.addPoint(px, py, pz);
+      if (pathType === PathType.Extrusion) {
+        job.boundingBox.update(px, py, pz);
+      }
+    });
 
     // `??` not `||`: an arc ending on X0, Y0 or Z0 used to silently keep the previous
     // coordinate. Safe now that the parser drops non-finite params -- `||` was also

src/interpreter/commands/linear-move.ts

@@ -1,3 +1,4 @@
+import { toMillimeters } from '../../units';
 import { PathType } from '../../path';
 import type { CommandHandler } from '../../interpreter';
 
@@ -11,8 +12,13 @@ import type { CommandHandler } from '../../interpreter';
  * G0 is for rapid moves (non-extrusion), G1 is for linear moves (with optional extrusion).
  */
 export const linearMove: CommandHandler = (command, job) => {
-  const { x, y, z, e, f } = command.params;
   const { state } = job;
+  const { units } = state;
+  const x = toMillimeters(command.params.x, units);
+  const y = toMillimeters(command.params.y, units);
+  const z = toMillimeters(command.params.z, units);
+  const e = toMillimeters(command.params.e, units);
+  const f = command.params.f;
 
   // discard zero length moves
   if (x === undefined && y === undefined && z === undefined) {

src/interpreter/commands/probe.ts

@@ -1,3 +1,4 @@
+import { toMillimeters } from '../../units';
 import { PathType } from '../../path';
 import type { CommandHandler } from '../../interpreter';
 
@@ -23,16 +24,17 @@ import type { CommandHandler } from '../../interpreter';
  */
 export const probe: CommandHandler = (command, job) => {
   const { state } = job;
+  const { units } = state;
   const { params } = command;
 
   if (params.p !== undefined) {
     return;
   }
 
   const { positionShift } = state;
-  const x = params.x === undefined ? undefined : params.x + positionShift.x;
-  const y = params.y === undefined ? undefined : params.y + positionShift.y;
-  let z = params.z === undefined ? undefined : params.z + positionShift.z;
+  const x = params.x === undefined ? undefined : toMillimeters(params.x, units)! + positionShift.x;
+  const y = params.y === undefined ? undefined : toMillimeters(params.y, units)! + positionShift.y;
+  let z = params.z === undefined ? undefined : toMillimeters(params.z, units)! + positionShift.z;
 
   if (x === undefined && y === undefined && z === undefined) {
     return;

src/interpreter/commands/set-position.ts

@@ -1,3 +1,4 @@
+import { toMillimeters } from '../../units';
 import type { CommandHandler } from '../../interpreter';
 
 /**
@@ -15,8 +16,12 @@ import type { CommandHandler } from '../../interpreter';
  * the axes.
  */
 export const setPosition: CommandHandler = (command, job) => {
-  const { x, y, z, e } = command.params;
   const { state } = job;
+  const { units } = state;
+  const x = toMillimeters(command.params.x, units);
+  const y = toMillimeters(command.params.y, units);
+  const z = toMillimeters(command.params.z, units);
+  const e = toMillimeters(command.params.e, units);
   const { positionShift } = state;
   const physical = job.resolvePosition();
 

src/state.ts

@@ -6,13 +6,13 @@ import { Units } from './units';
  * Tracks the current position, extrusion state, active tool, and units
  */
 export class State {
-  /** Current X position, or `undefined` until the axis is homed (G28) */
+  /** Current X position in millimeters, or `undefined` until the axis is homed (G28) */
   x: number | undefined = undefined;
-  /** Current Y position, or `undefined` until the axis is homed (G28) */
+  /** Current Y position in millimeters, or `undefined` until the axis is homed (G28) */
   y: number | undefined = undefined;
-  /** Current Z position, or `undefined` until the axis is homed (G28) */
+  /** Current Z position in millimeters, or `undefined` until the axis is homed (G28) */
   z: number | undefined = undefined;
-  /** Current extruder position, tracked by `applyExtrusion` and reset by G92 */
+  /** Current extruder position in millimeters, tracked by `applyExtrusion` and reset by G92 */
   e = 0;
   /**
    * Whether E parameters are relative distances (M83) rather than absolute

src/units.ts

@@ -3,3 +3,17 @@ export type Units = 'mm' | 'in';
 
 /** Millimeters per inch, for converting values from inch-based G-code */
 export const MM_PER_INCH = 25.4;
+
+/**
+ * Converts a command distance to millimeters
+ * @param value - A distance in the current units, or `undefined` when the command omits the word
+ * @param units - The units the value is expressed in
+ * @returns The distance in millimeters, or `undefined` for an omitted word
+ * @remarks
+ * Everything downstream of the interpreter works in millimeters; converting at
+ * the command boundary keeps inch files (G20) from leaking their units into
+ * the state, the paths or the rendered geometry.
+ */
+export function toMillimeters(value: number | undefined, units: Units): number | undefined {
+  return value === undefined ? undefined : value * (units === 'in' ? MM_PER_INCH : 1);
+}