prusa3d / PrusaSlicer

G-code generator for 3D printers (RepRap, Makerbot, Ultimaker etc.)
https://www.prusa3d.com/prusaslicer/
GNU Affero General Public License v3.0
7.41k stars 1.88k forks source link

new z-dither branch which implements feature described in issue #9388 #10391

Open LRaiz opened 1 year ago

LRaiz commented 1 year ago

I implemented the z-dithering algorithm that halves the layer stairstep effect on low-slopped surfaces. See https://github.com/prusa3d/PrusaSlicer/issues/9388 for the description of the idea. A new checkmark in the Advanced group of the Print Setting tab allows you to turn it on or off. Turning it on introduces additional thin layers at the periphery of regular layers. These layers are added not for the entire slice area but only where the surface slope is sufficiently low to accommodate nozzle passage at half-layer height. It makes the resulting surface with details rivaling the details of the surface printed with half-layer height but without a significant increase in print time. I expect that turning this option on in many cases will negate the necessity to have multiple layer height ranges. I also hope that it will allow increasing default layer height and thus speed up printing. On the other hand, it may necessitate fine-tuning other printing parameters. Consider it to be an experimental feature.

I have tested the implementation on several examples, but given its nature, further testing is required; it may also necessitate tuning some aspects of the resulting g-code (e.g. speed or width).

LRaiz commented 1 year ago

This pull request probably needs to remove the Overhangs label, just as it was removed on originating issue #9388. On the other hand, it may need to have the "feature request" label added.

mirlang commented 1 year ago

was eager to try this, but it gives tons of errors in compilation :/

LRaiz commented 1 year ago

In my limited tests, prints with z-dithering turned on showed the expected decrease in layer stairstep appearance. The geometric aspects of the new feature were working fine. However, the surface smoothness at the corners connecting z-dithering layers with nominal layers was not as good as I had hoped. Some tuning of z-dithered paths or their g-code parameters appears necessary to address smoothness at such corners.

LRaiz commented 1 year ago

@mirlang I didn't see any errors in my Windows environment. Are you on Windows? Make sure to fetch everything from the origin and build the master branch first. Use supplied build script as described in https://github.com/prusa3d/PrusaSlicer/blob/master/doc/How%20to%20build%20-%20Windows.md Then switch to the z-dither branch and build.

mirlang commented 1 year ago

Are you on Windows?

no, sry, I'm using linux... my c++ abilities are not that great, but those errors indicate missing includes oder variable definitions - i downloaded the diff of this pull request from github instead of cloning your repo, maybe something went wrong here (will try tomorrow) - alpha6 without this patch builds fine

errors from log:

/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:64:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:71:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:76:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:82:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:90:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:107:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:117:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:125:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:130:11: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:132:5: error: ‘indexed_triangle_set’ has not been declared
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/TriangleMeshSlicer.hpp:133:5: error: ‘indexed_triangle_set’ has not been declared
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.hpp:21:24: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.hpp:35:40: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:14:24: error: ‘indexed_triangle_set’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:29:17: error: ‘stl_vertex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:29:27: error: template argument 1 is invalid
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:29:27: error: template argument 2 is invalid
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:29:55: error: request for member ‘vertices’ in ‘mesh’, which is of non-class type ‘const int’
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:31:24: error: found ‘:’ in nested-name-specifier, expected ‘::’
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:31:22: error: ‘v’ has not been declared
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:33:36: error: expected initializer before ‘(’ token
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:33:36: error: could not convert ‘candidate_zs’ from ‘std::vector<float>’ to ‘bool’
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:33:36: error: expected ‘;’ before ‘(’ token
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:33:42: error: expected ‘)’ before ‘;’ token
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:39:9: error: ‘candidate_zs’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:44:5: error: ‘candidate_zs’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:56:31: error: request for member ‘indices’ in ‘mesh’, which is of non-class type ‘const int’
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:57:15: error: ‘stl_triangle_vertex_indices’ does not name a type
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:58:19: error: expected ‘;’ before ‘vertex’
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:61:21: error: ‘vertex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:125:21: error: ‘diff_ex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:129:38: error: ‘diff_ex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:134:21: error: ‘diff_ex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:138:38: error: ‘diff_ex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:143:35: error: ‘intersection_ex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:145:35: error: ‘intersection_ex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:147:35: error: ‘intersection_ex’ was not declared in this scope
/var/tmp/portage/media-gfx/prusaslicer-2.6.0_alpha6/work/PrusaSlicer-version_2.6.0-alpha6/src/libslic3r/ZDither.cpp:154:35: error: ‘error’ was not declared in this scope; did you mean ‘perror’?
LRaiz commented 1 year ago

It looks like you don’t have right base. I suspect your master branch would not compile either. Consult Linux instructions how to install and build in README.md

vovodroid commented 1 year ago

Hi Leonid!

I updated your code to latest beta2, resolved conflicts, removed various whitespaces and tabulation changes (probably your IDE has auto format enabled), and made code style more Prusa like.

Feel free to take branch https://github.com/vovodroidprusa/PrusaSlicer/tree/z-dither-LRaiz and update PR basing on it.

Most astonishing that it still works )))

