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

Updated narrow infill area threshold to prevent infill over bridges incorrectly drawn as concentric #2109

Conversation

igiannakas
Copy link
Contributor

@igiannakas igiannakas commented Sep 14, 2023

This fix is for issue #2081. Reducing the threshold results in less "erroneous" identification of concentric infill in areas where it is not really needed.

The current setting impacts bridges as concentric over a bridge results in a split in the bridge layers when printing.

Left - before / Right - after (concentric infill not added in that bridge).

Screenshot 2023-09-14 at 12 02 00

@samwiseg0
Copy link

samwiseg0 commented Sep 14, 2023

Would it make sense to expose this on the frontend so users could tweak the value if needed?

@igiannakas
Copy link
Contributor Author

Would it make sense to expose this on the frontend so users could tweak the value if needed?

Maybe - this feature aims to speed up printing by using concentric infill which is faster to print in smaller areas instead of a series of super small extrusions.

The biggest speed benefit is to avoid those supersmall extrusions - the default value was quite conservative resulting in large areas that could be meaningfully be printed fast with the regular infill to be printed using concentric (such as the bridge in the Voron cube). I think the value I'm proposing in the latest commit (1.3 instead of 3) is working well in striking that balance.

In general my preference would be to avoid adding more options for something that can be tuned well enough for the majority of cases, but its up to Softfever to decide :)

@Noisyfox
Copy link
Collaborator

Noisyfox commented Sep 16, 2023

Just a reminder, this NARROW_INFILL_AREA_THRESHOLD is a feature added by Bambu that achieves the similar goal as the "Ensure vertical shell thickness improvement" in https://github.com/prusa3d/PrusaSlicer/releases/tag/version_2.6.0-alpha5
which is controlled by the "Detect narrow internal solid infill" switch.

So if this value is touched, make sure the vertical shell infill is still properly filled with concentric pattern.

@igiannakas
Copy link
Contributor Author

It looks like it does - I’ll slice some more models but both the benchy and orca cube above are ok with it.

1 similar comment
@igiannakas
Copy link
Contributor Author

It looks like it does - I’ll slice some more models but both the benchy and orca cube above are ok with it.

@Noisyfox
Copy link
Collaborator

Noisyfox commented Sep 16, 2023

NARROW_INFILL_AREA_THRESHOLD 3:
image
NARROW_INFILL_AREA_THRESHOLD 1.3:
bc542c93aeda82e9fd392e06eaddf1e5

Was using the model from https://www.printables.com/model/475637-frog/files

@Noisyfox
Copy link
Collaborator

Noisyfox commented Sep 16, 2023

I think this might be harder to fix though... IMO the bambu's approach is just too rough and lacks proper shell detection. Might worth porting the "proper" algorithm from Prusa.

@igiannakas
Copy link
Contributor Author

I think this might be harder to fix though... IMO the bambu's approach is just too rough and lacks proper shell detection. Might worth porting the "proper" algorithm from Prusa.

I know. I was thinking whether a better fix would be to detect a bridging area underneath and switch to the regular infill for it. But I don’t know enough yet about the slicer structure to be able to implement it.

However, in the above picture- is that an issue? Regular infill there seems to be doing the trick well - it won’t really slow down the print and I can’t see the benefit over concentric quality wise as it’s a large ish internal area which could easily be filled with regular infill.

@igiannakas
Copy link
Contributor Author

I think this might be harder to fix though... IMO the bambu's approach is just too rough and lacks proper shell detection. Might worth porting the "proper" algorithm from Prusa.

True- that may be worth doing actually. Haven’t dug though the code so not sure how complex the retrofit would be. Currently working on porting the pressure equalizer function :)

@Noisyfox
Copy link
Collaborator

Noisyfox commented Sep 16, 2023

t won’t really slow down the print and I can’t see the benefit over concentric quality wise as it’s a large ish internal area which could easily be filled with regular infill.

Same issue as Prusa tried to fix: extrude over the air when using regular infill:
image

@igiannakas
Copy link
Contributor Author

Hm true.

t won’t really slow down the print and I can’t see the benefit over concentric quality wise as it’s a large ish internal area which could easily be filled with regular infill.

Same issue as Prusa tried to fix: extrude over the air when using regular infill: image

Hm I see, you’re right. Let’s spend the time to port this properly then. Happy to look at it but at first glance couldn’t find the place in the code that this is triggered on prusa slicer. Any pointers would be fantastic :)

@Noisyfox
Copy link
Collaborator

Hm true.

t won’t really slow down the print and I can’t see the benefit over concentric quality wise as it’s a large ish internal area which could easily be filled with regular infill.

Same issue as Prusa tried to fix: extrude over the air when using regular infill: image

Hm I see, you’re right. Let’s spend the time to port this properly then. Happy to look at it but at first glance couldn’t find the place in the code that this is triggered on prusa slicer. Any pointers would be fantastic :)

It was introduced and get mixed up with the "Extend sparse infill" feature in prusa3d/PrusaSlicer@3e85016.

I've done the porting of the later one (with some of the code that relate to the ensure vertical shell thickness commented out), so I think it's easier for me to do this as I've already read some of the related codes.

@Noisyfox
Copy link
Collaborator

Noisyfox commented Sep 16, 2023

The remaining question then is: do we want to keep the "detect narrow internal solid infill" option once the wall thickness is properly done? I'm not sure how this option would be useful besides this.

@SoftFever
Copy link
Owner

SoftFever commented Sep 16, 2023

detect narrow solid infill parameter is acutually pretty useful feature to speed up the printing.
It takes care of all small internal solid infills.
image

I was thinking whether a better fix would be to detect a bridging area underneath and switch to the regular infill for it.

I agree.
For the implementation, I didn't dig deep but I think it should relatively straigthforward to add a new surface type(stInternalOverBridge) and detect it by comparing it agaist lower_lower_layer ;-)
const Layer* lower_layer = m_layers[i-1];
in this place
https://github.com/SoftFever/OrcaSlicer/blob/4161b6cfa5e76542b9273d6a62e422cc208b8a82/src/libslic3r/PrintObject.cpp#L1769C41-L1769C41

What do you think? @igiannakas @Noisyfox

@igiannakas
Copy link
Contributor Author

detect narrow solid infill parameter is acutually pretty useful feature to speed up the printing. It takes care of all small internal solid infills. image

I was thinking whether a better fix would be to detect a bridging area underneath and switch to the regular infill for it.

I agree. For the implementation, I didn't dig deep but I think it should relatively straigthforward to add a new surface type(stInternalOverBridge) and detect it by comparing it agaist lower_lower_layer ;-) const Layer* lower_layer = m_layers[i-1]; in this place https://github.com/SoftFever/OrcaSlicer/blob/4161b6cfa5e76542b9273d6a62e422cc208b8a82/src/libslic3r/PrintObject.cpp#L1769C41-L1769C41

What do you think? @igiannakas @Noisyfox

Now that I’m done hopefully with porting the pressure equalizer feature I’ll try and do a bit more digging on this. I need to find a way to get Xcode debugging working as working with BBedit and printfs for debug is killing me :)

@igiannakas
Copy link
Contributor Author

Closing as fixed by pr #2382

@igiannakas igiannakas closed this Oct 11, 2023
@igiannakas igiannakas deleted the Detect-narrow-solid-infill-parameter-tweak branch October 11, 2023 17:17
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.

4 participants