[opensource-dev] Review Request: CHOP-624 header dependency improvements for build time speedup

[Brad Kittenbrink]

> On May 6, 2011, 3:19 a.m., Boroondas Gupte wrote:
> > indra/newview/llviewerregion.cpp, lines 833-839
> > <http://codereview.secondlife.com/r/289/diff/2/?file=1684#file1684line833>
> >
> >     > on further thought, is this a bug?
> >     > should it be 0.5 * (mImpl->mLandp->getMinZ() + mImpl->mLandp->getMaxZ()) instead?
> >     
> >     Oh, didn't notice that. Then again, I was merely verifying that you don't alter functionality, not whether the present functionality makes any sense.
> >     
> >     Looking at the lines above the one in question and the method name, it might indeed well be that this is a bug and that mdV[VZ] should be set to the mean of the mLandp's minZ and maxZ. However,
> >     1) without explanatory code comments and/or a specification of this method, we cannot tell for sure.
> >     2) Even if this is a bug, relying code might to some hackary to compensate for it, which would lead to overcompensation if this is fixed only here. So this would need careful further examination, first. (If this is a bug and isn't compensated for elsewhere, would its effect be observable somehow? If so, how?)
> >     3) Even if this is a bug, fixing it is out of scope for CHOP-624 and should not delay/block it.
> >     
> >     Conclusion: Good catch, but file a separate jira issue for tracking investigation and (if needed) fixing of that potential problem.
> 
> Brad Kittenbrink wrote:
>     Totally agreed.
> 
> Boroondas Gupte wrote:
>     Please link to that new jira once it's filed. I'm curious what'll become of that topic.

finally got around to writing this up: https://jira.secondlife.com/browse/VWR-25719


- Brad


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


On May 5, 2011, 5:21 p.m., Brad Kittenbrink wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/289/
> -----------------------------------------------------------
> 
> (Updated May 5, 2011, 5:21 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Sorry for the big diff here, but reworking a LOT of header dependencies to reduce complexity and help build time.  Started moving stuff out of public interfaces of classes to improve insulation (for example in llviewerregion.h and llagent.h for the biggest examples).
> 
> 
> This addresses bug CHOP-624.
>     http://jira.secondlife.com/browse/CHOP-624
> 
> 
> Diffs
> -----
> 
>   indra/llcommon/llapp.cpp UNKNOWN 
>   indra/llui/llbutton.h UNKNOWN 
>   indra/llui/llfloaterreg.h UNKNOWN 
>   indra/llui/llfocusmgr.h UNKNOWN 
>   indra/llui/llfocusmgr.cpp UNKNOWN 
>   indra/llui/lliconctrl.h UNKNOWN 
>   indra/llui/lllineeditor.h UNKNOWN 
>   indra/llui/llloadingindicator.h UNKNOWN 
>   indra/llui/llmultislider.cpp UNKNOWN 
>   indra/llui/llpanel.h UNKNOWN 
>   indra/llui/llprogressbar.h UNKNOWN 
>   indra/llui/llprogressbar.cpp UNKNOWN 
>   indra/llui/llslider.h UNKNOWN 
>   indra/llui/llstyle.h UNKNOWN 
>   indra/llui/llstyle.cpp UNKNOWN 
>   indra/llui/lltransutil.cpp UNKNOWN 
>   indra/llui/llui.h UNKNOWN 
>   indra/llui/llview.h UNKNOWN 
>   indra/llui/llviewborder.cpp UNKNOWN 
>   indra/llui/llwindowshade.h UNKNOWN 
>   indra/llxuixml/lltrans.h UNKNOWN 
>   indra/llxuixml/lltrans.cpp UNKNOWN 
>   indra/llxuixml/llxuiparser.h UNKNOWN 
>   indra/newview/llagent.h UNKNOWN 
>   indra/newview/llagent.cpp UNKNOWN 
>   indra/newview/llappviewer.cpp UNKNOWN 
>   indra/newview/lleventnotifier.h UNKNOWN 
>   indra/newview/llfloaterland.h UNKNOWN 
>   indra/newview/llfloaterland.cpp UNKNOWN 
>   indra/newview/llfloatersnapshot.cpp UNKNOWN 
>   indra/newview/llfolderviewitem.h UNKNOWN 
>   indra/newview/lllocationhistory.h UNKNOWN 
>   indra/newview/lloutputmonitorctrl.h UNKNOWN 
>   indra/newview/llpanelavatar.cpp UNKNOWN 
>   indra/newview/llpanelgroupgeneral.cpp UNKNOWN 
>   indra/newview/llpanelgrouproles.cpp UNKNOWN 
>   indra/newview/llpreviewgesture.cpp UNKNOWN 
>   indra/newview/llsidepaneliteminfo.cpp UNKNOWN 
>   indra/newview/lltooldraganddrop.cpp UNKNOWN 
>   indra/newview/llviewerchat.cpp UNKNOWN 
>   indra/newview/llviewerkeyboard.h UNKNOWN 
>   indra/newview/llviewermenu.cpp UNKNOWN 
>   indra/newview/llviewerparcelmgr.cpp UNKNOWN 
>   indra/newview/llviewerprecompiledheaders.h UNKNOWN 
>   indra/newview/llviewerregion.h UNKNOWN 
>   indra/newview/llviewerregion.cpp UNKNOWN 
>   indra/newview/llviewertexturelist.h UNKNOWN 
>   indra/newview/llviewerwindow.h UNKNOWN 
>   indra/newview/llviewerwindow.cpp UNKNOWN 
>   indra/newview/llvoavatar.cpp UNKNOWN 
>   indra/newview/llvotree.cpp UNKNOWN 
>   indra/newview/llvovolume.cpp UNKNOWN 
>   indra/newview/llworld.cpp UNKNOWN 
>   indra/newview/tests/llremoteparcelrequest_test.cpp UNKNOWN 
>   indra/newview/tests/llviewerhelputil_test.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/289/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Brad
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110509/32885f95/attachment.htm