RayTracing / raytracing.github.io

Main Web Site (Online Books)
https://raytracing.github.io/
Creative Commons Zero v1.0 Universal
8.89k stars 878 forks source link

Incorrect linear interpolation in Chapter 4.2 #1652

Open KDJDEV opened 2 weeks ago

KDJDEV commented 2 weeks ago

Chapter 4.2 describes linear interpolation as some function that outputs a color when given a value a that ranges from 0 to 1. However, the ray_color function does not make a range from 0 to 1.

color ray_color(const ray& r) {
    vec3 unit_direction = unit_vector(r.direction());
    auto a = 0.5*(unit_direction.y() + 1.0);
    return (1.0-a)*color(1.0, 1.0, 1.0) + a*color(0.5, 0.7, 1.0);
}

Because the code runs unit_vector on r.direction(), the y value no longer ranges from 1 to -1, and therefore a does not range from 0 to 1. If you just don't make unit_direction into a unit vector then it works as expected.

This is what I think the code should be:

color ray_color(const ray& r) {
    vec3 unit_direction = r.direction();
    auto a = 0.5*(unit_direction.y() + 1.0);
    return (1.0-a)*color(1.0, 1.0, 1.0) + a*color(0.5, 0.7, 1.0);
}

The current code still does an interpolation between a sort of white and a sort of blue, but it does not perform a linear interpolation between color(1.0, 1.0, 1.0) and color(0.5, 0.7, 1.0).

dimitry-ishenko commented 2 weeks ago

Because the code runs unit_vector on r.direction(), the y value no longer ranges from 1 to -1

@KDJDEV what makes you think that?

All that unit_vector does is divide vector coordinates by its length. So, if you had a vector {0,-5,0}, applying unit_vector to it would result in {0,-1,0}. Likewise, for vector {0,5,0} it would yield {0,1,0}.

KDJDEV commented 2 weeks ago

All that unit_vector does is divide vector coordinates by its length. So, if you had a vector {0,-5,0}, applying unit_vector to it would result in {0,-1,0}. Likewise, for vector {0,5,0} it would yield {0,1,0}.

r.direction() has multiple components not equal to 0. r.direction() for the first pixel in the top left is {-1.77333 0.995556 -1}. Notice that the y component is about equal to 1 which is what we want since initially a=1. The magnitude of this vector is about 2.266. This means that unit_vector(r.direction()) is equal to about {-0.78, 0.44, -0.44}. Now the y component no longer equals 1, and this messes up the interpolation.

Basically normalizing the vector makes it so the range of the y component is smaller than (-1, 1).

dimitry-ishenko commented 2 weeks ago

@KDJDEV Oh, I see what you are saying now... we are not using the entire range between the two colors.

The short answer is—it's not a big deal. The longer answer is—consider these points:

  1. This is just a fallback to give us nice gradient instead of a boring solid background color.

  2. Later on you will be able to set field of view and it will affect how much of the range will be used. Eg, here is the same image rendered with 60° and 160° FOV:

    file

  3. There is nothing that's stopping someone from passing vectors with $y$ values outside of the $[-1, 1]$ range. Maybe they are using different math, or want to achieve "anamorphic lens" effect, etc.

    If we simply remove normalization, this function will not be able to handle those cases. So, now we would need to add a check to make sure the value of $y$ is within the range.

I hope this clarifies things.

KDJDEV commented 2 weeks ago

This is helpful, thank you! It's true that you still get a gradient effect when you normalize it.

I still think it is rather confusing because the code doesn't do exactly what is discussed in the paragraph before, where the range of a is said to lerp over the range [0, 1]. image It says that a lerp is always of the form "... with a going from 0 to 1". This makes sense with the equation shown. However, in the code a doesn't go from 0 to 1, which is confusing for the reader.

This is definitely a minor thing, but I would add some text that clarifies why the code doesn't match up with the math in the previous paragraph.

dimitry-ishenko commented 2 weeks ago

TBH I don't see it the way you do (as being confusing), but nevertheless, thank you for your input. Maybe some day when we get through the issues that have been collecting dust for some time, we can revisit this. For now, I will close this issue.

hollasch commented 2 weeks ago

I'll add some commentary about LERP and extrapolation since this is our introduction of the function.