[Bf-cycles] Environment/lightprobe importance sampling patch

[Mike Farnsworth]

Thanks, Brecht!  I saw that this went in and the shadow bug got fixed,
but I apologize for not saying anything when that was all happening.
I have a good excuse, though, my wife was having a baby on Friday
morning. =)  (Mom and baby are home and in great shape.)

So, related to this patch was the whole HDR-range texture issue...I
think I'm going to take a crack at that and see if I can come up with
a way to allow full-float textures for certain ones.  It really only
makes sense for emission (reflectances should always be zero to one),
so I don't want to add overhead to all texture sampling.  I'm sure
there's a way to do it.

-Mike

On Sun, Jan 22, 2012 at 5:28 AM, Brecht Van Lommel
<brechtvanlommel at pandora.be> wrote:
> Problem should be fixed now, made a mistake in tweaking the patch. The
> memory errors I think will also be fixed, was a separate issue with
> nodes for which a fix was also committed.
>
> Brecht.
>
> On Sat, Jan 21, 2012 at 11:06 AM, Constantin Rahn <conz at vrchannel.de> wrote:
>> Hi,
>>
>> i have tested the importance sampling with one of my current scenes (Blender
>> 2.61.43676, Win 7 64). But I have the problem, that there is no shadow with
>> activated imp. sampling.
>>
>> Rendering without importance sampling (250spp, CPU):
>> http://www.vrchannel.de/blender/Test_is_off.jpg
>>
>> Rendering with importance sampling (~50spp, CPU), only activated the "sample
>> as lamp" (512) for the BG:
>> http://www.vrchannel.de/blender/Test_is_on.jpg
>>
>> Maybe it has to do with my node setup. I am using two different BG images,
>> one JPG as visible background and one HDRI for the lighting with different
>> power.
>> The node setup:
>> http://www.vrchannel.de/blender/Test_is_nodes.png
>>
>> I get no specific "importance sampling" massage in the console. But there
>> are some "Memoryblock free: attempt to free NULL pointer".
>> I can't send my scene, will try to reduce the problem to a simpler scene.
>>
>> Greetings and happy blending
>> Conz
>>
>> Am 20.01.2012 06:00, schrieb Mike Farnsworth:
>>
>> Okay, one more update.  This version hopefully fixes an issue where
>> any changes to nearly any setting were causing a re-render, because I
>> wasn't checking for opportunities to skip the background light sync.
>> This version seems to trigger fewer progressive render updates, but
>> still updates at the right times.
>>
>> Brecht, let me know when you think this might make it into trunk.
>>
>> Thanks,
>> Farny
>>
>> On Tue, Jan 17, 2012 at 10:59 PM, Mike Farnsworth
>> <mike.farnsworth at gmail.com> wrote:
>>
>> I've updated the patch to include the task split (max of 16384 shader
>> evaluations per task), as well as turn off the background
>> sample_as_light switch by default to not adversely impact performance
>> for those who haven't set a custom environment.  I don't know of any
>> other outstanding issues.
>>
>> -Farny
>>
>> On Tue, Jan 17, 2012 at 6:18 AM, Brecht Van Lommel
>> <brechtvanlommel at pandora.be> wrote:
>>
>> Great work! I'll do a proper review of it later.
>>
>> Regarding the task size, there's a function to split up tasks in the
>> device code. It's used in device_opencl.cpp, task_add, if there's a
>> CUDA issue with maximum size being exceeded a similar thing could be
>> done there.
>>
>> Brecht.
>>
>> On Mon, Jan 16, 2012 at 6:16 PM, Mike Farnsworth
>> <mike.farnsworth at gmail.com> wrote:
>>
>> Someone tried the patch and had a crash when using an importance
>> resolution greater than 1023.  (It's really just eating it with a
>> cuda_assert(cuLaunchGrid(cuDisplace, xblocks, 1)) when launching the
>> shader task.)  I had the same issue with my GTX 260, and I thought it
>> was just because that was an old(er) graphics card.  On my GTX 460 it
>> doesn't seem to have any issues with the importance resolution.  If
>> the GTX 560 Ti is crashing too, though, then I should probably to do
>> something about it.
>>
>> The issue is that when running a cuda task, you can't have 65536
>> blocks in any dimension.  We use a block size of 16, and with a
>> resolution of 1024x1024, that results in 65536 blocks in the X
>> direction.  The near-term workaround is to keep the resolution under
>> 1024, perhaps limited in the UI.
>>
>> The long-term solution is that I need to break up the background
>> evaluation task into, say, chunks of 128x128 and go through them one
>> at a time in order to cover the whole background.
>>
>> -Farny
>>
>> On Sun, Jan 15, 2012 at 11:35 PM, Mike Farnsworth
>> <mike.farnsworth at gmail.com> wrote:
>>
>> This patch includes a full implementation of importance sampling for
>> environments, including the MIS solution we talked about.  It adds two
>> items for the user to control: on/off switch for whether to treat the
>> environment as a light (off is old behavior), and a resolution control
>> for the importance map.  Here are a few notes about the patch:
>>
>> * For environments that are a solid color (or very slowly changing),
>> turning on the env light machinery is overkill, and will actually
>> result in reduced performance.  For anything complex (textures, sky,
>> noise, etc) it produces noticeably less noise for the same number of
>> passes, so it should be a solid advantage.
>>
>> * Any HDR environment textures are chopped down to SDR range by the
>> texture upload to device right now (nothing to do with my patch, of
>> course); this reduces the effectiveness of this new environment
>> sampling.  If we can fix that, the env light sampling will have an
>> even greater effect, not to mention potentially improved visuals in
>> general.  Just sayin'...
>>
>> * The patch includes a change where the displacement device function
>> now outputs to a float4 buffer, not a float3 buffer, because on my
>> older GPU that was causing completely screwed up sampling of the
>> background when building the importance map.  So, displacement and
>> shader tasks now have a slightly larger output buffer, but it now
>> works on both my older GPU (GTX 260, sm_13) and my newer GPU (GTX 460,
>> sm_21).
>>
>> * There is a commented #define that allows dumping the sampled
>> background and both parts of the importance map to /tmp for debugging
>> purposes.  Those sections of code can be removed, but they could come
>> in handy in the future.
>>
>> I'm going to post some before/after images to morecycles.blogspot.com
>> tonight, so you can see the difference in quality.  I apologize for
>> this taking longer than I had anticipated.  I had left in a
>> __int_as_float(f) where I should have just had an (int)f instead, and
>> it manifested itself with a zero PDF for the environment light, just
>> causing noise.  It took a while to dig through it and find it.
>> Brecht, let me know if you have any issues with the code, and I'll try
>> to turn around any suggestions you make ASAP so this can be included
>> in 2.62.  I understand the deadline is the 22nd?
>>
>> -Farny
>>
>> _______________________________________________
>> Bf-cycles mailing list
>> Bf-cycles at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-cycles
>>
>> _______________________________________________
>> Bf-cycles mailing list
>> Bf-cycles at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-cycles
>>
>>
>>
>> _______________________________________________
>> Bf-cycles mailing list
>> Bf-cycles at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-cycles
>>
>>
>>
>> _______________________________________________
>> Bf-cycles mailing list
>> Bf-cycles at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-cycles
>>
> _______________________________________________
> Bf-cycles mailing list
> Bf-cycles at blender.org
> http://lists.blender.org/mailman/listinfo/bf-cycles