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

Rings in Rendered Image for Realistic Camera #162

Closed
tlian7 opened this Issue Nov 26, 2017 · 8 comments

Comments

Projects
None yet
2 participants
@tlian7

tlian7 commented Nov 26, 2017

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

This comment has been minimized.

Owner

mmp commented Nov 26, 2017

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

This comment has been minimized.

tlian7 commented Nov 26, 2017

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

This comment has been minimized.

tlian7 commented Nov 29, 2017

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 added a commit that referenced this issue Dec 1, 2017

Improve robustness of Camera::GenerateRayDifferential
Reduce the frequency of times where the main ray makes it through but
one of the auxiliary rays is vignetted with the RealisticCamera:

1. Use a smaller spacing for the forward difference
2. Try again in the other direction if we fail the first time.

This helps with some of the radial artifacts that are sometimes present
with that camera.

Partially addresses issue #162.
@mmp

This comment has been minimized.

Owner

mmp commented Dec 1, 2017

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

This comment has been minimized.

tlian7 commented Dec 1, 2017

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

This comment has been minimized.

Owner

mmp commented Dec 1, 2017

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

This comment has been minimized.

tlian7 commented Dec 1, 2017

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 added a commit that referenced this issue Dec 1, 2017

@mmp

This comment has been minimized.

Owner

mmp commented Dec 1, 2017

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

@mmp mmp closed this Dec 1, 2017

mmp added a commit that referenced this issue Dec 2, 2017

Improved solution to the rings artifacts in RealisticCamera
Now, images have generally the same pixel values as regular projective
cameras again, (modulo the cos^4 falloff, which we want).

Issue #162.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment