[opensource-dev] Review Request: VWR-29083 Make SSAO work better

Celierra Darling celierra at gmail.com
Sun Dec 2 09:23:12 PST 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/612/#review1288
-----------------------------------------------------------


Hey Tofu! :3



indra/newview/app_settings/settings.xml
<http://codereview.secondlife.com/r/612/#comment1179>

    I still think it might be valuable to attenuate (HSV) saturation, but since it'd been stuck at 1.0 since forever and it doesn't look like it'll be exposed in Environment Settings anytime soon... *shrug*
    
    Used like this, RenderSSAOEffect might be entirely redundant with RenderSSAOFactor, if I remember correctly?



indra/newview/app_settings/shaders/class1/deferred/softenLightF.glsl
<http://codereview.secondlife.com/r/612/#comment1180>

    Where's the code where ssao_effect_mat had been constructed? (If it's not being used, it should probably be taken out too.)



indra/newview/app_settings/shaders/class1/deferred/sunLightSSAOF.glsl
<http://codereview.secondlife.com/r/612/#comment1181>

    This line can probably go too. (It got removed in class2 but not class1; maybe check for more?)



indra/newview/app_settings/shaders/class2/deferred/sunLightSSAOF.glsl
<http://codereview.secondlife.com/r/612/#comment1177>

    The 'if' here might be problematic.. I remember some cards were choking on branches, thus the convoluted lines that looked like new = old + int(conditional)*value.  (same for class1)
    
    Also, I could have sworn that there used to be comments here specifying what the non-mangled math originally was (a la the old softenLightF.glsl:206-214)....



indra/newview/app_settings/shaders/class2/deferred/sunLightSSAOF.glsl
<http://codereview.secondlife.com/r/612/#comment1178>

    Won't using norm.xyz*diff raw like this cause false positives (from self-occlusion)?  IIRC, that was why the old code used dot(samp-0.05*norm-pos, norm).  (todo for self: render this for one sample to check...)  (same for class1)



indra/newview/app_settings/shaders/class2/deferred/sunLightSSAOF.glsl
<http://codereview.secondlife.com/r/612/#comment1182>

    Out of curiosity, why a minimum, instead of ex. "(angrel>0) ? distrel : 0.0" ?  Seems odd in cases of 0 < angrel < distrel.  (No longer using the sphere assumption?)


- Celierra Darling


On Dec. 2, 2012, 3:49 a.m., Tofu Buzzard wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/612/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2012, 3:49 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Description
> -------
> 
> Use a different scheme for weighting SSAO samples, apply SSAO before fog is applied, fix a bug in the screen-space shadow/ssao smoothing offset where the 'checkerboard' stipple had been refactored incorrectly, change some default settings in line with the resulting visual changes.  Also, improve comments a bit. :3
> 
> 
> This addresses bug VWR-29083.
>     http://jira.secondlife.com/browse/VWR-29083
> 
> 
> Diffs
> -----
> 
>   doc/contributions.txt UNKNOWN 
>   indra/llrender/llshadermgr.h UNKNOWN 
>   indra/llrender/llshadermgr.cpp UNKNOWN 
>   indra/newview/app_settings/settings.xml UNKNOWN 
>   indra/newview/app_settings/shaders/class1/deferred/blurLightF.glsl UNKNOWN 
>   indra/newview/app_settings/shaders/class1/deferred/softenLightF.glsl UNKNOWN 
>   indra/newview/app_settings/shaders/class1/deferred/sunLightSSAOF.glsl UNKNOWN 
>   indra/newview/app_settings/shaders/class2/deferred/softenLightF.glsl UNKNOWN 
>   indra/newview/app_settings/shaders/class2/deferred/sunLightSSAOF.glsl UNKNOWN 
>   indra/newview/pipeline.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/612/diff/
> 
> 
> Testing
> -------
> 
> Been running with this for months on an assortment of nvidia hardware, linux only.
> 
> 
> Thanks,
> 
> Tofu Buzzard
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20121202/6c034c50/attachment-0001.htm 


More information about the opensource-dev mailing list