Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

> On March 30, 2011, 3:16 p.m., Boroondas Gupte wrote:
> > indra/llui/lllineeditor.cpp, lines 2294-2297
> > 
> >
> > ... this simple setter isn't implemented right at the declaration (i.e. 
> > in indra/llui/lllineeditor.h)?

Unfortunately this identical change is also in Storm-1094 and I don't want to 
make the MM crazy.  As I do not have much internet at the moment if someone 
would like to adjust both of these jiras to match please do so, but I don't see 
that this is a showstopper if not addressed.


> On March 30, 2011, 3:16 p.m., Boroondas Gupte wrote:
> > indra/newview/llbottomtray.cpp, lines 554-555
> > 
> >
> > If font isn't used later, this could be written as
> > 
> > mNearbyChatBar->getChatBox()->setFont(LLViewerChat::getChatFont());
> > 
> > (Not sure which variant is more readable.)

I was doing a bit of code stealing of my changes in other files, hence the two 
lines made more sense in this instance.


> On March 30, 2011, 3:16 p.m., Boroondas Gupte wrote:
> > indra/newview/llviewerchat.cpp, lines 38-39
> > 
> >
> > Whatever the reason for that comment is, I guess the idea is that you 
> > put new members below it, not above.
> > 
> > Also, should this be initialized to null here ...

This was fixed as well.


- Jonathan


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


On March 30, 2011, 5:43 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/244/
> ---
> 
> (Updated March 30, 2011, 5:43 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
>  Chat preferences > font size should increase size of input text in the chat 
> box
> 
> 
> This addresses bug STORM-1095.
> http://jira.secondlife.com/browse/STORM-1095
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 65ff7415f171 
>   indra/llui/lllineeditor.h 65ff7415f171 
>   indra/llui/lllineeditor.cpp 65ff7415f171 
>   indra/newview/llbottomtray.cpp 65ff7415f171 
>   indra/newview/llfloaterpreference.cpp 65ff7415f171 
>   indra/newview/llnearbychatbar.h 65ff7415f171 
>   indra/newview/llnearbychatbar.cpp 65ff7415f171 
>   indra/newview/llviewerchat.h 65ff7415f171 
>   indra/newview/llviewerchat.cpp 65ff7415f171 
> 
> Diff: http://codereview.secondlife.com/r/244/diff
> 
> 
> Testing
> ---
> 
> Change font size in preferences and see
> 1) Font size in chat input box changes to new size immediately
> 2) Font size is set to selected size when viewer is restarted
> 
> 
> 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-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

> On April 3, 2011, 8:46 p.m., Merov Linden wrote:
> > indra/newview/llviewerchat.cpp, lines 38-39
> > 
> >
> > Should be initialized to NULL.

I fixed that initialization to NULL issue and that change was in the last PO 
test build.


- Jonathan


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


On March 30, 2011, 5:43 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/244/
> ---
> 
> (Updated March 30, 2011, 5:43 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
>  Chat preferences > font size should increase size of input text in the chat 
> box
> 
> 
> This addresses bug STORM-1095.
> http://jira.secondlife.com/browse/STORM-1095
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 65ff7415f171 
>   indra/llui/lllineeditor.h 65ff7415f171 
>   indra/llui/lllineeditor.cpp 65ff7415f171 
>   indra/newview/llbottomtray.cpp 65ff7415f171 
>   indra/newview/llfloaterpreference.cpp 65ff7415f171 
>   indra/newview/llnearbychatbar.h 65ff7415f171 
>   indra/newview/llnearbychatbar.cpp 65ff7415f171 
>   indra/newview/llviewerchat.h 65ff7415f171 
>   indra/newview/llviewerchat.cpp 65ff7415f171 
> 
> Diff: http://codereview.secondlife.com/r/244/diff
> 
> 
> Testing
> ---
> 
> Change font size in preferences and see
> 1) Font size in chat input box changes to new size immediately
> 2) Font size is set to selected size when viewer is restarted
> 
> 
> 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] autobuild & VS 2010 support merged to viewer-development

[Nicky Perian]

Second Life 2.6.3 (225584) Apr  1 2011 02:02:52 (Project Viewer - VS2010)
Release Notes

You are at 255,034.0, 256,238.0, 28.7 in Gibson located at 
sim7019.aditi.lindenlab.com (216.82.40.115:13001)
DRTSIM-42 11.03.29.225233
Release Notes

CPU: Intel(R) Core(TM) i5 CPU 650  @ 3.20GHz (3192.22 MHz)
Memory: 8120 MB
OS Version: Microsoft Windows 7 64-bit Service Pack 1 (Build 7601)
Graphics Card Vendor: ATI Technologies Inc.
Graphics Card: ATI Radeon HD 5450 

Windows Graphics Driver Version: 8.17.0010.1043
OpenGL Version: 4.0.10207 Compatibility Profile Context

libcurl Version: libcurl/7.21.1 OpenSSL/0.9.8q zlib/1.2.5 c-ares/1.7.1
J2C Decoder Version: KDU v6.4.1
Audio Driver Version: FMOD version 3.75
Qt Webkit Version: 4.7.1 (version number hard-coded)
Voice Server Version: Vivox 3.2.0002.9361

Built with MSVC version 1600
Packets Lost: 1/8,654 (0.0%)
**

Hangs at downloading inventory.





From: Nicky Perian 
To: Oz Linden (Scott Lawrence) 
Cc: opensource-dev 
Sent: Sun, April 3, 2011 10:46:01 AM
Subject: Re: [opensource-dev] autobuild & VS 2010 support merged to 
viewer-development








From: Oz Linden (Scott Lawrence) 
To: Nicky Perian 
Cc: opensource-dev 
Sent: Sun, April 3, 2011 8:27:02 AM
Subject: Re: [opensource-dev] autobuild & VS 2010 support merged to 
viewer-development

On 2011-04-03 0:18, Nicky Perian wrote: 
Development viewer installed.
>1. crashed at startup because no settings_development.xml 
>delivered 
>from installer.
>2. copied settings_lindendeveloper.xml to settings_development.xml
>3. logged to aditi. login process took more than a minute maybe 
>close to 2 minutes.
>4. 10 minutes into online and still a ghost.
>
>5. attempted to tp home to resolve ghost of different sim and 
>crashed.
>
You didn't specify which platform you installed, or which version (there 
have been 4 since I posted my note).
sorry,
225662 win7/64bit
___
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] autobuild & VS 2010 support merged to viewer-development

[Mike Chase]

On Sun, 2011-04-03 at 10:57 -0400, Oz Linden (Scott Lawrence) wrote: 
> On 2011-04-03 9:27, Mike Chase wrote:
> > Quick pass through on Linux and all seemed ok. Profiles worked fine and
> > the annoying pause that previouly made the viewer unusable seems fixed.
> 
> Glad to hear that the problem there was in how some of the libraries 
> were built.  Fortunately, in our new build structure it will be much 
> easier for open developers to do library builds the same we that we do, 
> and therefor to contribute improvements and upgrades.
> 
> > I'll use it this evening for an event I'm hosting so it will get a
> > workout and I'll post more experiences then
> 
Used it all last night at my set.  Seemed pretty much solid overall.  I
had a few issues with voice (lost access to the voice server and then
only got half of it back; could hear but not speak).  That may have been
issues with the voice server. It was a one time occurrence.  Again this
is on Linux. Fedora 14 x64.

Mike

> Thanks!
> 


___
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] autobuild & VS 2010 support merged to viewer-development

[Opensource Obscure]

I ran both last vs2010 and v-d builds for many hours in the last
days, trying to use as much SL features as possible.

Performances and behaviour were quite satisfying.
I had no issues, except:

- unusual high inventory loading time in BlueSteel Sandbox 1
(see my report to server-beta mailing list) - no problems in
other regions

- texture loading seems to be decently quick, but maybe not
really optimal. I'm not sure about this. Anyway, the weekend may
not be the ideal time  to test this so I'm going to investigate further

A couple of other issues related to shadows / deferred rendering,
which aren't officially supported features - anyway:

- viewer crashed while enabling shadows and global illuminations
(this was an isolate case: both earlier and later, I could enable
and disable those advanced rendering features with no apparent issues)

- sometimes, ground disappears and sea can be seen (even
in no-water regions). Again, I think this only happened with
shadows enabled. A workaround seems to be disabling and
re-enabling shadows.

None of these issues seems to be "new".
-- 
Opensource Obscure
http://twitter.com/oobscure - http://opensourceobscure.com/lol
___
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 13, 2011, 5:34 a.m., Boroondas Gupte wrote:
> > indra/newview/llfloaterchat.cpp, lines 416-419
> > 
> >
> > If storing return values of functions (or inline computations), just so 
> > we don't have to call the function (or perform the calculation) twice to 
> > get its square is a reoccurring pattern, we might want a helper function 
> > that'll save us from having to introduce extra variables just for this 
> > purpose:
> > 
> > // Either
> > F32 sqr(F32 base) { return (base * base); } // Isn't something like 
> > this already available in the standard library or llmath? Maybe even as a 
> > template?
> > 
> > // and then here
> > F32 distance_squared = 
> > dist_vec_squared(pos_agent, chat.mPosAgent);
> > if (distance_squared > 
> > sqr(gAgent.getNearChatRadius()))
> > // ...
> > 
> > // or, if this mainly occurs comparisons between distances to other 
> > values
> > bool dist_vec_leq( LLVector3 first_position, LLVector3 second_position, 
> > F32 distance)
> > {
> > return ( dist_vec_squared(first_position, second_position) <= 
> > (distance * distance) );
> > }
> > 
> > // and then here
> > 
> > if (!dist_vec_leq(pos_agent, chat.mPosAgent, 
> > gAgent.getNearChatRadius()))
> > // ...
> > 
> > Off course, where intermediary variables help readability or 
> > understandability, we should prefer them, but I don't think that's the case 
> > here.
> 
> Boroondas Gupte wrote:
> [...] if this mainly occurs *at* comparisons [...]
> 
> Cron Stardust wrote:
> Good thoughts, however I think I'll have to defer to Oz on this one: I 
> don't feel qualified to say one way or the other in this case.  To me a 
> helper function would clean up several lines of code in the codebase, but is 
> it called for, and is it in the scope of this change?  I cannot say...  Oz?
> 
> Boroondas Gupte wrote:
> Should we tag the jira issue with the label "needs-design" or is that for 
> user-visible design only?

needs-design is just for UI

While conceptually I like the idea of the helper function, the main motivation 
for this is performance, so I'd say leave it as is without the helper function.


> On March 13, 2011, 5:34 a.m., Boroondas Gupte wrote:
> > indra/llmath/tests/llbbox_test.cpp, line 37
> > 
> >
> > While we're changing this anyway, braces around the assigned define 
> > value probably won't hurt:
> > #define APPROX_EQUAL(a, b)   (dist_vec_squared((a),(b)) < 1e-10)
> > 
> > They protect the define from unexpected operator precedence issues when 
> > used in certain contexts, so that it acts more similar to a function.
> > 
> > Though, one has to wonder anyway, why a define is used here rather than 
> > a function. (Which could be declared inline, if really, really necessary.)
> 
> Cron Stardust wrote:
> The parens makes perfect sense: the lesser-than operator is pretty low on 
> the precedence totem pole.  I'll add them to the next revision, after some 
> further discussion on some of the other topics.
> 
> As to why it's not a function: I have no idea.  C++ has way too many ways 
> of doing the same task! :P
> 
>
> 
> Boroondas Gupte wrote:
> It's almost like perl, isn't it? ;-)

It's not as good as perl :-)


> On March 13, 2011, 5:34 a.m., Boroondas Gupte wrote:
> > indra/newview/llselectmgr.cpp, lines 6596-6599
> > 
> >
> > Notwithstanding the comment above this code, I don't think the notion 
> > of a "factored minimal distance" makes any sense at all. (I might be 
> > mistaken, as I'm not a native speaker of English.)
> > 
> > If we can't think of a better name (e.g. based on the actual geometric 
> > meaning of this intermediary result), just describe what we're storing 
> > here, e.g. with an ugly name like half_sqrt_of_min_dist.
> > 
> > Because the variable is only used very locally and the computation of 
> > its value isn't too complicated, just naming it "tmp" or similar might also 
> > be acceptable, and still better than a name purporting a wrong or confusing 
> > meaning.
> 
> Boroondas Gupte wrote:
> Oh, and remove the trailing whitespace.
> 
> Cron Stardust wrote:
> Good call: I too have no idea what the meaning of "sqrt(min_dist) / 2" 
> truely is, and I am a native speaker of English!  I assumed that the sqrt was 
> what was doing the factoring above, but I could be wrong; factoring to me 
> means separating a number into its constituent factors, ie: 16 = 4 * 2 * 2.  
> (Note that I'm not taking it into its prime factors, just the more general 
> "factoring.") If my assumpti

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 April 4, 2011, 10:34 a.m.)


Review request for Viewer.


Changes
---

Following the review comments above, I parenthesized a #define to make it 
safer, adjusted some notes (and added a TODO) around some extremely obscure 
code that needs further attention but which is outside this scope.


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

Re: [opensource-dev] Review Request: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Boroondas Gupte]

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



indra/newview/llselectmgr.cpp


This comment only really makes sense to those aware of this change. Those 
reading the code later won't easily be able to make any sense of it.

As a rule of thumb, in-code comments should relate to the resulting code 
and commit messages to the change. If you feel you have to deviate from that, 
make it explicit. E.g. here, we could write:

// Replaces a call to dist_vec(), which uses fsqrtf. Thus 
that's what we use here, too.
F32 min_dist = fsqrtf(min_dist_squared);



indra/newview/llselectmgr.cpp


Even if we decided that it is out of scope to decide what "factor" might 
have meant here, we can avoid having to place a FIXME comment here by just 
using a variable name that avoids the term.

half_sqrt_of_min_dist is admittedly an ugly name and doesn't tell the 
reader why we would calculate it, but it is at least truthful and not 
misleading, so I'd really go for that for now.



indra/newview/llselectmgr.cpp


While we're touching this code anyway, put spaces around (i.e. on both 
sides of) the binary * operators (multiplication). That makes it easier to 
visually distinguish them from unary * operators (dereference).


- Boroondas


On April 4, 2011, 10:34 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated April 4, 2011, 10:34 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/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

Re: [opensource-dev] Review Request: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Cron Stardust]

> On April 4, 2011, 2:33 p.m., Boroondas Gupte wrote:
> > indra/newview/llselectmgr.cpp, lines 6591-6592
> > 
> >
> > This comment only really makes sense to those aware of this change. 
> > Those reading the code later won't easily be able to make any sense of it.
> > 
> > As a rule of thumb, in-code comments should relate to the resulting 
> > code and commit messages to the change. If you feel you have to deviate 
> > from that, make it explicit. E.g. here, we could write:
> > 
> > // Replaces a call to dist_vec(), which uses fsqrtf. 
> > Thus that's what we use here, too.
> > F32 min_dist = fsqrtf(min_dist_squared);

The comment is a warning to those who might notice that a few lines further 
down a different sqrt function is used.  Your edition is clearer though.


> On April 4, 2011, 2:33 p.m., Boroondas Gupte wrote:
> > indra/newview/llselectmgr.cpp, lines 6595-6597
> > 
> >
> > While we're touching this code anyway, put spaces around (i.e. on both 
> > sides of) the binary * operators (multiplication). That makes it easier to 
> > visually distinguish them from unary * operators (dereference).

How did I miss doing that? :P


> On April 4, 2011, 2:33 p.m., Boroondas Gupte wrote:
> > indra/newview/llselectmgr.cpp, lines 6594-6597
> > 
> >
> > Even if we decided that it is out of scope to decide what "factor" 
> > might have meant here, we can avoid having to place a FIXME comment here by 
> > just using a variable name that avoids the term.
> > 
> > half_sqrt_of_min_dist is admittedly an ugly name and doesn't tell the 
> > reader why we would calculate it, but it is at least truthful and not 
> > misleading, so I'd really go for that for now.

ugh.. still bugs me.  I agree that the FIXME is obnoxious, and your variable 
name (as ugly as I agree it is,) is only 4 characters longer.  Lemme try an 
idea in a parallel review of my own, let me know what you think.


- Cron


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


On April 4, 2011, 10:34 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated April 4, 2011, 10:34 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/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

[opensource-dev] Review Request: (STORM-941) IM log naming should go by SL name, not DN.

[Seth ProductEngine]

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

Review request for Viewer.


Summary
---

Fixed IM history to use the resident's user name for the log file name.
Added conversions from legacy names or SLURLs with avatar id to the user names 
in cases of logging P2P sessions and inventory offers.


This addresses bug STORM-941.
http://jira.secondlife.com/browse/STORM-941


Diffs
-

  indra/llui/llurlaction.h d30636c2a83a 
  indra/llui/llurlaction.cpp d30636c2a83a 
  indra/newview/llgiveinventory.cpp d30636c2a83a 
  indra/newview/llimview.cpp d30636c2a83a 
  indra/newview/llnotificationhandler.h d30636c2a83a 
  indra/newview/llnotificationhandlerutil.cpp d30636c2a83a 

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


Testing
---


Thanks,

Seth

___
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/#review546
---


- Cron


On April 4, 2011, 10:34 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated April 4, 2011, 10:34 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/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

Re: [opensource-dev] Review Request: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Cron Stardust]

> On April 4, 2011, 4:39 p.m., Cron Stardust wrote:
> >

Huh.. long winded speech became dust on pressing publish...  Here goes again:


- Cron


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


On April 4, 2011, 10:34 a.m., Cron Stardust wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
> ---
> 
> (Updated April 4, 2011, 10:34 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/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

Re: [opensource-dev] Review Request: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.

[Ricky]

ok, this is getting annoying.  Any idea why I am having issues posting to
the review?

Ricky
Cron Stardust

On Mon, Apr 4, 2011 at 4:48 PM, Cron Stardust  wrote:

>This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
>
> Huh.. long winded speech became dust on pressing publish...  Here goes again:
>
>
> - Cron
>
> On April 4th, 2011, 10:34 a.m., Cron Stardust wrote:
>   Review request for Viewer.
> By Cron Stardust.
>
> *Updated April 4, 2011, 10:34 a.m.*
> Description
>
> 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.
>
>   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!
>
>   *Bugs: * VWR-25126 
> Diffs
>
>- 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)
>
> View Diff 
>
___
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 trouble (was: Review Request: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.)

[Boroondas Gupte]

On 04/05/2011 01:49 AM, Ricky wrote:
> ok, this is getting annoying.  Any idea why I am having issues posting
> to the review?
Mayhaps you added comments to code-lines but didn't save them? Also,
comments with overlapping code-line ranges (e.g. one comment about lines
1-7 and one about lines 3-9 of the same file) sometimes eat each other.
If you want to see what would get published before hitting the "publish"
button, click "Edit Review" instead.

Cheers,
Boroondas
___
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 trouble (was: Review Request: Squared all dist_vec() based comparisons and other dist_vec() operations where sensible.)

[Ricky]

I was just doing a straight review, not a source review, and all the
text was gone.  Then I added a comment to my own empty review, and
poof, all but the first line are gone. Very strange.

I did paste in some code as part of an idea I was trying to talk
about, but that started on line 3 of the response, and on line 2 of
the original.  Too bad I forgot to copy before publish!

Well, I'll rewrite it again and post it here in response to the review thread.

Ricky
Cron Stardust
- Persevering in the face of perversity

On Mon, Apr 4, 2011 at 4:54 PM, Boroondas Gupte
 wrote:
> On 04/05/2011 01:49 AM, Ricky wrote:
>> ok, this is getting annoying.  Any idea why I am having issues posting
>> to the review?
> Mayhaps you added comments to code-lines but didn't save them? Also,
> comments with overlapping code-line ranges (e.g. one comment about lines
> 1-7 and one about lines 3-9 of the same file) sometimes eat each other.
> If you want to see what would get published before hitting the "publish"
> button, click "Edit Review" instead.
>
> Cheers,
> Boroondas
>
___
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.

[Ricky]

And since that failed too, I'll try the tried-and-true email list! Ha!

In llselectmgr.cpp (around lines 6586-6595) we've been beating our heads
against some rather... interesting... code.  I had tried adding
a variable to make things clearer, but I now suggest to jump sideways, and
try another route.  Consider the following replacement for the section under
consideration:
---
}
 // factor the distance into the displacement vector. This will get us
 // equally visible movements for both close and far away selections.
F32 min_dist = sqrt(fsqrtf(min_dist_squared)) / 2;
 displ_global.setVec(displ.mV[0] * min_dist,
displ.mV[1] * min_dist,
 displ.mV[2] * min_dist);

// equates to: Displ_global = Displ * M_cam_axes_in_global_frame
---
Yes, the double nested sqrt is rather strange to behold.  The inner fsqrtf
came from the definition of vec_dist() and I wanted it to match that
definition to make absolutely sure that this change is nothing more than a
refactor.

The inner sqrt is to make it the minimum distance.  The outer, and
its subsequent division by two, are what are so confusing: what does this
code do?  Why does it do it that way? The comment doesn't seem to shed much
more light than can be gained from reading the code; it doesn't give the
reasons behind the functionality.

Now, as to what fsqrtf is... According to math.h, it is part of a
multiple-level #define chain that results in the following: fsqrtf(x) =
sqrtf(x) = ((F32)sqrt((F64)(x)))

So aside from the extra type conversions, fsqrtf is practically the same as
sqrt.  Does that change anything?  Should I just make both sqrts one or the
other?

Or should I just leave this mess and move on to making sure the rest of the
patch is done? (Which it practically is, we are just quibbling over the last
stragglers as far as I can tell.)

Ricky
Cron Stardust

On Mon, Apr 4, 2011 at 4:48 PM, Cron Stardust  wrote:

>This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/199/
>
> Huh.. long winded speech became dust on pressing publish...  Here goes again:
>
>
> - Cron
>
> On April 4th, 2011, 10:34 a.m., Cron Stardust wrote:
>   Review request for Viewer.
> By Cron Stardust.
>
> *Updated April 4, 2011, 10:34 a.m.*
> Description
>
> 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.
>
>   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!
>
>   *Bugs: * VWR-25126 
> Diffs
>
>- 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)
>
> View Diff 
>
___
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] autobuild & VS 2010 support merged to viewer-development

[Trilo Byte]

Found something that appears to have broken in 225670 - multi-wearables.

Adding multiple attachments still works as expected, right click on an 
attachment in your inventory or the My Appearance tab and choose Add to add it 
to your currently worn outfit.  However, multiple layers is broken - the Add 
button is ghosted when selecting additional clothing layers in your SL 
inventory.  It does work as expected if selecting an additional clothing layer 
item from one of your established outfit folders in the My Appearance tab.

I'll start winding back through the builds to try and identify the version this 
was broken in, this is a major regression to the fashion set and hopefully this 
can be fixed before 2.6.3 goes to beta.

On Apr 1, 2011, at 3:08 PM, Oz Linden (Scott Lawrence) wrote:

> The autobuild and Visual Studio 2010 branch has been merged to 
> viewer-development
> 
> develop.py is dead, long live autobuild
> 
> We did not get the OPEN-50 changes in before this merge, but it will be 
> very high priority for early next week.
> 
> ___
> 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] autobuild & VS 2010 support merged to viewer-development

[Trilo Byte]

Last build with working multi-wearables (on Mac client, anyways) was 225589.  
I've created a JIRA issue for the bug - 
https://jira.secondlife.com/browse/VWR-25421

If any Windows/Linux users could please check this out and see if they can 
reproduce, I'd appreciate it.

TriloByte Zanzibar

On Apr 4, 2011, at 7:47 PM, Trilo Byte wrote:

> Found something that appears to have broken in 225670 - multi-wearables.
> 
> Adding multiple attachments still works as expected, right click on an 
> attachment in your inventory or the My Appearance tab and choose Add to add 
> it to your currently worn outfit.  However, multiple layers is broken - the 
> Add button is ghosted when selecting additional clothing layers in your SL 
> inventory.  It does work as expected if selecting an additional clothing 
> layer item from one of your established outfit folders in the My Appearance 
> tab.
> 
> I'll start winding back through the builds to try and identify the version 
> this was broken in, this is a major regression to the fashion set and 
> hopefully this can be fixed before 2.6.3 goes to beta.
> 
> On Apr 1, 2011, at 3:08 PM, Oz Linden (Scott Lawrence) wrote:
> 
>> The autobuild and Visual Studio 2010 branch has been merged to 
>> viewer-development
>> 
>> develop.py is dead, long live autobuild
>> 
>> We did not get the OPEN-50 changes in before this merge, but it will be 
>> very high priority for early next week.
>> 
>> ___
>> 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] Snowstorm PO review build

[Anya Kanevsky]

all approved!
thank you for testing help.

a

2011/3/31 Oz Linden (Scott Lawrence) 

>
> http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/oz_viewer-poreview/rev/225458/index.html
>
> Here are the issues addressed in that build:
>
> STORM-399  Users that has
> chatted within chat range of the user in-world are not added to Recent tab
> STORM-1072  [TRANSLATED BUT
> IN EN] in "Monde" > "Profil du lieu" > "À propos des terrains" > "Réglement"
> : "Estate / Full Region" is in english.
> STORM-1094  Chat preferences
> > font size should increase size of input text in IM window
> STORM-1095  Chat preferences
> > font size should increase size of input text in the chat box
> STORM-1108  Enable Hints
> menu entry/preference is confusing
> VWR-25269  [#STORM-1106] "Set
> scripts to running" on an object containing no scripts produces a "unknown
> notification" error
>
>
> ___
> 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

[opensource-dev] UI produces just grey boxes

[Glimmering Sands]

Dear Developers,

Since November last I have been unable to build a a version of the
viewer that does not cause the UI to suffer from what I might call
severe grey fatigue syndrome.  Please see the attached screen shots to
understand what I mean.  I am building using Xcode Version 3.2.3.  I
have pulled fresh sources from mercurial, but to avail.  Always I am
left with this unusable situation.  I am certain it must be something
quite simple, but I am at a loss for what it might be.  Has anyone
else seen this before, or even better, might some kind sole know how I
can rid myself of this problem and finally switch to a newer viewer?

For those who would like to suggest I simply download a prebuilt
binary, this will simply not do.  I have my own custom modifications
to how the viewer handles scripts (I am a scripter, after all, and
then some) for which I cannot be without.  And to anticipate the
question, yes, I have attempted to build from pristine source.  I have
also moved out of the way /Applications/Second Life.app on the off
chance that there was something from there that was being picked up.
I have also dispatched every file located in ~/Library that I could
find associated with Second Life.  Clearly none of this helped as I am
still sending this message.

Thank you to all who read my plight and thank you and kiss to those
who might provide me the solution,

Hugs,

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