Bugs in edgy rect placement in updateBinSize() (#56)

* fixes both bugs in #55

* Change default square option from true to false and remove outdated comment

---------

Co-authored-by: Shen Yiming <soimy@163.com>

Author: JohannesHoffmann

src/maxrects-bin.ts

@@ -13,7 +13,7 @@ export class MaxRectsBin<T extends IRectangle = Rectangle> extends Bin<T> {
     public options: IOption = {
         smart: true,
         pot: true,
-        square: true,
+        square: false,
         allowRotation: false,
         tag: false,
         exclusiveTag: true,
@@ -327,11 +327,19 @@ export class MaxRectsBin<T extends IRectangle = Rectangle> extends Bin<T> {
         if (this.stage.contain(node)) return false;
         let tmpWidth: number = Math.max(this.width, node.x + node.width - this.padding + this.border);
         let tmpHeight: number = Math.max(this.height, node.y + node.height - this.padding + this.border);
+        let tmpFits: boolean = !(tmpWidth > this.maxWidth || tmpHeight > this.maxHeight);
         if (this.options.allowRotation) {
             // do extra test on rotated node whether it's a better choice
             const rotWidth: number = Math.max(this.width, node.x + node.height - this.padding + this.border);
             const rotHeight: number = Math.max(this.height, node.y + node.width - this.padding + this.border);
-            if (rotWidth * rotHeight < tmpWidth * tmpHeight) {
+            const rotFits: boolean = !(rotWidth > this.maxWidth || rotHeight > this.maxHeight);
+            // only compare when both rects will fit into bin
+            if (tmpFits && rotFits && rotWidth * rotHeight < tmpWidth * tmpHeight) {
+                tmpWidth = rotWidth;
+                tmpHeight = rotHeight;
+            }
+            // if rot fits and tmpFits not then do not compare area and set rot directly (some cases area of not rotated is smaller but will not fit)
+            if (rotFits && !tmpFits) {
                 tmpWidth = rotWidth;
                 tmpHeight = rotHeight;
             }
@@ -343,7 +351,8 @@ export class MaxRectsBin<T extends IRectangle = Rectangle> extends Bin<T> {
         if (this.options.square) {
             tmpWidth = tmpHeight = Math.max(tmpWidth, tmpHeight);
         }
-        if (tmpWidth > this.maxWidth + this.padding || tmpHeight > this.maxHeight + this.padding) {
+        tmpFits = !(tmpWidth > this.maxWidth || tmpHeight > this.maxHeight);
+        if (!tmpFits) {
             return false;
         }
         this.expandFreeRects(tmpWidth + this.padding, tmpHeight + this.padding);

test/maxrects-bin.spec.js

@@ -33,6 +33,15 @@ describe("no padding", () => {
         expect(position.y).toBe(0);
     });
 
+    test("edge case: only rotated version fits and should be set", () => {
+        const edgeCaseBin = new MaxRectsBin(256, 1024, 0, {allowRotation: true, pot: false});
+        edgeCaseBin.add(260, 80);
+        edgeCaseBin.add(260, 80);
+        edgeCaseBin.add(260, 80);
+        edgeCaseBin.add(260, 80);
+        expect(edgeCaseBin.rects).toHaveLength(4);
+    });
+
     test("report/set bin dirty status", () => {
         bin.add(200, 100, {});
         expect(bin.dirty).toBe(true); // add element to bin will render bin dirty
@@ -219,6 +228,18 @@ describe("padding", () => {
         expect(bin.rects.length).toBe(1);
     });
 
+    test("edge case: multiple rects with slightly bigger size then maxWidth should be placed rotated", () => {
+        const edgeCaseBin = new MaxRectsBin(256, 1024, padding, {allowRotation: true, pot: false, square: false, smart: true});
+        edgeCaseBin.add(260, 80);
+        edgeCaseBin.add(260, 80);
+        edgeCaseBin.add(260, 80);
+        edgeCaseBin.add(260, 80);
+
+        expect(edgeCaseBin.rects).toHaveLength(4);
+        expect(edgeCaseBin.rects[3].rot).toBeTruthy();
+        expect(edgeCaseBin.rects[3].width).toBe(80);
+    });
+
     test("monkey testing", () => {
         // bin = new MaxRectsBin(1024, 1024, 40);
         let rects = [];