Skip to content

Disable SAFC and retract for Hilbert curve #9592

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vovodroid
Copy link
Contributor

@vovodroid vovodroid commented May 10, 2025

This PR enable small area flow compensation for solid Rectilinear/Monotonic patterns only and disable retract for solid/top infill Hilbert curve pattern.

It resolves Under extrusion on Hilbert curve first layer with small area flow compensation. #4617 and Create as a check marked option Enable small area flow compensation for first layer #7201.
Replaces Disable small area flow compensation on first layer (Fixes #4617) #5838 and Retract on top layer option. #6188.

Before:
image
image

@kisslorand
Copy link
Contributor

@vovodroid
I was working on fixing the same issue for "Archimedean Chords" when I saw this PR. It is unnecessary to make another PR so please add "Archimedean Chords" also.

It's the same issue with "Concentric" pattern also but I do not know how to fix SAFC only for the curved parts. :(
{776B46E1-A4BB-48FD-BAB6-0CEE42A33741}

@vovodroid
Copy link
Contributor Author

Do Archimedean Chords demand this? Actually it consist of both short and long lines. Doesn't short line benefit form SAFC?

@kisslorand
Copy link
Contributor

kisslorand commented May 22, 2025

In the case of Archimedean Chords I wouldn't call it "benefit from SAFC" but rather "suffer from SAFC".

Flow of Archimedean chords with SAFC:
{46F06AC9-668B-47D9-B1C7-33B501193213}

Flow of Archimedean chords without SAFC:
{47E3DA74-C7B5-4278-9D6E-1F99D7416345}

On the above examples speed is the same but flow differs drastically.

Archimedean chords are made by many many tiny lines in Klipper where there's no ARC support (hence ARC has to be disabled for Klipper). I discovered this bug by stripping my hair why YOLO flow calibration gave me wrong results. It was because I had SAFC enabled (and some other stuff that I will make a PR for the fix).

@vovodroid
Copy link
Contributor Author

So probably disable only if no arc support is enabled?

@kisslorand
Copy link
Contributor

If ARC is enabled it will still suffer from SAFC because a surface with Archimedean Chords pattern will have many tiny arcs with different radiuses.

{980996BC-A802-4C56-9757-2AC66210F275}

@kisslorand
Copy link
Contributor

Actually the real problem is the implementation of SAFC. Whilst the idea is great, the implementation of it is wrong. SAFC doesn't check for small area but only checks small line segments in a solid infill. It should check the distance between the two walls where the infill segment goes, that's how you determine "small areas".
The fix in this PR only avoids the issue that SAFC brings, it is not a fix for the core of the problem.

@vovodroid
Copy link
Contributor Author

So may be do just opposite - enable only for Rectilinear/Monotonic/Lines infill?

@kisslorand
Copy link
Contributor

YESSS!
That's an even better idea! I think that SAFC was born having only those infills in mind, infills with only straight lines.
Maybe a good idea to update SAFC description also to let the users know it will be effective only with infill patterns that exclusively consist of straight lines/segments.

@vovodroid
Copy link
Contributor Author

@mjonuschat , @TheSlashEffect , @Noisyfox - what do you think?

@vovodroid vovodroid closed this May 27, 2025
@vovodroid vovodroid force-pushed the hilbert-flow-retract-pr branch from 661af9b to 4545132 Compare May 27, 2025 12:15
@vovodroid vovodroid reopened this May 27, 2025
@vovodroid
Copy link
Contributor Author

@kisslorand I change PR to what we were talked about.

@kisslorand
Copy link
Contributor

kisslorand commented May 27, 2025

I would refactor a bit the _needSAFC() function for the following reasons:

  • make a list (array) of the infill patterns that can be easily edited to remove or add new future infills
  • remove the SAFC on the first layer regardless of the infill pattern (there was a PR that removed SAFC on the first layer because SAFC causes first layer adhesion problems)

Here's what I did in my build after I cherry-picked your commit:

bool GCode::_needSAFC(const ExtrusionPath &path)
{
    if (!m_small_area_infill_flow_compensator || !m_config.small_area_infill_flow_compensation.value || this->on_first_layer())
        return false;

    static const InfillPattern supported_patterns[] = {
        InfillPattern::ipRectilinear,
        InfillPattern::ipAlignedRectilinear,
        InfillPattern::ipMonotonic,
        InfillPattern::ipMonotonicLine,
    };

    return std::any_of(std::begin(supported_patterns), std::end(supported_patterns), [&](const InfillPattern pattern) {
        return path.role() == erSolidInfill && this->config().internal_solid_infill_pattern == pattern ||
               path.role() == erTopSolidInfill && this->config().top_surface_pattern == pattern;
    });
}

It's tested and working on local Windows build.
I really like how it turned out creating a whitelist for the infill patterns permitted for SAFC.

@vovodroid
Copy link
Contributor Author

Great, thanks!

I took your implementation, besides still using SAFC on first layer for supported patterns. Actually it works very good on first layer.

@kisslorand
Copy link
Contributor

Great, I will try also first layer with SAFC.

@vovodroid
Copy link
Contributor Author

All this mess started from using SAFC for Hilbert curve on first layer. I tried it, but actually got solid first layer.

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.

Under extrusion on Hilbert curve first layer with small area flow compensation.
2 participants