search

fel88 / DeepNestPort

DeepNest C# Port
MIT License
78 stars 34 forks source link

Random gaps between items #12

Closed 9swampy closed 3 years ago

9swampy commented 3 years ago

Hi.

First of all, great work. I'd fought with the original DeepNest repo but just wasn't getting a successful build then found this port. C# being my preferred language the port's really helpful.

I've already forked (https://github.com/9swampy/DeepNestPort/tree/develop - hopefully you'll be happy to take a PR in due course), applied some core standards and added persisted configuration settings (PlacementType, PopulatonSize etc) but am having problems nesting complex shapes without leaving gaps.

If I use the Demo table and load simple rectangles then no gaps are left, as expected. If I load up actual complex shapes I'm getting random gaps between some items; some no gaps, some with gaps - when there's very clearly no need for any of the gaps at all.

Could you point me in the direction of what I should look in to to see where the gaps are coming from?

Cheers

fel88 commented 3 years ago

Hi, Could you please provide your input data (e.g. dxf files) so that I can reproduce your gaps problem A screenshot will also be useful

9swampy commented 3 years ago

With Debug rectangles everything's nice and tight: image

But with some real complex shapes I get random gaps in between: image

Run nesting on two of these parts and note during optimisation sometimes they nest tight but they ultimately settle on a nest with a gap per screenie above: SideWall.zip

9swampy commented 3 years ago

Actually I'm seeing the same behaviour just with some of the sample dxfs you've included in the repo. 4.dxf won't load at all; kills the application completely.

fel88 commented 3 years ago

4.dxf won't load at all; kills the application completely.

'closing threshold' in settings.xml was too small by default. 4.dxf should load fine now. image

I'll try to fix the gaps issue ASAP

9swampy commented 3 years ago

With the commit you've already done I'm seeing big improvements.

Load one of the 6.dxf and 6 of the 5.dxfs and nest. The 5's don't fill the space in the 6.

image

9swampy commented 3 years ago

...ooh and 4 loads now; can nest. The preview doesn't show on the Debug tab though.

9swampy commented 3 years ago

I've rebased on top of your commits today and pushed my develop branch. It does a bit of tidying up (nothing controversial but sorry, I'm a bit OCD on standards) and adds persisted settings and export to dxf (shamelessly plagiarised from @Daniel-t-1 but it seems to be working so far for my samples).

Would merge or consider a PullRequest?

fel88 commented 3 years ago

I think a better way is to create separate PRs for dxf export, persisted settings and tidying up. Because I'm not sure about stylecop stuff. Also I can add dxf export myself, no problem.

9swampy commented 3 years ago

Sure. The stylecop is just fixing most of the warnings that come up in your Visual Studio. Mostly with white space and standardised code layout. It can help a lot further with best practice coding which i didn't do because that would need a lot more care to make sure it doesn't break any logic. You really should consider adding the analysers: saves you a load of effort and time.

No problem though if you would rather hold back. Really appreciate what you've done already!

With peristed settings and dxf export it's working much better for me. Found the Simplify setting which has helped with concave/void filling a lot but doesn't seem quite right yet. If you have time to look in to it would be great. I've been playing with the rectangle with 1 wavy face to try understand why it doesn't nest two wavy sides together.