LRaiz commented 1 year ago

Hi,

I picked up your changes and added some more changes of my own. Before pushing the update to GitHub, I want to check if the following is okay - for all changed files I removed trailing whitespaces and replaced tabs with spaces.

Thanks, ~ LR

On Thu, May 18, 2023 at 1:28 PM Vovodroid @.***> wrote:

Hi Leonid!

I updated your code to latest beta2, removed various whitespaces and tabulation changes (probably your IDE has auto format enabled), and made code style more Prusa like.

Feel free to take branch https://github.com/vovodroidprusa/PrusaSlicer/tree/z-dither-LRaiz and update PR basing on it.

— Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/pull/10391#issuecomment-1553381957, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPJMNAJQR4PNHAT55JFB3XGZL3RANCNFSM6AAAAAAXDF623U . You are receiving this because you authored the thread.Message ID: @.***>

vovodroid commented 1 year ago

for all changed files I removed trailing whitespaces and replaced tabs with spaces.

This adds a lot of "noise" when you compare PR to master, and causes endless conflicts in the future. Actually https://github.com/prusa3d/PrusaSlicer/wiki/Contribution-guidelines says

Don't do anything not directly related to that. Do not fix comment-typos in all files that you see, don't unify whitespace usage,

LRaiz commented 1 year ago

Thanks. Unfortunately I did not come across the contribution guidelines earlier. - LR Sent from my iPhoneOn May 22, 2023, at 2:14 AM, Vovodroid @.***> wrote:

for all changed files I removed trailing whitespaces and replaced tabs with spaces.

This adds a lot of "noise" when you compare PR to master, and causes endless conflicts in the future. Actually https://github.com/prusa3d/PrusaSlicer/wiki/Contribution-guidelines says Don't do anything not directly related to that. Do not fix comment-typos in all files that you see, don't unify whitespace usage,

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

vovodroid commented 1 year ago

Unfortunately I did not come across the contribution guidelines earlier. -

No problem, just check my code that it didn't broke desired functionality.

LRaiz commented 1 year ago

I found more changes to be necessary on top of yours. I will take some time to test before pushing to GitHub.~ LRSent from my iPadOn May 22, 2023, at 6:09 AM, Vovodroid @.***> wrote:

Unfortunately I did not come across the contribution guidelines earlier. -

No problem, just check my code that it didn't broke desired functionality.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

LRaiz commented 1 year ago

I committed @vovodroid changes and some additional by myself to LRaiz/PrusaSlicer z-dither branch. Some comment lines are taken from the master branch instead of the @vovodroid version.

LRaiz commented 1 year ago

I am not experienced with pull requests. Do I need to merge z-dither to master, push it to LRaiz, and open another pull request? @vovodroid

vovodroid commented 1 year ago

You should do nothing, until your branch is updated and there are no conflicts. You can see what will be merged into master here https://github.com/prusa3d/PrusaSlicer/pull/10391/files

LRaiz commented 1 year ago

I am confused. My branch is already updated, and conflicts are resolved. You approved the changes. The are no changes to master that aren't merged into z-dither. Do I need to do something to merge the z-dither branch into the PrusaSlicer master? If the answer is yes, then does my previous comment reflect the correct procedure?

vovodroid commented 1 year ago

You approved the changes.

My approvement means nothing, I'm not code maintainer.

