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

Fix small area infill flow compensation (graph version) causing some lines to not extrude (#4374) #4399

Open
wants to merge 5,873 commits into
base: nightly_dev
Choose a base branch
from

Conversation

TheSlashEffect
Copy link

Hello 3D printing crew

I found the issue that the user was presenting in #4374. The problem was that the actual graph points data structure and begin_idx, end_idx usage were inconsistent in this function. As a result, all extrusion lengths above the max value we account for in the graph (~3.0mm in our case) were not interpolated and got a multiplication factor of 0 instead (the default in this function). I am not sure why the graph implementation works as it does but these changes make this function comply with how the graph and its data structures are currently implemented.

Below is the user's scenario as an example, working as expected.
Screenshot from 2024-08-12 18-39-02

Additional fixes:

  • I changed the function's default y_value to 1.0f, so the default behaviour leaves the extrusion value as it was.
  • The equality comparison of extrusion length and the graph's data points should be done by checking how close to the value it is instead of equality, because of floating point arithmetics. (I)
  • Some return instead of break statements to help with readability a bit.

(I): This can cause some discrepancies if we have 2 lines with very close but not equal lengths, as they will both get assigned the same extrusion factor. We could remove this check or make it compare from the left side only, but then again it is a small performance optimization. I leave this decision to the owner :)
(II): A question on the feature; should the feature's function not be monotonically non-decreasing? I personally cannot think of a case where a shorter line should extrude more than a longer one.

macdylan and others added 30 commits January 3, 2024 15:09
- fix value of branch diameter angle of the organic support
- fix overhang speed values
Turn off feature 'use_surface' for new text object
…reported in prusa3d#11721.

The temporary variable used for reordering ExPolygons wasn't cleared, and it contained empty ExPolygons from previous reordering. This caused those empty ExPolygons replaced actual ExPolygons for infill.

Another issue was that m_fill_expolygons was reordered, but m_fill_expolygons_bboxes were left untouched.
 . arcs merged into a new structure (ArcPolyline)
…ence the warning.

The warning was there because std::string_view::data() returns a pointer to a buffer that is not necessarily null-terminated. So, strncpy shouldn't be used on non-null-terminated buffers, but we always relied only on the buffer length, so it couldn't cause any issues.
Actually, all those deprecated functions were internally called those new functions. So there isn't any risk to use them directly.
This variable hasn't been used since the beginning.
…_progress() inside check_objects_after_cut().
…te_compatible_internal().

This variable hasn't been used since the beginning.
…nymous> may be used uninitialized in this function).

This warning was shown because the previous code was triggering a bug in GCC. More about the bug can be found here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86465.
@supermerill
Copy link
Owner

Thanks, merged.

@supermerill supermerill force-pushed the nightly_dev branch 8 times, most recently from 97361d8 to 2aea702 Compare October 15, 2024 10:17
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.