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

Avoid division in JD calc, when possible #17945

Merged
merged 1 commit into from
May 11, 2020

Conversation

XDA-Bam
Copy link
Contributor

@XDA-Bam XDA-Bam commented May 10, 2020

Description

There's no need for a division, if the axis is not limiting. This solution typically requires three to four multiplications instead of three to four divisions. In the worst case (all axis limiting in descending order), four divisions + four multiplications can be necessary, but that should be exceedingly rare.

Technically, the if (unit_vec[idx]) { check is not required anymore, since a divide by zero is avoided by limit_value * ABS(unit_vec[idx]) > settings.max_acceleration_mm_per_s2[idx]. I kept the check nonetheless, to avoid another multiplication when possible.

Benefits

Typically saves three to four divides per block planned and only requires three to four multiplications in return.

Related Issues

None.

There's no need for a division, if the axis is not limiting. This solution typically requires four multiplications instead of four divisions. In the worst case (all axis limiting in descending order), four divisions + four multiplications can be necessary, but that should be exceedingly rare.
@thinkyhead
Copy link
Member

Now, to just make those curves smoother somehow…

@thinkyhead thinkyhead merged commit e22e076 into MarlinFirmware:bugfix-2.0.x May 11, 2020
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request May 29, 2020
jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
njibhu pushed a commit to njibhu/Marlin that referenced this pull request Aug 24, 2020
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants