Cycles: Light sampling threshold inadvertently clamps negative lamps
Closed, ResolvedPublic

Description

System Information
Windows 10, Nvidia GTX 980

Blender Version
Broken: 2.78b

Short description of error

Not sure this is truly a bug, but: It's possible to make a negative lamp in Cycles by giving a light a negative value for strength. However, when using light sampling threshold, the negative power will fall below the threshold and sampling will constantly get canceled for the negative light

Exact steps for others to reproduce the error
Add a lamp with a negative strength, note the difference in effect between light sampling threshold of 0 (disabled), vs any larger value (enabled).

A test blend with a negative lamp is located here:

Sergey Sharybin (sergey) triaged this task as "Normal" priority.Mon, Mar 20, 12:12 PM
Sergey Sharybin (sergey) claimed this task.

Don't really think it's a bug. Negative colors and strength is going to be always problematic. What is the reason you use negative strength on lamps?

@Brecht Van Lommel (brecht), @Lukas Stockner (lukasstockner97), shouldn't we disable possibility of setting negative light strength?

I don't think that we should explicitly disable negative lights - after all, they don't really cause any problems for regular use and there's no real chance of using them by accident, so I don't see the harm in keeping them as an "undocumented feature" as long as we don't officially support them.

As for the light threshold - I guess adding a fabsf() wouldn't noticably change performance, but of course some benchmarks are needed to check that.

So far we've supported negative light strengths, though in practice I would suggest to never use it. So if we want remove it at some point that's fine with me, but maybe better for 2.8.

Adding the fabsf() here should be fine, I wouldn't even bother to benchmark it, clearing the sign bit is super fast.

@Brecht Van Lommel (brecht), this is just weird to forbid negative color but allow negative light.. Would be better if both are allowed or both are disallowed. But i'm fine keeping things as-is and just use fabs() for now.