Re: [opensource-dev] Review Request: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Oz Linden (Scott Lawrence)]

On 2011-03-11 14:21, Lance Corrimal wrote:
> Am Freitag, 11. März 2011 schrieb Hitomi Tiponi:
>> Thanks for that Jonathan. Having seen that I am worried that it may
>> be a little too indistinct, especially as this would be a useful
>> feature when sailing, when the blue of the sea is the background.
>> But I also realise you don't want it to become a dominant feature
>> of the mini-map.
>>
>>
> there is a color choice in preferences for the color of "my chat",
> would that not be the logical choice for a circle on the map that
> indicates the range of "my chat"?

I rather like that idea
___
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: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Oz Linden]

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

(Updated March 12, 2011, 6:33 a.m.)


Review request for Viewer.


Changes
---

corrected the jira link - just put the id in, not the url


Summary
---

I looking at the code, trying to find out where/how to add a new feature, when 
I tripped across one of these and it lit my mental warning bells off.  Vector 
distance comparisons should, IMHO, always be done squared.  So I did some 
greppin, manual analysis, and some careful modification, and here's the result.


This addresses bug VWR-25126.
http://jira.secondlife.com/browse/VWR-25126


Diffs
-

  doc/contributions.txt 344d4c6d7d7e 
  indra/llcharacter/llbvhloader.cpp 344d4c6d7d7e 
  indra/llmath/tests/llbbox_test.cpp 344d4c6d7d7e 
  indra/newview/llagent.cpp 344d4c6d7d7e 
  indra/newview/llfloaterchat.cpp 344d4c6d7d7e 
  indra/newview/llhudeffectlookat.cpp 344d4c6d7d7e 
  indra/newview/llhudeffectpointat.cpp 344d4c6d7d7e 
  indra/newview/llmaniprotate.cpp 344d4c6d7d7e 
  indra/newview/llmanipscale.cpp 344d4c6d7d7e 
  indra/newview/llnetmap.cpp 344d4c6d7d7e 
  indra/newview/llpanelpeople.cpp 344d4c6d7d7e 
  indra/newview/llselectmgr.cpp 344d4c6d7d7e 
  indra/newview/llspeakers.cpp 344d4c6d7d7e 
  indra/newview/llviewerchat.cpp 344d4c6d7d7e 
  indra/newview/llvoicevivox.cpp 344d4c6d7d7e 
  indra/newview/llworld.cpp 344d4c6d7d7e 

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


Testing
---

Compiled a test viewer and used it, undertaking some of my normal activities.  
Results felt good, but are currently anecdotal.  Any suggestions on how to 
properly measure this (or even some actual measurement from those already 
instrumented to measure these things,) would be great!


Thanks,

Cron

___
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: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Oz Linden]

> On March 11, 2011, 3:53 a.m., Boroondas Gupte wrote:
> > indra/newview/llselectmgr.cpp, lines 6574-6587
> > 
> >
> > Memory reuse is good, I guess, but having variable names that only 
> > describe the variable's content correctly part of the time bothers me.

Agree with Boroondas here - the variable used inside the loop should be 
min_dist_squared, not min_dist.

Leave the memory optimization to the compiler.


> On March 11, 2011, 3:53 a.m., Boroondas Gupte wrote:
> > indra/newview/llselectmgr.cpp, lines 6589-6594
> > 
> >
> > Off course, this variable already changed meaning during execution 
> > before your change, so ... meh.

but since you're touching it anyway, fix it


- Oz


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


