-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add setting for extra_infill_lines_to_support_skins #19446
Conversation
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.
Clang-Tidy
found issue(s) with the introduced code (1/1)
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.
Hi!
Again, thanks for the extensive contribution!
(Please also see my review of the engine part.)
Here, I'd like to disable it entirely for lightning infill. In my tests, it didn't change anything, and this feature doesn't make a lot of sense in that case, since that type of infill is already meant to address these issues on its own.
(I'm less inclined to want to change it for gradual infills steps, as it works, and makes a little more sense.)
For the rest, it seems to work quite nicely! I did run into something I think is at the very least a bit counterintuitive if I multiply the # of infill lines (see the pictures). I don't think this can do much harm, but OTOH, I don't think those lines are needed there.
(how it is with this feature off, for comparison):
)
'There's something wrong with that bot.'
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.
Done. Please take another look
Description
This is the config necessary for the fix of #19388
Type of change
How Has This Been Tested?
Tested with Ultimaker/CuraEngine#2122