[BUG] G02/G03 commands execution speed issues

[Ohmarinus]

Bug Description

When executing G02/G03 commands (curves with X, Y, I and J variables) the machine (homebrew Pen Plotter) has speed issues slowing down, and speeding up beyond the max set acceleration settings. See this youtube video:
https://www.youtube.com/watch?v=-qqhaZyxPzU

These speed-ups and sudden slowdowns are causing quite noticeable artifacts which I would like to remove. gcode files without G02/G03 commands draw out fine and without trouble.

My Configurations

Marlin_config_2052_marinus.zip

Hardware:
Mega2560 + Ramps 1.4
3x TMC2208 drivers
LCD panel + SD card

This issue has been persistent since I started working with the plotter, I have tried different Mega2560 boards, different Ramps boards, different stepper drivers, different SD cards. but nothing made a difference.

Steps to Reproduce

I have printed this line art file that I created with my own hacked script from gcodetools:
antonio_grass__0001.gcode.zip

The issue only happens with G02/G03 commands, it is confirmed not to be an issue with G01 commands. As you can see, the values in the GCODE file are rounded to 2 decimals, I tried to make them as small as possible but it didn't help. Also, I have tried to see if a 32-bit controller made any difference, but it didn't. So there must be something going on in the firmware causing this behaviour.

I expect the speeds to be consistent so, move speeds of Z should be F3000 and draw speeds of XY should be F2400, which is correct as you see in the file. I expect jitter and S_CURVE speeds to be consistent with the acceleration settings set in the config. Not to speed up and slow down as much as you see in the video.

See the video for the actual behaviour (as mentioned before):
https://www.youtube.com/watch?v=-qqhaZyxPzU

Additional Information

This issue has been persistent since I started using bugfix versions in May/June 2019. I have not found a single version of MarlinFW that worked correctly with G02/G03 commands and I would love to find a fix for this. I am using a pen plotter, but I can imagine these issues will have severe consequences for CNC routers as the bits cannot handle this stress of suddenly speeding up.

Since more and more people are using Marlin for projects that are not directly related to 3D printing, but also for CNC, pen plotter and laser cutter projects, I think it is worth it to investigate what the issue is and how this can be fixed. I am willing to experiment a bit so if anyone has a solution of what I can try, let me know.

Thanks for your time and energy and this wonderful piece of firmware.

ps
A small side-question would be if it is possible to make Marlin lightweight for non 3D-printer machines with custom configs since a pen plotter doesn't need temperature control for example. I am using D8 for case fan control when the steppers are turned on. Also, the entire bed leveling part can be discarded when working with a pen plotter.

[ellensp]

I've seen something like this before #15295

ie they recommend to change G2_G3.cpp#L219 and G2_G3.cpp#L239 and replace seg_length with 0.0

Can you give this a try?

[Ohmarinus]

I've seen something like this before #15295

ie they recommend to change G2_G3.cpp#L219 and G2_G3.cpp#L239 and replace seg_length with 0.0

Can you give this a try?

Oh my god, this was the solution.. I cannot believe it was this simple. I have rebuilt the machine's controller at least ten times the past few months to try out every config there is. Many people said it was the 8-bit controller bugging out, but I couldn't believe it. Countless hours of trying to fix this and it was only two values in the MarlinFW that needed change.

To prove I made a video and I will link this thread under the other thread as well!

The resolution has improved greatly as well. I think it only needs a little bit of extra tuning to remove jitter as some lines are not round or straight but wobbly around mostly the starting point somehow. See it working here:
https://www.youtube.com/watch?v=KbKefd_6oGs

[thinkyhead]

Interesting! The difference is that instead of the arc loop providing the seg_length value the length will be calculated inside of planner.buffer_line. So, the final result won't actually be 0.0.

So we just need to compare these length calculations and correct them. If we get the seg_length right here then it saves on calculation in the planner.

[thinkyhead]

On a second look, I think 0.0 is the right value at line 239. So let's make that the patch.

[FormerLurker]

You may need to reopen this. The segment interpolation loop is also using an incorrect segment length in some cases. These lines:

Marlin/Marlin/src/gcode/motion/G2_G3.cpp

Lines 117 to 118 in e0e87ca

	uint16_t segments = FLOOR(mm_of_travel / seg_length);
	NOLESS(segments, min_segments);

