new z-dither branch which implements feature described in issue #9388 #10391
Conversation
|
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. |
|
was eager to try this, but it gives tons of errors in compilation :/ |
|
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. |
|
@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. |
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: |
|
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 |
|
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 ))) |
|
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
<#10391 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAPJMNAJQR4PNHAT55JFB3XGZL3RANCNFSM6AAAAAAXDF623U>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
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
|
|
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: ***@***.***>
|
No problem, just check my code that it didn't broke desired functionality. |
|
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: ***@***.***>
|
|
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. |
src/libslic3r/PrintObject.cpp
Outdated
| @@ -296,6 +302,7 @@ void PrintObject::prepare_infill() | |||
| // Decide what surfaces are to be filled. | |||
| // Here the stTop / stBottomBridge / stBottom infill is turned to just stInternal if zero top / bottom infill layers are configured. | |||
| // Also tiny stInternal surfaces are turned to stInternalSolid. | |||
|
|
|||
There was a problem hiding this comment.
Just empty string, could be removed.
There was a problem hiding this comment.
Removed.
The change was delayed because I was looking into failures of fff_print_tests built with debug (but not release). Turns out that master branch behaves exactly the same, so I stopped checking.
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.
But master branch does take into account So I would suggest to check layer height and split base layer in such way, that no min limit is violated. |
|
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
<#10391 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAPJMIY7AGO7I3YX4GLU2TXH5DYPANCNFSM6AAAAAAXDF623U>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Though master allows setting layer height out of min limit (but anyway limits maximum to nozzle diameter), but variable layers do respect these limits. |
|
Hi, z-dither option sometimes causes overhangs to be printed in the air (project attached): Without z-dither: |
|
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
<#10391 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAPJMMK3QAWRDTYNJRLXADXINQ4FANCNFSM6AAAAAAXDF623U>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
1. Check and filter out tiny areas made during z-dithering 2. Prefer to use better bahaving classic perimeters for dithered layers 3. Computed dithered overhangs only if support is enabled 4. Turning support on/off in the presence of z-dithering requires reslicing in order to account for potential change in number of layers. 5. Accounted for dithered layers effect on skirt-height handling by GCode.
|
Commit 7f1f6bd addresses the issue pointed out by @vovodroid and contains a number of additional fixes/enhancements. |
Decreasing layer height is good for overhangs as it increases line overlap. |
|
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
<#10391 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAPJMJBPYKUC6LMSHZY73DXM7XNXANCNFSM6AAAAAAXDF623U>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Why? I'm talking about overhangs, not bridges. By a way, latest commit looks quite good! |
|
The illustration included in the original issue #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. |
|
Z-dither breaks support for attached STL, especially organic:
|
|
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. 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. |
|
But without Z-dithering it's ok:
|
|
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. |
|
Here is attached project with z-dither enabled:
|
| // arachne width averaging is not so good for thin areas of dithered layers becuase it results in routh attachment/seams of | ||
| // dithered layers to non-dithered (regular) layers. On the other hand for very narrow dithered layers classic algothim | ||
| // sometimes makes no perimeters at all and in this case we better employ arachne algorithm (even if user selected classic). | ||
| if (!use_arachne || dithered_layer) |
There was a problem hiding this comment.
It's better not to switch Arachne/Classic calls order, neither reformat parameter list, as it causes problem with further merges.
There was a problem hiding this comment.
While changing the formatting of parameters could be done and is not a big deal, the alternative to switching Arachne/Classic calls order was considered and turned out to be much worse.
There was a problem hiding this comment.
Even restore format will be good, as I had problem in merging another PR over dithering. But what is problem with call order? Aren't two calls mutually exclusive? I guess this
if (dithered_layer && m_perimeters.empty() || !dithered_layer && use_arachne)
PerimeterGenerator::process_arachne(
.........
else
PerimeterGenerator::process_classic(
should work, or do I miss something?
There was a problem hiding this comment.
For dithered layers, Classic paths connect to adjacent non-dithers layers much better than Arachne. This is because Classic tends to create closed perimeter loops, while for narrow regions, Arachne may produce open contours. On the other hand, at times, Classic produces nothing, while Arachne is still capable of making open perimeters. My code for dithered layers attempts to use Classic first, then if Classic fails, use Arachne. Observe m_perimeters.empty() as a check for Classic failure and two if statements as opposed to if/else.
The change in formatting was made by Visual Studio while complying with .clang-format. I will restore the original formatting and attempt to improve the comment describing the logic of the changed code.
1. Check and filter out tiny areas made during z-dithering 2. Prefer to use better bahaving classic perimeters for dithered layers 3. Computed dithered overhangs only if support is enabled 4. Turning support on/off in the presence of z-dithering requires reslicing in order to account for potential change in number of layers. 5. Accounted for dithered layers effect on skirt-height handling by GCode.
I implemented the z-dithering algorithm that halves the layer stairstep effect on low-slopped surfaces. See #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).