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.

Sign up for GitHub

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

Jump to bottom

Define proper and portable constants #151

Closed
hollasch opened this issue Aug 17, 2019 · 4 comments
Closed

Define proper and portable constants #151

hollasch opened this issue Aug 17, 2019 · 4 comments
Assignees
@ronaldfw
Labels
area: code book 1: One Weekend
Milestone
v3

Comments

@hollasch
Copy link
Collaborator

The current code base uses M_PI and MAXFLOAT. Both of these should be simple C++ constants defined somewhere. This may induce the need for a new common header (constants.h?).

Also note that MAXFLOAT, in addition to not being portable, also defines the maximum representable finite floating-point value. It should really be replaced with infinity (of the appropriate precision).
@hollasch hollasch self-assigned this Aug 17, 2019
vchizhov referenced this issue in vchizhov/InOneWeekend-1 Aug 23, 2019
@vchizhov
Addressed issues: #65, #67, #69
ecc5663 
Compiles with no warnings on MSVC 2019 with Warning Level: /W4.

- provided CMakeLists.txt (tested only on Windows 10 with MSVC 2019 though, create the project in /build so that it would be ignored by the gitignore)
- changed M_PI with a constexpr pi<T>() function
- substituted random_double() and drand48() with my own random() function using the Mersenne twister from the standard library
- use fstream rather than cout, so that redirecting would not be required
- fixed double/float mismatch

Removed random.h since it is redundant - I substituted random_double() with my own random() implementation using the Mersenne twister from the standard library. The random_double() in random.h used rand() % (RAND_MAX + 1.0); which is suboptimal (fails multiple statistical tests).
@hollasch hollasch transferred this issue from RayTracing/InOneWeekend Aug 30, 2019
@hollasch hollasch added this to the post-v2 milestone Aug 30, 2019
@ronaldfw
Copy link
Contributor

@hollasch Mind if I take this? I will try to make all code compile and run on Windows/Linux/Mac and I think this is part of that.

I'll assign to myself, but please reassign if you have already started or want to work on this.

@ronaldfw ronaldfw assigned ronaldfw and unassigned hollasch Sep 24, 2019
@hollasch
Copy link
Collaborator Author

Sweet — thanks Ronald!

@ronaldfw
Copy link
Contributor

Fixed on the future branch.

@hollasch
Copy link
Collaborator Author

Now in the development branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ronaldfw ronaldfw

Labels
area: code book 1: One Weekend
Projects
None yet
Milestone
v3
Development

No branches or pull requests
2 participants
@hollasch @ronaldfw