On March 12, 2011, 6:33 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated March 12, 2011, 6:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> I looking at the code, trying to find out where/how to add a new feature, 
> when I tripped across one of these and it lit my mental warning bells off.  
> Vector distance comparisons should, IMHO, always be done squared.  So I did 
> some greppin, manual analysis, and some careful modification, and here's the 
> result.
> 
> 
> This addresses bug VWR-25126.
> http://jira.secondlife.com/browse/VWR-25126
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 344d4c6d7d7e 
>   indra/llcharacter/llbvhloader.cpp 344d4c6d7d7e 
>   indra/llmath/tests/llbbox_test.cpp 344d4c6d7d7e 
>   indra/newview/llagent.cpp 344d4c6d7d7e 
>   indra/newview/llfloaterchat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectlookat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectpointat.cpp 344d4c6d7d7e 
>   indra/newview/llmaniprotate.cpp 344d4c6d7d7e 
>   indra/newview/llmanipscale.cpp 344d4c6d7d7e 
>   indra/newview/llnetmap.cpp 344d4c6d7d7e 
>   indra/newview/llpanelpeople.cpp 344d4c6d7d7e 
>   indra/newview/llselectmgr.cpp 344d4c6d7d7e 
>   indra/newview/llspeakers.cpp 344d4c6d7d7e 
>   indra/newview/llviewerchat.cpp 344d4c6d7d7e 
>   indra/newview/llvoicevivox.cpp 344d4c6d7d7e 
>   indra/newview/llworld.cpp 344d4c6d7d7e 
> 
> Diff: http://codereview.secondlife.com/r/199/diff
> 
> 
> Testing
> ---
> 
> Compiled a test viewer and used it, undertaking some of my normal activities. 
>  Results felt good, but are currently anecdotal.  Any suggestions on how to 
> properly measure this (or even some actual measurement from those already 
> instrumented to measure these things,) would be great!
> 
> 
> Thanks,
> 
> Cron
> 
>

___
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] PO Test Build (223545)

[Ardy Lay]

* STORM-28 : As a User, I want the ability to send my calling card to 
others (additional fixes)
Tried it a few times.  Seems to have worked but the message I got back 
appeared to be an IM from the recipient (I had an IM session open with 
them at the time), not a notice from the system:

[06:22] Second Life: Inventory item offered
[06:22] Sampler (sample.resident): Sampler (sample.resident) received 
your inventory offer.


* STORM-357 : Gestures button is in the pressed state after drag-n-drop 
but gestures list isn't visible

Button seems to behave better when dragged now.  Not sticking down.

* STORM-399 : Users that has chatted within chat range of the user 
in-world are not added to Recent tab
Unfortunately, objects that chat are in the list as (???)(???).  I see 
users being added to the list and the times seem to be incrementing.


* STORM-1019 : Add ability to display beacons for Media on a Prim objects
Works but console text overwrites or displaces text for scripted with 
touch beacons.  Also, I would prefer the console text to be presented in 
the same sequence as the check boxes on the floater, or, perhaps not 
displayed at all.  The beacons are disabled when the floater is closed.


* STORM-1021 : Viewer shows "L$300" instead of object IM details if 
object sends an IM from another region
"Mini-profile" of an Object IM from an object on a neighboring region 
has all the same details as one for an object in the region I am in.  I 
can now find those objects and identify their owner.


* STORM-1064 : People sidebar panel elements have inconsistent widths 
and map is slightly too tall

Looks better now.


Cheers,
- Merov


___
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


___
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: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Oz Linden]

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


If the comments here are addressed, I think this is probably a good idea.  I 
agree that making changes that might make code less clear is usually not 
justified by claims of performance benefit, but done carefully this change can 
almost certainly be beneficial and not cost any real clarity.


indra/llcharacter/llbvhloader.cpp


I think it would be clearer to either add a new constant 
POSITION_MOTION_THRESHOLD_SQUARED or to write these like

 ... < (POSTION_MOTION_THRESHOLD*POSITION_MOTION_THRESHOLD).

(and same convention elsewhere)


- Oz


