mmp / pbrt-v4

Source code to pbrt, the ray tracer described in the forthcoming 4th edition of the "Physically Based Rendering: From Theory to Implementation" book.
https://pbrt.org
Apache License 2.0
2.88k stars 450 forks source link

Problem with --cropwindow and bdpt #347

Closed rainbow-app closed 1 year ago

rainbow-app commented 1 year ago

Hello.

I'm using v3, but writing to v4 because it's affected too.

AddSplats from the pixels outside of crop are not added (not even considered to be added) to the crop pixels.

My solution: Edit Render() to loop over all pixels of the full image, and modify few places here and there, like skip t>1 strategies if outside of crop. Replace sampleBounds with full res in two ifs that are commented in the (v3) book as "Return zero importance for out of bounds points" (otherwise importance pdf is not normalized!??)

Pretty obvious fix. Works, but is very slow (obviously), only like 30% faster, which defeats the purpose of this option. What would be the proper way to fix it?

Sorry if it's a known bug.

mmp commented 1 year ago

Hm, I'm not sure there is a bug there: is the issue that if you render a cropwindow, then there are fewer successful splats from light paths, so image quality is worse?

On one hand, that could be argued to be a bug, since pbrt does its best to do the exact same computation when crop windows are used, in part for debugging and reproducability, and in part so that rendering can be farmed across machines. However, light paths in BDPT don't fit into that well; pbrt traces a single light path per image sample and those are where splats may come from. So fewer image samples means fewer light paths. Of course, there is this weirdness in that the light paths for one pixel may (and usually do) actually contribute splats to different pixels.

Always tracing all of the light paths seems like an expensive alternative (consider the limit of a single-pixel crop window out of a 100 million pixel image.) So I'm inclined to say it's working as expected and that that's how it goes with BDPT.

However, perhaps I'm misunderstanding the issue!

rainbow-app commented 1 year ago

image quality is worse?

Yes, misunderstanding. My bad, I should have done it one step at a time, and with images. First step:

Problem description: Crop from big image and cropwindow image are visually (very) different.

Images: 10spp, crop from full: parallelepiped-cr10 10spp, --cropwindow: parallelepiped-cw10 100spp, --cropwindow: parallelepiped-cw100

How to reproduce: Use the below pbrt file (v3; and the screenshots are also from v3). We're inside an empty parallelepiped, with a narrow-angle spotlight (a pt-difficult case).

I used the following commands to get the above screenshots (rename filenames a bit). ./pbrt --cropwindow 0.4333 0.6250 0.474 0.6074 parallelepiped.pbrt convert parallelepiped.pfm -level 0,2% -quality 97 parallelepiped.jpg convert -crop 368x144+832+512 parallelepiped-full10.jpg parallelepiped-cr10.jpg


pbrt file: Sampler "random" "integer pixelsamples" [100] Integrator "bdpt" LookAt 14.5 1.5 -20 12 -1 50 0 1 0 Camera "perspective" "float fov" [50]

Film "image" "integer xresolution" [1920] "integer yresolution" [1080] "string filename" "parallelepiped.pfm"

WorldBegin

Material "matte" "color Kd" [.5 .5 .5] "float sigma" [0]

LightSource "spot" "point from" [15 1 -20] "point to" [15 0.5 50] "float coneangle" [5] "float conedeltaangle" [5] "color I" [10000 10000 10000]

Shape "trianglemesh" "point P" [0 0 0 25 0 0 25 0 -70 0 0 -70] "integer indices" [ 0 1 2 2 3 0] Shape "trianglemesh" "point P" [25 0 -70 25 5 -70 0 5 -70 0 0 -70] "integer indices" [ 0 1 2 2 3 0] Shape "trianglemesh" "point P" [0 0 0 0 5 0 25 5 0 25 0 0] "integer indices" [ 0 1 2 2 3 0] Shape "trianglemesh" "point P" [0 5 0 0 5 -70 25 5 -70 25 5 0] "integer indices" [ 0 1 2 2 3 0] Shape "trianglemesh" "point P" [0 0 0 0 0 -70 0 5 -70 0 5 0] "integer indices" [ 0 1 2 2 3 0] Shape "trianglemesh" "point P" [25 0 0 25 5 0 25 5 -70 25 0 -70] "integer indices" [ 0 1 2 2 3 0]

WorldEnd

rainbow-app commented 1 year ago

I only now figured it. For --cropwindow we have fewer splats, and their frequency of occurrence is inversely proportional to ratio of areas of full image and crop, Acrop_ratio (e.g. for the crop values that I mentioned previously, Acrop_ratio=39.1304).

Fix two things

Images

Crop from full image, the reference to compare to. This is regenerated, but should be (almost) the same as the first image in previous post. 10spp. hall-y-mis-F-full-crop pfm

(fixed) --cropwindow 10spp. Due to lack of data, we have higher noise. hall-y-mis-C-10spp pfm

(fixed) --cropwindow 40spp. hall-y-mis-C-40spp pfm

(fixed) --cropwindow 10spp. No MIS; shows that MIS works. hall-z-nomis-10

And two high-spp images.

Crop from full, 100spp hall-y-mis-F-full-100spp-crop pfm

(fixed) --cropwindow 400spp. hall-y-mis-C-400spp pfm

Disclaimer

The above example code is only for specific scene/test (it's modified slightly from my heavily-stripped down version of pbrt; images are also from my version). You'll need to generalize/test yourself.

Hope it helps.

mmp commented 1 year ago

Nice find! Thanks for figuring the issue out and how to fix it. I've made the corresponding fixes in pbrt-v4 in 2b5837a862d7f79f4c2dd48acb2038e737b019a7; that test scene now renders correctly even with crop windows. Please double check my version of the fix to make sure it looks good to you.

rainbow-app commented 1 year ago

This whole ray tracing stuff was for some hobby of mine, and now I put the hobby aside. I finished the ray tracing part of the hobby (few months ago), so I'm not interested in pbrt anymore (especially in v4, because I based my version on v3).

As I wrote in another issue, I don't know anything about other parts of pbrt (like layered materials, volume scattering, etc). So I cannot evaluate your fix professionally. As to that particular test scene -- I fully trust you that it now works. So, the issue may be closed.

I'm glad I contributed something. Let also me thank you (and other related people) for this your pbrt system. Hopefully I'll finish my hobby someday and ray tracing will add a nice shine to it :)

mmp commented 1 year ago

Sounds good and I'm happy to hear pbrt has been useful for you!

rainbow-app commented 1 year ago

I clicked on the link, and looked at diffs, and noticed that you divide by splatScale after the second loop, while I multiply.

I double-checked, what I showed in June is correct.

rainbow-app commented 1 year ago

I also noticed a possible confusion: you already use the word splatScale in v4 to mean a different thing. May be better to rename it here to something about sensor area crop ratio?