Merge pull request #306 from xyz-tools/fix/layer-logic

Update layer logic

Author: remcoder

src/__tests__/job.ts

@@ -11,7 +11,7 @@ test('it has an initial state', () => {
 });
 
 describe('.isPlanar', () => {
-  test('returns true if all extrusions are on the same plane', () => {
+  test('if all extrusions are on the same plane (Z=0)', () => {
     const job = new Job();
 
     append_path(job, PathType.Extrusion, [
@@ -23,22 +23,33 @@ describe('.isPlanar', () => {
       [5, 6, 0]
     ]);
 
-    expect(job.isPlanar()).toEqual(true);
+    expect(job.isPlanar).toEqual(true);
   });
 
-  test('returns false if any extrusions are on a different plane', () => {
+  test('if all extrusions are on the same plane (Z=1)', () => {
     const job = new Job();
 
     append_path(job, PathType.Extrusion, [
-      [0, 0, 0],
-      [1, 2, 0]
+      [0, 0, 1],
+      [1, 2, 1]
     ]);
+    append_path(job, PathType.Extrusion, [
+      [1, 2, 1],
+      [5, 6, 1]
+    ]);
+
+    expect(job.isPlanar).toEqual(true);
+  });
+
+  test('if any extrusion path has a Z value that exceeds the default tolerance', () => {
+    const job = new Job();
+
     append_path(job, PathType.Extrusion, [
       [1, 2, 0],
       [5, 6, 1]
     ]);
 
-    expect(job.isPlanar()).toEqual(false);
+    expect(job.isPlanar).toEqual(false);
   });
 
   test('ignores travel paths', () => {
@@ -58,12 +69,29 @@ describe('.isPlanar', () => {
       [5, 6, 0]
     ]);
 
-    expect(job.isPlanar()).toEqual(true);
+    expect(job.isPlanar).toEqual(true);
   });
 });
 
 describe('.layers', () => {
-  test('returns null if the job is not planar', () => {
+  test('returns empty list if no paths are present', () => {
+    const job = new Job();
+
+    expect(job.layers).toEqual([]);
+  });
+
+  test('returns empty list if no extrusion is present', () => {
+    const job = new Job();
+
+    append_path(job, PathType.Travel, [
+      [0, 0, 0],
+      [1, 2, 0]
+    ]);
+
+    expect(job.layers).toEqual([]);
+  });
+
+  test('returns empty list if the job is not planar', () => {
     const job = new Job();
 
     append_path(job, PathType.Extrusion, [
@@ -76,16 +104,17 @@ describe('.layers', () => {
     ]);
 
     expect(job.layers).toEqual([]);
+    expect(job.isPlanar).toEqual(false);
   });
 
-  test('paths without z changes are on the same layer', () => {
+  test('extrusions with same Z value are on the same layer', () => {
     const job = new Job();
 
     append_path(job, PathType.Extrusion, [
       [0, 0, 0],
       [1, 2, 0]
     ]);
-    append_path(job, PathType.Travel, [
+    append_path(job, PathType.Extrusion, [
       [5, 6, 0],
       [5, 6, 0]
     ]);
@@ -119,7 +148,7 @@ describe('.layers', () => {
     expect(layers[1].paths.length).toEqual(1);
   });
 
-  test('travel paths moving z under the default tolerance are on the same layer', () => {
+  test('travel with a z component is still on the same layer', () => {
     const job = new Job();
 
     append_path(job, PathType.Extrusion, [
@@ -128,7 +157,7 @@ describe('.layers', () => {
     ]);
     append_path(job, PathType.Travel, [
       [5, 6, 0],
-      [5, 6, LayersIndexer.DEFAULT_TOLERANCE - 0.01]
+      [5, 6, 42]
     ]);
 
     const layers = job.layers;
@@ -146,7 +175,7 @@ describe('.layers', () => {
       [0, 0, 0],
       [1, 2, 0]
     ]);
-    append_path(job, PathType.Travel, [
+    append_path(job, PathType.Extrusion, [
       [5, 6, 0],
       [5, 6, 0.09]
     ]);
@@ -190,6 +219,38 @@ describe('.layers', () => {
   test('extrusions after travels are on the same layer', () => {
     const job = new Job();
 
+    append_path(job, PathType.Extrusion, [
+      [0, 0, 0],
+      [1, 2, 0]
+    ]);
+    append_path(job, PathType.Travel, [
+      [5, 6, 0],
+      [5, 6, 2]
+    ]);
+    append_path(job, PathType.Travel, [
+      [5, 6, 2],
+      [5, 6, 0]
+    ]);
+    append_path(job, PathType.Travel, [
+      [5, 6, 0],
+      [5, 6, 2]
+    ]);
+    append_path(job, PathType.Extrusion, [
+      [5, 6, 0],
+      [5, 6, 0]
+    ]);
+
+    const layers = job.layers;
+
+    expect(layers).not.toBeNull();
+    expect(layers).toBeInstanceOf(Array);
+    expect(layers.length).toEqual(1);
+    expect(layers[0].paths.length).toEqual(5);
+  });
+
+  test('extrusions with a new Z value after travels are on a new layer', () => {
+    const job = new Job();
+
     append_path(job, PathType.Extrusion, [
       [0, 0, 0],
       [1, 2, 0]
@@ -220,7 +281,7 @@ describe('.layers', () => {
     expect(layers[1].paths.length).toEqual(1);
   });
 
-  test('initial travels are on the same layer as the first extrusion', () => {
+  test('travel paths before the first extrusion are not indexed', () => {
     const job = new Job();
 
     append_path(job, PathType.Travel, [
@@ -244,7 +305,61 @@ describe('.layers', () => {
 
     expect(layers).not.toBeNull();
     expect(layers.length).toEqual(1);
-    expect(layers[0].paths.length).toEqual(4);
+    expect(layers[0].paths.length).toEqual(1);
+  });
+
+  test('layer z must equal path z', () => {
+    const job = new Job();
+
+    append_path(job, PathType.Extrusion, [
+      [5, 6, 2],
+      [5, 6, 2]
+    ]);
+
+    const layers = job.layers;
+
+    expect(layers).not.toBeNull();
+    expect(layers.length).toEqual(1);
+    expect(layers[0].z).toEqual(2);
+  });
+
+  test('layer z must equal extrusion path z', () => {
+    const job = new Job();
+
+    append_path(job, PathType.Extrusion, [
+      [5, 6, 2],
+      [5, 6, 2]
+    ]);
+
+    append_path(job, PathType.Travel, [
+      [5, 6, 4],
+      [5, 6, 4]
+    ]);
+
+    const layers = job.layers;
+
+    expect(layers).not.toBeNull();
+    expect(layers.length).toEqual(1);
+    expect(layers[0].z).toEqual(2);
+  });
+
+  test('layer z must equal path z, for second layer', () => {
+    const job = new Job();
+
+    append_path(job, PathType.Extrusion, [
+      [5, 6, 2],
+      [5, 6, 2]
+    ]);
+    append_path(job, PathType.Extrusion, [
+      [5, 6, 4],
+      [5, 6, 4]
+    ]);
+
+    const layers = job.layers;
+
+    expect(layers).not.toBeNull();
+    expect(layers.length).toEqual(2);
+    expect(layers[1].z).toEqual(4);
   });
 });
 

src/indexers.ts

@@ -62,9 +62,9 @@ export class TravelTypeIndexer extends Indexer {
 /**
  * Error thrown when attempting to index a non-planar path
  */
-export class NonPlanarPathError extends NonApplicableIndexer {
+export class NonPlanarExtrusionError extends NonApplicableIndexer {
   constructor() {
-    super("Non-planar paths can't be indexed by layer");
+    super('Non-planar extrusions cannot be indexed by layer');
   }
 }
 
@@ -101,41 +101,40 @@ export class LayersIndexer extends Indexer {
       path.travelType === PathType.Extrusion &&
       path.vertices.some((_, i, arr) => i > 3 && i % 3 === 2 && Math.abs(arr[i] - arr[i - 3]) > this.tolerance)
     ) {
-      throw new NonPlanarPathError();
+      throw new NonPlanarExtrusionError();
     }
 
-    if (this.indexes[this.indexes.length - 1] === undefined) {
-      // Create the first layer at the current Z height (which is always 0 bc the gcode origin is at 0,0,0)
-      this.createLayer(0);
-    }
+    // new layers are only created when extruding
+    if (path.travelType === PathType.Extrusion) {
+      const newZ = path.vertices[2];
+      const lastZ = this.lastLayer?.z;
 
-    if (
-      path.travelType === PathType.Extrusion &&
-      this.lastLayer().paths.some((p) => p.travelType === PathType.Extrusion)
-    ) {
-      if (path.vertices[2] - (this.lastLayer().z || 0) > this.tolerance) {
-        this.createLayer(path.vertices[2]);
+      // either this is the first extrusion path
+      // or this is an extrusion path that is higher than the last layer
+      if (!this.lastLayer || newZ - lastZ > this.tolerance) {
+        this.createLayerAt(newZ);
       }
     }
-    this.lastLayer().paths.push(path);
+
+    this.lastLayer?.paths.push(path);
   }
 
   /**
    * Gets the last layer in the indexes
    * @returns The most recent layer
    */
-  private lastLayer(): Layer {
+  private get lastLayer(): Layer {
     return this.indexes[this.indexes.length - 1];
   }
 
   /**
    * Creates a new layer at the specified Z height
    * @param z - Z height for the new layer
    */
-  private createLayer(z: number): void {
-    const layerNumber = this.indexes.length;
-    const height = z - (this.lastLayer()?.z || 0);
-    this.indexes.push(new Layer(this.indexes.length, [], layerNumber, height, z));
+  private createLayerAt(z: number): void {
+    const lastZ = this.lastLayer?.z || 0;
+    const height = z - lastZ;
+    this.indexes.push(new Layer(z, height));
   }
 }
 

src/job.ts

@@ -1,7 +1,14 @@
 import { Path } from './path';
 import { State } from './state';
 import { Layer } from './layer';
-import { TravelTypeIndexer, LayersIndexer, ToolIndexer, Indexer, NonApplicableIndexer } from './indexers';
+import {
+  TravelTypeIndexer,
+  LayersIndexer,
+  ToolIndexer,
+  Indexer,
+  NonApplicableIndexer,
+  NonPlanarExtrusionError
+} from './indexers';
 import { BoundingBox } from './bounding-box';
 
 /**
@@ -135,10 +142,10 @@ export class Job {
   }
 
   /**
-   * Checks if the job contains planar layers
+   * Checks if the job contains planar extrusion layers
    * @returns True if the job contains at least one layer, false otherwise
    */
-  isPlanar(): boolean {
+  get isPlanar(): boolean {
     return this.layers.length > 0;
   }
 
@@ -159,9 +166,12 @@ export class Job {
           throw e; // If the error is not a NonApplicableIndexer, it will be thrown.
         }
 
-        console.warn('Non-planar path detected; clearing layer index');
-        this._layers = [];
+        if (e instanceof NonPlanarExtrusionError) {
+          console.warn('Non-planar path detected; clearing layer index');
+          this._layers = [];
+        }
 
+        // Remove the indexer that cannot handle this path
         const i = this.indexers.indexOf(indexer);
         this.indexers.splice(i, 1);
       }

src/layer.ts

@@ -6,30 +6,16 @@ import { Path } from './path';
  * Contains information about the layer number, paths, height, and Z position
  */
 export class Layer {
-  /** Layer number (0-based index) */
-  public layer: number;
   /** Array of paths in this layer */
-  public paths: Path[];
-  /** Line number in the G-code file where this layer starts */
-  public lineNumber: number;
-  /** Height of this layer */
-  public height: number = 0;
-  /** Z position of this layer */
-  public z: number = 0;
+  paths: Path[] = [];
 
   /**
    * Creates a new Layer instance
-   * @param layer - Layer number
-   * @param paths - Array of paths in this layer
-   * @param lineNumber - Line number in G-code file
    * @param height - Layer height (default: 0)
    * @param z - Z position (default: 0)
    */
-  constructor(layer: number, paths: Path[], lineNumber: number, height: number = 0, z: number = 0) {
-    this.layer = layer;
-    this.paths = paths;
-    this.lineNumber = lineNumber;
-    this.height = height;
-    this.z = z;
-  }
+  constructor(
+    public z: number,
+    public height: number = 0
+  ) {}
 }