fix(selection-strategies): fix issue where rectangle index was referr…

…ing to incorrect index, causing
tyschroed · Jan 27, 2020 · 79c255e · 79c255e
1 parent 4cbf3bd
commit 79c255e
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 113 deletions.
diff --git a/__tests__/guillotine-packer.ts b/__tests__/guillotine-packer.ts
@@ -37,39 +37,85 @@ Array [
 test('pack multiple bins', () => {
   const items = [
     {
-      width: 30,
-      height: 40
+      width: 4,
+      height: 3
     },
     {
-      width: 30,
-      height: 40
+      width: 4,
+      height: 3
+    },
+    {
+      width: 4,
+      height: 3
+    },
+    {
+      width: 4,
+      height: 3
+    },
+    {
+      width: 4,
+      height: 3
     }
   ]
 
-  const result = packer({
-    binHeight: 30,
-    binWidth: 40,
-    items
-  })
+  const result = packer(
+    {
+      binHeight: 4,
+      binWidth: 8,
+      items
+    },
+    {
+      kerfSize: 0,
+      sortStrategy: SortStrategy.Area,
+      splitStrategy: SplitStrategy.ShortAxisSplit,
+      selectionStrategy: SelectionStrategy.BEST_AREA_FIT
+    }
+  )
 
   expect(result).toMatchInlineSnapshot(`
 Array [
   Array [
     Object {
       "bin": 1,
-      "height": 30,
+      "height": 4,
       "item": Object {},
-      "width": 40,
+      "width": 3,
       "x": 0,
       "y": 0,
     },
+    Object {
+      "bin": 1,
+      "height": 4,
+      "item": Object {},
+      "width": 3,
+      "x": 3,
+      "y": 0,
+    },
   ],
   Array [
     Object {
       "bin": 2,
-      "height": 30,
+      "height": 4,
       "item": Object {},
-      "width": 40,
+      "width": 3,
+      "x": 0,
+      "y": 0,
+    },
+    Object {
+      "bin": 2,
+      "height": 4,
+      "item": Object {},
+      "width": 3,
+      "x": 3,
+      "y": 0,
+    },
+  ],
+  Array [
+    Object {
+      "bin": 3,
+      "height": 4,
+      "item": Object {},
+      "width": 3,
       "x": 0,
       "y": 0,
     },
@@ -97,7 +143,7 @@ test('should rotate items if it results in more efficent packing', () => {
       ]
     },
     {
-      kerfSize: 2,
+      kerfSize: 0,
       sortStrategy: SortStrategy.Area,
       splitStrategy: SplitStrategy.ShortAxisSplit,
       selectionStrategy: SelectionStrategy.BEST_AREA_FIT

diff --git a/__tests__/selection-strategies.ts b/__tests__/selection-strategies.ts
@@ -38,15 +38,12 @@ test('best area fit', () => {
 
   expect(selected).toMatchInlineSnapshot(`
 Object {
-  "index": 1,
-  "selectedRectangle": Object {
-    "bin": 0,
-    "height": 30,
-    "id": "2",
-    "width": 30,
-    "x": 0,
-    "y": 0,
-  },
+  "bin": 0,
+  "height": 30,
+  "id": "2",
+  "width": 30,
+  "x": 0,
+  "y": 0,
 }
 `)
 })
@@ -57,15 +54,12 @@ test('best long side fit', () => {
 
   expect(selected).toMatchInlineSnapshot(`
 Object {
-  "index": 1,
-  "selectedRectangle": Object {
-    "bin": 0,
-    "height": 30,
-    "id": "2",
-    "width": 30,
-    "x": 0,
-    "y": 0,
-  },
+  "bin": 0,
+  "height": 30,
+  "id": "2",
+  "width": 30,
+  "x": 0,
+  "y": 0,
 }
 `)
 })
@@ -102,15 +96,12 @@ test('best short side fit', () => {
 
   expect(selected).toMatchInlineSnapshot(`
 Object {
-  "index": 2,
-  "selectedRectangle": Object {
-    "bin": 0,
-    "height": 200,
-    "id": "3",
-    "width": 10,
-    "x": 0,
-    "y": 0,
-  },
+  "bin": 0,
+  "height": 200,
+  "id": "3",
+  "width": 10,
+  "x": 0,
+  "y": 0,
 }
 `)
 })
diff --git a/src/pack-strategy.ts b/src/pack-strategy.ts
@@ -1,5 +1,5 @@
 import debugLib from 'debug'
-const debug = debugLib('guillotine-packer')
+const debug = debugLib('guillotine-packer:pack-strategy')
 import { Rectangle, Item } from './types'
 import { SelectionStrategy, GetSelectionImplementation } from './selection-strategies'
 import { SortDirection, SortStrategy, GetSortImplementation } from './sort-strategies'
@@ -44,8 +44,8 @@ export function PackStrategy({
   const freeRectangles: Rectangle[] = []
 
   const createBin = () => {
-    debug(`creating bin ${binCount}`)
     binCount++
+    debug(`creating bin ${binCount}`)
     freeRectangles.push({
       width: binWidth,
       height: binHeight,
@@ -66,7 +66,7 @@ export function PackStrategy({
   }
 
   const splitRectangle = ({ rectangle, item }: { rectangle: Rectangle; item: Item }) => {
-    return splitter.split(rectangle, item).filter(r => r.width !== 0 && r.height !== 0)
+    return splitter.split(rectangle, item).filter(r => r.width > 0 && r.height > 0)
   }
 
   const getSelectionOption = (item: Item) => {
@@ -75,7 +75,7 @@ export function PackStrategy({
     if (!rectangle) {
       return null
     }
-    const splitRectangles = splitRectangle({ rectangle: rectangle.selectedRectangle, item })
+    const splitRectangles = splitRectangle({ rectangle: rectangle, item })
     return {
       rectangle,
       splitRectangles,
@@ -136,12 +136,14 @@ export function PackStrategy({
         item: otherItemProps,
         width,
         height,
-        x: rectangle.selectedRectangle.x,
-        y: rectangle.selectedRectangle.y,
-        bin: rectangle.selectedRectangle.bin
+        x: rectangle.x,
+        y: rectangle.y,
+        bin: rectangle.bin
       }
       debug('packed item', packedItem)
-      freeRectangles.splice(rectangle.index, 1, ...splitRectangles)
+      debug('free rectangles pre split', freeRectangles)
+      const rectIndex = freeRectangles.findIndex(r => r === rectangle)
+      freeRectangles.splice(rectIndex, 1, ...splitRectangles)
       debug('free rectangles post split', freeRectangles)
       return packedItem
     })

diff --git a/src/selection-strategies.ts b/src/selection-strategies.ts
@@ -1,85 +1,43 @@
 import { Rectangle, Item } from './types'
 
-type SelectionResult = {
-  selectedRectangle: Rectangle
-  index: number
-}
-
 export enum SelectionStrategy {
   BEST_SHORT_SIDE_FIT,
   BEST_LONG_SIDE_FIT,
   BEST_AREA_FIT
 }
 
 abstract class SelectionImplementation {
-  abstract selectFromAvailable(
-    freeRectangles: Rectangle[],
-    itemToPlace: Item
-  ): SelectionResult | null
-  select(freeRectangles: Rectangle[], itemToPlace: Item) {
-    return this.selectFromAvailable(
-      freeRectangles.filter(
+  abstract generateSortValue(freeRectangle: Rectangle, itemToPlace: Item): number
+  select(freeRectangles: Rectangle[], itemToPlace: Item): Rectangle | null {
+    const [bestRect] = freeRectangles
+      .filter(
         freeRect =>
           freeRect.width - itemToPlace.width >= 0 && freeRect.height - itemToPlace.height >= 0
-      ),
-      itemToPlace
-    )
+      )
+      .map(r => ({ rectangle: r, sortValue: this.generateSortValue(r, itemToPlace) }))
+      .sort((a, b) => (a.sortValue > b.sortValue ? 1 : -1))
+
+    return bestRect ? bestRect.rectangle : null
   }
 }
 
 class BestShortSideFit extends SelectionImplementation {
-  selectFromAvailable(freeRectangles: Rectangle[], itemToPlace: Item) {
+  generateSortValue(freeRectangle: Rectangle, itemToPlace: Item) {
     const { width, height } = itemToPlace
-    const [rect] = freeRectangles
-      .map((freeRect, rectIndex) => ({
-        shortSideLeftover: Math.min(freeRect.width - width, freeRect.height - height),
-        rectangleIndex: rectIndex
-      }))
-      .sort((a, b) => (a.shortSideLeftover > b.shortSideLeftover ? 1 : -1))
-    if (!rect) {
-      return null
-    }
-    return {
-      selectedRectangle: freeRectangles[rect.rectangleIndex],
-      index: rect.rectangleIndex
-    }
+    return Math.min(freeRectangle.width - width, freeRectangle.height - height)
   }
 }
 
 class BestLongSideFit extends SelectionImplementation {
-  selectFromAvailable(freeRectangles: Rectangle[], itemToPlace: Item) {
+  generateSortValue(freeRectangle: Rectangle, itemToPlace: Item) {
     const { width, height } = itemToPlace
-    const [rect] = freeRectangles
-      .map((freeRect, rectIndex) => ({
-        longSideLeftover: Math.max(freeRect.width - width, freeRect.height - height),
-        rectangleIndex: rectIndex
-      }))
-      .sort((a, b) => (a.longSideLeftover > b.longSideLeftover ? 1 : -1))
-    if (!rect) {
-      return null
-    }
-    return {
-      selectedRectangle: freeRectangles[rect.rectangleIndex],
-      index: rect.rectangleIndex
-    }
+    return Math.max(freeRectangle.width - width, freeRectangle.height - height)
   }
 }
 
 class BestAreaFit extends SelectionImplementation {
-  selectFromAvailable(freeRectangles: Rectangle[], itemToPlace: Item) {
-    const [rect] = freeRectangles
-      .map((freeRect, rectIndex) => ({
-        area: freeRect.width * freeRect.height,
-        rectangleIndex: rectIndex
-      }))
-      .sort((a, b) => (a.area > b.area ? 1 : -1))
-    if (!rect) {
-      return null
-    }
-    return {
-      selectedRectangle: freeRectangles[rect.rectangleIndex],
-      index: rect.rectangleIndex
-    }
+  generateSortValue(freeRectangle: Rectangle) {
+    return freeRectangle.width * freeRectangle.height
   }
 }
 

diff --git a/src/split-strategies.ts b/src/split-strategies.ts
@@ -1,49 +1,49 @@
 import debugLib from 'debug'
-const debug = debugLib('guillotine-packer')
+const debug = debugLib('guillotine-packer:split-strategies')
 import { Rectangle, Item } from './types'
 
 // implementations based off of http://pds25.egloos.com/pds/201504/21/98/RectangleBinPack.pdf
 
+const createSplitRectangle = (rectangle: Rectangle) => ({ ...rectangle, splitFrom: rectangle.id })
+
 abstract class Splitter {
   constructor(public kerfSize: number) {}
   abstract split(rectangle: Rectangle, item: Item): Rectangle[]
   protected splitHorizontally(rectangle: Rectangle, item: Item): Rectangle[] {
     debug(`splitting ${rectangle.id} horizontally`)
     const rectangle1 = {
-      ...this._split(rectangle),
+      ...createSplitRectangle(rectangle),
       x: rectangle.x + item.width + this.kerfSize,
       width: rectangle.width - item.width - this.kerfSize,
       height: item.height,
       id: 'sh-r1'
     }
     const rectangle2 = {
-      ...this._split(rectangle),
+      ...createSplitRectangle(rectangle),
       y: rectangle.y + item.height + this.kerfSize,
       height: rectangle.height - item.height - this.kerfSize,
       id: 'sh-r2'
     }
-    debug('horizontal rectangles', [rectangle1, rectangle2])
     return [rectangle1, rectangle2]
   }
   protected splitVertically(rectangle: Rectangle, item: Item): Rectangle[] {
     debug(`splitting ${rectangle.id} vertically`)
     const rectangle1 = {
-      ...this._split(rectangle),
+      ...createSplitRectangle(rectangle),
       y: rectangle.y + item.height + this.kerfSize,
       width: item.width,
       height: rectangle.height - item.height - this.kerfSize,
       id: 'sh-r1'
     }
     const rectangle2 = {
-      ...this._split(rectangle),
+      ...createSplitRectangle(rectangle),
       x: rectangle.x + item.width + this.kerfSize,
       y: rectangle.y,
       width: rectangle.width - item.width - this.kerfSize,
       id: 'sh-r2'
     }
     return [rectangle1, rectangle2]
   }
-  private _split = (rectangle: Rectangle) => ({ ...rectangle, splitFrom: rectangle.id })
 }
 
 class ShortAxisSplit extends Splitter {