New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

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

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

This comment has been minimized.

Show comment
Hide comment

whosawhatsis commented Apr 5, 2014

+1

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 6, 2014

Member

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

Member

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

This comment has been minimized.

Show comment
Hide comment
@luke321

luke321 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!

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

This comment has been minimized.

Show comment
Hide comment
@ledvinap

ledvinap Apr 7, 2014

Collaborator

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?

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

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 18, 2014

Member

@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

Member

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 added a commit that referenced this issue Apr 19, 2014

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 19, 2014

Member

Done! Let's wait for the complaints.

Member

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-

This comment has been minimized.

Show comment
Hide comment
@kefir-

kefir- 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- 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-

This comment has been minimized.

Show comment
Hide comment
@kefir-

kefir- 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

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 milestones: 1.1.2, 1.1.1 Apr 24, 2014

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 24, 2014

Member

Thanks @kefir-

Member

alexrj commented Apr 24, 2014

Thanks @kefir-

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 28, 2014

Member

@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?

Member

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-

This comment has been minimized.

Show comment
Hide comment
@kefir-

kefir- 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? :)

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

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 29, 2014

Member

Yeah, I'll check that.

Member

alexrj commented Apr 29, 2014

Yeah, I'll check that.

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 29, 2014

Member

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.

Member

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

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 29, 2014

Member

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

Member

alexrj commented Apr 29, 2014

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

alexrj added a commit that referenced this issue Apr 29, 2014

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 29, 2014

Member

Fixed and added regression test. Thanks! :)

Member

alexrj commented Apr 29, 2014

Fixed and added regression test. Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment