Skip to content
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

Optimise sandwich wall mode when perimeters are shared with multiple islands(Arachne only) #1352

Merged
merged 6 commits into from
Jun 29, 2023

Conversation

igiannakas
Copy link
Contributor

@igiannakas igiannakas commented Jun 15, 2023

PR containing the change in approach to handling sandwich mode when using Arachne engine.

In a nutshell instead of treating the perimeters as inside to outside and re-ordering the last extrusion it flips it to treat them as outside to inside and swaps the order of printing the third wall with the outer perimeter so the order is internal, external, internal and then the remaining internal walls from the outside - in.

This means that when less than 3 extrusions exist the wall is treated as outside to inside, leading to a better surface finish as there is no variation due to printing an external perimeter with and without an adjoining internal perimeter

This is a revised version of the PR #751 addressing issue #745 with more extensive logic on the wall ordering to ensure that all cases where an external perimeter is present on an island are re-ordered correctly, even if there are multiple external perimeters within the same island.

}
if(outer >-1 && first_internal>-1 && second_internal>-1){ //found perimeters to re-order?
swapped_extrusions[0] = ordered_extrusions[outer]; // get the extrusions that need re-ordering into temporary array.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to use swap function here instead of the a temporary vector.

@SoftFever
Copy link
Owner

The overall logic seems fine to me. It could be optimized or simplified. I have some suggestions in my comment. Let me know if you want me to commit changes into this branch directly. If not, I'm happy to let you do it yourself.

@igiannakas
Copy link
Contributor Author

The overall logic seems fine to me. It could be optimized or simplified. I have some suggestions in my comment. Let me know if you want me to commit changes into this branch directly. If not, I'm happy to let you do it yourself.

I couldn’t get the swap function for some reason to work consistently - I’m sure it’s a debug issue but couldn’t find a solution to it. I’d appreciate your help on that please.

@SoftFever
Copy link
Owner

@igiannakas
I can't make the swap function work either.
So I have to use a temporary variable :)
Please take a look at my changes and let me know if you have any issues with the changes

@SoftFever SoftFever changed the title Sandwich wall mode can be optimized when perimeters are shared with multiple islands Optimise sandwich wall mode when perimeters are shared with multiple islands(Arachne only) Jun 28, 2023
@igiannakas
Copy link
Contributor Author

@igiannakas
I can't make the swap function work either.
So I have to use a temporary variable :)
Please take a look at my changes and let me know if you have any issues with the changes

Looks great, love the clean up especially on the reordering - moving away from allocating an array of 3 to a single one is a good move! Looking good to go I think?

@SoftFever
Copy link
Owner

Cool. Good to merge.

Currently this optimisation apply to Arachne, do you want to optimise the classic wall generator(in process_classic() function)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants