Cycles: remove min bounces, modify RR to terminate later.
ClosedPublic

Authored by Brecht Van Lommel (brecht) on Aug 3 2017, 4:06 AM.

Details

Summary

This patch removes the min bounces settings, instead making Russian roulette
termination use a sqrt() for the termination probability to do path termination
a bit later on average. This also matches typical view transforms a bit, giving
more importance to dark corners.

Diff Detail

Repository
rB Blender

Some scenes render faster with more noise, others slower with less noise. For the benchmark scenes it seems to be a good trade-off. Brightness differences in fishy cat are due to indirect sample clamp.

Tests with GTX 1080 on Linux:

BeforeAfterAfter (equal time)
BMW
24.77s17.41s24.86s
Classroom
17.71s12.36s17.68s
Fishy Cat
23.98s20.57s23.93s
Koro
29.98s26.12s30.12s
Pabellon
20.77s24.12s20.78s

Generally looks good, but see inlined comment.

Also wondering whether the tests are still working with this? And can you see some possible downsides?

intern/cycles/kernel/kernel_path_state.h
205–207

I really propose using proper statements (starting with capital and ending with fullstop) for all comments. Often you need to expand comments, or make more comprehensive explanations, where such an obscure statements will make it difficult to follow. Having different format for comprehensive and "simple" comments is also weird.

Lots of regression tests have different noise now, but seem to be ok.

As for downsides, there will be scenes that render worse with this. For example in a scene where you always need at least N bounces to reach a light, having an explicit min setting could help (maybe a tunnel with the only light a few corners away). Or there can be scenes where not using sqrt() gives a better estimate, where for example indirect light is less significant compared to direct light and terminating more would be better. It's always going to be a heuristic, empirically this one just seems better on average.

intern/cycles/kernel/kernel_path_state.h
205–207

Ok, I'll fix the comment.

After checking the tests in detail, this change exposes an existing issue in the shadow catcher where it doesn't work correctly with RR. The checkboard texture is not supposed to be visible here:

Before (min bounces 0)Before (min bounces 8)After

The low light threshold RR is probably disabled with shadow catchers for the same reason. The workaround would be to disable RR or at least raise the minimum bounces for shadow catchers. To properly fix this issue I think deeper shadow catcher changes are needed, like postponing the shadowed/unshadowed division until after rendering. Right now the shadow catcher integration is not unbiased and quite sensitive to sampling algorithms. There may be a simpler fix but I couldn't find one without introducing other issues.

@Brecht Van Lommel (brecht), guess we are not in a hurry. I'd like to gather regressions tests for various shadow catcher setups. Also, think we can do proper shadow catcher pass, with maybe a bit of compositing trickery get_pass_rect() to preserve progressive feedback on how shadow will look like. And we can also keep memory usage lowe-ish, if we only allocate memory for active tiles for intermediate shadow catcher data. Need to think about this, and maybe will poke once i'm back to Amsterdam.

Workaround for shadow catcher, fix comments.

Now the shadow catcher ctests are unchanged, I think we could commit this
as is and then handle shadow catcher bugs elsewhere.

I'm fine with current workadound and dealing with catcher later. So please go ahead an commit. But also nice to start documenting such changes in https://wiki.blender.org/index.php/Dev:Ref/Release_Notes/2.80 so artists know what's going on with all those settings.

This revision is now accepted and ready to land.Aug 4 2017, 10:48 PM
This revision was automatically updated to reflect the committed changes.

Ok, release notes and tests updated.