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

Unnecessary moves cause extra moves and longer moves, with no benefits #1990

Closed
kefir- opened this Issue Apr 29, 2014 · 6 comments

Comments

Projects
None yet
3 participants
@kefir-

kefir- commented Apr 29, 2014

I'm testing latest git head with the same files and config used in #1907 (comment), and I've noticed that slic3r adds some seemingly completely unnecessary moves. They're not a big problem for this part, but I'm reporting this because the logic doesn't really make sense, and this might be a useful test case. I'll try to describe with some screenshots, and include the gcode.

This is the last island of this layer, and the perimeter is almost done. It is being drawn counter-clockwise:
canvas-perimeter-almost-done-2

This is how it looks when it's done, finishing in the lower right corner:
canvas-perimeter-done-2

Here is the unnecessary move, for some reason the nozzle is moved from the lower right corner to the lower left corner. I don't see any good argument for doing this move:
canvas-unnecessary-move-3

Here the infill is started, moving clockwise from the lower left corner:
canvas-infill-started-2

Here we can see what it looks like when the layer is done, excluding the final retract in the lower left corner. As you can see, the infill also overlaps itself a bit, looking a little like some of the artifacts seen in #1875:
canvas-infill-done-2

This is the first move of the next layer. It may not be possible to see, but it is moving to the right. That means that the unnecessary move done above was doubly unnecessary, because it has added extra distance to this move as well. Interestingly, the optimal case seems to be just to drop the unecessary move every time.
canvas-next-layer-longer-move-2

The very first layer is special, but the next 19 layers seem to do this. Here's the gcode:

http://kefirshare.dreamhosters.com/bridge-torture-test_m2_20140429-230956_1.1.2-dev.gcode

@kefir- kefir- changed the title from Unnecessary move on every layer to Unnecessary moves cause extra moves and longer moves, with no benefits Apr 29, 2014

@ledvinap

This comment has been minimized.

Show comment
Hide comment
@ledvinap

ledvinap Apr 29, 2014

Collaborator

The infill part is property of current implementation of medial-axis. It does not find loops correctly(or at all?), so it finds path from some entry point (selected by its relationship to surrounding polygon). The resulting path is most probably 'correct' result of math used ...

Collaborator

ledvinap commented Apr 29, 2014

The infill part is property of current implementation of medial-axis. It does not find loops correctly(or at all?), so it finds path from some entry point (selected by its relationship to surrounding polygon). The resulting path is most probably 'correct' result of math used ...

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 29, 2014

Member

Yes, I think once the problem causing the extra tiny segments is fixed, that gap fill will be detected as a loop (there's already logic for that in _fill_gaps() and it will be started at the most convenient point instead at the entry point on the left.

Member

alexrj commented Apr 29, 2014

Yes, I think once the problem causing the extra tiny segments is fixed, that gap fill will be detected as a loop (there's already logic for that in _fill_gaps() and it will be started at the most convenient point instead at the entry point on the left.

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj Apr 29, 2014

Member

Still, I love this linear gap fill. :)

Member

alexrj commented Apr 29, 2014

Still, I love this linear gap fill. :)

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj May 21, 2014

Member

@kefir-, I think 08279ec fixes this. Let me know when you have a chance! Thank you!

Member

alexrj commented May 21, 2014

@kefir-, I think 08279ec fixes this. Let me know when you have a chance! Thank you!

@kefir-

This comment has been minimized.

Show comment
Hide comment
@kefir-

kefir- May 21, 2014

As far as I can tell, the unnecessary moves still happen with my test part. The artifacts where the infill returns to the starting point seem to be gone, however, so that looks much better.

kefir- commented May 21, 2014

As far as I can tell, the unnecessary moves still happen with my test part. The artifacts where the infill returns to the starting point seem to be gone, however, so that looks much better.

@alexrj

This comment has been minimized.

Show comment
Hide comment
@alexrj

alexrj May 26, 2014

Member

Thank you for testing @kefir-. I now fixed loop detection and I confirm that unnecessary moves are gone since the internal loop is started at the nearest point:

schermata 2014-05-26 a 12 08 17

Member

alexrj commented May 26, 2014

Thank you for testing @kefir-. I now fixed loop detection and I confirm that unnecessary moves are gone since the internal loop is started at the nearest point:

schermata 2014-05-26 a 12 08 17

@alexrj alexrj closed this May 26, 2014

alexrj added a commit that referenced this issue May 26, 2014

@alexrj alexrj added the Fixed label May 26, 2014

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