On March 12, 2011, 6:33 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated March 12, 2011, 6:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> I looking at the code, trying to find out where/how to add a new feature, 
> when I tripped across one of these and it lit my mental warning bells off.  
> Vector distance comparisons should, IMHO, always be done squared.  So I did 
> some greppin, manual analysis, and some careful modification, and here's the 
> result.
> 
> 
> This addresses bug VWR-25126.
> http://jira.secondlife.com/browse/VWR-25126
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 344d4c6d7d7e 
>   indra/llcharacter/llbvhloader.cpp 344d4c6d7d7e 
>   indra/llmath/tests/llbbox_test.cpp 344d4c6d7d7e 
>   indra/newview/llagent.cpp 344d4c6d7d7e 
>   indra/newview/llfloaterchat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectlookat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectpointat.cpp 344d4c6d7d7e 
>   indra/newview/llmaniprotate.cpp 344d4c6d7d7e 
>   indra/newview/llmanipscale.cpp 344d4c6d7d7e 
>   indra/newview/llnetmap.cpp 344d4c6d7d7e 
>   indra/newview/llpanelpeople.cpp 344d4c6d7d7e 
>   indra/newview/llselectmgr.cpp 344d4c6d7d7e 
>   indra/newview/llspeakers.cpp 344d4c6d7d7e 
>   indra/newview/llviewerchat.cpp 344d4c6d7d7e 
>   indra/newview/llvoicevivox.cpp 344d4c6d7d7e 
>   indra/newview/llworld.cpp 344d4c6d7d7e 
> 
> Diff: http://codereview.secondlife.com/r/199/diff
> 
> 
> Testing
> ---
> 
> Compiled a test viewer and used it, undertaking some of my normal activities. 
>  Results felt good, but are currently anecdotal.  Any suggestions on how to 
> properly measure this (or even some actual measurement from those already 
> instrumented to measure these things,) would be great!
> 
> 
> Thanks,
> 
> Cron
> 
>

___
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: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Oz Linden (Scott Lawrence)]

On 2011-03-11 11:43, Ricky wrote:
Thanks for the review! Yes, I am familiar with those "principles of 
optimization" - yet it seemed to just feel wrong to leave it alone... :P


As to the variables that are initialized with high numbers, there has 
to be a better way: if these were standard for-loops, I would just 
initialize the first value with the first item in the list, and then 
start the loop at the second value, if any.  However, these are a 
iterator style I am not yet familiar enough with to bend that far. 
 Any suggestions?


When I write a for loop that has multiple control variables, I use comma 
expressions.  For example, if I wanted to iterate over selections and 
operate on the first MAX_SELECTIONS or until there were not any more one 
could put the count outside the loop:


   S32 counter = 0;
   LLObjectSelection::root_iterator  it;
   for (it  =  getSelection()->root_begin();
 it  !=  getSelection()->root_end();
 ++it)
   {
if (counter>= MAX_SELECTIONS)
{
break;
}
// whatever it is the loop does

counter++;
   }


One could make that better by moving the counter test up into the second 
expression in the for loop, but I'd take the extra step of also putting 
the initialization and increments of both variables into the for:



   S32 counter;
   LLObjectSelection::root_iterator  it;
   for (it  =  getSelection()->root_begin(), counter = 0;
 it  !=  getSelection()->root_end()&&  counter<  MAX_SELECTIONS;
 ++it, ++counter)
   {
// whatever it is the loop does
   }

I think that makes it clearer that both variables are controlling the loop.

Comma expressions are not seen all that often (this sort if thing is the 
only really good application of them I can think of), but I've never 
found a compiler that didn't support them correctly.


___
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: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Jonathan Yap]

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

(Updated March 12, 2011, 7:51 a.m.)


Review request for Viewer.


Changes
---

Added 2nd ring at 100 meters.
Changed menu wording.
Internally, made the ring drawing code into a separate function.


Summary
---

Add optional range ring to the mini-map -- one centered on you with a radius of 
20m to show local chat range.

By default the range ring is off.

To turn it on you right click on the mini-map and pick the menu entry "Range 
Ring".


This addresses bug Storm-1068.
http://jira.secondlife.com/browse/Storm-1068


Diffs (updated)
-

  indra/newview/app_settings/settings.xml aed94e854443 
  indra/newview/llnetmap.h aed94e854443 
  indra/newview/llnetmap.cpp aed94e854443 
  indra/newview/skins/default/colors.xml aed94e854443 
  indra/newview/skins/default/xui/en/menu_mini_map.xml aed94e854443 

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


Testing
---

Tested with another avatar.  When they are outside the ring they cannot see my 
local chat and when they are inside it they can, so the ring's radius is set 
correctly.

Flew over various types of land to make sure my color choice (blue @10%) was 
always visible.

Enabled range ring, logged out and back on; range ring is still present on 
mini-map.

Panned mini-map, range ring remains centered over avatar.

Used mouse wheel to zoom in and out as far as possible.  Size of range ring 
varied with change in zoom level.


Thanks,

Jonathan

___
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: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Jonathan Yap]

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

(Updated March 12, 2011, 7:52 a.m.)


Review request for Viewer.


