prusa3d / Prusa-Firmware

Firmware for Original Prusa i3 3D printer by PrusaResearch
GNU General Public License v3.0
2.02k stars 1.05k forks source link

[ENHANCEMENT] Adjust MM_PER_ARC_SEGMENT default value #2606

Closed FormerLurker closed 3 years ago

FormerLurker commented 4 years ago

Printer type - All Prusa Printers MMU Upgrade - NA (this is purely a printer firmware request)

Is your enhancement related to a problem? Please describe. I have been experimenting with a new algorithm I created that converts G0-G1 commands into G2-G3 commands to eliminate stutter introduced by serial streaming and to compress the GCode files by approx 40-60%. However,the conservative value of 1 for MM_PER_ARC_SEGMENT creates very noticeable flat edges that should clearly be curves. This results in a noticable decrease of print quality around tight curves (around 1mm or less) when using the arc commands, even though the tool paths are practically identical (+- 0.025mm). Longer gentler arcs turn out fantastic, and are indistinguishable from those printed using G1 commands. Please let me know if you need to see comparisons.

Describe the solution you'd like I would recommend a value of at most 0.1mm for quality that is comparable to the output of PrusaSlicer when using G2/G3, as long as that won't affect printer performance in your opinion. If it's possible to go even lower, down to the resolution of the stepper, that would be ideal if it doesn't negatively affect printer performance. In any case, I believe 1mm is too conservative. Since most people are not using G2/G3 in everyday printing, I believe this has been overlooked.

Here is the relevant setting in the Configuration_adv.h file: https://github.com/prusa3d/Prusa-Firmware/blob/66ffb8489ac34cd62605ea744e262c429f7c479f/Firmware/Configuration_adv.h#L296

I would be more than willing to extensively test a build that contained the altered setting. I'm running an MK2.5 w MMU2.0. I am planning to compile a version with that setting modified as soon as possible, but wanted to reach out first.

Thank you very much for taking the time to read this, and for all of your efforts!

FormerLurker commented 4 years ago

I've been experimenting with the MM_PER_ARC_SEGMENT and N_ARC_CORRECTION defines. I increased the N_ARC_CORRECTION setting inversely as I decreased the value of MM_PER_ARC_SEGMENT so that the same number of corrections per mm are being applied.

I have determined that MM_PER_ARC_SEGMENT = 0.1 will NOT work. There is a large amount of stuttering when using that setting, which results in unacceptable print quality impacts. I'm not sure if there is a problem calculating acceleration or if the CPU is maxed. I tried printing an object of similar radius using 0.1mm line segments (G1 commands), and it printed without stutter. It still could be the extra overhead added by mc_arc call in motion_control.cpp, but I'm not sure yet. I should add that other people using Marlin 2+ have tested MM_PER_ARC_SEGMENT = 0.1 and even MM_PER_ARC_SEGMENT = 0.01 with no stuttering, though that could be board or buffer related. I will try to get more info about their setup.

A value of 0.25 resulted in acceptable looking small curves, but there was a bit of stutter when printing large curves that produced unacceptable quality. 0.5 worked better still (no noticeable stutter on my test Benchy), but still resulted in some flat edges for shapes that look like smooth curves when printing with G1. I suspect that turning up the print speed might cause stuttering issues even with a value of 0.5. I will try to test this as well.

I'm working on some small modifications to the motion_control.cpp file that allows larger arcs to be printed with the default value of 1mm per segment, but that allows a smaller value to be used for very tight curves. I'm also planning to try out a MIN_SEGMENTS_PER_ARC define as an alternative. I will append my results to this issue.

wavexx commented 4 years ago

I'd love to see comparisons. Can you share some gcode too, before & after?

There's some margin to improve the in the gcode parsing, but we're definitely close to the limit. Since I print a ton of mechanical parts, regular arcs are incredibly frequent and I'd love to avoid artificially reducing the quality of the model just to keep the printing speed high. If there's any code you're willing to share I'd love to be a tester ;)

FormerLurker commented 4 years ago

@wavexx, yes, I'm working on comparisons and will 100% post the relevant ones(I'm growing a collection, lol). I tried a few code tweaks that worked well except for some edge cases, like implementing a simple MIN_SEGMENTS_PER_ARC. After reviewing the Marlin 2 code, I like their method of defining a minimum number of segments for an entire circle, which avoids adding unnecessary segments for small curves with a large radius. I also noticed some over-extrusion, especially in cases where the number of segments is small, so I am adding a correction factor now. I am also trying to create a test shape via scad.

I will fork the repo and insert my code once I'm done tinkering.

FormerLurker commented 4 years ago

@wavexx, I have created this gist which contains two gcode files sliced for the Mk2.5 w/mmu 2.0. You might need to change the start/end gcode if you have a different model printer.