Will cause the length to be incorrect due to the floor and noless calculations. The segment length will need to be calculated after the number of segments is finalized, once upon entering the loop (if it is entered) and once at the end for the final segment.

You could use hypot in the for loop where i = 1 on the initial and destination coordinates, then reuse that value for all but the final segment. The same thing could be done for the final segment since the small angle approx of sin and cos mean it won't always be the same length. Unfortunately, multiplying the radius and theta would give the length of the arc, not the segment, though the difference is slight unless theta is large.

This may belong in a separate issue (I will open if you want), but I figured I'd mention it here. I'm seeing some poorly interpolated segments using the small angle approximation for arcs of a small radius (enough to cause some overlap problems that result in an unsightly bulge in the middle of a circle). Could this deviation also result in each segment within the loop having a slightly different length?

I've been testing getting rid of N_ARC_CORRECTION and using actual cos and sin functions to calculate sin_T and cos_T before the loop (only if segments > 1, and only calculated one time per call to G2_G3), and have been getting much better results without any additional slowdown. However, I'm running my modifications on an older fork using a Rambo 1.3, so not sure how broadly that applies. If the small angle approximation can be removed without too much negative impact, you might get an accurate enough segment length to use for the final segment too (provided floating point issues don't accumulate too quickly). I realize there is a tradeoff here between getting the first segment in the planner faster and reducing the total number of calculations, so I'm not sure what works best in the general case. I will continue testing this.

Also, if you would like some additional test gcode containing G2/G3, let me know. I have loads.

Thank you very much!

[Ohmarinus]

@FormerLurker I think indeed this could use a new issue however, the issue might be that these issues influence each other so it's good you mention it here. I think that the arc G02/G03 feature definitely could use some more testing and improvement. Of course the main focus of Marlin is 3D printing, but it's a great piece of firmware and the versatility is a great addition to XY-related machines. I see more and more people building and buying pen plotters and CNC machines for hobby use and Marlin is very valuable in some of these projects because it allows G02/G03 gcode to be used.

I hope that the devs also see the value of this and keep developing the arc support. I'm not sure if I should reopen this issue, I'm not an experienced Github user and am not very knowledgeable etiquette-wise.

[FormerLurker]

@Ohmarinus, I posted because here because I received some issues in my arc welder plugin repository related to slowdowns while printing arcs. Though the original issue was regarding Prusa's fork, later posters were using Marlin, and one of the users also noticed the same slowdowns while printing G2/G3. I suggested the seg_length fix that was committed recently, but this user already had the fix on line 233 applied. After changing seg_length to 0 on line 216, the issue went away, which is why I posted here.

Regarding etiquette, I also am not all that knowledgeable, and each repo has its own culture. I would gladly open a new issue if that is considered better here (perhaps it is since your particular issue was solved), but posted here because the information directly related to this issue, and to code changes mentioned here, especially since @ellensp mentioned this exact code change on line 216 that solved the problem for the user in my issues repo. Just trying to share that info here.

[thinkyhead]

See if this simple correction helps:

  uint16_t segments = FLOOR(mm_of_travel / seg_length);
- NOLESS(segments, min_segments);
+ if (segments < min_segments) {
+   segments = min_segments;
+   seg_length = mm_of_travel / segments;
+ }

The last segment already passes 0 so the planner calculates the true length of that one.

[FormerLurker]

Will do, thanks @thinkyhead! If I see any further problems I will create a new issue.

[eyal0]

@thinkyhead That solution is not sufficient. The line seg_length = mm_of_travel / segments; must be after the closing brace. I will test a solution and provide it. #18141

[eyal0]

Tested that and it, too, was not sufficient. I'll keep testing.

[eyal0]

Ah, I see the issue. The problem is that plan_arc uses a small-angle approximation for the values of sine and cosine. sin(x) is approximately x and cos(x) is approximately 1-0.5*sq(x). Once every N_ARC_CORRECTION steps in the arc, the code will just do a real sine and cosine calculation to get the right answer. So long as that number isn't too big (it's 25 by default) and so long as the angles are small enough, this is a close enough approximation. But that means that the length of the segment changes each time. Grr!

