betterway-tinyman / packer

3D Bin Packing with multiple Wrappers (Boxes)
BSD 3-Clause "New" or "Revised" License
123 stars 51 forks source link

Packer crashes if an instance is larger than the largest wrapper #2

Closed gwijnja closed 8 years ago

gwijnja commented 8 years ago

Steps to reproduce:

  1. Go to the packer demo website
  2. Add an instance with WxHxL = 1x1x2
  3. Add a wrapper with WxHxL = 1x1x1
  4. Press 'Pack'

Expected result: an error or warning informing you that one of the instances does not fit in any wrappers

Actual result: the website becomes kind of 'greyed out' and does not respond to any mouse clicks. No errors. The only way to continue is press F5 and start over again.

gwijnja commented 8 years ago

Hmm, actually it also crashes if the instance is 1x1x1 and the wrapper is 2x2x2.

samchon commented 8 years ago

I will find and fix the bug on in next weekend.

gwijnja commented 8 years ago

It gets stranger. If you use the sample.xml included in the demo (also the starting set if you visit the demo packer website), and change the Notebook height from 40 to 41, it crashes. But if you remove the Large box (40x40x15), the packing succeeds... while the notebook doesn't fit in any box. So yes there's a bug somewhere.

Thanks for looking into this issue samchon, I really like your packer!

samchon commented 8 years ago

If an Instance is greater than a Wrapper, then program (logic) doesn't pack it.

https://github.com/betterwaysystems/packer/blob/master/ts/src/bws/packer/Packer.ts#L112-L114 https://github.com/betterwaysystems/packer/blob/master/ts/src/bws/packer/WrapperGroup.ts#L154-L155 https://github.com/betterwaysystems/packer/blob/master/ts/src/bws/packer/Wrapper.ts#L285-L299

It's the problem of UI-part. When UI requests packing, it blocks screen (unable to click anything) and unblocks the screen when packing result is arrived. However, when an Instance is greater than any Wrapper, the packing result can be reached and the screen doesn't be unlocked.

It's the reason why the bug occurs. As the reason was detected, Bug on the Demo will be fixed soon. If you want to utilize the logic only, then you can use it now.

gwijnja commented 8 years ago

If an Instance is greater than a Wrapper, then program (logic) doesn't pack it.

But if the instance is 1x1x1 and the wrapper is 2x2x2, the screen is also not unlocked.

samchon commented 8 years ago

Without reference to the size of instance, only an instance will not packed.

I found reason of the bug. It also will be fixed soon. Thanks for your reporting.

Anyway, if you want to execute the packing on logic side, reference the code: https://github.com/betterwaysystems/packer/blob/master/ts/src/bws/packer/main.ts#L61-L85

gwijnja commented 8 years ago

Was looking through the code and also found this error, in Wrap.ts on line 202 the variable is named orietation and in the code of the function it is called orientation.

There are some more places where this may cause problems: https://github.com/betterwaysystems/packer/search?utf8=%E2%9C%93&q=orietation

I will also try to do some more testing and debugging next week. Not very familiar with TypeScript yet, but maybe I can make some pull requests if I find anything else.

samchon commented 8 years ago

The bugs are fixed.

Now, packing only an Instance is possible. When no Wrapper to pack, then program does not be freezed, but pop-ups an alert message.

However, when removes Notebook-PC from sample data, a new bug has occurred in 2nd wrapper. \o/