hair sidedness bug

I forgot one important case in the get_light_side function - some closures receive light from both sides simultaneously (like hair).

This patch makes the 4 cases explicit:
   None: surface doesn't need eval
   Front: surface is sensitive to light on the same side as the viewer
   Back: surface is sensitive to light on the opposite side of the viewer
   Both: surface is sensitive to light on both sides

This fixes backlighting on hair - previously only eval_reflect was being called because get_light_side was filtering out lights coming from behind.

[aconty, 2010-02-10 19:57:21]

Now it looks clear. The only thing I don't like is using eval_sidedness of None
for singular closures. Now I'm using sidedness to choose visible lights and this
is going to break it. Can you avoid setting None for singular closures? We
really need that information for MIS. Singularity is already reflected on the
scattering label.

[ckulla, 2010-02-10 19:59:37]

I'm not sure I understand - we were already returning None from get_light_side()
for singular closures before.

Can you setup a test case for me to look at?

[aconty, 2010-02-10 21:07:38]

It breaks with my other review, so nothing that you could actually reproduce
now. But the case is this. Think of a reflective plane that is reflecting a
sphere light. That reflection comes from the indirect part of the MIS, but if
you set the sidedness to None, my other patch will put the light out of the loop
and therefore won't be accounted for (no reflection)

[ckulla, 2010-02-10 21:23:15]

But the behavior for singular closures does not change in this review. So it
sounds like something is already broken.

[aconty, 2010-02-10 21:32:10]

Is not broken now, because we don't use the sidedness to discard lights from the
light loop. We use it to discard direct sampling, but not when sampling the
closures.

But now, with my added extra check for sidedness in the light estimation, this
will prevent the light to arrive to both parts of the MIS.

Plus I think it is confusing to mix two concepts, sidedness and singularity.
Even if it didn't break anything, it would do sooner or later.

[ckulla, 2010-02-10 21:39:00]

Right - what I meant is that its already broken in your other review, regardless
of this change.

Let's discuss this more in the other code review.

Since the behavior doesn't change for now - can I commit this?

[aconty, 2010-02-10 21:49:56]

That's right, but if you commit that how are we going to fix it?

[aconty, 2010-02-10 21:55:28]

No, wait a minute. Singular had already been taken out of MIS for optimization.
This is not going to break anything. I still would change it, but it is safe to
commit. Go ahead.

[ckulla, 2010-02-10 21:57:56]

On 2010/02/10 21:49:56, aconty wrote:
> That's right, but if you commit that how are we going to fix it?

We could have get_light_side return front/back but then skip the eval loop for
singular closures for instance.

Whatever we come up with needs to be coordinated to your other fix. This change
is only about fixing the hair.

[aconty, 2010-02-10 22:03:23]

> Whatever we come up with needs to be coordinated to your other fix. This
change
> is only about fixing the hair.

No problem, go ahead. As I said in the previous reply we moved singular out of
the light loop some time ago, so my code won't break anything.