Changes
---

Slight tweak to menu name.


Summary
---

Add optional range ring to the mini-map -- one centered on you with a radius of 
20m to show local chat range.

By default the range ring is off.

To turn it on you right click on the mini-map and pick the menu entry "Range 
Ring".


This addresses bug Storm-1068.
http://jira.secondlife.com/browse/Storm-1068


Diffs (updated)
-

  indra/newview/skins/default/xui/en/menu_mini_map.xml aed94e854443 

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


Testing
---

Tested with another avatar.  When they are outside the ring they cannot see my 
local chat and when they are inside it they can, so the ring's radius is set 
correctly.

Flew over various types of land to make sure my color choice (blue @10%) was 
always visible.

Enabled range ring, logged out and back on; range ring is still present on 
mini-map.

Panned mini-map, range ring remains centered over avatar.

Used mouse wheel to zoom in and out as far as possible.  Size of range ring 
varied with change in zoom level.


Thanks,

Jonathan

___
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: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Jonathan Yap]

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

(Updated March 12, 2011, 8 a.m.)


Review request for Viewer.


Changes
---

Trouble with incremental diffs.  Applying all diffs as one file.


Summary
---

Add optional range ring to the mini-map -- one centered on you with a radius of 
20m to show local chat range.

By default the range ring is off.

To turn it on you right click on the mini-map and pick the menu entry "Range 
Ring".


This addresses bug Storm-1068.
http://jira.secondlife.com/browse/Storm-1068


Diffs (updated)
-

  doc/contributions.txt aed94e854443 
  indra/newview/app_settings/settings.xml aed94e854443 
  indra/newview/llnetmap.h aed94e854443 
  indra/newview/llnetmap.cpp aed94e854443 
  indra/newview/skins/default/colors.xml aed94e854443 
  indra/newview/skins/default/xui/en/menu_mini_map.xml aed94e854443 

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


Testing
---

Tested with another avatar.  When they are outside the ring they cannot see my 
local chat and when they are inside it they can, so the ring's radius is set 
correctly.

Flew over various types of land to make sure my color choice (blue @10%) was 
always visible.

Enabled range ring, logged out and back on; range ring is still present on 
mini-map.

Panned mini-map, range ring remains centered over avatar.

Used mouse wheel to zoom in and out as far as possible.  Size of range ring 
varied with change in zoom level.


Thanks,

Jonathan

___
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] PO Test Build (223545)

[Erin Mallory]

storm- 28: seems to work okay
storm-357: fail
Storm-399: fail object names do not seem to work properly if DN is on 
storm-1019: fail: see ardy's comments
storm-1021: seems to work so far
storm-1064: seems alright...  


  ___
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] Outstanding patches since Januari...

[Aleric Inglewood]

This is just a reminder that there are still six patches of me waiting
on the reviewboard:

https://codereview.secondlife.com/r/88/
https://codereview.secondlife.com/r/92/
https://codereview.secondlife.com/r/80/
https://codereview.secondlife.com/r/95/
https://codereview.secondlife.com/r/81/
https://codereview.secondlife.com/r/91/

[This one also, but that one is currently being processed:

https://codereview.secondlife.com/r/93/

Finally there are,

https://codereview.secondlife.com/r/167/
https://codereview.secondlife.com/r/99/ ]

Those six, together with a bunch of others that
were processed in Januari were already done
in December last year - but Oz told me not to post
them because he wouldn't have 'reviewers' ready,
so I waited three weeks. After being posted, seven
more weeks have passed-- and all that time it seems
to make no sense to do anything else:

- Firstly, there are still patches out standing, what's the use to add
more? It makes no sense to submit patches faster than they are
processed.
- Secondly, one of the reasons for the "trial" (I never decided to
really come back, yet, I just wanted to give Oz' project 'snowstorm' a
fair chance) is that I wanted to avoid putting a lot of time into
viewer 2, only to have wasted that time when the trial failed; so I
wanted to avoid putting time into snowstorm when it the current state
is unsatisfactory.

Thus, I have been working on other projects for 10 weeks now(!) (not
viewer or even SL related), and it doesn't seem that things are
improving.
Two weeks ago I informed Oz about my upcoming decision, that I will be
forced to take, to leave the snowstorm project when nothing changed
(and before that I let him know my dissatisfaction plenty of times of
course). I said I'd stick around one more month (that's till the end
of March).

