Skip to content

Commit 1f3920e

Browse files
committed
MSVC warning-clean pass
Bumping up warnings to max with explicit exceptions for the Microsoft Visual C++ compiler. Found a number of issues: - Some simple signed/unsigned mismatches - Some unused function parameters. I've disabled this warning generally, as we often have virtual functions that may not use some parameters that other derived classes do. Still this found some functions that _never_ use some parameters. - Found a member variable data shadowing issue in quad::pdf_value(). Renamed parameters `o` and `v` to `origin` and `direction` for better clarity and to fix the shadowing. - Found what looks remnants of earlier code versions in cos_cubed.cc. Ensured that the book never develops the unnecessary code, so deleted the unnecessary parts. - Fixed some minor whitespace issues.
1 parent 3856245 commit 1f3920e

File tree

9 files changed

+55
-40
lines changed

9 files changed

+55
-40
lines changed

CMakeLists.txt

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,26 @@ include_directories(src)
9595
message (STATUS "Compiler ID: " ${CMAKE_CXX_COMPILER_ID})
9696

9797
if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
98-
add_compile_options("/we 4265") # Class has virtual functions, but its non-trivial destructor is not virtual
98+
# /we #### - treat warning as error
99+
# /wd #### - Disable warning
100+
# /w3 #### - set warning to level 3
101+
add_compile_options("/Wall") # Enable all warnings
99102
add_compile_options("/w3 5038") # Data member will be initialized after [other] data member
103+
add_compile_options("/we 4265") # Class has virtual functions, but its non-trivial destructor is not virtual
100104
add_compile_options("/we 5204") # Class has virtual functions, but its trivial destructor is not virtual
105+
add_compile_options("/wd 4100") # unreferenced formal parameter
106+
add_compile_options("/wd 4365") # conversion from 'size_t' to 'const int __int64', signed/unsigned mismatch
107+
add_compile_options("/wd 4457") # index X range checked by comparison on this line
108+
add_compile_options("/wd 4464") # feeds call on this line
109+
add_compile_options("/wd 4514") # Ureferenced inline function has been removed
110+
add_compile_options("/wd 4626") # assignment operator was implicitly defined as deleted
111+
add_compile_options("/wd 4710") # Function not inlined
112+
add_compile_options("/wd 4711") # Function selected for automatic inline expansion
113+
add_compile_options("/wd 4820") # N bytes padding added after data member
114+
add_compile_options("/wd 5027") # move assignment operator was implicitly defined as deleted
115+
add_compile_options("/wd 5045") # will insert Spectre mitigation for memory load if /Qspectre switch specified
116+
add_compile_options("/wd 5219") # implicit conversion from 'unsigned __int64' to 'double', possible loss of data
117+
add_compile_options("/wd 5267") # definition of implicit assignment operator is deprecated; has user-provided dtor
101118
elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
102119
add_compile_options(-Wnon-virtual-dtor) # Class has virtual functions, but its destructor is not virtual
103120
add_compile_options(-Wreorder) # Data member will be initialized after [other] data member

books/RayTracingTheNextWeek.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2742,13 +2742,13 @@
27422742
private:
27432743
point3 Q;
27442744
vec3 u, v;
2745+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
2746+
vec3 w;
2747+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
27452748
shared_ptr<material> mat;
27462749
aabb bbox;
27472750
vec3 normal;
27482751
double D;
2749-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
2750-
vec3 w;
2751-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
27522752
};
27532753
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
27542754
[Listing [quad-w]: <kbd>[quad.h]</kbd> Caching the quadrilateral's w value]

books/RayTracingTheRestOfYourLife.html

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -904,12 +904,12 @@
904904
}
905905

