-
-
Notifications
You must be signed in to change notification settings - Fork 968
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
Improvements in Inner Outer Inner wall ordering logic #6138
Conversation
…avel moves when printing neighbouring features
Updating status to ready to review. Further enhancements may come from future testing, however it is in a place now to review and get feedback. |
…scenarios where multiple external perimeters are contained in the same island.
I hope that once this one and #5996 are merged a new beta of Orca will be released for everyone to test it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @igiannakas
I had a rough overview of the changes, and have some comments of the code structure, could you take a look?
I need more time to deep dive into the algos.
Meanwhile, do you by any chance measured/noticed the performance impact?
Awesome thank you for the feedback, much appreciated!!! I’ll get on to them next week when back home. Performace wise I have not noticed anything huge; however I’m using a MacBook Pro M1 Pro which never struggled really with slicing anyway. Have been using it since I opened the PR with reasonable complex models (incl multi color). That being said, I’m not expecting a huge hit as the tree is “pruning” itself - start with external contour, search perimeters in the same island for nearest inset index 1, store them, then search for inset index 2, store etc. if the perimeter is already stored, it doesn’t search for it again so the perimeter list reduces as we scan through the list - the first scan is the most expensive and it’s bound by the number of inset index 1 perimeters available. If that makes sense :) I’m expecting more of a hit on models with multiple perimeter inset index 1 in the same island. For the rest (as in simple islands with 1 inset index perimeter) it shouldn’t make any difference as the perimeter list is small. Also as the perimeter generation is running multi threaded it’s somewhat ok but a test with a slower computer may be useful. |
…mber already present. Removed squaredDistance and replaced with Eigen library call.
…enamed for more clarity on their purpose.
Hope the below commentary helps with the review Softfever! findAllTouchingPerimeters Function: This function is identifies perimeters that are in close proximity to a set of reference perimeters. Given a collection of perimeters, each with an associated position and inset index (which indicates whether the perimeter is external or internal), the function searches for other perimeters that are "touching" i.e. are near the reference perimeters within a specified distance threshold. This function creates a list of perimeters that are within a certain distance from a reference set, distinguishing between external and internal perimeters based on their inset index. The function finds perimeters of a specific inset index (e.g., first internal perimeters) that are close to the reference. reorderPerimetersByProximity Function: Optimization Strategy: The function begins with the outermost perimeters and finds the closest internal perimeters that should follow in the sequence. It prioritizes larger, dominant perimeters that are more impactful on the overall process (as we want the largest amount of extrusion to have happened before we print the external perimeter to ensure the nozzle is primed properly). bringContoursToFront function: Once we have a sequenced outer-inner list of perimeters based on proximity, we then run the IOI re-ordering as before, moving the inset index 2+ perimeter ahead of the inset index 0 + 1. This part has remained unchanged, and is slightly simplified due to the more consistent ordering outputs from the above revised algorithm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Thank you!
Awesome thank you :) I’ll possibly carry on refining this over the next few months as it’s my primary print mode too :) |
* Improvements in Inner Outer Inner wall ordering logic * Updated to BFS algorithm, made ordering more robust and corrected edge cases * Doc updates * Refinements in perimeter sorting * Removal of touch threshold and code debugging to improve sequencing * Code cleanup * Code refinements on perimeter distance thresholds * Extend perimeter re-ordering to more than inset index 2, to reduce travel moves when printing neighbouring features * Refinements to IOI perimeter re-ordering algorithm to improve travel scenarios where multiple external perimeters are contained in the same island. * Documentation updates * Removed unnecessary code * Removed bespoke to_points function and replaced with ExtrusionLine member already present. Removed squaredDistance and replaced with Eigen library call. * Refactor code to move distancing functions to the multipoint class. Renamed for more clarity on their purpose.
Introducing distance checking when reordering perimeters for inner outer inner to catch more edge cases.
Testing is in progress hence marking as draft
Explanation of changes / issues addressed:
The examples below use Outer Inner to better showcase the identified issue:
Before - Layer 41-46
Before - Layer 48-49
Now the algorithm re-orders the external walls always setting the external wall of the model as first in the list for IOI wall ordering. I have not made this change in the Outer Inner ordering mode, as the clustering algorithm works well there and it doesn't matter quality wise that the hole is printed before the contour, as both start with the external perimeter first.
For example:
In the below we can see the external wall printed with no neighbouring perimeters at all
That is because the internal walls next to the outer benchy shell are associated to the hole (i.e. the "pole" on the benchy)
Note how the IOI algorithm before tried to fix this by at least finding one internal perimeter to print - it just did not know proximity so the selected line was not always optimal.
In this PR I have implemented a new perimeter re-ordering algorithm that reallocates the first internal, second internal and so forth, perimeters back to the external perimeter.
After - the external contour now gets ownership of the internal perimeters, on the assumption that external model wall quality is more important than "hole" surface finish.
This also helps with more complicated shapes, like the below
Before: The outer wall is printed after a long travel which isn't necessary as it has plenty of perimeters next to it to use. However these were allocated to a "hole" so it cannot re-order using the IOI sequence and instead falls back to OI.
After: - the external contour (wall) takes ownership of the perimeters.
How this PR works:
The ordering algorithm before executing the IOI re-ordering sequence, traverses the ordered extrusions list and checks that neighbouring perimeters to the external contour perimeter are ordered in the right way. If they are not, it re-orders the internal perimeters, up to the second internal perimeter and attaches them to the external contour.
How are we checking for proximity?
Minimum distance
I've experimented with the Hausdorf distance algorithm and the minimum path segment distance algorithm to define proximity. As we are looking for perimeters that are touching the external contour, the minimum distance algorithm of any two points between the base perimeter and the candidate perimeter for reordering yielded better results.
Algorithm explanation:
This revision to the perimeter ordering has also enabled the reduction in complexity of the old heuristics performed to attempt to find the right perimeter sequence based on inset indexes, as now the perimeters are ordered in the right way, and in an optimal manner for IOI ordering from the beginning.
Finally, unfortunately as the seam data is not present in this code class (they are generated later in the slicing pipeline) we cannot use the start and end points of the extrusion as mechanisms to re-order the perimeters so travel between the start and end points is minimised. So we are left with trying to find a proxy that helps with sequencing - in this case it is proximity and perimeter length.
This change affects Arachne perimeter generation only, as it is the one susceptible to suboptimal perimeter ordering for IOI mode compared to classic. This is because of the variable line widths that Arachne is using, resulting in perimeters being shared between islands instead of gap fill operations to fill the space between the islands.
Credits:
Credit to discord user KillSwitch who's worked with me to replicate the ordering issue he was facing plus ideas on how this could be addressed and for being the test subject :) STL above was provided by KillSwitch when replicating the issue.