Summary: | Fix building of GLSL demos which use M_PI when M_PI is not defined by math.h | ||
---|---|---|---|
Product: | Mesa | Reporter: | Jon Turney <jon.turney> |
Component: | Demos | Assignee: | mesa-dev |
Status: | RESOLVED MOVED | QA Contact: | |
Severity: | minor | ||
Priority: | medium | ||
Version: | git | ||
Hardware: | x86 (IA32) | ||
OS: | Cygwin | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
Patch to fix building of GLSL demos which use M_PI when M_PI is not defined by math.h
Remove the various definitions of M_PI, except in compiler.h Add an autoconf check that math.h defines M_PI Add an autoconf check that math.h defines M_PI Remove the various definitions of M_PI, except in compiler.h Remove the various definitions of M_PI |
Description
Jon Turney
2009-10-05 06:16:28 UTC
Committed to the 7.6 branch. Thanks. NAK!!! The only systems where I have seen M_PI not defined is MSVC. On MSVC it is not defined /by default/, but it can be enabled by defining _USE_MATH_DEFINES. It would be much better to define _USE_MATH_DEFINES from the build system than pollute the code with this cruft. (In reply to comment #2) > NAK!!! The only systems where I have seen M_PI not defined is MSVC. On MSVC > it is not defined /by default/, but it can be enabled by defining > _USE_MATH_DEFINES. It would be much better to define _USE_MATH_DEFINES from > the build system than pollute the code with this cruft. I'm building with gcc under Cygwin. Your point is well made, but... $ grep -r "#define M_PI" * | wc -l 48 Just a historical note: I recall needing the #ifndef M_PI stuff many years ago on some Unix variants (HP-UX?), before Mesa was running on Windows. If you want to revert the patch and fix the compile flags, that's OK w/ me. As Jon points out, a lot of other demos do this too. I'll see if I can cook up a patch to do this 'properly' (or at least 'better') For the compiler/header combination I have, -std=c99 -U__STRICT_ANSI__ seems to be needed to let math.h defined M_PI bugzilla-daemon@freedesktop.org writes: > http://bugs.freedesktop.org/show_bug.cgi?id=24318 > > --- Comment #2 from Ian Romanick <idr@freedesktop.org> 2009-10-05 > > NAK!!! The only systems where I have seen M_PI not defined is > MSVC. On MSVC it is not defined /by default/, but it can be enabled > by defining _USE_MATH_DEFINES. It would be much better to define > _USE_MATH_DEFINES from the build system than pollute the code with > this cruft. Not that using _USE_MATH_DEFINES sounds like a bad idea, but I don't see M_PI mentioned in the n1336 draft. I'm not sure where I got n1336, but probably somewhere here: http://www.open-std.org/jtc1/sc22/wg14/ Thus, I would argue that *both* this and the build system define should be used. -tom Reopening so I don't forget about this IIRC when compiling with gcc -ansi, M_PI isn't defined. In my math.h, its definition is wrapped in '#if defined __USE_BSD || defined __USE_XOPEN' Created attachment 30659 [details] [review] Remove the various definitions of M_PI, except in compiler.h Created attachment 30660 [details] [review] Add an autoconf check that math.h defines M_PI Attached a couple of patches with an effort to do this better. As written, this falls back to explicitly defining M_PI using -D o the compiler command line if it can't make math.h define it. This probably isn't a good idea. Probably it should do nothing, but this probably breaks stuff on some targets which should include compiler.h but doesn't? Visual C projects will need updating to use _USE_MATH_DEFINES. I have assumed that the precision of the value of M_PI is not significant, as it varies from place to place, which should be the case anyhow, as these values are only a fall-back (In reply to comment #8) > IIRC when compiling with gcc -ansi, M_PI isn't defined. > > In my math.h, its definition is wrapped in '#if defined __USE_BSD || defined > __USE_XOPEN' I have the same. I believe that -U__STRICT_ANSI__ is the way to access these definitions if you want to use -ansi, but this may be incorrect. Created attachment 35298 [details] [review] Add an autoconf check that math.h defines M_PI Created attachment 35299 [details] [review] Remove the various definitions of M_PI, except in compiler.h (In reply to comment #11) > Attached a couple of patches with an effort to do this better. Refreshed patches for git master > As written, this falls back to explicitly defining M_PI using -D o the compiler > command line if it can't make math.h define it. This probably isn't a good > idea. Changed this so ./configure will fail with an error if we can't work out how to get math.h to define M_PI Created attachment 37144 [details] [review] Remove the various definitions of M_PI Updated patch for separate mesa-demos git respository. The autoconf changes don't seem to be required anymore as the gcc flag -ansi is no longer used, so M_PI is defined. -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/demos/issues/2. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.