I'm not blaming anyone in particular - I'm sure that the snowstorm
team is heavily undermanned - but the result is the same: this open
source project is a failure.
The only solution that I can think of given the current situation is
to trust good coders more and given them (me included) direct write
access to viewer-development, so I can work at my own pace. A given
fact is that when I work at my own pace you can not keep up with
reviewing and testing... you have to leave that to me and trust that I
do a good job. We did that with snowglobe and as you know all those
patches were good (in the end everything was ported from snowglobe to
snowstorm, too). I simply cannot work like this: I want to work on a
project that I CAN work on at my own (high) pace.

I don't have a lot of hope anymore, but fair is fair... if those six
patches are in viewer-development by the end of the month then I'll
give it a bit more chance.
This mail is just informative. Do with it what you want.

Working currently many hours per day on http://m4ri.sagemath.org/ ,
Aleric
___
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: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Boroondas Gupte]

> On March 12, 2011, 7:16 a.m., Oz Linden wrote:
> > indra/llcharacter/llbvhloader.cpp, line 1199
> > 
> >
> > I think it would be clearer to either add a new constant 
> > POSITION_MOTION_THRESHOLD_SQUARED or to write these like
> > 
> >  ... < (POSTION_MOTION_THRESHOLD*POSITION_MOTION_THRESHOLD).
> > 
> > (and same convention elsewhere)

For this specific line, you probably mean ">", not "<", do you?

Also, what's the convention are you proposing? Grouping the multiplied values 
together by braces? Or leaving away the spaces around the multiplication 
operator?


- Boroondas


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


On March 12, 2011, 6:33 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated March 12, 2011, 6:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> I looking at the code, trying to find out where/how to add a new feature, 
> when I tripped across one of these and it lit my mental warning bells off.  
> Vector distance comparisons should, IMHO, always be done squared.  So I did 
> some greppin, manual analysis, and some careful modification, and here's the 
> result.
> 
> 
> This addresses bug VWR-25126.
> http://jira.secondlife.com/browse/VWR-25126
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 344d4c6d7d7e 
>   indra/llcharacter/llbvhloader.cpp 344d4c6d7d7e 
>   indra/llmath/tests/llbbox_test.cpp 344d4c6d7d7e 
>   indra/newview/llagent.cpp 344d4c6d7d7e 
>   indra/newview/llfloaterchat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectlookat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectpointat.cpp 344d4c6d7d7e 
>   indra/newview/llmaniprotate.cpp 344d4c6d7d7e 
>   indra/newview/llmanipscale.cpp 344d4c6d7d7e 
>   indra/newview/llnetmap.cpp 344d4c6d7d7e 
>   indra/newview/llpanelpeople.cpp 344d4c6d7d7e 
>   indra/newview/llselectmgr.cpp 344d4c6d7d7e 
>   indra/newview/llspeakers.cpp 344d4c6d7d7e 
>   indra/newview/llviewerchat.cpp 344d4c6d7d7e 
>   indra/newview/llvoicevivox.cpp 344d4c6d7d7e 
>   indra/newview/llworld.cpp 344d4c6d7d7e 
> 
> Diff: http://codereview.secondlife.com/r/199/diff
> 
> 
> Testing
> ---
> 
> Compiled a test viewer and used it, undertaking some of my normal activities. 
>  Results felt good, but are currently anecdotal.  Any suggestions on how to 
> properly measure this (or even some actual measurement from those already 
> instrumented to measure these things,) would be great!
> 
> 
> Thanks,
> 
> Cron
> 
>

___
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-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

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

(Updated March 12, 2011, 4:33 p.m.)


Review request for Viewer.


Summary
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug storm-1025.
http://jira.secondlife.com/browse/storm-1025


Diffs
-

  indra/newview/llbottomtray.h 3d2e71443c58 
  indra/newview/llbottomtray.cpp 3d2e71443c58 
  indra/newview/llchatbar.h 3d2e71443c58 
  indra/newview/llchatbar.cpp 3d2e71443c58 

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


Testing
---


Thanks,

Jonathan

___
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-1025 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

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