The whole point of using the approximation is so that the Arduino can do fewer cosine and sine calculations, which take a while. But if we pass 0 into the buffer_line function each time then we'll be doing one extra sqrt() each time. That's not great either! From this post, it seems that sqrt() is about twice as fast as sine and cosine so one sqrt each time surely beats one each of cosine and sine each time! But still, can we do better? I'll keep trying.

[eyal0]

Yes, we can at least do a little better. The segment length is assumed to be the length of the arc divided by the number of seconds. But that's wrong, that's computing the length of an arc. What we really want is the length of a chord. That is, the perimeter of a a regular polygon is not the same as the circumference of a circle.

This also needs fixing. If you want to draw an arc and approximate it with segments, you don't necessarily want those segments to be inside the arc. You don't want a circumscribed regular polygon. You also don't want the opposite. You want something in the middle.

[FormerLurker]

@eyal0,

The problem is that plan_arc uses a small-angle approximation for the values of sine and cosine

I've noticed VERY large deviations from the actual arc when using the small angle approximations that can even cause the extruded filament to overlap with other lines already drawn. I have been testing using actual sin and cos instead of the small angle approximation, and the placement of the line segments ends up matching the actual arc nearly exactly. The correction in the interpolation loop seems completely unnecessary if actual sin/cos is calculated in all situations I've tested since the rounding error is so small. I've not noticed any difference in performance, but this may not be universal.

What we really want is the length of a chord

I've done a lot of testing on this, and the chord length is usually very very close to the arc length. At one point I added extrusion corrections added to arc welder to account for the difference between chord and arc length. The differences even on files that only contained arcs was a small fraction of a percent. However, in cases where there aren't enough segments to really approximate the arc (like a single segment of 1mm for semi-circle), this could be quite important. Since all this is dependent on the interpolation settings used, it is probably a good idea to take care of this.

Regarding the half-circumscribed and half-inscribed arc, that is an interesting idea. You could not do this with a single segment, unless I am missing something. I'm guessing you'd want to equalize the area above and below the arc somehow.

[eyal0]

I have been testing using actual sin and cos instead of the small angle approximation, and the placement of the line segments ends up matching the actual arc nearly exactly.

Yup, you can get that by just settings N_ARC_CORRECTION to 0 in the Configuration_adv.h file.

Regarding the half-circumscribed and half-inscribed arc,

Right, so here's what you'd do. First, you'd find a shape that has the same perimeter as the original arc. So look as this shape, where the path that you want to take is some part of that circle.

Then you need to set up your start point so that it will be where the perimeter and the circumference cross. Finally, you travel around the perimeter to your destination. The very first and last segments will be shorter than normal but the rest will be full length. I'll try to see if it can be done without adding too much calculation overhead.

[FormerLurker]

Interesting idea regarding the half-inscribed circle (not sure if there is a technical term for this)!

Yup, you can get that by just settings N_ARC_CORRECTION to 0 in the Configuration_adv.h file.

Not exactly, because that would trigger a sin and cos for every segment. I'm suggesting we replace the small angle approximation with the actual sin and cos, then ditch N_ARC_CORRECTIONS (or just make it really really high).

[eyal0]

Ah, I see. The point of that is to avoid doing a sin and cosine upfront because potentially those can take a while and the printer will stall while waiting for the answer. So what they want to do instead is use the approximation first, get a few segments (25) queued up, and then do the sine and cosine, which take a little longer.

According to the link that I sent above, a cosine is only 10 times slower than a regular addition and we're doing a bunch of those already so it seems like it shouldn't be so big a deal to just do the cosine and sine upfront. But I haven't measured, maybe it did cause a problem.

I'll keep thinking about it. I have some ideas!

[eyal0]

It's pretty hard to get something both accurate and fast. We could do a perfect job of cutting the arc up into a polygon but it would require extra sine and cosines and the comment there says that it would take too long.

If the problem is the approximation for sine and cosine, you could extend those by a term. It's a MacLaurin series. So sin(x) is about x - x³/6 and cos(x) could be 1 - x²/2 + x⁴/24.

[FormerLurker]

I figured that polygon would be difficult to calculate. Perhaps this is not really our problem to solve? Instead, this could be the responsibility of the slicer or post-processor. We can assume (especially since only one product does this today) that the arcs are intended to be transformed into inscribed segments, and just forget about that one for now? Alternatively, another arc interpolation define could be added so that faster hardware can use better approximations?

Another option is something I implemented, but removed because it didn't seem to make a difference on my hardware: Use actual sin and cosine if the angle is not small, as defined by some constant. The real problem here is that the error of the small angle approximation grows quite fast as the angle increases. I don't think it would be difficult to calculate an angle that would guarantee so many digits of precision (say the precision of a float in Arduino). If the extra term improves accuracy and is faster than sin/cos, we could use another cutoff and use the extra term for that interval, then finally use sin and cos. This would cost a few less than compares with theta, but that shouldn't be too bad.

Ideas?

Edit: It looks like the divergence for the small angle approximation for sin is different from cos, and sin diverges much faster. I will see what constants it takes to get at least 6 digits of precision for each.

[eyal0]

The Maclaurin series is here: https://blogs.ubc.ca/infiniteseriesmodule/units/unit-3-power-series/taylor-series/maclaurin-expansion-of-sinx/

sin(x) = x - x³/6 + x⁵/120 - ...

So if you keep going until the denominator is 6 digits long, you're done. But at some point, it's just faster to compute the real sin!

I like your idea of making the preprocessor do it. That's not too bad.

[eyal0]

Just adding one more term to sin_T made a difference. Using the same 3D benchy as before, with N_ARC_CORRECTION set to 7, here's the length of the little segments making up an arc:

length 1.001972
length 1.001972
length 1.001970
length 1.001972
length 1.001984
length 1.001970
length 0.998226
length 1.001962
length 1.001976
length 1.001966
length 1.001983
length 1.001973
length 1.001981
length 0.998217
length 1.001971
length 1.001971
length 1.001970
length 1.001977
length 1.001970
length 0.998754 vs 1.001568

Notice how after 7 of them, it has to correct for the error. Now I'll change:

sin_T = theta_per_segment,

to:

sin_T = theta_per_segment - theta_per_segment*theta_per_segment*theta_per_segment/6,

Everything else the same, here are the segments now:

ength 1.001438
length 1.001427
length 1.001434
length 1.001428
length 1.001438
length 1.001432
length 1.001432
length 1.001435
length 1.001423
length 1.001440
length 1.001425
length 1.001433
length 1.001434
length 1.001432
length 1.001437
length 1.001427
length 1.001430
length 1.001435
length 1.001431
length 1.001425

See how they're much more uniform? Not bad and a pretty easy fix. Just two more multiplications and one division.

[FormerLurker]

I like your idea of making the preprocessor do it. That's not too bad.

Arc Welder already does this, at least within the resolution. Turns out it's the easiest way to go!

Regarding the table, that sin approximation REALLY made a huge difference. I may try the same thing without the extra cos term just to see what happens.

From your table, I calculate a max deviation of 0.0009% from the average, which is excellent! That's way beyond what is required to calculate acceleration, no?

I think it would be interesting to run a monte carlo simulation for different sized arcs and segment lengths and see what the worst case scenario is regarding deviation from true sin/cos. If the results are similar to the above, I'd say it's smooth sailing and case closed on the approximation of sin/cos. Great work!

[AnHardt]

The real problem behind this is: Marlin can't draw arcs.
As long as we need to split arcs into a lot of straight segments, whatever algorithm we use for that, it would better be done in the slicer/cam.
How about drawing real arcs (as far steppers can do that)?
How about bresenham for ellipses or something like that? (Setting this up is ugly. 8 Oktants, complicated start and end conditions. Stepping it, needs a stepper interrupt extension. ... But it would draw circles!)

[eyal0]

As long as we need to split arcs into a lot of straight segments, whatever algorithm we use for that, it would better be done in the slicer/cam

The reason not to do it in the Slicer is that it makes the gcode file much longer. For cases where the buffer size on the printer is limited or time to read from disk is limited, this is a problem!

Marlin's solution is not bad: Draw a series of segments. Marlin is limited in processing power so doing a good calculation need to be weighed against the processor time.

Bresenham is for a grid but we're not fitting to a grid. So I think that it isn't relevant. I think that fitting a polygon within the circle is not bad, though fitting a polygon on the circle would be a better fit but with a good bit more math, like maybe 2-3 more sin/cos operations.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.