Page MenuHome

Cycles: Fix correlation issues in certain cases
ClosedPublic

Authored by Sergey Sharybin (sergey) on Nov 26 2016, 5:33 PM.

Details

Summary

There were two cases where correlation issues were obvious:

  • File from T38710 was giving issues in 2.78a again
  • File from T50116 was having totally different shadow between sample 1 and sample 32.

Use some more simplified version of CMJ hash which seems to give
nice randomized value which solves the correlation.

This commit will break all unit test files, but it's a bug fix
so perhaps OK to commit this.

Proper science paper about hash function is coming.

Diff Detail

Repository
rB Blender

Event Timeline

Sergey Sharybin (sergey) retitled this revision from to Cycles: Fix correlation issues in certain cases.
Sergey Sharybin (sergey) updated this object.
Lukas Stockner (lukasstockner97) added inline comments.
intern/cycles/kernel/kernel_random.h
126

This seems weird, you use the i uninitialized tmp_rng to initialize itself. I guess you meant cmj_hash_simple(dimension,*rng)?

intern/cycles/kernel/kernel_random.h
126

Ah, last minute optimization which for some reason was not visible at the renders =\

Good spot!

Update patch to address mistake spotted by Lukas

This revision is now accepted and ready to land.Nov 27 2016, 7:44 AM

There is some correlation in the hash function itself.

Original CMJ hash:

Current simplified version of CMJ hash:

Weirdly enough this was still solving the issue.

Here's some other function i came up with:

Seems much better and it is same number of math operations.

Sergey Sharybin (sergey) edited edge metadata.

Use updated hash function

Benchmark results:

BMW: +0.5%
Classroom: +0.6%
Fishy Cat: +0.2%
Koro: +1.7%
Barcelona: -0.4%

For the Koro it's a bit too much, but i've got upcoming AVX2 optimization for hair which will compensate for that.

That seems quite acceptable, especially if solving this correlation helps with convergence in some cases.

I can't judge it well without running the tests, but if that plot tells the whole story then we can entirely replace cmj_hash() with cmj_hash_simple(), which would speed up CMJ sampling and branched path tracing.

This revision was automatically updated to reflect the committed changes.