mmp / pbrt-v3

Source code for pbrt, the renderer described in the third edition of "Physically Based Rendering: From Theory To Implementation", by Matt Pharr, Wenzel Jakob, and Greg Humphreys.
http://pbrt.org
BSD 2-Clause "Simplified" License
4.89k stars 1.19k forks source link

Sphere mapping discontinuity may not be handled properly #302

Open AlcEccentric opened 3 years ago

AlcEccentric commented 3 years ago

Hi,

The code snippet named "Handle sphere mapping discontinuity for coordinate differentials" in function SphericalMapping2D::Map may not be able to handle the discontinuity of the t texture coordinate in sphere mapping properly.

I think that code snippet assumes the absolute value of (dstdx)[1] or (dstdy)[1] is less than 1, but this assumption does not hold as you set delta as 0.1.

Here is the function:

Point2f SphericalMapping2D::Map(const SurfaceInteraction &si, Vector2f *dstdx,
                                Vector2f *dstdy) const {
    Point2f st = sphere(si.p);
    // Compute texture coordinate differentials for sphere $(u,v)$ mapping
    const Float delta = .1f; // **DELTA IS 0.1**
    Point2f stDeltaX = sphere(si.p + delta * si.dpdx);
    *dstdx = (stDeltaX - st) / delta;
    Point2f stDeltaY = sphere(si.p + delta * si.dpdy);
    *dstdy = (stDeltaY - st) / delta;

    // Handle sphere mapping discontinuity for coordinate differentials
    if ((*dstdx)[1] > .5) // **THE VALUE HERE COULD BE CLOSE TO 10.0** 
        (*dstdx)[1] = 1 - (*dstdx)[1];
    else if ((*dstdx)[1] < -.5f)
        (*dstdx)[1] = -((*dstdx)[1] + 1);
    if ((*dstdy)[1] > .5)
        (*dstdy)[1] = 1 - (*dstdy)[1];
    else if ((*dstdy)[1] < -.5f)
        (*dstdy)[1] = -((*dstdy)[1] + 1);
    return st;
}

I tested it with a scene using spherical mapping. The log showed the value of (dstdx)[1] could be something like 9.999742. In this case, the final value of (dstdx)[1] was -8.999742 after the code handled the discontinuity. In my opinion, this final value is not desired.

I am a beginner in this field, so I may be wrong here. If I am wrong, hope you could briefly explain why that code snippet could still handle discontinuity properly in the example I mentioned.

Thanks.