(Updated March 12, 2011, 4:42 p.m.)


Review request for Viewer.


Changes
---

Ignore this and the previous message--just changing the jira number from vwr to 
storm.  And yes, the diffs are messed up and I don't know how to fix that.


Summary (updated)
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug storm-1025.
http://jira.secondlife.com/browse/storm-1025


Diffs
-

  indra/newview/llbottomtray.h 3d2e71443c58 
  indra/newview/llbottomtray.cpp 3d2e71443c58 
  indra/newview/llchatbar.h 3d2e71443c58 
  indra/newview/llchatbar.cpp 3d2e71443c58 

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


Testing
---


Thanks,

Jonathan

___
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: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Cron Stardust]

> On March 11, 2011, 3:53 a.m., Boroondas Gupte wrote:
> > indra/newview/llnetmap.cpp, line 337
> > 
> >
> > Maybe add a short comment here, that this value is meant to be 
> > overwritten in the loop below it.

Good idea!  I'll do so.


> On March 11, 2011, 3:53 a.m., Boroondas Gupte wrote:
> > indra/newview/llpanelpeople.cpp, lines 161-162
> > 
> >
> > For clarity, please rename these variables to dist1_squared and 
> > dist2_squared, too. Or eliminate them by calling dist_vec_squared right in 
> > the return statement:
> > return dist_vec_squared(item1_pos, me_pos) < 
> > dist_vec_squared(item2_pos, me_pos);
> > 
> > (A bit long, but still shorter than the lines right above it.)

I think I'll go with the idea of remove the extraneous variables.  I don't 
think it makes the code that much less readable or understandable, and it 
leaves less for the compiler to have to optimize away. (Not that that last is a 
reason that can stand up on its own; I simply use it as spice for a stronger 
reason...)


> On March 11, 2011, 3:53 a.m., Boroondas Gupte wrote:
> > indra/newview/llselectmgr.cpp, lines 6574-6587
> > 
> >
> > Memory reuse is good, I guess, but having variable names that only 
> > describe the variable's content correctly part of the time bothers me.
> 
> Oz Linden wrote:
> Agree with Boroondas here - the variable used inside the loop should be 
> min_dist_squared, not min_dist.
> 
> Leave the memory optimization to the compiler.

No problem.  I was just using what I had on hand. :)


> On March 11, 2011, 3:53 a.m., Boroondas Gupte wrote:
> > indra/newview/llselectmgr.cpp, lines 6589-6594
> > 
> >
> > Off course, this variable already changed meaning during execution 
> > before your change, so ... meh.
> 
> Oz Linden wrote:
> but since you're touching it anyway, fix it

QSL: I'm on it! :D


- Cron


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


On March 12, 2011, 6:33 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated March 12, 2011, 6:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> I looking at the code, trying to find out where/how to add a new feature, 
> when I tripped across one of these and it lit my mental warning bells off.  
> Vector distance comparisons should, IMHO, always be done squared.  So I did 
> some greppin, manual analysis, and some careful modification, and here's the 
> result.
> 
> 
> This addresses bug VWR-25126.
> http://jira.secondlife.com/browse/VWR-25126
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 344d4c6d7d7e 
>   indra/llcharacter/llbvhloader.cpp 344d4c6d7d7e 
>   indra/llmath/tests/llbbox_test.cpp 344d4c6d7d7e 
>   indra/newview/llagent.cpp 344d4c6d7d7e 
>   indra/newview/llfloaterchat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectlookat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectpointat.cpp 344d4c6d7d7e 
>   indra/newview/llmaniprotate.cpp 344d4c6d7d7e 
>   indra/newview/llmanipscale.cpp 344d4c6d7d7e 
>   indra/newview/llnetmap.cpp 344d4c6d7d7e 
>   indra/newview/llpanelpeople.cpp 344d4c6d7d7e 
>   indra/newview/llselectmgr.cpp 344d4c6d7d7e 
>   indra/newview/llspeakers.cpp 344d4c6d7d7e 
>   indra/newview/llviewerchat.cpp 344d4c6d7d7e 
>   indra/newview/llvoicevivox.cpp 344d4c6d7d7e 
>   indra/newview/llworld.cpp 344d4c6d7d7e 
> 
> Diff: http://codereview.secondlife.com/r/199/diff
> 
> 
> Testing
> ---
> 
> Compiled a test viewer and used it, undertaking some of my normal activities. 
>  Results felt good, but are currently anecdotal.  Any suggestions on how to 
> properly measure this (or even some actual measurement from those already 
> instrumented to measure these things,) would be great!
> 
> 
> Thanks,
> 
> Cron
> 
>

___
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: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Cron Stardust]

> On March 12, 2011, 7:16 a.m., Oz Linden wrote:
> > indra/llcharacter/llbvhloader.cpp, line 1199
> > 
> >
> > I think it would be clearer to either add a new constant 
> > POSITION_MOTION_THRESHOLD_SQUARED or to write these like
> > 
> >  ... < (POSTION_MOTION_THRESHOLD*POSITION_MOTION_THRESHOLD).
> > 
> > (and same convention elsewhere)
> 
> Boroondas Gupte wrote:
> For this specific line, you probably mean ">", not "<", do you?
> 
> Also, what's the convention are you proposing? Grouping the multiplied 
> values together by braces? Or leaving away the spaces around the 
> multiplication operator?

Oz, I was thinking along those lines (creating or changing the constant,) but I 
wasn't sure if it would be within the scope of this.  After further thought, 
and evaluating the input thus far, I've come to the conclusion that it is 
within the scope.  A new diff incorporating the notes thus far mentioned will 
be forthcoming.


- Cron


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


On March 12, 2011, 6:33 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated March 12, 2011, 6:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> I looking at the code, trying to find out where/how to add a new feature, 
> when I tripped across one of these and it lit my mental warning bells off.  
> Vector distance comparisons should, IMHO, always be done squared.  So I did 
> some greppin, manual analysis, and some careful modification, and here's the 
> result.
> 
> 
> This addresses bug VWR-25126.
> http://jira.secondlife.com/browse/VWR-25126
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 344d4c6d7d7e 
>   indra/llcharacter/llbvhloader.cpp 344d4c6d7d7e 
>   indra/llmath/tests/llbbox_test.cpp 344d4c6d7d7e 
>   indra/newview/llagent.cpp 344d4c6d7d7e 
>   indra/newview/llfloaterchat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectlookat.cpp 344d4c6d7d7e 
>   indra/newview/llhudeffectpointat.cpp 344d4c6d7d7e 
>   indra/newview/llmaniprotate.cpp 344d4c6d7d7e 
>   indra/newview/llmanipscale.cpp 344d4c6d7d7e 
>   indra/newview/llnetmap.cpp 344d4c6d7d7e 
>   indra/newview/llpanelpeople.cpp 344d4c6d7d7e 
>   indra/newview/llselectmgr.cpp 344d4c6d7d7e 
>   indra/newview/llspeakers.cpp 344d4c6d7d7e 
>   indra/newview/llviewerchat.cpp 344d4c6d7d7e 
>   indra/newview/llvoicevivox.cpp 344d4c6d7d7e 
>   indra/newview/llworld.cpp 344d4c6d7d7e 
> 
> Diff: http://codereview.secondlife.com/r/199/diff
> 
> 
> Testing
> ---
> 
> Compiled a test viewer and used it, undertaking some of my normal activities. 
>  Results felt good, but are currently anecdotal.  Any suggestions on how to 
> properly measure this (or even some actual measurement from those already 
> instrumented to measure these things,) would be great!
> 
> 
> Thanks,
> 
> Cron
> 
>

___
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] Release 2.5.2 candidate is available

[Trilo Byte]

One request - at least for the Mac client (not sure if the other platform 
builds are set up this way).  Please give pre-release builds an appropriate 
name (ie Project, Development, Developer, etc).  If I were to follow the 
instructions and drag the application into my applications folder, it would 
overwrite the official release version.  Since the Installer dmg file is 
read-only, the workaround is to drag the file to another location and then 
rename before pulling it into applications.

