keremdemirer / 3dbinpackingjs

javascript conversion of https://github.com/wknechtel/3d-bin-pack/
83 stars 26 forks source link

Overlapping boxes #3

Open luenib opened 7 years ago

luenib commented 7 years ago

I undocumented commented out line 1143 // layerthickness = layers[bestite].layerdim; and now I see the list of boxes and their location inside the container. the problems is that some boxes overlap.

e.g.

Box1: 3,3,2 Box2: 3,2,1 Box3: 3,1,1 Container: 3,3,3

The location of the boxes are: Box1: 0,0,0 Box2: 0,0,2 Box3: 0,0,2

As you can see, Box2 and Box3 are located in the same position.

Do you have an idea of what could be the problem?

Thanks

keremdemirer commented 7 years ago

Can you try with this commit? https://github.com/keremdemirer/3dbinpackingjs/commit/6536c0a0384f0b19c294892242808fe85f0a1a73

luenib commented 7 years ago

Thanks for the response. I ran the same test and this is what I got.

Box locations: Box1: 0,1,0 (rotated: 3,2,3) Box2: 0,0,0 (rotated: 3,1,2) Box3: 0,0,2 (3,1,1)

CORRECT!

I'll test with 9 more and come back with the results.

ninankara commented 7 years ago

Hi luenib,

how can you make this program running well? I download the solution and click the index.html to run the program, and I also insert the same test input as your, but mine did not work.

Can you help me? THank you.

luenib commented 7 years ago

ninankara, in order to see the positions of the boxes, you need to uncomment line 1143 of pack1.js

I ran 2 more tests but this time they failed. These are the numbers: TEST A)

     DIM      POS
B1 | 3 3 2 | 0 0 0 | 3 3 2
B2 | 3 2 1 | 0 0 2 | 2 3 1
B3 | 3 1 1 | 1 0 3 | 1 3 1
B4 | 2 1 1 | 0 0 3 | 1 2 1
B5 | 1 2 1 | 1 0 3 | 1 2 1
B6 | 2 2 1 | 0 0 2 | 2 2 1
B7 | 1 1 1 | 0 0 3 | 1 1 1

Finding 1) You'll notice that Box4 (dim: 2,1,1; pos: 0,0,3) shares position with Box7 (dim:1,1,1; pos: 0,0,3)

Finding 2) Box3 and Box5 also share the position in the container.

TEST B) I tried to stack 4 boxes dimension 2,2,2 in a 2,8,2 container. One can easily imagine a single column of boxes as a result but the code returns nothing.

keremdemirer commented 7 years ago

I made a new commit that reverts the changes made by last merge. This is the initial ported version of original repo.

Unfortunately I do not have time to contribute to this project. Always open to pull request though.

Can you please confirm this version has more close results?

ninankara commented 7 years ago

Hi All,

so i tried the current commit by cloning the project, and like luenib said, the result is wrong. Then I tried the other commit, and as he said , the result was different and might be correct. However, I tried to insert my case again from the original paper with the 6536c0a commit, and the result is different actually.

This is what I got (6536c0a commit): 1 40 52 36 0 0 0 40 52 36 2 40 52 36 0 0 36 40 52 36 3 40 52 36 0 0 0 40 52 36 4 70 104 24 0 0 0 70 104 24 5 70 104 24 0 0 24 70 104 24 6 70 104 24 0 0 48 70 104 24 7 70 104 24 0 0 72 70 104 24 8 14 104 48 0 0 0 48 104 14 9 14 104 48 0 0 14 48 104 14

This is the result from the current repo: 1 40 52 36 0 0 0 52 36 40 2 40 52 36 0 0 40 52 36 40 3 40 52 36 0 0 0 52 36 40 4 70 104 24 0 0 0 70 104 24 5 70 104 24 0 0 24 70 104 24 6 70 104 24 0 0 48 70 104 24 7 70 104 24 0 0 72 70 104 24 8 14 104 48 0 0 0 48 104 14 9 14 104 48 0 0 14 48 104 14

the result from paper is:

  1. 70 104 24 0 0 0 104 24 70
  2. 14 104 48 0 0 70 104 48 14
  3. 70 104 24 0 24 0 104 24 70
  4. 70 104 24 0 48 0 104 24 70
  5. 14 104 48 0 48 70 104 48 14
  6. 70 104 24 0 72 0 104 24 70

list of unpacked boxes 7 40 52 36 8 40 52 36 9 40 52 36

hmm i couldnt tell which one is better, but surely something need to be double checked and revised . 💃

luenib commented 7 years ago

I think I found it.

Inside the function expression findsmallestz, change the line while (!scrapmemb.pos == null) { with while (scrapmemb.pos != null) {

The problem with the original line is that !scrapmemb.pos returns FALSE, not NULL, so it will never go inside the while.

Also, comment out line 1143 of the original commit. That will let you see the coordinates of the boxes.

I modified other things while trying to find the bug, but I think the evaluation of scrapmemb.pos in findsmallestz is what hit the nail in the head.

I ran 5 tests and all came out good.

EDIT: Another thing: in line 973 replace strox = boxlist[cboxi].cox; with strcox = boxlist[cboxi].cox;

luenib commented 7 years ago

@ninankara @keremdemirer

Were you guys able to get better results with the modifications?

RichMie commented 1 year ago

Thanks for this correction! It works great!

keremdemirer commented 1 year ago

It would be great if someone create a PR ^^