Book 1: Image 8 contains incorrect rendering after Section 8.3

[ronnieholm]

The image rendered at the end of Section 8.3 appears to be the result of fixing shadow acne in Section 8.4, and should probably be placed at the end of Section 8.4.

Placing the image after Section 8.3 causes one to go hunt through the code for bugs because the image is too dark.

[hollasch]

We'll double check this during the v4.0.0 sanity pass.

[johannmeyer]

I am unable to reproduce image 7 in sec 8.21(The first render of a diffuse sphere) and image 8 in sec 8.22 (gamma correction). My images are much darker than the images shown but removing the shadow acne by setting t_min = 1e-3 corrects it mostly but the produced images are then lighter than image 7 or 8. Image 9 is reproducible though.

[hollasch]

Thank you for the additional information!

[webanck]

I concur, image 8 (section 8.3) is much closer to the result I get after taking care of shadow acne (section 8.4), even with the gamma correction.

[ISchenhaiqing]

I fix this bug.

if (root < t_min || t_max < root) {
        root = (-half_b + sqrtd) / a;
        if (root < t_min || t_max < root)
            return false;
    }

[sphere.h] Ray-sphere intersection code (now)

if (root <= t_min || t_max <= root) {
        root = (-half_b + sqrtd) / a;
        if (root <= t_min || t_max <= root)
            return false;
    }

[sphere.h] Ray-sphere intersection code (after fixing) The inconsistency between the 2020 version and the 2016 version results in a different energy transfer that makes the current result darker than the previous one.

[tysonliddell]

Confirming @ISchenhaiqing's fix yields an image that seems to match Image 7 in the book much more closely.

Before the fix

After the fix

[samuel-cavalcanti]

@hollasch , is the darker sphere the correct one ?

my rendered diffuse sphere	Image 7: First render of a diffuse sphere

[hollasch]

I believe the right image is the correct one.

[hollasch]

The latest code in the dev-major branch looks like this:

if (!ray_t.contains(root)) {
    root = (-half_b + sqrtd) / a;
    if (!ray_t.contains(root))
        return false;
}

This uses the new interval::contains() method, which internally returns true for points on the closed interval. This means that the negation tests for points outside the open interval, yielding the dark image above (I think; I haven't verified this).

For the time being, I'll make the test more explicit instead of relying on the definition of interval::contains() method. This should correct the code in the dev-major branch, but I'll leave this issue open, to be fully verified with the v4.0.0-release milestone that goes through and regenerates all images one-by-one.

[trevordblack]

Instead of this refactor, maybe we should add helper functions

[a, b] - contains(a,b) (a, b) - inside(a,b) {(-inf, a) , (b, inf)} - outside(a,b)

[hollasch]

Brilliant! I really like the inside naming. I thought of close-contains and open-contains, which are horrible. I prefer !contains instead of outside, as it's simpler and means a smaller API surface. I'll make these changes and we'll see how it looks.

[hollasch]

Ugh. inside reverses the meaning. For example, ray_t.contains(x) makes sense, but ray_t.inside(x) is misleading. Thinking...

• surrounds
• encloses
• envelops

I think I'll go with surrounds.

[hollasch]

Another random thought: bool inside (interval, double) and bool on (interval, double). I like this approach less, though.