Bug 145630 - Use atan2 function instead of atan
Summary: Use atan2 function instead of atan
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: reviewed:2022
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2021-11-11 15:04 UTC by Hossein
Modified: 2022-09-13 10:44 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hossein 2021-11-11 15:04:25 UTC
If you use atan(x) to calculate the output θ, you have to think about different situations that may occur, because with the same value of x, there are multiple values of θ, and it would be your responsibility to handle that, and find out what is the suitable value of θ.

But, if you have two values y and x, and you want to calculate atan(y / x), there is a better choice: using atan2(y, x); atan2(y, x) can handle the values from all the 4 different quadrants:

https://en.wikipedia.org/wiki/Atan2

Instances of the similar situations can be found by invoking:

git grep atan\(

Within the results of the grep, you can find places that atan2() is better suited. You should take care about the conditions that may be present (or missing) after the atan() in which check the signature of the  and make sure that the code works correctly after removing them.
Comment 1 Hossein 2021-11-17 15:42:26 UTC
Blog post related to this EasyHack in the LibreOffice Development Blog:
https://dev.blog.documentfoundation.org/2021/11/17/use-atan2-function-instead-of-atan-easyhack/

Example commit (hwpfilter/source/hcode.cxx)
https://git.libreoffice.org/core/+/35c85effecb5a615a361c1b7d92d08447bc83423%5E!/
Comment 2 Arjun Badola 2022-01-26 09:34:55 UTC
I'm working on this
Comment 3 Hossein 2022-01-29 10:48:06 UTC
(In reply to Arjun Badola from comment #2)
> I'm working on this
This is multi-hacker, so please just work on it, and don't assign it to yourself.
Comment 4 Hossein 2022-06-16 13:35:34 UTC
Re-evaluating the EasyHack in 2022

This enhancement is still relevant. There are remaining atan invocations where atan2 is better suited.
Comment 5 Maciej Sikorski 2022-07-13 10:59:33 UTC
I have fixed some of it. https://gerrit.libreoffice.org/c/core/+/137012
Comment 6 Hossein 2022-09-13 10:44:12 UTC
I am closing down this issue, as the remaining instances need very careful checks to make sure changing atan -> atan2 actually works, and not leads to incorrect behavior change.