Re: [opensource-dev] Review Request: STORM-1818: Fix for viewer crash when entering chars other than [a-z0-9-_. ] into the grid-selection combo.
---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/563/
---
(Updated March 24, 2012, 5:33 a.m.)
Review request for Viewer.
Changes
---
Rebuilt as STORM-1818 with repository pulled from viewer-release (updated
diff-file).
Summary (updated)
-
STORM-1818: Fix for viewer crash when entering chars other than [a-z0-9-_. ]
into the grid-selection combo.
Description (updated)
---
FIX FOR STORM-1818 (https://jira.secondlife.com/browse/storm-1818) "Abnormal
program termination when I enter an URL to the grid selection combobox
(login-screen) and press enter."
Repository with fix: https://bitbucket.org/MartinRJ/storm-1818/
About:
When entering a malformed grid dns address, the viewer needs to catch the
malformed address. Currently it throws an error in a subroutine of the
grid-address-handling (llviewernetwork.cpp, function 'addGrid') and crashes
without warning due to a thrown error ("throw LLInvalidGridName(grid);").
This fix adds code to:
notifications.xml (a warning message text/notification: "IllegalGridName"),
llviewernetwork.h (a function which checks whether a given grid exists, by name
(string): bool getGridExists),
llpanellogin.cpp (an include for llviewernetwork.h, and a check against an
invalid grid dns address in function onSelectServer).
Todo: Maybe edit the notification message, and update the language files.
This addresses bug STORM-1818.
http://jira.secondlife.com/browse/STORM-1818
Diffs (updated)
-
indra/newview/llpanellogin.cpp acfb0781d850
indra/newview/llviewernetwork.h acfb0781d850
indra/newview/skins/default/xui/en/notifications.xml acfb0781d850
Diff: http://codereview.secondlife.com/r/563/diff/diff
Testing
---
Test plan: Insert a malformed grid dns address (e.g.
http://grid.francogrid.org:8002 which works with some TPVs) into the grid
selection combo on the login screen.
Expected behaviour: a popup shows up, informing about the malformed address,
and the grid selection combo swaps back to the default grid (Agni).
Thanks,
MartinRJ Fayray
___
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: STORM-1818: Fix for viewer crash when entering chars other than [a-z0-9-_. ] into the grid-selection combo.
---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/563/#review1177
---
It seems to me that the better fix would be to support the URLs, as you say
some TPVs do, that are currently causing trouble.
- Jonathan Yap
On March 24, 2012, 5:33 a.m., MartinRJ Fayray wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/563/
> ---
>
> (Updated March 24, 2012, 5:33 a.m.)
>
>
> Review request for Viewer.
>
>
> Description
> ---
>
> FIX FOR STORM-1818 (https://jira.secondlife.com/browse/storm-1818) "Abnormal
> program termination when I enter an URL to the grid selection combobox
> (login-screen) and press enter."
>
> Repository with fix: https://bitbucket.org/MartinRJ/storm-1818/
>
> About:
> When entering a malformed grid dns address, the viewer needs to catch the
> malformed address. Currently it throws an error in a subroutine of the
> grid-address-handling (llviewernetwork.cpp, function 'addGrid') and crashes
> without warning due to a thrown error ("throw LLInvalidGridName(grid);").
>
> This fix adds code to:
> notifications.xml (a warning message text/notification: "IllegalGridName"),
> llviewernetwork.h (a function which checks whether a given grid exists, by
> name (string): bool getGridExists),
> llpanellogin.cpp (an include for llviewernetwork.h, and a check against an
> invalid grid dns address in function onSelectServer).
>
> Todo: Maybe edit the notification message, and update the language files.
>
>
> This addresses bug STORM-1818.
> http://jira.secondlife.com/browse/STORM-1818
>
>
> Diffs
> -
>
> indra/newview/llpanellogin.cpp acfb0781d850
> indra/newview/llviewernetwork.h acfb0781d850
> indra/newview/skins/default/xui/en/notifications.xml acfb0781d850
>
> Diff: http://codereview.secondlife.com/r/563/diff/diff
>
>
> Testing
> ---
>
> Test plan: Insert a malformed grid dns address (e.g.
> http://grid.francogrid.org:8002 which works with some TPVs) into the grid
> selection combo on the login screen.
> Expected behaviour: a popup shows up, informing about the malformed address,
> and the grid selection combo swaps back to the default grid (Agni).
>
>
> Thanks,
>
> MartinRJ Fayray
>
>
___
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: STORM-1818: Fix for viewer crash when entering chars other than [a-z0-9-_. ] into the grid-selection combo.
> On March 24, 2012, 7:22 a.m., Jonathan Yap wrote:
> > It seems to me that the better fix would be to support the URLs, as you say
> > some TPVs do, that are currently causing trouble.
You can still login to non-Linden Lab grids via the --login URI parameter
https://wiki.secondlife.com/wiki/Viewer_parameters
But the grid selection combo doesn't support adding custom grids with a
'corrupt' format at all, as it seems the whole viewer is NOT designed for
non-Linden Lab grids, see here:
https://jira.secondlife.com/browse/VWR-28570?focusedCommentId=315423&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-315423
The hack to support these non-Linden Lab grid-format-urls as suggested here is
an attempt to try parsing input data of any given format - be it valid or not -
and a guessing game what the users input might mean, and that's in my honest
opinion more of a 'political' question for the Linden Lab managment, and not a
crash-fix-issue:
https://jira.secondlife.com/browse/STORM-1818?focusedCommentId=314515&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-314515
Please test the fix/ship it.
Thanks in advance. Kind regards, MartinRJ
- MartinRJ
---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/563/#review1177
---
On March 24, 2012, 5:33 a.m., MartinRJ Fayray wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/563/
> ---
>
> (Updated March 24, 2012, 5:33 a.m.)
>
>
> Review request for Viewer.
>
>
> Description
> ---
>
> FIX FOR STORM-1818 (https://jira.secondlife.com/browse/storm-1818) "Abnormal
> program termination when I enter an URL to the grid selection combobox
> (login-screen) and press enter."
>
> Repository with fix: https://bitbucket.org/MartinRJ/storm-1818/
>
> About:
> When entering a malformed grid dns address, the viewer needs to catch the
> malformed address. Currently it throws an error in a subroutine of the
> grid-address-handling (llviewernetwork.cpp, function 'addGrid') and crashes
> without warning due to a thrown error ("throw LLInvalidGridName(grid);").
>
> This fix adds code to:
> notifications.xml (a warning message text/notification: "IllegalGridName"),
> llviewernetwork.h (a function which checks whether a given grid exists, by
> name (string): bool getGridExists),
> llpanellogin.cpp (an include for llviewernetwork.h, and a check against an
> invalid grid dns address in function onSelectServer).
>
> Todo: Maybe edit the notification message, and update the language files.
>
>
> This addresses bug STORM-1818.
> http://jira.secondlife.com/browse/STORM-1818
>
>
> Diffs
> -
>
> indra/newview/llpanellogin.cpp acfb0781d850
> indra/newview/llviewernetwork.h acfb0781d850
> indra/newview/skins/default/xui/en/notifications.xml acfb0781d850
>
> Diff: http://codereview.secondlife.com/r/563/diff/diff
>
>
> Testing
> ---
>
> Test plan: Insert a malformed grid dns address (e.g.
> http://grid.francogrid.org:8002 which works with some TPVs) into the grid
> selection combo on the login screen.
> Expected behaviour: a popup shows up, informing about the malformed address,
> and the grid selection combo swaps back to the default grid (Agni).
>
>
> Thanks,
>
> MartinRJ Fayray
>
>
___
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: STORM-1818: Fix for viewer crash when entering chars other than [a-z0-9-_. ] into the grid-selection combo.
---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/563/#review1179
---
I would like to point out that having all those linden-internal grids in there
is utterly useless unless you are inside the linden lab network. How about
adding a proper grid manager that supports actually accessible grids instead?
- Lance Corrimal
On March 24, 2012, 5:33 a.m., MartinRJ Fayray wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/563/
> ---
>
> (Updated March 24, 2012, 5:33 a.m.)
>
>
> Review request for Viewer.
>
>
> Description
> ---
>
> FIX FOR STORM-1818 (https://jira.secondlife.com/browse/storm-1818) "Abnormal
> program termination when I enter an URL to the grid selection combobox
> (login-screen) and press enter."
>
> Repository with fix: https://bitbucket.org/MartinRJ/storm-1818/
>
> About:
> When entering a malformed grid dns address, the viewer needs to catch the
> malformed address. Currently it throws an error in a subroutine of the
> grid-address-handling (llviewernetwork.cpp, function 'addGrid') and crashes
> without warning due to a thrown error ("throw LLInvalidGridName(grid);").
>
> This fix adds code to:
> notifications.xml (a warning message text/notification: "IllegalGridName"),
> llviewernetwork.h (a function which checks whether a given grid exists, by
> name (string): bool getGridExists),
> llpanellogin.cpp (an include for llviewernetwork.h, and a check against an
> invalid grid dns address in function onSelectServer).
>
> Todo: Maybe edit the notification message, and update the language files.
>
>
> This addresses bug STORM-1818.
> http://jira.secondlife.com/browse/STORM-1818
>
>
> Diffs
> -
>
> indra/newview/llpanellogin.cpp acfb0781d850
> indra/newview/llviewernetwork.h acfb0781d850
> indra/newview/skins/default/xui/en/notifications.xml acfb0781d850
>
> Diff: http://codereview.secondlife.com/r/563/diff/diff
>
>
> Testing
> ---
>
> Test plan: Insert a malformed grid dns address (e.g.
> http://grid.francogrid.org:8002 which works with some TPVs) into the grid
> selection combo on the login screen.
> Expected behaviour: a popup shows up, informing about the malformed address,
> and the grid selection combo swaps back to the default grid (Agni).
>
>
> Thanks,
>
> MartinRJ Fayray
>
>
___
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: STORM-1818: Fix for viewer crash when entering chars other than [a-z0-9-_. ] into the grid-selection combo.
> On March 24, 2012, 8:08 a.m., Lance Corrimal wrote:
> > I would like to point out that having all those linden-internal grids in
> > there is utterly useless unless you are inside the linden lab network. How
> > about adding a proper grid manager that supports actually accessible grids
> > instead?
@Lance this is only a review of my crash fix, no public discussion. It is only
about this severe bug (abnormal program termination). This is not a public user
group meeting where we discuss features, please bear in mind that you just
demanded me - a volunteer contributor - to add a proper grid manager:
"How about adding a proper grid manager"?
The answer is: no! Of course not!
- MartinRJ
---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/563/#review1179
---
On March 24, 2012, 5:33 a.m., MartinRJ Fayray wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/563/
> ---
>
> (Updated March 24, 2012, 5:33 a.m.)
>
>
> Review request for Viewer.
>
>
> Description
> ---
>
> FIX FOR STORM-1818 (https://jira.secondlife.com/browse/storm-1818) "Abnormal
> program termination when I enter an URL to the grid selection combobox
> (login-screen) and press enter."
>
> Repository with fix: https://bitbucket.org/MartinRJ/storm-1818/
>
> About:
> When entering a malformed grid dns address, the viewer needs to catch the
> malformed address. Currently it throws an error in a subroutine of the
> grid-address-handling (llviewernetwork.cpp, function 'addGrid') and crashes
> without warning due to a thrown error ("throw LLInvalidGridName(grid);").
>
> This fix adds code to:
> notifications.xml (a warning message text/notification: "IllegalGridName"),
> llviewernetwork.h (a function which checks whether a given grid exists, by
> name (string): bool getGridExists),
> llpanellogin.cpp (an include for llviewernetwork.h, and a check against an
> invalid grid dns address in function onSelectServer).
>
> Todo: Maybe edit the notification message, and update the language files.
>
>
> This addresses bug STORM-1818.
> http://jira.secondlife.com/browse/STORM-1818
>
>
> Diffs
> -
>
> indra/newview/llpanellogin.cpp acfb0781d850
> indra/newview/llviewernetwork.h acfb0781d850
> indra/newview/skins/default/xui/en/notifications.xml acfb0781d850
>
> Diff: http://codereview.secondlife.com/r/563/diff/diff
>
>
> Testing
> ---
>
> Test plan: Insert a malformed grid dns address (e.g.
> http://grid.francogrid.org:8002 which works with some TPVs) into the grid
> selection combo on the login screen.
> Expected behaviour: a popup shows up, informing about the malformed address,
> and the grid selection combo swaps back to the default grid (Agni).
>
>
> Thanks,
>
> MartinRJ Fayray
>
>
___
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: STORM-1818: Fix for viewer crash when entering chars other than [a-z0-9-_. ] into the grid-selection combo.
> On March 24, 2012, 8:08 a.m., Lance Corrimal wrote:
> > I would like to point out that having all those linden-internal grids in
> > there is utterly useless unless you are inside the linden lab network. How
> > about adding a proper grid manager that supports actually accessible grids
> > instead?
>
> MartinRJ Fayray wrote:
> @Lance this is only a review of my crash fix, no public discussion. It is
> only about this severe bug (abnormal program termination). This is not a
> public user group meeting where we discuss features, please bear in mind that
> you just demanded me - a volunteer contributor - to add a proper grid manager:
> "How about adding a proper grid manager"?
> The answer is: no! Of course not!
I didn't "demand" anything from you, I was just suggesting something. Calm down
a bit.
- Lance
---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/563/#review1179
---
On March 24, 2012, 5:33 a.m., MartinRJ Fayray wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/563/
> ---
>
> (Updated March 24, 2012, 5:33 a.m.)
>
>
> Review request for Viewer.
>
>
> Description
> ---
>
> FIX FOR STORM-1818 (https://jira.secondlife.com/browse/storm-1818) "Abnormal
> program termination when I enter an URL to the grid selection combobox
> (login-screen) and press enter."
>
> Repository with fix: https://bitbucket.org/MartinRJ/storm-1818/
>
> About:
> When entering a malformed grid dns address, the viewer needs to catch the
> malformed address. Currently it throws an error in a subroutine of the
> grid-address-handling (llviewernetwork.cpp, function 'addGrid') and crashes
> without warning due to a thrown error ("throw LLInvalidGridName(grid);").
>
> This fix adds code to:
> notifications.xml (a warning message text/notification: "IllegalGridName"),
> llviewernetwork.h (a function which checks whether a given grid exists, by
> name (string): bool getGridExists),
> llpanellogin.cpp (an include for llviewernetwork.h, and a check against an
> invalid grid dns address in function onSelectServer).
>
> Todo: Maybe edit the notification message, and update the language files.
>
>
> This addresses bug STORM-1818.
> http://jira.secondlife.com/browse/STORM-1818
>
>
> Diffs
> -
>
> indra/newview/llpanellogin.cpp acfb0781d850
> indra/newview/llviewernetwork.h acfb0781d850
> indra/newview/skins/default/xui/en/notifications.xml acfb0781d850
>
> Diff: http://codereview.secondlife.com/r/563/diff/diff
>
>
> Testing
> ---
>
> Test plan: Insert a malformed grid dns address (e.g.
> http://grid.francogrid.org:8002 which works with some TPVs) into the grid
> selection combo on the login screen.
> Expected behaviour: a popup shows up, informing about the malformed address,
> and the grid selection combo swaps back to the default grid (Agni).
>
>
> Thanks,
>
> MartinRJ Fayray
>
>
___
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: STORM-1823 - Replace current specular model with Normalized Blinn-Phong specularity
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/565/#review1182 --- Ship it! Been running with this patch for about two days with no negative effect. Performance is more or less the same with much improved visuals, exceptionally well in bright lit areas (where previously, the light would almost wash out my avatar in white light) - Cindy Chidester On March 21, 2012, 1:01 p.m., Geenz Spad wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/565/ > --- > > (Updated March 21, 2012, 1:01 p.m.) > > > Review request for Viewer. > > > Description > --- > > For a while now, Second Life's deferred renderer has had a somewhat "toonish" > looking specular model, as opposed to other platforms which try to go for > more physically accurate looking models, such as blinn-phong and similar. > > This feature changes the specular model to a technique called normalized > blinn-phong. The specular model applies the usual blinn-phong term (assume > NdotV to the power of n), multiplied by a normalization function of: > ((n + 2) * (n + 4)) / (8 * PI * (powf(2, -n/2) + n)) > Where n is the specular exponent. > > Gamma correction is also applied to the result to bring the visual results > more in line with what you see in high end game engines such as Unreal Engine > 3 and Frostbite 2, and stored in a lookup texture. > > Test plan can be found in the JIRA. > Repository can be found here: https://bitbucket.org/Geenz/viewer-nbp > > > This addresses bug STORM-1823. > http://jira.secondlife.com/browse/STORM-1823 > > > Diffs > - > > doc/contributions.txt b32595c5170f92ac2dbab635955b1b86634f1475 > indra/newview/app_settings/settings.xml > b32595c5170f92ac2dbab635955b1b86634f1475 > indra/newview/app_settings/shaders/class1/deferred/multiPointLightF.glsl > b32595c5170f92ac2dbab635955b1b86634f1475 > indra/newview/app_settings/shaders/class1/deferred/pointLightF.glsl > b32595c5170f92ac2dbab635955b1b86634f1475 > indra/newview/app_settings/shaders/class1/deferred/softenLightF.glsl > b32595c5170f92ac2dbab635955b1b86634f1475 > indra/newview/app_settings/shaders/class2/deferred/softenLightF.glsl > b32595c5170f92ac2dbab635955b1b86634f1475 > indra/newview/llviewercontrol.cpp b32595c5170f92ac2dbab635955b1b86634f1475 > indra/newview/pipeline.h b32595c5170f92ac2dbab635955b1b86634f1475 > indra/newview/pipeline.cpp b32595c5170f92ac2dbab635955b1b86634f1475 > > Diff: http://codereview.secondlife.com/r/565/diff/diff > > > Testing > --- > > * Tested performance characteristics - both Normalized Blinn-Phong and the > previous model seem to have the same performance > * Checked the perceived "brightness" of the new highlights vs. the old ones - > new ones seem much brighter at higher shiny values than the old ones, and > seem much less "cartoonish" as such > * Checked the impact on different bumpiness values at the test rig at Hippo > Hollow - noticeable visual improvement across all objects (though low shiny > seems to have a much more subtle light reflectance term - this is expected > for normalized blinn-phong) > * Tested different LUT resolutions - best looking seems to be 512x128 with > seemingly no measurable decrease in performance > > You can find the latest build here: > http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/oz_project-8/rev/251711/index.html > > > Thanks, > > Geenz Spad > > ___ 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
