RESOLVED FIXED 15076
deg2rad has multiple definitions 
https://bugs.webkit.org/show_bug.cgi?id=15076
Summary deg2rad has multiple definitions
Rob Buis
Reported 2007-08-25 08:09:01 PDT
deg2rad constant is used in multiple places, it would be nice IMHO to have one shared contant.
Attachments
First attempt (8.58 KB, patch)
2007-08-25 08:14 PDT, Rob Buis 		mjs: review-
DetailsFormatted DiffDiff
Proposed patch (10.04 KB, patch)
2007-10-09 23:58 PDT, Andrew Wellington 		darin: review-
DetailsFormatted DiffDiff
Proposed patch 2 (10.43 KB, patch)
2007-10-10 17:52 PDT, Andrew Wellington 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2007-08-25 08:14:22 PDT
Created attachment 16118 [details] First attempt I think MathExtras.h is the best place for reuse, but am open to other suggestions :) Cheers, Rob.
Nikolas Zimmermann
Comment 2 2007-08-26 04:18:17 PDT
Heya Rob, sounds like a nice attempt to unify these. Some comments: - I'd rather define two inlined deg2rad / rad2deg functions instead of exposing a #define'd constant. and let all callers use these. - Maybe adding deg2grad, grad2deg is also a good idea, while you're at it. (at least SVG needs it) Greetings, Niko
Maciej Stachowiak
Comment 3 2007-08-28 21:22:56 PDT
At the very least, the constant should be defined as a static const double instead of a #define. It does seem like the inline function would read better if used everywhere.
Maciej Stachowiak
Comment 4 2007-08-28 21:23:19 PDT
Comment on attachment 16118 [details] First attempt r- to consider me and Niko's comments, but otherwise looks great.
Andrew Wellington
Comment 5 2007-10-09 23:58:29 PDT
Created attachment 16609 [details] Proposed patch Proposed patch based on Rob's original patch.
Darin Adler
Comment 6 2007-10-10 07:44:58 PDT
Comment on attachment 16609 [details] Proposed patch This patch adds conversions to/from double in code that was previously dealing entirely with floats. That will make the code slower. Instead there should be overloaded versions of these functions for both float and double.
Andrew Wellington
Comment 7 2007-10-10 17:52:50 PDT
Created attachment 16621 [details] Proposed patch 2 Now with float versions as well
Eric Seidel (no email)
Comment 8 2007-10-11 18:42:00 PDT
Comment on attachment 16621 [details] Proposed patch 2 This looks great to me.
Andrew Wellington
Comment 9 2007-10-11 18:58:45 PDT
Landed in r26378
Note You need to log in before you can comment on or make changes to this bug.