906906
int main() {
907-
int N = 10000;
907+
unsigned int N = 10000;
908908
double sum = 0.0;
909909

910910
// iterate through all of our samples
911911
std::vector<sample> samples;
912-
for (int i = 0; i < N; i++) {
912+
for (unsigned int i = 0; i < N; i++) {
913913
// Get the area under the curve
914914
auto x = random_double(0, 2*pi);
915915
auto sin_x = sin(x);
@@ -927,9 +927,9 @@
927927
double half_sum = sum / 2.0;
928928
double halfway_point = 0.0;
929929
double accum = 0.0;
930-
for (int i = 0; i < N; i++){
930+
for (unsigned int i = 0; i < N; i++){
931931
accum += samples[i].p_x;
932-
if (accum >= half_sum){
932+
if (accum >= half_sum) {
933933
halfway_point = samples[i].x;
934934
break;
935935
}
@@ -1991,15 +1991,15 @@
19911991
#include <iomanip>
19921992
#include <math.h>
19931993

1994-
double f(double r1, double r2) {
1994+
double f(double r2) {
19951995
// auto x = cos(2*pi*r1)*2*sqrt(r2*(1-r2));
19961996
// auto y = sin(2*pi*r1)*2*sqrt(r2*(1-r2));
19971997
auto z = 1 - r2;
19981998
double cos_theta = z;
19991999
return cos_theta*cos_theta*cos_theta;
20002000
}
20012001

2002-
double pdf(double r1, double r2) {
2002+
double pdf() {
20032003
return 1.0 / (2.0*pi);
20042004
}
20052005

@@ -2008,9 +2008,8 @@
20082008

20092009
auto sum = 0.0;
20102010
for (int i = 0; i < N; i++) {
2011-
auto r1 = random_double();
20122011
auto r2 = random_double();
2013-
sum += f(r1, r2) / pdf(r1, r2);
2012+
sum += f(r2) / pdf();
20142013
}
20152014

20162015
std::cout << std::fixed << std::setprecision(12);
@@ -2706,7 +2705,7 @@
27062705

27072706

27082707
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
2709-
virtual double pdf_value(const point3& o, const vec3& v) const {
2708+
virtual double pdf_value(const point3& origin, const vec3& direction) const {
27102709
return 0.0;
27112710
}
27122711

@@ -2743,13 +2742,13 @@
27432742

27442743

27452744
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
2746-
double pdf_value(const point3& origin, const vec3& v) const override {
2745+
double pdf_value(const point3& origin, const vec3& direction) const override {
27472746
hit_record rec;
2748-
if (!this->hit(ray(origin, v), interval(0.001, infinity), rec))
2747+
if (!this->hit(ray(origin, direction), interval(0.001, infinity), rec))
27492748
return 0;
27502749

2751-
auto distance_squared = rec.t * rec.t * v.length_squared();
2752-
auto cosine = fabs(dot(v, rec.normal) / v.length());
2750+
auto distance_squared = rec.t * rec.t * direction.length_squared();
2751+
auto cosine = fabs(dot(direction, rec.normal) / direction.length());
27532752

27542753
return distance_squared / (cosine * area);
27552754
}
@@ -2763,11 +2762,11 @@
27632762
private:
27642763
point3 Q;
27652764
vec3 u, v;
2765+
vec3 w;
27662766
shared_ptr<material> mat;
27672767
aabb bbox;
27682768
vec3 normal;
27692769
double D;
2770-
vec3 w;
27712770
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
27722771
double area;
27732772
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
@@ -3484,14 +3483,14 @@
34843483

34853484

34863485
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
3487-
double pdf_value(const point3& o, const vec3& v) const override {
3486+
double pdf_value(const point3& origin, const vec3& direction) const override {
34883487
// This method only works for stationary spheres.
34893488

34903489
hit_record rec;
3491-
if (!this->hit(ray(o, v), interval(0.001, infinity), rec))
3490+
if (!this->hit(ray(origin, direction), interval(0.001, infinity), rec))
34923491
return 0;
34933492

3494-
auto cos_theta_max = sqrt(1 - radius*radius/(center1 - o).length_squared());
3493+
auto cos_theta_max = sqrt(1 - radius*radius/(center1 - origin).length_squared());
34953494
auto solid_angle = 2*pi*(1-cos_theta_max);
34963495

34973496
return 1 / solid_angle;
@@ -3586,12 +3585,12 @@
35863585

35873586

35883587
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
3589-
double pdf_value(const point3& o, const vec3& v) const override {
3590-
auto weight = 1.0/objects.size();
3588+
double pdf_value(const point3& origin, const vec3& direction) const override {
3589+
auto weight = 1.0 / objects.size();
35913590
auto sum = 0.0;
35923591

35933592
for (const auto& object : objects)
3594-
sum += weight * object->pdf_value(o, v);
3593+
sum += weight * object->pdf_value(origin, direction);
35953594

35963595
return sum;
35973596
}

src/TheNextWeek/bvh.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class bvh_node : public hittable {
2727
bvh_node(const std::vector<shared_ptr<hittable>>& src_objects, size_t start, size_t end) {
2828
// Build the bounding box of the span of source objects.
2929
bbox = aabb::empty;
30-
for (int object_index=start; object_index < end; ++object_index)
30+
for (size_t object_index=start; object_index < end; ++object_index)
3131
bbox = aabb(bbox, src_objects[object_index]->bounding_box());
3232

3333
int axis = bbox.longest_axis();

src/TheNextWeek/quad.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ class quad : public hittable {
7878
private:
7979
point3 Q;
8080
vec3 u, v;
81+
vec3 w;
8182
shared_ptr<material> mat;
8283
aabb bbox;
8384
vec3 normal;
8485
double D;
85-
vec3 w;
8686
};
8787

8888

src/TheRestOfYourLife/cos_cubed.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@
1515
#include <iomanip>
1616
#include <math.h>
1717

18-
double f(double r1, double r2) {
18+
double f(double r2) {
1919
// auto x = cos(2*pi*r1)*2*sqrt(r2*(1-r2));
2020
// auto y = sin(2*pi*r1)*2*sqrt(r2*(1-r2));
2121
auto z = 1 - r2;
2222
double cos_theta = z;
2323
return cos_theta*cos_theta*cos_theta;
2424
}
2525

26-
double pdf(double r1, double r2) {
26+
double pdf() {
2727
return 1.0 / (2.0*pi);
2828
}
2929

@@ -32,9 +32,8 @@ int main() {
3232

3333
auto sum = 0.0;
3434
for (int i = 0; i < N; i++) {
35-
auto r1 = random_double();
3635
auto r2 = random_double();
37-
sum += f(r1, r2) / pdf(r1, r2);
36+
sum += f(r2) / pdf();
3837
}
3938

4039
std::cout << std::fixed << std::setprecision(12);

src/TheRestOfYourLife/estimate_halfway.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ bool compare_by_x(const sample& a, const sample& b) {
2727
}
2828

2929
int main() {
30-
int N = 10000;
30+
unsigned int N = 10000;
3131
double sum = 0.0;
3232

3333
// iterate through all of our samples
3434
std::vector<sample> samples;
35-
for (int i = 0; i < N; i++) {
35+
for (unsigned int i = 0; i < N; i++) {
3636
// Get the area under the curve
3737
auto x = random_double(0, 2*pi);
3838
auto sin_x = sin(x);
@@ -50,9 +50,9 @@ int main() {
5050
double half_sum = sum / 2.0;
5151
double halfway_point = 0.0;
5252
double accum = 0.0;
53-
for (int i = 0; i < N; i++){
53+
for (unsigned int i = 0; i < N; i++){
5454
accum += samples[i].p_x;
55-
if (accum >= half_sum){
55+
if (accum >= half_sum) {
5656
halfway_point = samples[i].x;
5757
break;
5858
}

src/TheRestOfYourLife/hittable_list.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class hittable_list : public hittable {
5353
aabb bounding_box() const override { return bbox; }
5454

5555
double pdf_value(const point3 &o, const vec3 &v) const override {
56-
auto weight = 1.0/objects.size();
56+
auto weight = 1.0 / objects.size();
5757
auto sum = 0.0;
5858

5959
for (const auto& object : objects)

src/TheRestOfYourLife/quad.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,13 @@ class quad : public hittable {
7777
return true;
7878
}
7979

80-
double pdf_value(const point3& origin, const vec3& v) const override {
80+
double pdf_value(const point3& origin, const vec3& direction) const override {
8181
hit_record rec;
82-
if (!this->hit(ray(origin, v), interval(0.001, infinity), rec))
82+
if (!this->hit(ray(origin, direction), interval(0.001, infinity), rec))
8383
return 0;
8484

85-
auto distance_squared = rec.t * rec.t * v.length_squared();
86-
auto cosine = fabs(dot(v, rec.normal) / v.length());
85+
auto distance_squared = rec.t * rec.t * direction.length_squared();
86+
auto cosine = fabs(dot(direction, rec.normal) / direction.length());
8787

8888
return distance_squared / (cosine * area);
8989
}
@@ -96,11 +96,11 @@ class quad : public hittable {
9696
private:
9797
point3 Q;
9898
vec3 u, v;
99+
vec3 w;
99100
shared_ptr<material> mat;
100101
aabb bbox;
101102
vec3 normal;
102103
double D;
103-
vec3 w;
104104
double area;
105105
};
106106

0 commit comments

Comments
 (0)