Closed saumyaj3 closed 6 months ago
Looks a bit like a band-aid 😬 any chance to find a root cause to this ? And maybe instead of looking for the exact same areas in the other list, we should make a difference between the fractional and non-fractional ?
The root cause is a bit tricky: As I see it, root cause is because, tree calculation is done by combining the meshes and treating them as a group but for tipgenrator two meshes are used, creates a confusion because there is only one tree or supports for (all nodes) but multiple tip calculation. We can create a spinoff ticket to fix this
Please do so, maybe this bug hides something else. @ThomasRahm could we have your opinion on this ?
I am not sure what the goal of this change is, so my input here could be way off:
I think that the idea to calculate the fractional support from the finished support/roof areas is flawed. I don't like extrapolating information that was thrown away before. I think that the areas that require fractional support should be tracked separately.
As the current implementation of fractional support area calculation causes issues in my cradle branch, I am currently trying to solve this anyway. If it works I will do an alternative pull request this weekend Monday and reference it here.
On another related note: I did take a quick look at how fractional support is implemented and am wondering: The fractional config seems global, but is dependent on Support Top Distance, which can be set per model. Am I missing something or does this mean that fractional support distance is not working correctly when models with different Support Top Distance are present?
@ThomasRahm Thanks for your answer. You are quite right in assuming that the fractional layer support is a bit ad-hoc. We tried to make something simple but it fired back at us with many bugs that we are fixing one by one. On the other hand, we know that "normal" support is not really satisfying with its current implementation, so we don't really want to give a lot of effort on it. I think we should either rewrite it from scratch, or drop it to use tree support which gives much better results in most cases (IMHO).
About the per-model Z-distance, I think this has been taken care of, but I will definitely do some tests this morning to make sure of it.
I see you could make your PR this morning. I will add a ticket so that we can review it, and hopefully the band-aid we are discussing here will not be required.
@ThomasRahm Well apparently you were right about the per-model Z distance not being applied properly. I have added a new bug ticket for this, but I'm afraid this one would require too much work to be fixed for 5.7.
I will review your PR now, but this may take some time because I haven't seen the tree support code yet.
Issue fixed by https://github.com/Ultimaker/CuraEngine/pull/2054
Description
Refine fractional support roof layer logic in TreeSupportTipGenerator
This update refines the layer logic in the TreeSupportTipGenerator. It streamlines the handling of additions to the support fractional roof by checking for existing paths before adding new ones.
CURA-11735