Cycles: Overhaul ensure_valid_reflection to fix issues with normal- and bumpmapping

Authored by Lukas Stockner (lukasstockner97) on Sun, Oct 21, 4:08 AM.



This function is supposed to prevent the black artifacts caused by strong normal- or bumpmapping, but failed in some cases.

Now the code correctly handles all test files and previous issues I am aware of and also has extensive comments describing
the algorithm and the math behind it.

Basically, the main problem was that there can be multiple valid solutions that fulfil the reflection angle criterium,
but I had assumed that only one would exist and therefore simply picked the first solution with a positive term in srqt().
Now, the code uses additional validity checks and a simple heuristic to pick the best valid solution.

Additionally, the code messed up very shallow reflections even if the normal map strength was zero due to the constant
limit for the outgoing ray angle, which caused shallow incoming rays to fail the initial test even when reflected directly
on Ng. Now, the code accounts for this by reducing the threshold in the case of a shallow incoming ray, ensuring that at
least N=Ng is always a valid solution.

Diff Detail

rB Blender

The relevant bug reports are T56209, T56175 and possibly T57109 (didn't check yet).

Here are two scenes that I'd like to add to the tests to cover this feature:

Also, I'm not sure about the comment, it's fairly long - might be better to put a more detailed version into a PDF and link it in the code.

Brecht Van Lommel (brecht) requested changes to this revision.Mon, Oct 22, 7:44 PM

Thanks a lot for looking into this! The explanation makes sense to me, and it seems to work well.

My only comment is that the OSL code needs to be updated to match.

This revision now requires changes to proceed.Mon, Oct 22, 7:44 PM
This revision was not accepted when it landed; it landed in state Needs Revision.Thu, Oct 25, 2:51 PM
This revision was automatically updated to reflect the committed changes.