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

Apply scarf seam and speed gradient in surface mode #2158

Merged
merged 7 commits into from
Oct 31, 2024

Conversation

wawanbreton
Copy link
Contributor

CURA-12176

The addSplitWall method is now a template so that it can handle either ExtrusionLine or Polygon objects. It also takes a function as parameter because the way we add extrusion lines is not the same for surface mode and normal mode.
In order to simplify this function, I added a PathAdapter class to work with different types of paths with more transparency.

I am not completely satisfied of this fix. The initial decision I took of handling the scarf seam while adding the extrusion moves now appears very bad to me. This should have been a LayerPlan post-processing function similar to what gradual flow does. I suggest adding a ticket to refactor this, or I fear we will discover other bad interactions between scarf seam, speed gradient and gradual flow, and in the future if we want to add more similar behaviors, it will really be a mess.

wawanbreton and others added 5 commits October 28, 2024 14:42
CURA-12176

The addSplitWall method is now a template so that it can handle either
ExtrusionLine or Polygon objects. It also takes a function as parameter
because the way we add extrusion lines is not the same for surface mode
and normal mode.
In order to simplify this function, I added a PathAdapter class to work
with different types of paths with more transparency.
Copy link
Member

@rburema rburema left a comment

Choose a reason for hiding this comment

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

... I can really see why you want this whole thing refactored now 😅

Anyway, I have a couple of small remarks, otherwise the code seems solid (just likely to fall over if we add any more to it).

include/LayerPlan.h Outdated Show resolved Hide resolved
include/LayerPlan.h Outdated Show resolved Hide resolved
src/LayerPlan.cpp Outdated Show resolved Hide resolved
src/LayerPlan.cpp Outdated Show resolved Hide resolved
src/LayerPlan.cpp Show resolved Hide resolved
include/LayerPlan.h Outdated Show resolved Hide resolved
include/LayerPlan.h Show resolved Hide resolved
@HellAholic HellAholic merged commit e16bbe1 into 5.9 Oct 31, 2024
@HellAholic HellAholic deleted the CURA-12176_scarf-seam-with-surface-mode branch October 31, 2024 14:31
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.

3 participants