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

Don't use bridge flow over dense infill #4374

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jschuh
Copy link
Contributor

@jschuh jschuh commented Jun 10, 2020

Updated description: The current version of this patch automatically disables bridge flow over sufficiently dense infill (and when the upper extrusions are thick enough). The benchmark threshold for disabling bridge infill is spanning a 3mm gap at 0.2mm layer height. I'm entirely open to adjusting those thresholds, but they seem to work well after a few years of regular use.

Old description (no longer accurate):

This fixes issue #3814 by adding a bridge_infill_threshold setting to control when bridge infill is applied. The change works by computing the average void area for the infill below the solid layer, and flagging the solid layer as stInternalBridge only if it's larger than the threshold value. Assuming a reasonable threshold is chosen, this results in both reduced print times and stronger parts with no apparent reduction in print quality.

The default bridge_infill_threshold value is 25mm², which has been reliable in all my testing with PLA and PETG, and means that on .4mm infill extrusion width you will get bridge infill at 15% infill density but not at 20% infill density.

I've been carrying this change in my local build and printing with it for the better part of a year with good results. It pairs very well with my perimeter fan speed controls in PR #2921, which I rely on when printing any structural parts.

bubnikv added a commit that referenced this pull request Feb 10, 2021
Add setting to control when bridge infill is used #4374

Squashed commit of the following:

commit 56e5c7756a683e80324521736215d0678748a8cb
Merge: eaf6e0d 33215bb
Author: Vojtech Bubnik <bubnikv@gmail.com>
Date:   Wed Feb 10 12:07:59 2021 +0100

    Merge branch 'bridge_infill_threshold' of https://github.com/jschuh/PrusaSlicer into jschuh-bridge_infill_threshold

commit 33215bb
Merge: 8b60613 fe4f1b1
Author: Justin Schuh <jschuh@users.noreply.github.com>
Date:   Tue Dec 8 16:55:09 2020 -0800

    Merge branch 'master' into bridge_infill_threshold

commit 8b60613
Author: Justin Schuh <jschuh@users.noreply.github.com>
Date:   Tue Jun 9 20:01:15 2020 -0700

    Add setting to control when bridge infill is used

    Bridge infill will be applied only if the average area of the voids
    between infill lines (computed from the infill density and infill
    extrusion width) is greater than the bridge_infill_threshold setting.
@bubnikv
Copy link
Collaborator

bubnikv commented Feb 10, 2021

I don't think that this pull request fixes #3814 . Those are two independent topics.

I see some sense in what you have done, however I would rather have the threshold hard coded and pretty low. I suppose for 50% infill the bridging is not really needed.

@jschuh
Copy link
Contributor Author

jschuh commented Feb 13, 2021

Happy to hardcode it and remove the preference, but how strongly do you feel about that 50% infill threshold? I ask because my feeling is that it's less about the infill percentage and more about the length of the unsupported span between infill extrusions. So, I think it still makes sense to calculate from the extrusion width, which would look something like this:

unsupported_span = extrusion_width / infill_percentage - extrusion_width

Based on my testing 1mm would be a conservative span threshold, which works out to 29% infill percentage for a .4mm extrusion width, and 55% infill percentage for a 1.2mm extrusion width. Accepting that, these are obviously just estimates, because the exact length of the span depends on the infill pattern and the angle of the layer above, but I've found this calculation to be good enough.

@jschuh jschuh changed the title Add setting to control when bridge infill is used Don't use bridge flow over dense infill Feb 13, 2021
@jschuh
Copy link
Contributor Author

jschuh commented Feb 13, 2021

I removed the setting, now calculate the threshold as described in my last comment, and rolled forward to the latest trunk.

@bubnikv, does this seem reasonable, or would you prefer to skip the extrusion width calculation and just use a very conservative infill density?

@jschuh
Copy link
Contributor Author

jschuh commented Mar 4, 2021

@bubnikv, I think I've found a very simple and correct solution. The bridge_over_infill() function already had a threshold check near the end to to avoid bridging over voids below a certain width. So, I just moved that threshold calculation up earlier in the code, and also use it to avoid bridging over voids in the infill below the same width.

This is also a much simpler patch, because my earlier version was erroneously performing redundant threshold checks inside the loop, rather than checking before to avoid the loop entirely. And I moved the check entirely inside bridge_over_infill, so no other files or methods are modified.

@jschuh
Copy link
Contributor Author

jschuh commented Jul 15, 2021

I just squashed and rebased to today's HEAD to make this an easy merge. It's only a 15 line change, localized to a single file, and no longer introduces new configuration options.