From: @. Sent: 17 June 2021 18:08 To: @. Reply to: @. Cc: @.; @.*** Subject: Re: [fel88/DeepNestPort] Random gaps between items (#12)

I think a better way is to create separate PRs for dxf export, persisted settings and tidying up. Because I'm not sure about stylecop stuff. Also I can add dxf export myself, no problem.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/fel88/DeepNestPort/issues/12#issuecomment-863412400, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAD7WLU7HVCY6BV3MUHGT5LTTITWFANCNFSM462WW4EQ.

[ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/fel88/DeepNestPort/issues/12#issuecomment-863412400", "url": "https://github.com/fel88/DeepNestPort/issues/12#issuecomment-863412400", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

fel88 commented 3 years ago

You really should consider adding the analysers: saves you a load of effort and time.

I'll try, but it is important to keep compatibility with the original code, That's why the code is such a mess right now.

Found the Simplify setting which has helped with concave/void filling a lot but doesn't seem quite right yet. If you have time to look in to it would be great. I've been playing with the rectangle with 1 wavy face to try understand why it doesn't nest two wavy sides together.

Simplify method does interesting stuff: image

It increases the area of the original polygon during simplification. I thing this is the reason for your issue with two wavy sides.

fel88 commented 3 years ago

Load one of the 6.dxf and 6 of the 5.dxfs and nest. The 5's don't fill the space in the 6.

My results:

Result 1 Result 2
image image
9swampy commented 3 years ago

Yeah, turning Simplify ON/OFF gets me to the same as you've screenied with the 6 5's and a 6 - but somethings not right. With Simplify ON parts are butting correctly up to the material edge and to each other, image

but with it OFF I still get the gap noted in the first screenie when a flat edge is against the material edge. That can't be right. image

Just 6 5's DeepNestPort comes back with: image

but the original DeepNest comes back with: image

Maybe not end of the world with these demo pieces but with my real world nest the impact is ~15% more material.

Appreciate the pointers and feedback. I'll keep looking in to it; and if you can shed any more light it would be most welcome.

9swampy commented 3 years ago

image

SvgNestConfig:CurveTolerance = 0.1; (down from 0.72)

A little more investigation to do but i've also got the original DepNest debuggable now too so i can step through side by side with your port; see where it deviates...

Got the original DeepNest down to: image

fel88 commented 3 years ago

A little more investigation to do but i've also got the original DepNest debuggable now too so i can step through side by side with your port; see where it deviates...

It would be great. Thank you for digging this up. Apparently, there is a bug somewhere in my code

9swampy commented 3 years ago

I've done quite a bit of work on this and have pushed code that'll improve the situation fir for my purposes. I appreciate you were reluctant to take on StyleCop type changes but the codes there if you want to have a look. I've cherry picked your commits over too.

image

image

image

image

image

This is all exposed through the UI so when the Curve Tolerance setting or Use Hull Approximation settings change the user can see the approximations that will get used in the nestings.

Note that almost invariably the approximation is much improved but still has less points than the original approximation; there is more inertia building the approximations before starting the nest but once the nest is off and running it's actually faster so you catch up pretty quickly.

Happy to close the issue; I have an answer. Thanks for the pointers.

9swampy commented 3 years ago

Ooh, forgot:

image

Can match the original DeepNest but still have to take Curve Tolerance lower than in the original so start up time is borderline unacceptable, but a very small compromise and it's back to little more than the kerf I'd get on the laser anyhow; can live with that :)

fel88 commented 3 years ago

I really like your idea about nfp clipping using a convex hull in order to reduce simplification artifacts. I've integrated it into my code as an option.

BTW, It's strange that you have too many points after simplifying in the picture above.

My results: image image

9swampy commented 3 years ago

Hmm... yeah. Maybe I took too early a commit to do the comparison. The simplifications may then be up a few points but still much improved. Saw you've cherry picked over; happy to have added value. Hope you consider breaking away from the legacy code base because it's getting really hard to merge. Have a look at the commit following on from the reverse cherry picks of your adaption of mine on yours. Putting ClipToHull on the UI wouldn't make sense in my code because of the caching mechanism I put in (which you didn't copy over) but the caching mechanism helped massively keeping the UI responsive.

fel88 commented 3 years ago

Hope you consider breaking away from the legacy code base because it's getting really hard to merge.

I think we shouldn't entangle our repositories too much

Putting ClipToHull on the UI wouldn't make sense in my code because of the caching mechanism I put in (which you didn't copy over) but the caching mechanism helped massively keeping the UI responsive.

Cache is good, but simpification contour draw is just auxiliary debug feature. In normal mode it doesn't affect UI response. Besides your cache based on HashCode.Combine which is not available in my framework.