[opensource-dev] Review Request: Local Bitmap Browser implementation.
Oz Linden
oz at lindenlab.com
Wed Jun 22 13:39:02 PDT 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/347/#review811
-----------------------------------------------------------
indra/llmath/llvolumemgr.cpp
<http://codereview.secondlife.com/r/347/#comment810>
Unnecessary early returns.
LLVolume* vol = NULL;
if ( lod <= NUM_LODS && ! mVolumeLODs[lod].isNull() )
{
vol = mVolumeLODs[lod];
}
return vol;
indra/newview/llfloaterlocalbitmap.cpp
<http://codereview.secondlife.com/r/347/#comment812>
(many instances of this throughout)
All these clauses should reverse the sense of the tests and only have a single break at the end of each case clause.
The use of early returns is unhelpful when debugging - set a variable and return at the bottom.
indra/newview/llfloaterlocalbitmap.cpp
<http://codereview.secondlife.com/r/347/#comment813>
What's wrong with combining these ifs and not needing the continues?
I know this is getting repetitious....
indra/newview/llfloaterlocalbitmap.cpp
<http://codereview.secondlife.com/r/347/#comment814>
Nice logging !
indra/newview/llfloaterlocalbitmap.cpp
<http://codereview.secondlife.com/r/347/#comment815>
Really a global comment...
don't put braces all on one line, please. See https://wiki.secondlife.com/wiki/Coding_Standard#Braces
- Oz
On June 18, 2011, 8:04 a.m., Vaalith Jinn wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/347/
> -----------------------------------------------------------
>
> (Updated June 18, 2011, 8:04 a.m.)
>
>
> Review request for Viewer.
>
>
> Summary
> -------
>
> Local Bitmap Browser is a mechanism to locally load images into the viewer, track them and optionally (per each image)
> have it check if the image has been overwritten locally and if so - update it in the viewer and inworld.
>
> This change affects build menu by adding a way to open the floater there.
> This change also affects the texture picker - adding tabs that lets the user choose between the "Server" (regular inventory) and "Local" tabs (list of locally added files).
>
>
> This addresses bug STORM-64.
> http://jira.secondlife.com/browse/STORM-64
>
>
> Diffs
> -----
>
> doc/contributions.txt 6a3e7e403bd1
> indra/llmath/llvolumemgr.h 6a3e7e403bd1
> indra/llmath/llvolumemgr.cpp 6a3e7e403bd1
> indra/newview/CMakeLists.txt 6a3e7e403bd1
> indra/newview/llfloaterlocalbitmap.h PRE-CREATION
> indra/newview/llfloaterlocalbitmap.cpp PRE-CREATION
> indra/newview/lltexturectrl.h 6a3e7e403bd1
> indra/newview/lltexturectrl.cpp 6a3e7e403bd1
> indra/newview/llviewerfloaterreg.cpp 6a3e7e403bd1
> indra/newview/llviewerobjectlist.h 6a3e7e403bd1
> indra/newview/llviewertexturelist.h 6a3e7e403bd1
> indra/newview/llvovolume.h 6a3e7e403bd1
> indra/newview/skins/default/xui/en/floater_local_bitmap.xml PRE-CREATION
> indra/newview/skins/default/xui/en/floater_texture_ctrl.xml 6a3e7e403bd1
> indra/newview/skins/default/xui/en/menu_viewer.xml 6a3e7e403bd1
>
> Diff: http://codereview.secondlife.com/r/347/diff
>
>
> Testing
> -------
>
> Texture/Sculptmap/Avatar Layer show.
> Texture/Sculptmap/Avatar Layer update.
> Multiple sculpties update.
> Opening multiple browser floaters works correctly. (two possible since one can be opened from menu above, one from texture picker. all of them + texture picker play nice.)
> Tested adding over 200 images at the same time.
>
>
> Thanks,
>
> Vaalith
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110622/91bef788/attachment-0001.htm
More information about the opensource-dev
mailing list