The first file is a Benchy that contains gyroid infill and Archimedean Chord top and bottom fill, so it is just chalked full of curves. The second file is the output of my Arc Compression algorithm. I recommend comparing the two in a gcode analyzer (either simplify or nc viewer, it's difficult to find analyzers that fully support G2/G3) so you can see the differences. The arc version is about half of the size. If you have a file you'd like to test, feel free to send me gcode (link to gist or a download link is preferred) I will convert it for you. I recommend models using relative extrusion, since absolute extrusion is not as well tested (and way more complicated).

I've been tweaking and running test prints with my modified firmware all day. I've finally got mostly good results for all of the edge cases without any stutter. I just added extrusion compensation (regular polygons have shorter circumferences than circles), but I'm not 100% sure I got it right. I adjusted the E axis absolute position via position[E_AXIS] = original_e;, where original_e was the non-adjusted target absolute e coordinate after I called plan_buffer_line, so hopefully that works to keep absolute E lined up. The actual factor is usually quite small (tenths of a percent or less), but I'll see for sure what the difference is after a few tests.

Edit: Setting position was not correct.

FormerLurker commented 4 years ago

So, I wrote a correction for extrusion caused by the difference in interpolated line segment length vs the real arc length, but I don't know how to ensure that the new absolute position doesn't mess up the next extrusion. I changed this line:

https://github.com/prusa3d/Prusa-Firmware/blob/60466c09935c7a5b8f4a8c7e71d51bccead4920f/Firmware/motion_control.cpp#L141

to this:

      // Ensure last segment arrives at target location.
#ifdef ARC_EXTRUSION_CORRECTION
    // 20200417 - FormerLurker - adjust the final absolute e coordinate based on our extruder correction
    // Save the original E absolute position
    float original_e = target[E_AXIS];
    arc_target[E_AXIS] += extruder_per_segment;
    plan_buffer_line(target[X_AXIS], target[Y_AXIS], target[Z_AXIS], arc_target[E_AXIS], feed_rate, extruder);
    // Is this the way we save the change to absolute e based on the correction?  Please Confirm
    // Set the absolute e position to the original expected e value so as to not over extrude on the next segment
    target[E_AXIS] = original_e;

You can see there is a new define ARC_EXTRUSION_CORRECTION that controls the new code block. I previously adjusted extruder_per_segment with a correction factor. Things are all good until after the call to plan_buffer_line, where the E_AXIS will be off by the sum of the individual segment corrections. That's why I added this right after the call to plan_buffer_line:

target[E_AXIS] = original_e;

I also see in the call to mc_arc within prepare_arc_move here:

https://github.com/prusa3d/Prusa-Firmware/blob/60466c09935c7a5b8f4a8c7e71d51bccead4920f/Firmware/Marlin_main.cpp#L9331-L9333

that the current_position is being set to destination (called target in mc_arc).

My question is this: Since I have altered the amount of extrusion in the G2/G3 command, will adjusting the target[E_AXIS] to the original target[E_AXIS] (as in the above code) account for this, or will it mess the next extrude? If this isn't the right way to apply a correction, what is?

Thanks!

FormerLurker commented 4 years ago

OK, so I'm still not 100% sure about fixing the absolute position of the E_AXIS within the planner, but my test prints are looking OK so far. I have been working on a visualization of what the planner is doing with arc commands to get a better idea of what's going on when the defines change. It has been quite illuminating. Here's what I've come up with using the layer at 8.6mm from Benchy, sliced at 0.2mm with gyroid infill and Archimedean Chord top and bottom fill printed with a 0.4mm nozzle. I chose this layer because it has a combination of tight and large curves. I'll be showing a rendering for various scenarios that show the tool-paths and endpoints (rendered with NC Viewer):

Source Gcode file outputted by PrusaSlicer:

image

Output of my algorithm to convert G1s to G2/G3 where possible:

image

As you can see, there are lots of curves on this layer, but relatively few gcodes. The tool paths are within +-0.0250mm of the original file (usually much closer).

Here is the output from the latest release candidate of the Prusa firmware using the default value for MM_PER_ARC_SEGMENT:

image

I generated the gcode to produce the above output by replacing the calls to plan_buffer_line with a function that outputs the line segments as gcode instead. As you can see, the approximation is quite poor, and doesn't match the original shape very well at all. Some of the previously rounded corners are now quite sharp and unpleasant looking.

Here is the output created by changing MM_PER_ARC_SEGMENT to 0.1, which is approximately equal to the minimum segment length in the original source gcode file:

image

It's easy to see why I experienced so much stuttering when printing with that setting. It's hard to tell from the image, but those black lines are actually individual points.
Using MM_PER_ARC_SEGMENT = 0.5 was a bit better, but there are still corners that appear flat, and there is still a number of unnecessarily small segments:

image

This is the output of a modification I've made that is closer to what Marlin 2.0 does (I've not examined the output of Marlin 2.0's routine, so I'm not 100% sure how close they are). I created a new define (exists in Marlin 2.0) called MIN_ARC_SEGMENTS that specifies the minimum number of segments in a full circle. I used a value of 32 for this example:

image

As you can see, this looks much closer to the original output from PrusaSlicer. The maximum spacing between any two segments of an arc here is 1MM unless that would create fewer facets than MIN_ARC_SEGMENTS in a full circle drawn with the same radius. In that case there are always at least ceil((MIN_ARC_SEGMENTS) * (fabs(angular_travel) / (2 * M_PI))) segments.

The resulting print looked good. I was able to increase the speed to 200% and didn't see any stuttering. There are no noticeable flat edges that should be curves. I'll post a compare of the prints later because I need to fully print some of the examples I show above.

Thoughts? Ideas?

FormerLurker commented 3 years ago

I'm going to close this issue since Pull Request 2657 covers this now.

Thanks!