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.88k stars 1.19k forks source link

Rings in Rendered Image for Realistic Camera #162

Closed tlian7 closed 6 years ago

tlian7 commented 6 years ago

When I render with the Realistic camera, I often get these rings overlaid on the image. Here's an example of what I mean.

I've been trying to figure out why this is happening, but since there are 64 rings I'm guessing it has something to do with sampling the exit pupil bounds. Have you seen this before, and if you have do you have any fixes?

mmp commented 6 years ago

I've seen that before as well :-(. (And have not yet chased it down.) I think that your hypothesis about the problem is likely correct. If you're able to make any progress debugging it, that'd be fantastic; otherwise I'll get to it eventually.

tlian7 commented 6 years ago

Thanks for the speedy reply! I've been trying to debug it, so I'll update here if I find out what the issue is. :)

tlian7 commented 6 years ago

I think I may know what the issue is here, but maybe someone can tell me if this makes sense.

If we look at camera.cpp, I believe the rings are caused by rays that for: "Float wt = GenerateRay(sample, rd);" return a weight of >0 (successfully passes through the lens) but for the differentials: Float wtx = GenerateRay(sshift, &rx); Float wty = GenerateRay(sshift, &ry); return a weight of 0 (do not pass through lens).

These erroneous rays cluster around the edges of the 64 bounds. The reason this happens is as follows...

Say you have a ray that is near the border of a bound, but still has an rIndex of 1. It samples the lens at certain point (pLens). It makes it through the lens successfully, so GenerateRay returns something nonzero.

However, the shift in the differential bumps it over so that it's now in a different bound; its rIndex is now 2. It still has the same pLens but this corresponds to a completely different location on the lens (pRear) since the pupil bounds are now different. It's unlikely to make it through the lens with this new direction (pFilm to pRear) so GenerateRay returns a 0 for the ray differentials, and the whole ray returns a weight of 0.

As a result of this, many of the rays near the edge of the bounds are given a ray weight of 0, and we see 64 dark rings around the image. I think this artifact is exacerbated by small exit pupils with less overlap between bounds, since it becomes more unlikely that the differential, "bumped" ray would pass through the lens with its new pupil bounds.

Does this explanation make sense? If it does, I have a few ideas for hack-y fixes but I'll have to think more to find a more elegant solution.

mmp commented 6 years ago

Nice debugging!

Note that it still does the remapping from the [0,1] lens sample in CameraSample::pLens to the new bounding box in the next region over), so I don't think it's using the same pRear between both, but I think that in any case, there are still plenty of rays where the auxiliary ray will be vignetted but the main one won't, just because a given point inside the exit pupil bounds of one bucket may be in the exit pupil but the corresponding point in the next one over may not be.

In any case, I just pushed a fix that tries to make the central differencing in Camera::GenerateRayDifferential be less prone to hitting this. (It still happens, but quite rarely--around 1 in a million in with the attached test case. Not enough to cause any image artifacts.)

x.pbrt.txt

If you have a cleaner fix in mind, I'm all ears!

While this fix eliminates the very visible lines at the boundaries, there is still some banding. An update on that to come in a bit...

tlian7 commented 6 years ago

Thanks for the fix! I did a hack-y fix as well to get rid of the rings, but your fix is much cleaner. Even with either of our fixes, I do still see the banding, as you mentioned. In fact, I was looking at it today trying to figure out why it was still happening.

Out of curiosity, did you know why the banding is still happening? I've been scratching my head over it this morning.

Thank so much!

mmp commented 6 years ago

Here's the rest of it (which took a few hours to figure out...)

A clue is that if the "simpleweighting" parameter is set to false, then those go way. (But then the image generally gets much dimmer, since it's not directly measuring radiance anymore.)

A second clue came when I tried modifying the exit pupil bounds computation at the end of the constructor to make much bigger bounds, to see if being more conservative helped. The rings didn't disappear, but the image got dimmer (only with the simpleweighting, though.)

Thinking about it in terms of Monte Carlo sampling, it should be fine if we sample over a wider region of the domain where much of it is zero (as would happen with the expanded bounds where more rays will be vignetted); it all cancels out since we divide by the PDF, and the PDF will be relatively smaller if we expand the sampling domain. With the non-simple weighting, each ray is weighted by exitPupilBoundsArea, which accounts for that term.

With the simpleweighting case, the exit pupil bounds in different buckets end up having different areas, both because the size of the exit pupil changes but also because it may be a better or worse fit for a bounding box depending on if it's stretched out, etc. In turn, they have varying levels of effectiveness at giving unvignetted rays when they're sampled, and in turn, because there's no PDF correction, the ones that are a worse fit and have more vignetted rays are... darker, naturally (in retrospect).

Changing the end of RealisticCamera::GenerateRay to something like: if (simpleWeighting) return cos4Theta * exitPupilBoundsArea / exitPupilBounds[0].Area();

Seems to fix things. The divide by exitPupilBounds[0].Area is an attempt to normalize the result (otherwise the image gets super dim, but ring free).

That's a little unsatisfying since the image is still ~1.5x-2x darker than a regular perspective projection camera, and its brightness varies as the aperture size is adjusted. (Not proportionally to the aperture area, but proportionally to how well the aperture's exit pupil is fit by bounding boxes.)

I think that the best thing to do is probably to check in that fix and then add an entry to the FAQ about this and to probably also print a warning when simpleweighting is used. (But am open to better suggestions!)

tlian7 commented 6 years ago

Thanks for the clear and detailed reply @mmp . Your explanation makes a lot of sense, and scaling the cos4Theta term by the exit pupil area did exactly as you said it was. Even if the brightness varies according to the exit pupil, I'm still very glad we have a fix to get artifact free images now!

mmp commented 6 years ago

Great. I've pushed that fix (and will think about whether there's something better to do, but better to fix the rings for starters, I think.)