> On June 13, 2011, 8:20 a.m., Oz Linden wrote:
> > indra/newview/lltexlayer.cpp, line 518
> > <http://codereview.secondlife.com/r/152/diff/4/?file=1013#file1013line518>
> >
> >     what's with the minus one here?

mUploadFailCount counts failures, while BAKE_UPLOAD_ATTEMPTS is the number of 
attempts. When doUpload() is called for the last attempt, mUploadFailCount 
equals BAKE_UPLOAD_ATTEMPTS - 1, since the last failure hasn't occurred yet. 
This tries to do the last upload via the old asset store (UDP), just in case 
the HTTP capability isn't working. Does this still makes sense, or will the 
asset store not be supported soon anyway?


> On June 13, 2011, 8:20 a.m., Oz Linden wrote:
> > indra/newview/lltexlayer.h, line 369
> > <http://codereview.secondlife.com/r/152/diff/4/?file=1012#file1012line369>
> >
> >     This would be more clear as an enum: when looking at a call to this 
> > method, a value 'true' or 'false' is much less informative that 
> > 'isHighestRes' or 'isLowRes'

I'm not sure that an enum would make this clearer. This is never called with 
hard-coded values, and the value for the only call to this c'tor is taken from 
the result (BOOL) of mTexLayerSet::isLocalTextureDataFinal. Converting the BOOL 
to an enum, just to check later for equality to one of the enum values seems 
redundant to me.


- Thickbrick


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


On March 1, 2011, 8:58 a.m., Thickbrick Sleaford wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/152/
> -----------------------------------------------------------
> 
> (Updated March 1, 2011, 8:58 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> When a bake upload fails, the viewer doesn't retry it, and subsequently 
> doesn't send a AgentSetAppearance message. This can happen without the user 
> being aware, leaving the avatar looking good on their screen, but not updated 
> to the same outfit on other people's screens. The avatar will remain in that 
> state until the user does something that causes a rebake (manually rebake or 
> change outfit.) The solution here is to retry the upload after a small delay.
> 
> What this diff changes: when a full-res upload fails, retry to upload it 
> after a 5s delay, up to 5 times (in case the cap is available, last attempt 
> is via the old asset store.) Also, some clearer log messages. This implements 
> an old *FIX: comment:
>     // *FIX: retry upload after n seconds, asset server could be busy
> 
> This isn't needed for low res uploads, because they don't block subsequent 
> full-res uploads (mNeedsUpload isn't set to FALSE in 
> LLTexLayerSetBuffer::doUpload in low-res uploads.)
> 
> 
> This addresses bug VWR-24889.
>     http://jira.secondlife.com/browse/VWR-24889
> 
> 
> Diffs
> -----
> 
>   indra/newview/llassetuploadresponders.h 767feb16f05f 
>   indra/newview/llassetuploadresponders.cpp 767feb16f05f 
>   indra/newview/lltexlayer.h 767feb16f05f 
>   indra/newview/lltexlayer.cpp 767feb16f05f 
> 
> Diff: http://codereview.secondlife.com/r/152/diff
> 
> 
> Testing
> -------
> 
> Attempted outfit changes using a problematic connection (not recently used 
> outfits to avoid using cached bakes). Looked for "Baked full res texture 
> upload for <region name> failed" log messages, observed the subsequent 
> retries and successful upload for that region. Observed that eventually the 
> fully-baked avatar is visible to other users.
> 
> 
> Thanks,
> 
> Thickbrick
> 
>

_______________________________________________
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

Reply via email to