Separate logic to complete each island from "avoid crossing perimeters" #1907

Closed
kefir- opened this Issue Apr 4, 2014 · 15 comments

Projects

None yet

5 participants

@kefir-
kefir- commented Apr 4, 2014

As discussed in issue #278, printing of perimeters and infill in this order:

perimeter->next island->perimeter->first island->infill->next island->infill

is suboptimal and probably never a requirement, whereas

perimeter->infill->next island->perimeter->infill 

would be a better approach. I don't see any use cases where the first approach is superior.

The option "avoid crossing perimeters" was introduced, and when this option is enabled, the optimal ordering happens. But this option does more than that, and can also introduce other issues in some cases, like potentially much longer moves that can result in ooze.

Since "avoid crossing perimeters" seems unlikely to become slic3r's default behaviour without an option to disable it, I'd like to suggest that slic3r should always complete an island before moving to the next one, even when this option is disabled. For me the option is currently a workaround that solves one issue but sometimes introduces other ones. Solving this ordering appears to be a generic fix to optimizing moves and print ordering anyway.

@whosawhatsis

+1

@alexrj
Owner
alexrj commented Apr 6, 2014

I agree with you. People will complain, but still I think it's a good change.

@alexrj alexrj added this to the 1.1.1 milestone Apr 6, 2014
@luke321
luke321 commented Apr 7, 2014

+1 when using avoid crossing perimeters + only retrect when crossing perimeters the nozzle can crash when moving a long distance over infill and often the extruder oozes into the infill which leads to gaps when starting the next printer section after the travel move.

so finishing island after island withouth the need of using avoid crossing perimeters would be really nice!

@ledvinap
Collaborator
ledvinap commented Apr 7, 2014

Maybe decreasing CROSSING_PENALTY (https://github.com/alexrj/Slic3r/blob/master/lib/Slic3r/GCode/MotionPlanner.pm#L24) will work as expected. With low CROSSING_PENALTY planner will still try to avoid perimeters if good path is close and it will use right angle (aproximately) to cross them.
Currently penalty region is 3mm (https://github.com/alexrj/Slic3r/blob/master/lib/Slic3r/GCode/MotionPlanner.pm#L14) and crossing penalty is 20, so planer will use path that is up to 60mm longer.

Any test file around?

@alexrj
Owner
alexrj commented Apr 18, 2014

@ledvinap, not sure about your comment.. @kefir- is referring to https://github.com/alexrj/Slic3r/blob/6f3844c1ba87610a29356e91081811c35d815ddc/lib/Slic3r/GCode/Layer.pm#L149 whose behavior should be the default regardless of avoid_crossing_perimeters

@alexrj alexrj added a commit that referenced this issue Apr 19, 2014
@alexrj Always do one island at time instead of doing that only when avoid_cr…
…ossing_perimeters is enabled. #1907
1e5dcd8
@alexrj
Owner
alexrj commented Apr 19, 2014

Done! Let's wait for the complaints.

@alexrj alexrj closed this Apr 19, 2014
@alexrj alexrj added the Done label Apr 19, 2014
@kefir-
kefir- commented Apr 20, 2014

Nice, I can't wait to try it out!! And I'll try to help you fend off any complaints with good reasoning! :-)

@kefir-
kefir- commented Apr 24, 2014

I found a case where this logic doesn't appear to work quite as expected. It may be related to thin infill logic or something like that. Here's some screenshots of a layer that does perimeter -> next island -> perimeter -> infill -> first island -> infill with both 1.1.1 and 1.1.2-dev as of yesterday.

This first picture shows that the perimeter of both the first and second island are done, and no infill has yet been started.
canvas1

In this second picture, you can see that the infill is almost done on both layers. This is the same layer as the first picture.
canvas3

gcode with embedded config: http://kefirshare.dreamhosters.com/bridge-torture-test_m2_20140423-232035_1.1.2-dev.gcode
STL: http://kefirshare.dreamhosters.com/bridge-torture-test_m2.stl

@alexrj alexrj reopened this Apr 24, 2014
@alexrj alexrj modified the milestone: 1.1.2, 1.1.1 Apr 24, 2014
@alexrj
Owner
alexrj commented Apr 24, 2014

Thanks @kefir-

@alexrj
Owner
alexrj commented Apr 28, 2014

@kefir-, I need config.ini exported from File->Export config, as the embedded one isn't complete. Also maybe the issue was fixed in current master?

@kefir-
kefir- commented Apr 28, 2014

Sure, here's a variant. I think the only difference from the first case was the use of brim, here there's no brim. And with this config the issue is still reproduced with current master fetched after your post about 30 minutes ago.

http://kefirshare.dreamhosters.com/slic3r-config-PLA-careful-bugreport.ini

Are you adding the full config to the embedded comments, or should I create an issue for that? :)

@alexrj
Owner
alexrj commented Apr 29, 2014

Yeah, I'll check that.

@alexrj
Owner
alexrj commented Apr 29, 2014

The problem is, Slic3r now supports distinct configs inside a single print (per-object and per-object part), so that one will represent a random one.

@alexrj
Owner
alexrj commented Apr 29, 2014

Or maybe, all of them should be appended...

@alexrj alexrj added a commit that referenced this issue Apr 29, 2014
@alexrj Bugfix: gap fill was not inserted in the correct order before leaving…
… island. Includes regression test. #1907
913ab54
@alexrj
Owner
alexrj commented Apr 29, 2014

Fixed and added regression test. Thanks! :)

@alexrj alexrj closed this Apr 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment