Bug 146479 - Use basegfx fTools for converting the angle unit
Summary: Use basegfx fTools for converting the angle unit
Status: NEW
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: target:7.4.0 reviewed:2022 target:7.5.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2021-12-29 21:15 UTC by Hossein
Modified: 2022-11-25 00:42 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-12-29 21:15:23 UTC
There are many places in the code that converting the angle unit is needed. Either from radians (rad) to degrees (deg), or vice versa. Some of these unit conversions are now done using direct usage of arithmetic operations (*/). A better approach would be using basegfx fTools methods. See 'include/basegfx/numeric/ftools.hxx' in the LibreOffice source code.

For example, consider this expression:

    3.14159265359/180.0*fAngle

One can easily write it in a better form as:

    basegfx::deg2rad(fAngle)

There are even other interesting usages of this function:

    double fRad = basegfx::deg2rad(nMSORotationAngle / 60000.0)

can be written as:

    double fRad = basegfx::deg2rad<60000>(nMSORotationAngle);

See these example commits:

https://git.libreoffice.org/core/+/b0ce4e848f786a2f452a6a6e426df4b95af4da3c%5E%21/

https://git.libreoffice.org/core/+/7ff2c9cfc5fa8c261b1f7f959172f60255fcf617%5E%21/

This search provides some instances of the issue, although not all of the results are related:

    git grep -F 3.14 *.cxx

A more restricted search can be:

    git grep -F 3.14 *.cxx | grep 180

Please also take a look at bug 145759.
Comment 1 Commit Notification 2021-12-29 21:27:42 UTC
Ramreiso Kashung committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8a044b089e1ec3da3a3bc7f1a18555f7e19a4fd0

tdf#146479 - Use basegfx fTools for converting the angle unit

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 2 Commit Notification 2022-01-06 23:20:55 UTC
VaibhavMalik4187 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d3308503a8c320b4a1c3f6156a6b7e6a71a5806d

tdf#146479 - Use basegfx fTools for converting the angle unit

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 3 Hossein 2022-02-08 12:21:34 UTC
A related blog post:

Use basegfx to convert angle unit - EasyHack
https://dev.blog.documentfoundation.org/2021/12/31/use-basegfx-to-convert-angle-unit-easyhack/
Comment 4 Hossein 2022-06-16 13:41:57 UTC
Re-evaluating the EasyHack in 2022

This is still relevant, as there are many places in the code that needs change. For example, see these results:

git grep -F "/ 180.0" *.cxx
Comment 5 Commit Notification 2022-11-24 18:24:12 UTC
Leonid Ryzhov committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6379414ca34527fbe69df2035d49d651655317cd

tdf#146479 Use basegfx fTools for converting the angle unit

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 BogdanB 2022-11-24 19:35:29 UTC
(In reply to Hossein from comment #4)
> Re-evaluating the EasyHack in 2022
> 
> This is still relevant, as there are many places in the code that needs
> change. For example, see these results:
> 
> git grep -F "/ 180.0" *.cxx

Hossein, with today core I found just one line needing changes. You have mentioned many places. Is the search term wrong?
sc/source/core/opencl/op_math.cxx:    ss << "    return arg0 * M_PI / 180.0;\n";
Comment 7 Hossein 2022-11-25 00:42:51 UTC
(In reply to BogdanB from comment #6)
> (In reply to Hossein from comment #4)
> > Re-evaluating the EasyHack in 2022
> > 
> > This is still relevant, as there are many places in the code that needs
> > change. For example, see these results:
> > 
> > git grep -F "/ 180.0" *.cxx
> 
> Hossein, with today core I found just one line needing changes. You have
> mentioned many places. Is the search term wrong?
> sc/source/core/opencl/op_math.cxx:    ss << "    return arg0 * M_PI /
> 180.0;\n";
I don't exactly remember the number of instances 5 month ago, but that 1 instance might be enough for you to experiment changing the code. This is the main purpose of the EasyHacks.

After you have done 1 or 2 instance of each EasyHack, you should find other issues to work on. This is the path that I suggest:
https://wiki.documentfoundation.org/Development/GetInvolved#Roadmap_for_personal_growth