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

[CURA-10407] Introduce fractional layer-height support gaps #1955

Merged
merged 60 commits into from
Oct 27, 2023

Conversation

rburema
Copy link
Member

@rburema rburema commented Sep 21, 2023

Support gaps (z distance) between the model(s) and the support can be non-multiples of layer-heights now.

rburema and others added 3 commits September 21, 2023 15:38
First see if the part of the support (interface only for now) that should (perhaps) be a bit lower if the support z gap is not a multiple of the layer height, can actually be lowered at all without upending the code too much. This turns out to be possible. Next is actually setting the proper gap height (or what's left of the gap after already dropping down to the layer that needs to be a fractional height.)

Proof of concept mostly in the sense that the quality of the code added may not reflect our standards, and would also be missing functionality (for instance; what if we print without support-interface).

part of CURA-11041, which is the proof-of-concept for CURA-10407
Instead of half-height layers in each area that could potentially be affected by non-layer-height modulo support z distance, actually calculate the proper z-offset that is needed for the fractional layer.

Proof of concept mostly in the sense that the quality of the code added may not reflect our standards, and would also be missing functionality (for instance; what if we print without support-interface, or tree-support at all).

part of CURA-11041, which is the proof-of-concept for CURA-10407
@github-actions
Copy link
Contributor

github-actions bot commented Sep 21, 2023

Unit Test Results

26 tests  ±0   26 ✔️ ±0   15s ⏱️ +6s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ±0 

Results for commit 4a09451. ± Comparison against base commit d952305.

♻️ This comment has been updated with latest results.

include/sliceDataStorage.h Outdated Show resolved Hide resolved
casperlamboo and others added 22 commits September 25, 2023 17:24
…ine-fill.

Part of the partial layer-height support distance proof of concept: CURA-11041
belatedly part of CURA-11041

Co-authored-by: Casper Lamboo <[email protected]>
Implementation is not fully desired since it has some downsides
1. When a layer-part is partially a roof the whole outer/inner wall uses the inner/outer wall roofing print-configuration
2. Some logic is duplicated, namely the function that calculates the `filled_area_above`. This function previously lived in `SkinInfillAreaComputation`. It's not logical to create a `SkinInfillAreaComputation` instance just to re-use this utility. Proposal to fix this is to move the logic of calculating the `filled_area_above` to a more central location, this will be done in a seperate ticket.
3. The `inset0_roofing_config` and `insetX_roofing_config` contain `line_width` properties. Changing the line-widths here doesn't actually change the line width. The line widths can only be changed through the `insetX_config` and `inset0_config` configs

CURA-11110
rburema and others added 6 commits October 17, 2023 17:50
Take care of the ordering before it gets to all the order optimizers.

part of CURA-10407
There's some code duplication that needs to be consolidated, so refactor the 'copy' (or original rather) so it's like the others and can be easily combined into one funciton. While that was being done, found that the booleans weren't flipped after the order of the input array was changed. (In order to print the lower bits _first_, _then_ the 'normal' support.

part of CURA-10407
Take care of the order optimizer w.r.t. partially fractional-height support-layers.

part of CURA-10407
@rburema rburema marked this pull request as ready for review October 18, 2023 07:42
Copy link
Contributor

@wawanbreton wawanbreton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not much to say, just a few very minor remarks. Didn't get into the detailed behavior of the whole thing, but it seems to be designed as to carefully not break anything existing.

include/LayerPlan.h Outdated Show resolved Hide resolved
include/SupportInfillPart.h Outdated Show resolved Hide resolved
include/pathPlanning/GCodePath.h Outdated Show resolved Hide resolved
src/FffGcodeWriter.cpp Show resolved Hide resolved
src/LayerPlan.cpp Outdated Show resolved Hide resolved
casperlamboo and others added 3 commits October 24, 2023 22:14
The code was attempting to access an index that was out of bounds of the array. Add guards to prevent this access.

CURA-11041
@wawanbreton
Copy link
Contributor

I'm afraid the current implementation breaks the supports with a 0 Z-distance (which works properly in 5.5):
image
image

@wawanbreton
Copy link
Contributor

Also there is a strange behavior: when I set a 0.2mm Z-distance with a 0.1mm layer height, I get only one empty layer between support and model. On 5.5 I get two empty layers as I would expect.

@rburema
Copy link
Member Author

rburema commented Oct 25, 2023

The last two may be related -- there could be an off-by-one error?

@wawanbreton
Copy link
Contributor

Yeah maybe :) It looks like the "intermediate" layer height grows in the wrong direction. When reducing the gap, the layer becomes sparser, which should be the opposite.

@rburema
Copy link
Member Author

rburema commented Oct 25, 2023

... haha oh no, that's a bit embarrassing. It does sort of explain all the other bugs as well though.

@wawanbreton
Copy link
Contributor

Yeah I think it's all the same problem, and it should be easy to fix.

@wawanbreton
Copy link
Contributor

Forget my last comment... When the gap grows, the intermediate layer becomes sparser. That is expected. But when it finally become null-height, then it is printed as if it was full height.

@rburema
Copy link
Member Author

rburema commented Oct 25, 2023

I see. Still probably an easy fix, just avoid the 'exactly zero' scenario.

@wawanbreton
Copy link
Contributor

Just tested new version : 0mm gap works again, but now it does not take care about the "full" layers for the gap: actual maximum distance between support and model is at most 0.99*layer_height

@rburema
Copy link
Member Author

rburema commented Oct 26, 2023

Last commit should fix that, I think. Tested it with support roof on and 0.2 layer height, gaps of 0.4, 0.45, 0.55, 0.6 all gave the expected results (z gaps of; 2 full layers, 2+ layers, 2+ layers, 3 full layers respectively).

edit Oh, and 0mm works as well. The top layer does get split into parts 'just below model' and other parts, but I'm not sure if that's a problem, as it does have continuity with the other behaviour.

@wawanbreton wawanbreton merged commit 43d9532 into main Oct 27, 2023
18 checks passed
@wawanbreton wawanbreton deleted the CURA-11041_fracional_support_gap branch October 27, 2023 13:55
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.

5 participants