Skip to content

Bugs in edgy rect placement in updateBinSize() #55

Open
@JohannesHoffmann

Description

Hi,

i found two edge cases where the library is not placing the rects as intended.

Unrotated rect will not fit, rotated will fit but it moves to a new bin.

This is a test for maxrects-bin.ts describing the edge case:

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);
});

The reason for this bug is here:

if (rotWidth * rotHeight < tmpWidth * tmpHeight) {
tmpWidth = rotWidth;
tmpHeight = rotHeight;
}
.
The comparison checks only the areas and not if a rect fits or not. The edge case is that the unrotated rect will not fit but has the smaller area then the rotated version which will fit.

With Padding, four rects only placeable rotated. First three rotate, fourth not. This is should be impossible.

This is a test for maxrects-bin.ts describing the edge case:

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);
    });

This bug occures only on the given options. The fourth rect is not placed rotated when width is bigger then maxWidth + padding

Here i am not sure why this happens. I guess that the reason is in here:

if (tmpWidth > this.maxWidth + this.padding || tmpHeight > this.maxHeight + this.padding) {
return false;
}

The tmpHeight and tmpWidth values are without padding compared to the maxWidth and maxHeight plus padding.

I will provide a PR with the fixes.

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions