Re: [opensource-dev] Review Request: Fix for STORM-1854.

[Boroondas Gupte]

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

Ship it!


Ship It!

- Boroondas Gupte


On Nov. 30, 2012, 5:17 p.m., Log Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/609/
> ---
> 
> (Updated Nov. 30, 2012, 5:17 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Description
> ---
> 
> This is a fix for STORM-1854. It prevents an assert failure that happens 
> during viewer startup on linux systems with a sufficiently new version of the 
> fontconfig library (I think fontconfig 1.9). It should not impact mac or 
> windows because those platforms do not use llwindowsdl.cpp file, which was 
> the only file changed.
> 
> 
> This addresses bug STORM-1854.
> http://jira.secondlife.com/browse/STORM-1854
> 
> 
> Diffs
> -
> 
>   indra/llwindow/llwindowsdl.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/609/diff/
> 
> 
> Testing
> ---
> 
> This change builds successfully on all three platforms (Teamcity). I have 
> verified that the startup error no longer occurs on Ubuntu 12.10. Ideally we 
> would test that Linux systems with older versions of fontconfig can also 
> still start the viewer, but I don't have easy access to one of those. 
> 
> 
> Thanks,
> 
> Log Linden
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: Fix for STORM-1854.

[Boroondas Gupte]

> On Dec. 1, 2012, 4 p.m., Nicky Perian wrote:
> > https://bitbucket.org/log_linden/viewer-storm-1854/commits/cf7ad502c7c3/ 
> > does not match this diff. result should be &result

Got fixed in 
https://bitbucket.org/log_linden/viewer-storm-1854/commits/b418be80903520c492e1173f3afbc4021cad5d07


- Boroondas


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


On Nov. 30, 2012, 5:17 p.m., Log Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/609/
> ---
> 
> (Updated Nov. 30, 2012, 5:17 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Description
> ---
> 
> This is a fix for STORM-1854. It prevents an assert failure that happens 
> during viewer startup on linux systems with a sufficiently new version of the 
> fontconfig library (I think fontconfig 1.9). It should not impact mac or 
> windows because those platforms do not use llwindowsdl.cpp file, which was 
> the only file changed.
> 
> 
> This addresses bug STORM-1854.
> http://jira.secondlife.com/browse/STORM-1854
> 
> 
> Diffs
> -
> 
>   indra/llwindow/llwindowsdl.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/609/diff/
> 
> 
> Testing
> ---
> 
> This change builds successfully on all three platforms (Teamcity). I have 
> verified that the startup error no longer occurs on Ubuntu 12.10. Ideally we 
> would test that Linux systems with older versions of fontconfig can also 
> still start the viewer, but I don't have easy access to one of those. 
> 
> 
> Thanks,
> 
> Log Linden
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

[opensource-dev] Review Request: OPEN-152 Fog is broken on water, and extra-broken on water in deferred rendering

[Tofu Buzzard]

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

Review request for Viewer.


Description
---

Made deferred water use the atmospheric helper shader functions properly, made 
fog calculation view-relative rather than absolute-position in deferred and 
non-deferred windlight shaders.


Diffs
-

  doc/contributions.txt UNKNOWN 
  indra/newview/app_settings/shaders/class1/deferred/waterF.glsl UNKNOWN 
  indra/newview/app_settings/shaders/class1/deferred/waterV.glsl UNKNOWN 
  indra/newview/app_settings/shaders/class1/environment/waterV.glsl UNKNOWN 
  indra/newview/llviewershadermgr.cpp UNKNOWN 

Diff: http://codereview.secondlife.com/r/611/diff/


Testing
---

Tested with/without deferred rendering, with various sky presets.


Screenshots
---

before/after comparisons
  http://codereview.secondlife.com/r/611/s/1/


Thanks,

Tofu Buzzard

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: OPEN-152 Fog is broken on water, and extra-broken on water in deferred rendering

[Tofu Buzzard]

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

(Updated Dec. 2, 2012, 2:52 a.m.)


Review request for Viewer.


Description
---

Made deferred water use the atmospheric helper shader functions properly, made 
fog calculation view-relative rather than absolute-position in deferred and 
non-deferred windlight shaders.


Diffs
-

  doc/contributions.txt UNKNOWN 
  indra/newview/app_settings/shaders/class1/deferred/waterF.glsl UNKNOWN 
  indra/newview/app_settings/shaders/class1/deferred/waterV.glsl UNKNOWN 
  indra/newview/app_settings/shaders/class1/environment/waterV.glsl UNKNOWN 
  indra/newview/llviewershadermgr.cpp UNKNOWN 

Diff: http://codereview.secondlife.com/r/611/diff/


Testing
---

Tested with/without deferred rendering, with various sky presets.


Screenshots
---

before/after comparisons
  http://codereview.secondlife.com/r/611/s/1/


Thanks,

Tofu Buzzard

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: OPEN-152 Fog is broken on water, and extra-broken on water in deferred rendering

[Tofu Buzzard]

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

(Updated Dec. 2, 2012, 2:53 a.m.)


Review request for Viewer.


Description
---

Made deferred water use the atmospheric helper shader functions properly, made 
fog calculation view-relative rather than absolute-position in deferred and 
non-deferred windlight shaders.


This addresses bug OPEN-152.
http://jira.secondlife.com/browse/OPEN-152


Diffs
-

  doc/contributions.txt UNKNOWN 
  indra/newview/app_settings/shaders/class1/deferred/waterF.glsl UNKNOWN 
  indra/newview/app_settings/shaders/class1/deferred/waterV.glsl UNKNOWN 
  indra/newview/app_settings/shaders/class1/environment/waterV.glsl UNKNOWN 
  indra/newview/llviewershadermgr.cpp UNKNOWN 

Diff: http://codereview.secondlife.com/r/611/diff/


Testing
---

Tested with/without deferred rendering, with various sky presets.


Screenshots
---

before/after comparisons
  http://codereview.secondlife.com/r/611/s/1/


Thanks,

Tofu Buzzard

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

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

[Tofu Buzzard]

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

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

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

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

[Celierra Darling]

---
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


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


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


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


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


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


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
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges