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 <kf6...@gmail.com> 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 <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) > > View Diff <http://codereview.secondlife.com/r/199/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
