Cycles: Add "Max Bounce" control for lamps
AbandonedPublic
Actions

Authored by Thomas Dinges (dingto) on Oct 30 2014, 10:51 AM.

Details

Reviewers

Sergey Sharybin (sergey)

Group Reviewers

Cycles

Summary

Title says it all, this way we can have lamps that only contribute to X amount of bounces. 0 = only direct light, 1 = 1 bounce and so on...

Only implemented for Branched Path "Sample All Lights" atm, adding this to one lamp sampling would require more conditionals in some places, not sure it's worth it.

Diff Detail

Event Timeline

Thomas Dinges (dingto) updated this revision to Diff 2789.Oct 30 2014, 10:51 AM

Thomas Dinges (dingto) retitled this revision from to Cycles: Add "Max Bounce" control for lamps in Branched Path integrator.

Thomas Dinges (dingto) updated this object.

Thomas Dinges (dingto) added a reviewer: Cycles.

Thomas Dinges (dingto) added projects: BF Blender, Cycles.

Awesome!

How about a slightly less technical tooltip? "Maximum number of times that rays from this light may bounce." perhaps? Although that doesn't really help explain what a 'bounce' is...

Ronny G (nutel) added a subscriber: Ronny G (nutel).Oct 30 2014, 7:47 PM

seems to work

(2 area lamps)

Derek Barker (lordodin) added a subscriber: Derek Barker (lordodin).Oct 31 2014, 6:31 PM

Thomas Dinges (dingto) added a reviewer: Sergey Sharybin (sergey).Oct 31 2014, 9:55 PM

Generally code looks ok (didn't try to compile yet tho). but not sure why exactly it's only limited to branched path tracer only with sample all direct only? From quick think it seems you can skip them in all cases (quite similar to the low light threshold patch). Just maybe picking the light would need to become smarter a bit.

intern/cycles/blender/addon/properties.py
660	Think it'll make more sense to keep max bounces at max possible value, so this way if for some reason you'll be setting integrator bounces to more than 128 bounces you don't need to go changing all the lights in your scene.
intern/cycles/kernel/kernel_light.h
683	Did you test asm output/performance with using inlined function?

Feature works for regular Pathtrace / Random light now as well.
Less technical tooltip.
Set default to 1024.

Independently of this patch I think we can refactor the light classes a bit once, and make light_sample() more modular. There are cases where we know that it's a triangle (so unnecessary if check), and in volume code where we call this twice after another, we redundantly calculate this stuff too. But that's for later.

Didn't have chance to look in details yet, but think we should consider renaming light_select_reached_max_bounces() to something like light_is_effective() or so. That way we can easily implement light low threshold in the same exact function. And maybe do some more early out optimization in there.

I don't mind renaming it, but not sure how this could be re-used? Your Low light threshold patch is based on ls->pdf which we only know *after* lamp_light_sample().

Checked on trying to make checks more concentrated in a single place, kinda possible but probably doesn't worth it. Maybe with some clearer mind we'll find out simple way to do that.

Things to be done before commit:

There are some inline comments
You're missing initialization in Light::Light().
Did you check the speed changes in existing files?

P.S. When you'll finally install arc? That's not difficult and make reading the patches _WAY_ easier.

intern/cycles/kernel/kernel_light.h
651–657	Not sure why select?
657	Move `bounce` before `ls`, so this way input parameters are all comes before output one.
680	Use UNLIKELY? Same applies to the checks below.

Addressed most review points.

select, because we retrieve the info from a "selected" light, not a random one. Basically just consistent with naming we already use in e.g "light_select_num_samples".
Did not run speed tests yet, although I would not expect a noticeable change.

Will run final checks and commit tomorrow.

Thomas Dinges (dingto) retitled this revision from Cycles: Add "Max Bounce" control for lamps in Branched Path integrator to Cycles: Add "Max Bounce" control for lamps.Nov 4 2014, 11:07 PM

Thomas Dinges (dingto) abandoned this revision.Nov 6 2014, 11:36 AM