Improve ExtrusionLine::simplify, eliminating many very-short extrusion segments which led to blemishes in thin-wall models sliced with arachne#11811
Open
sethml wants to merge 9 commits intoprusa3d:masterfrom
Conversation
This was referenced Dec 1, 2023
|
I just did a test print using the dev build of OrcaSlicer/OrcaSlicer#3014 and can confirm that this change fixes #11573. |
Downstream code contains assumptions that closed ExtrusionLines are not degenerate. Open ExtrusionLines already are not reduced below 2 points by ExtrusionLine::simplify(). Change copied from: OrcaSlicer/OrcaSlicer@aa3cdc5
This reverts commit 6faffb5.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change makes ExtrusionLine::simplify() able to remove more very-short segments, which arachne seems to tend to produce frequently on thin-walled models with curvature. Interestingly, Cura does not seem to have this problem. From what I can see, when Arachne was ported over from Cura, a significant amount of logic was added to polyline simplification, but some of that logic was flawed. In particular, the simplification code in Cura seems to be in PolygonRef::simplify(), but during porting logic was added to avoid removing a point prior to a long segment and instead replace the two prior points with the intersection of the prior and next segments, but only if that intersection does not move the points too much. Unfortunately that test compared the intersection to the point at the other end of the long segment, thus the length test would always fail and the point would never be removed.
I made the following changes:
While working with this code I noticed a few potential issues/future improvements:
I believe this fixes #11807. Here's a photo of the model from that issue with this change:

I think it probably also fixes #11573. I'll do a test in a moment.