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.

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

Incorrect linear interpolation in Chapter 4.2 #1652

Open
KDJDEV opened this issue Nov 9, 2024 · 6 comments
Open

Incorrect linear interpolation in Chapter 4.2 #1652

KDJDEV opened this issue Nov 9, 2024 · 6 comments

Comments

@KDJDEV
Copy link

KDJDEV commented Nov 9, 2024

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
Copy link
Contributor

dimitry-ishenko commented Nov 9, 2024

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}.

@dimitry-ishenko dimitry-ishenko self-assigned this Nov 9, 2024
@KDJDEV
Copy link
Author

KDJDEV commented Nov 9, 2024

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
Copy link
Contributor

dimitry-ishenko commented Nov 9, 2024

@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

  1. 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
Copy link
Author

KDJDEV commented Nov 10, 2024

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
Copy link
Contributor

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.

@dimitry-ishenko dimitry-ishenko closed this as not planned Won't fix, can't repro, duplicate, stale Nov 10, 2024
@hollasch hollasch reopened this Nov 10, 2024
@hollasch
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants