CuraEngine icon indicating copy to clipboard operation
CuraEngine copied to clipboard

Brim overhaul: New features and Bug fixes

Open BagelOrb opened this issue 2 years ago • 6 comments

A. With this PR it is possible to have separate brims for each material: image 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: image

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: image

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: image With this PR: image

E. Also this PR includes a setting to prevent external only brims from touching internal holes using the Brim Inside Avoid Margin: image image image

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. image

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: https://github.com/Ultimaker/Cura/issues/11644 https://github.com/Ultimaker/Cura/issues/10753 https://github.com/Ultimaker/Cura/issues/10543 https://github.com/Ultimaker/Cura/issues/8995 https://github.com/Ultimaker/Cura/issues/7800 https://github.com/Ultimaker/Cura/issues/6447 https://github.com/Ultimaker/Cura/issues/6386 https://github.com/Ultimaker/Cura/issues/6118

For our reference: PP-36

BagelOrb avatar Mar 17 '22 16:03 BagelOrb

Watch out: I vaguely remember that when you switch the Skirt/Brim Extruder from one of the extruders to Not Overridden, the linked status (image) of all skirt/brim settings didn't update properly, which leads to weird behavior when changing any of those settings.

BagelOrb avatar Aug 08 '22 14:08 BagelOrb

Visual explanation of a single step in the core algorithm: image image

BagelOrb avatar Aug 08 '22 15:08 BagelOrb

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. Because I had to reimplement some of the features which were broken, those issues have now been fixed. E.g. Prime tower brim being generated when it was turned off.

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:

  1. it would be more difficult for me to implement in the front end due to my limited experience in the frontend.
  2. limiting the brims by the build area means that only the parts close to the edge would get less brim
  3. the core algorithm limits the brim lines by doing a boolean difference with the covered_area and extending this to doing an intersection with the allowed_areas was fairly straightforward.

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.

BagelOrb avatar Aug 08 '22 17:08 BagelOrb

Unit Test Results

25 tests  ±0   25 :heavy_check_mark: ±0   14s :stopwatch: -1s   1 suites ±0     0 :zzz: ±0    1 files   ±0     0 :x: ±0 

Results for commit 4f053c78. ± Comparison against base commit 657742e5.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Aug 12 '22 16:08 github-actions[bot]

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: image

BagelOrb avatar Aug 12 '22 16:08 BagelOrb

I've verified the settings bug: Even though the linked symbol (image) isn't displayed when the brim settings aren't linked to a single extruder anymore, the settings still behave as if they are linked.

Steps:

  • Adhesion Type = Brim, Skirt/Brim Extruder = Not Overridden, set left extruder to 0.8mm
  • Set Brim width of left extruder to 4
  • Set Brim width of right extruder to 8
  • Switch extruder tabs a couple of times

Actual:

  • Brim width of right extruder is reset to 4
  • Brim line count of left extruder is 5
  • Brim line count of right extruder is 9
  • print preview shows 5 brim lines for both extruders

Expected:

  • Brim width of right extruder remains at 8
  • Brim line count of left extruder is 5
  • Brim line count of right extruder is 20
  • print preview shows 5 brim lines for left extruder and 20 for right extruder

Extra info: brim_width_issues_brim_per_material.zip Even though the linked symbol is not present it seems like under the hood it still works as if the settings were linked. Perhaps the linked status was never implemented such that it could be updated by other settings values.

image The white material (right extruder) doesn't exhibit the right number of brim lines.

I think that the print preview problem is part of the same frontend problem and not a CuraEngine problem. It makes sense that if the frontend doesn't disambiguate between the two extruders for the brim settings than it will also not send the separate settings to the engine. The engine code does request the brim line count setting from each extruder:

src/SkirtBrim.cpp:56
        const ExtruderTrain& extruder = extruders[extruder_nr];
        [...]
        line_count[extruder_nr] = extruder.settings.get<int>(is_brim? "brim_line_count" : "skirt_line_count");

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.

BagelOrb avatar Aug 12 '22 17:08 BagelOrb