STORM-1014 Viewer crash in LLSurface::getWaterHeight
No problems, but I imagine you'd need godmode powers to be able to test.
STORM-1026 Viewer crahes while trying to reset Graphics quality
Can't test, the viewer still doesn't support the GPU in my Mac (I don't believe 
SL recognizes any GPU released for Mac since last October).
STORM-1046 Crash in LWorld::removeRegion
No idea how to test
STORM-1047 Crash at LLViewerObjectList::renderObjectsForMap(LLNetMap &) 
[secondlife-bin llviewerobjectlist.cpp]
No trouble using map.  Pan/zoom enable/disable features and items works as 
expected.
STORM-1049 Crash at LLViewerFetchedTexture::forceToSaveRawImage(int,bool) 
[secondlife-bin llviewertexture.cpp]
No idea how to test
STORM-1052 Crash at LLVOCacheEntry::~LLVOCacheEntry() line 138
No idea how to test
STORM-1053 Crash at LLTextureCache::writeToCache line
No idea how to test

Overall, very stable/no crashes.

TriloByte Zanzibar

On Mar 11, 2011, at 1:51 PM, Oz Linden (Scott Lawrence) wrote:

> The build link is: 
> 
>  
> http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/viewer-pre-release/rev/223426/index.html
> 
> This build has no feature changes; it is a set of crash fixes:
> 
> STORM-1014 Viewer crash in LLSurface::getWaterHeight
> STORM-1026 Viewer crahes while trying to reset Graphics quality
> STORM-1046 Crash in LWorld::removeRegion
> STORM-1047 Crash at LLViewerObjectList::renderObjectsForMap(LLNetMap &) 
> [secondlife-bin llviewerobjectlist.cpp]
> STORM-1049 Crash at LLViewerFetchedTexture::forceToSaveRawImage(int,bool) 
> [secondlife-bin llviewertexture.cpp]
> STORM-1052 Crash at LLVOCacheEntry::~LLVOCacheEntry() line 138
> STORM-1053 Crash at LLTextureCache::writeToCache line
> 
> the plan, subject to testing over the next few days, is to push that to the 
> release channel early next week.
> 
> Community testing in the mean time is appreciated.
> 
> ___
> 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

___
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: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Cron Stardust]

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

(Updated March 12, 2011, 11:54 p.m.)


Review request for Viewer.


Changes
---

Switched to using *_SQUARED constants instead of multiplied constants, and 
cleaned up a few other minor issues noted during review.


Summary
---

I looking at the code, trying to find out where/how to add a new feature, when 
I tripped across one of these and it lit my mental warning bells off.  Vector 
distance comparisons should, IMHO, always be done squared.  So I did some 
greppin, manual analysis, and some careful modification, and here's the result.


This addresses bug VWR-25126.
http://jira.secondlife.com/browse/VWR-25126


Diffs (updated)
-

  doc/contributions.txt 344d4c6d7d7e 
  indra/llcharacter/llbvhloader.cpp 344d4c6d7d7e 
  indra/llcommon/indra_constants.h 344d4c6d7d7e 
  indra/llmath/tests/llbbox_test.cpp 344d4c6d7d7e 
  indra/newview/llagent.cpp 344d4c6d7d7e 
  indra/newview/llfloaterchat.cpp 344d4c6d7d7e 
  indra/newview/llhudeffectlookat.cpp 344d4c6d7d7e 
  indra/newview/llhudeffectpointat.cpp 344d4c6d7d7e 
  indra/newview/llmaniprotate.cpp 344d4c6d7d7e 
  indra/newview/llmanipscale.cpp 344d4c6d7d7e 
  indra/newview/llnetmap.cpp 344d4c6d7d7e 
  indra/newview/llpanelpeople.cpp 344d4c6d7d7e 
  indra/newview/llselectmgr.cpp 344d4c6d7d7e 
  indra/newview/llspeakers.cpp 344d4c6d7d7e 
  indra/newview/llviewerchat.cpp 344d4c6d7d7e 
  indra/newview/llviewermessage.cpp 344d4c6d7d7e 
  indra/newview/llviewerparceloverlay.cpp 344d4c6d7d7e 
  indra/newview/llvoicevivox.cpp 344d4c6d7d7e 
  indra/newview/llworld.cpp 344d4c6d7d7e 

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


Testing
---

Compiled a test viewer and used it, undertaking some of my normal activities.  
Results felt good, but are currently anecdotal.  Any suggestions on how to 
properly measure this (or even some actual measurement from those already 
instrumented to measure these things,) would be great!


Thanks,

Cron

___
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