Do I need to do something to merge the z-dither branch into the PrusaSlicer master?

Just wait whether admins/maintainers will agree to take your PR into the slicer code. As you can see there are other 126 pending PRs.

vovodroid commented 1 year ago

By a way, z-dither code doesn't respect minimal exctruder layer height - min_layer_height.

LRaiz commented 1 year ago

It is not clear how z-dither should deal with _min_layerheight. I suspect this parameter is there to guard against user mistakes leading to exceedingly long print times. On the other hand, printer moving mechanisms are capable of z-increments/resolution considerably smaller than the current limit of 0.07mm. Since dithered layer height may be 1/4 of a non-dithered layer height, it seems unreasonable to limit z-dithering to layer heights greater than 0.28 mm. Besides, such non-dithered layers would be taller than _max-layerheight of 0.25 mm. Even though some limitations may be imposed in the future, I am inclined not to do it just yet.

vovodroid commented 1 year ago

I suspect this parameter is there to guard against user mistakes leading to exceedingly long print times

I thinks it's related to material properties, nozzle form, etc. For instance, very thin layer with small volumetric volume rate could lead to filament "frying". And if you have not flat nozzle like

image

thick layer could absorb it. So I guess it's better to respect minimum extruder height.

printer moving mechanisms are capable of z-increments/resolution considerably smaller than the current limit of 0.07mm.

Actually there is no such limit. In most printers by default Marlin is capable of moving to distance 0.015 mm, so it's possible to have such layer (and with z-lift it could be as little as 0.0025), but question is print quality.

I successfully printed small PLA bolt with 0.05 layer.

LRaiz commented 1 year ago

What do you mean by respect minimum layer height? Do you mean to warn if layer_height / 4 < min_layer_height? With the default value of min_layer_height = 0.07, it would trigger warnings for layers thinner than 0.28 mm. For several reasons, I am not convinced this is desirable. Z-dithering indeed encourages using layer heights greater than the current default max_layer_height = 0.25. But specific values and corresponding recommendations should be developed after testing.

vovodroid commented 1 year ago

As I mentioned, there is no such thing as 0.07 limit, it could be in specific printer profile, but neither Marlin, nor hardware has such strict limit.

Do you mean to warn if layer_height / 4 < min_layer_height?

No, I mean don't create layers with layer_height < min_layer_height. It should be up to user which minimal layer height to set, if going to thinner layer causes problems in his case.

LRaiz commented 1 year ago
vovodroid commented 1 year ago

Extruder advanced parameters on the printer settings tab default to min_layer_height = 0.07 and max_layer_height = 0.25

Yes, it's hardcoded, but it's not universal limit. Imagine someone printing with 0.6 nozzle and wood or carbon filament. I'm not sure you can go 0.07 and especially less. The same applies to small nozzles, like 0.2. It wouldn't be good print 0.3 layer with such nozzle.

  • All layer height restrictions are imposed by the master branch

But master branch does take into account min_layer_height, for example in variable layers. Nevertheless users expect that slicer never goes beyond min/max limits, that's the meaning of these values.

So I would suggest to check layer height and split base layer in such way, that no min limit is violated.

LRaiz commented 1 year ago

If desired layer_height > min_layer_height limitation as well as layer_height < max_layer_height should be implemented in master branch. Such limitations are not currently enforced by master. If the master branch implements then than z-dithering will get them as well. I don’t understand the reasoning to implement them specifically for z-dithering but not master.

On Thu, May 25, 2023 at 8:09 AM Vovodroid @.***> wrote:

Extruder advanced parameters on the printer settings tab default to min_layer_height = 0.07 and max_layer_height = 0.25

Yes, it's hardcoded, but it's not universal limit. Imagine someone printing with 0.6 nozzle and wood or carbon filament. I'm not sure you can go 0.07 and especially less. The same applies to small nozzles, like 0.2. It would be good print 0.3 layer with such nozzle.

  • All layer height restrictions are imposed by the master branch

But master branch does take into account min_layer_height, for example in variable layers. Nevertheless users expect that slicer never goes beyond min/max limits, that's the meaning of these values.

So I would suggest to check layer height and split base layer in such way, that no min limit is violated.

— Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/pull/10391#issuecomment-1562794252, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPJMIY7AGO7I3YX4GLU2TXH5DYPANCNFSM6AAAAAAXDF623U . You are receiving this because you authored the thread.Message ID: @.***>

vovodroid commented 1 year ago

Such limitations are not currently enforced by master.

Though master allows setting layer height out of min limit (but anyway limits maximum to nozzle diameter), but variable layers do respect these limits.

vovodroid commented 1 year ago

Hi, z-dither option sometimes causes overhangs to be printed in the air (project attached): image

Without z-dither: image

Body.zip

LRaiz commented 1 year ago

Interesting example. Thanks for sharing.

On Sun, May 28, 2023 at 10:50 AM Vovodroid @.***> wrote:

Hi, z-dither option sometimes causes overhangs to be printed in the air (project attached): [image: image] https://user-images.githubusercontent.com/8691781/241567691-24092f67-633a-4588-bf42-067468bfd3c0.png

Without z-dither: [image: image] https://user-images.githubusercontent.com/8691781/241567737-f2a5ee55-6440-4e72-b04e-f1bd3be08349.png

Body.zip https://github.com/prusa3d/PrusaSlicer/files/11584890/Body.zip

— Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/pull/10391#issuecomment-1566162907, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPJMMK3QAWRDTYNJRLXADXINQ4FANCNFSM6AAAAAAXDF623U . You are receiving this because you authored the thread.Message ID: @.***>

LRaiz commented 11 months ago

Commit 7f1f6bde0f44e64d10ff548e6d897c0596388402 addresses the issue pointed out by @vovodroid and contains a number of additional fixes/enhancements. There is still one remaining question. It is not clear to me how beneficial z-dithering would be for handling overhangs. For now, I limited the creation of dithered overhangs only to prints that generate support. I expect testing to show if it is sufficient or if a separate config switch is needed.

vovodroid commented 11 months ago

It is not clear to me how beneficial z-dithering would be for handling overhangs.

Decreasing layer height is good for overhangs as it increases line overlap.

LRaiz commented 11 months ago

Dithered layers on down-facing surfaces are different because they not bridging between supported non-overhanging areas and not overlapping non-dithered layers. They are supported only by support structure.

On Sun, Jun 25, 2023 at 4:14 AM Vovodroid @.***> wrote:

It is not clear to me how beneficial z-dithering would be for handling overhangs.

Decreasing layer height is good for overhangs as it increases line overlap.

— Reply to this email directly, view it on GitHub https://github.com/prusa3d/PrusaSlicer/pull/10391#issuecomment-1605926435, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPJMJBPYKUC6LMSHZY73DXM7XNXANCNFSM6AAAAAAXDF623U . You are receiving this because you authored the thread.Message ID: @.***>

vovodroid commented 11 months ago

They are supported only by support structure.

Why? I'm talking about overhangs, not bridges.

By a way, latest commit looks quite good!

LRaiz commented 11 months ago

The illustration included in the original issue https://github.com/prusa3d/PrusaSlicer/issues/9388 makes it apparent that dithered layers on downward-facing surfaces do not overlap regular layers below and thus need a support structure to keep them from falling.

vovodroid commented 11 months ago

Z-dither breaks support for attached STL, especially organic:

image

S10eCase.zip

LRaiz commented 11 months ago

I do not see indications that the problem reported by @vovodroid is related to z-dithering. When I run master branch using debug executable and select organic support, the executable generates an assert error. Assert I guess this issue is better reported to Prusa developers. I also suggest including parameter configuration to assure that the problem is reproduced. On the other hand z-dither RelWithDebug executable does not assert but makes support trees taller than object. Crazy. Capture

By the way, for my print parameters, I got no indication that support was needed, and g-code without support was computed fine with z-dither.

vovodroid commented 11 months ago

But without Z-dithering it's ok:

image

LRaiz commented 11 months ago

My result using RelWithDebug with no dithering is very different. The difference must be attributable to the difference in other printing parameters since @vovodroid did not provide his Config. Still, the fact that without z-dithering, Debug executable asserts using the same set of parameters indicates that newly released organic supports still have some issues to iron out. These issues need to be addressed before the z-dithering investigation. Capture

vovodroid commented 11 months ago

Here is attached project with z-dither enabled:

image

S10eCase-zdither.zip