CURA-12361 add skin support #2269
Conversation
CURA-12361 Remove an intermediate structure that was heavily used and actually useless
wawanbreton
changed the base branch from
main
to
CURA-12833_improve-bridge-lines-direction
CURA-12361 The FffGcodeWriter unit test is now failing because the skin doesn't have support infill. The new skin support feature can't be enabled easily because it requires the above layer to be properly processed. which requires a full rewrite of this test.
rburema
left a comment
rburema
left a comment
There was a problem hiding this comment.
Code looks good! I appreciate the comments, that helps.
I've got some small cosmetic changes (most importantly, remove the debug-code that writes to a folder it assumes exists...)
Lastly, I had two remarks that I don't think we should necessarily act on right now, but want to have mentioned:
- The
bridge.cppfile has a lot of classes now; we should consider breaking that up into chunks, probably in a folder. - The
const_castis a bit of a smell and scares me a little 😅
I would love that indeed. I didn't do it in the first place because those structures are very specific to this algorithm, but I do prefer when each file contains single structures/classes. I will do that.
It is a bit scary, hence the comment. But in this context it is perfectly safe. The object is const to allow it being used from the threads that process the layers, and this piece of code properly handles locking between these threads. So yes, quite surprising and scary, but this is one of the cases where |
|
I did the refactoring to move the intermediate structures in their own files, and converted most of them to classes. That makes things even more clear. |
…direction' into CURA-12361_add-skin-support
CURA-12361 Some compilers don't support it for some reason, and it not more useful.
HellAholic
dismissed
rburema’s stale review
Changes have been implemented as requested
This PR merges the following features:
Into a single one that is taking the best of each: we now add a briding layer at the top of the infill, but also expand the area to make sure that all the bridging lines will be supported by an infill line below. This should provide a proper supporting layer for the skin above, whatever the shape of the mesh/infill.
Requires Ultimaker/Cura#21206
CURA-12361