I see the current iteration of this patch as effectively a bug fix, because the existing code already skips bridging for open voids less than three times the bridge flow width. So, this patch just extends that same logic to voids in the infill (e.g. for the typical 0.4mm nozzle with a 0.45mm infill extrusion width this would skip bridging for anything over ~26% infill).

I've been printing with this iteration of the patch for the last few months, and I just get faster prints with no loss in quality when printing dense infill (which makes sense since it just applies the existing void threshold).

@viper0078
Copy link

I usually used a 25% infill. So, I agree with the 26% proposal. My reasons for wanting this feature are described in #7324.
I hope this feature will be implemented soon.
Is it not possible in Ver 2.4?

@bubnikv
Copy link
Collaborator

bubnikv commented Dec 10, 2021

Sorry it did not make it into 2.4. We need to roll out 2.4. We should look into the pull request after the 2.4 release.

I agree that for dense infills the current bridging flow over sparse infill is counter productive. For sparse infill, the current behavior is beneficial compared to other slicers, which don't do it.

We need to re-evaluate with both the thick and the new "thin" bridges. The "thin" bridges are what everybody in the industry is doing, the "thick" bridges are what Slic3r always did.

@jschuh jschuh force-pushed the bridge_infill_threshold branch 2 times, most recently from 94422c8 to 77412a4 Compare December 10, 2021 18:35
@scotfor
Copy link

scotfor commented Jan 10, 2022

I don't think that this pull request fixes #3814 . Those are two independent topics.

I see some sense in what you have done, however I would rather have the threshold hard coded and pretty low. I suppose for 50% infill the bridging is not really needed.

I think the threshold should NOT be hard-coded. It needs to be exposed as a setting.

@jschuh
Copy link
Contributor Author

jschuh commented Jan 10, 2022

I think the threshold should NOT be hard-coded. It needs to be exposed as a setting.

The minimum void width that will cause the slicer to use the bridge flow is hardcoded (to 3mm) in the current code. So, it doesn't really make sense to add a separate configuration value for just this one specific case. An earlier version of this PR did use a configuration setting, but in retrospect that was an error on my part, because I didn't understand the code well enough at the time.

@scotfor
Copy link

scotfor commented Jan 10, 2022

I think the threshold should NOT be hard-coded. It needs to be exposed as a setting.

The minimum void width that will cause the slicer to use the bridge flow is hardcoded (to 3mm) in the current code. So, it doesn't really make sense to add a separate configuration value for just this one specific case. An earlier version of this PR did use a configuration setting, but in retrospect that was an error on my part, because I didn't understand the code well enough at the time.

Maybe. I would use the threshold to shut bridge infill off completely. Given the number of posts about bridge infill, I think there is little doubt that it is broken the way it is. I just want it gone from my prints. From a tool design perspective it is bad design to have something appear ( bridge infill - where did that come from?) that there are no setting for. I can use settings to get solid infill to bridge properly.

@jschuh
Copy link
Contributor Author

jschuh commented Jan 11, 2022

@scotfor, I'm unclear on what you're proposing, and regardless this PR seems the wrong place for a different proposal. I don't make any decisions for this project (I'm just an independent, open source contributor), and I authored this PR for addressing a specific issue.

My personal suggestion is that you open a new issue specific to the problem you're seeing. I've found the development team to generally be responsive to clear, cogent explanations of problems, along with supporting data. Something that seems obvious in your specific usage could be incomprehensible to someone else—or your understanding of the issue may simply be incorrect. That's why it's so important to be clear in your communication, and respectful to those you're communicating with.

@scotfor
Copy link

scotfor commented Jan 11, 2022

@scotfor, I'm unclear on what you're proposing, and regardless this PR seems the wrong place for a different proposal. I don't make any decisions for this project (I'm just an independent, open source contributor), and I authored this PR for addressing a specific issue.

My personal suggestion is that you open a new issue specific to the problem you're seeing. I've found the development team to generally be responsive to clear, cogent explanations of problems, along with supporting data. Something that seems obvious in your specific usage could be incomprehensible to someone else—or your understanding of the issue may simply be incorrect. That's why it's so important to be clear in your communication, and respectful to those you're communicating with.

I do have another issue open: #7728 (comment)
Lets leave it at that. Thanks for your replies.

@jschuh
Copy link
Contributor Author

jschuh commented May 15, 2023

@Godrak, any chance you'd take a look at this one? It had earlier iterations, but for a while now it just disables bridge flow over sufficiently dense infill (and when the upper extrusions are thick enough).

TheSlashEffect added a commit to TheSlashEffect/PrusaSlicer-SmallAreaInfillFlowCompensation that referenced this pull request Aug 12, 2024
- Make the input range checks consistent with start_idx, end_idx of existing implementation
- Make default extrusion factor 1.0f to prevent similar issues
- Small readability improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants