Brim overhaul: New features and Bug fixes #1613
Conversation
|
Watch out: I vaguely remember that when you switch the Skirt/Brim Extruder from one of the extruders to Not Overridden, the linked status ( |
|
Visual explanation of a single step in the core algorithm: |
Why is this PR so big?Good question! It seems to include a lot more functionality, while the philosophy is to create one PR per feature. What happened here? The core algorithm works on both closed polygons and open polylines, which meant I had to rewrite large parts of the code. The order in which brim lines were printed used to be controlled in a way which was totally unintelligible to me, so when I redid the implementation I made use of the relatively new order_constraints which can be passed to the order optimizer. This solved several bugs in the order in which brim lines were printed. Because the different materials could have different size brims the grey borders would have to change or in general how the brims interact with the border of the build plate. For a couple of reasons I decided to limit the brims to the build area, rather than to limit the build area based on the brim size:
This update to the allowed areas also benefits tree support, which also limits itself to the allowed areas on the print bed. It used to be the case that when Brim On Outside Only was selected then enclosing parts with other parts inside of them would get a brim after all, because we had no way of knowing whether the external brim of the inside part could interact with the outer part. Because we now have the possibility for brim polygons to be cut up into polylines we can remove all sections of the internal brim which would otherwise overlap with the outer part. However, because we might overextrude a little bit the external brim of the enclosed part could still mess up the edge where the brim meets the enclosing part. I therefore had to introduce the Avoidance Margin to be able to limit that effect. TL;DR: because I revised the core algorithm a lot of other stuff had to change as well and I didn't want to reintroduce bugs in the reimplementation. |
Simple code, but slow and dirty approximation.
Not overridden means any extruder (which appears on the first layer) will get a brim in that material
This reverts commit 671be8d. That functionality is no longer needed.
The function getLayerOutlines was the only user of this field. However, that function never took brim into account, so neither should it take the prime tower brim into account.
It is technically more correct and faster to compute the allowed areas bsaed on the total printable area than based on some heuristic offset.
The code for raft outline generation tried to use the variable primeTower.outer_poly_first_layer, was used to be primeTower.outer_poly.offset(brim_distance). However, outer_poly_first_layer was removed in favor of a local variable in brim.cpp. Since when printing a raft there is no brim being printed (currently), we might as well use outer_poly here. The raft doesn't need to take any brim into account.
BagelOrb
force-pushed
the
brim_per_material_optimized_order
branch
from
665ea66
to
0a1bb7d
Compare
The disallowed areas are given in the coordinate system of the left extruder (?), whereas the global border is defined as the union of the extruder borders of each extruder. In order to retrieve the border of a single extruder, we mut perform some intersection to get it out of the global border.
For low values of brim_inside_margin th shield, which is outside of the shiled can still generate brim inside of the holes of the model, though. Ignoring this issue for now.
It allows for removing the brim inside holes completely if set to a high value, or just keep it a small distance away from internal holes
Shields are always on the outside; we should never go beyond a parts wall and print brim on the inside of the part to support the shield.
This prints faster. It's okay because the unintended overextrusion which gets propagated inwards still has room to go and doesn't affect the first layer outline of the actual parts. Reimplements 108e6b3
I've verified with git blame that all the commit messages which made sense to me were also implemented in the new brim implementation
They are done. I hope my const correctness skills are on game.
When multiple parts were laid inside each other like russion dolls, then some bad XOR voodoo would happen and some regions in between two parts would get fully filled with brim rather than no brim being generated anywhere on the inside of parts.
BagelOrb
force-pushed
the
brim_per_material_optimized_order
branch
from
0a1bb7d
to
9a35852
Compare
|
Rebased on main. (Old branch is still available here: brim_per_material_optimized_order_old_unrebased Most conflicts were due to spdlog, code style and Simplify. One conflict had to do with the raft generation for the prime tower, which used to account for a possible brim, but not anymore, which is no problem, since we don't generate any brim when we have a raft. Verified to be compiling and tested to be working correctly: |
|
I've verified the settings bug: Steps:
Actual:
Expected:
Extra info:
I think that the print preview problem is part of the same frontend problem and not a CuraEngine problem. This setting issue is too deep into the frontend settings system for me to solve. Please review the issue and try to solve it before or after merging this PR - or amend the frontend PR to include a fix. I'm sorry I'm leaving you with a PR which still has an issue in it. |
The variable 'is_last' is a perfectly 'normal' part of the object, not a cached value, mutext, or whatever that's part of the 'meta-data' of the object instead of what it 'is'. It makes way more sense to remove the const qualifier(s) in this instance. part of CURA-9066
Also make formula more clear. (And slightly better -- if we ever had a ridiculus amount of extruders, it would become incorrect.) part of CURA-9066
Replace redundant booleans with EPlatformAdhesion, replace 'manual' variant (save both Polygons* and int) with std::variant<Polygons*, int>. part of CURA-9066
Makes it more clear that you can also just get it for all extruders. pat of CURA-9066
part of CURA-9066
…nt-end. ... but should probably be an actual setting now at some point. part of CURA-9066
part of CURA-9066
A. With this PR it is possible to have separate brims for each material:
All brim options are now limited to the Skirt/Brim Extruder, which can be set to Not Overriden, which means they are not limited to any extruder, which means that a brim in each material will be printed.
B. Furthermore, preventing outside of buildplate extrusion is now done in the engine:
C. That way we don't bother the user anymore with grey areas which are determined by the adhesion - ony those due to clips and due to extruders not being able to reach the whole bed:
D. Furthermore, this PR includes a small fix for the prime tower brim being generated even though it was turned off.
Cura 4.13.0:
With this PR:
E. Also this PR includes a setting to prevent external only brims from touching internal holes using the Brim Inside Avoid Margin:
F. SliceDataStorage::getMachineBorder is amended with the disallowed areas, so tree support now also cannot overlap with the clips anymore and will always stay within the printable areas.
G. The ooze shield and draft shield used to be printed right through the prime tower if they were unfortunately position wrt one another. There would be double extrusion in those places, which means the the prime tower could topple more easily. The ooze and draft shield now avoid the prime tower so that they don't intersect.
H. The order in which the brim lines are printed is enforced more strictly and is controlled more verbosely in code. This solved some bugs which caused in between brim lines to be printed after more outer and after more inner brim lines.
Implementation
Because the brims can now interfere with each other, with models, or with the disallowed areas, the brim lines are cut up into polylines. This has implications for how the brim lines are stored, which meant that I had to redo a large part of the implementation of the brim.
SkirtBrim.cpp got a major overhaul. The primary brim algorithm is reimplemented. Also I've reimplemented the code for the secondary skirt to ensure the minimal length constraint for each extruder.
Also FffGcodeGenerator::processSkirtBrim got overhauled. It enforces the printing order constraints more strictly, but takes longer to compute. The slice time is generally increased by a couple of seconds, but since this is only applied to a single layer these extra seconds don't scale with larger models.
The printing order of a brim is from inside to outside in order to reduce the impact of extrusion propagation on the outlines of the first layer of the model. Skirts are printed more efficiently travel-time-wise by printing them outside to inside.
There might be some bugs relating to the printing order of brims which are now fixed. The printing order is now enforced through order_constraints.
The order in which the support brim lines are printed is less strictly enforced, since support doesn't require much accuracy. The OrderOptimizer determines the order in between the normal brim when to most efficiently print them travel-time-wise.
The brim next to the ooze and draft shield is largely unaffected.
I've used the following setup a lot for testing various aspects of the brim. It might come in handy.
complex_brim_scene.3mf.renamed.zip
ffff.3mf.renamed.zip
These issues are probably solved by this PR:
Ultimaker/Cura#11644
Ultimaker/Cura#10753
Ultimaker/Cura#10543
Ultimaker/Cura#8995
Ultimaker/Cura#7800
Ultimaker/Cura#6447
Ultimaker/Cura#6386
Ultimaker/Cura#6118
For our reference: PP-36