Merge pull request #2 from usmanpakistan/55-Bugs-edgy-rectplacement-u…

…pdateBinSize fixes both bugs in soimy#55
usmanpakistan · Apr 26, 2024 · a0a51e6 · a0a51e6
2 parents 28805ab + 70ccf4b
commit a0a51e6
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 2 deletions.
diff --git a/src/maxrects-bin.ts b/src/maxrects-bin.ts
@@ -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);

diff --git a/test/maxrects-bin.spec.js b/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}); //why square in maxrects-packer false and in maxrects-bin: 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 = [];