Add brim_overlap option to allow control of brim bonding strength. Fix #4174. #4957
Conversation
…fault value matches current brim distance.
|
✅ Build Slic3r 1.3.0-master-2400 completed (commit d601c83934 by @ianmackinnon) |
ianmackinnon
changed the title
|
@ianmackinnon please rebase. Additionally, once you've saved a config the defaults aren't going to show for you. |
| auto exported {gcode.str()}; | ||
| REQUIRE(exported.size() > 0); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a really basic test -- would really prefer testing that the spacing is what is expected.
Perhaps some parsing of the GCode to look at the brim lines?
|
@ianmackinnon see the review comment, please. |
|
@ianmackinnon #4997 has some code around testing the brim generation itself (it also fixes a minor off-by-one error I found when writing the test). It looks like you're pushing the entire brim out by that little amount; I think that referring to the gap in some small absolute unit like mm or maybe um would be more understandable by users than percentage, as you're basically changing the inflate/deflate scale numbers. @VanessaE any thoughts about this? |
|
@lordofhyphens Thanks for the feedback and linking that issue with the relevant test. When I get a moment I'll rebase and add a proper test. I imagine that the ability to control the gap will be important only to a small subset of users, so in my opinion the priority should be to not alter current behaviour. -3.13% is not a neat number but it is the value that matches current behaviour and functionally I think this default in the existing code works great for the vast majority of prints. I also think it makes sense for the size of the gap to scale with the print width rather than be an absolute value. So my preference would be to stick with a percentage and keep that number as the default but if others feel strongly it should have a different unit or default value I'm happy to try it out. |
|
I favor an exact measure for this e.g. mm. µm might be okay, though I guess this would be the only option to use that. |
This is my first pull request. Please let me know if there is anything in terms of content or protocol that I should do differently.
The value I have specified as the default (-3.13%, eg. a very slight separation, a negative overlap) maintains the current brim distance although it's not a very round number.
I have added a “default” option in
PrintConfig.cppbut the default value still shows as-3.13%in the Print Settings panel. I'm not sure what I would have to change to make this showdefaultinstead.