[opensource-dev] Review Request: (STORM-380) There is a little delay in sound when gesture first time played

[Boroondas Gupte]

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


I'm a bit worried about the loop-nesting. E.g.


indra/newview/llgesturemgr.cpp
<http://codereview.secondlife.com/r/231/#comment422>

    in update, stepGesture is called in a loop



indra/newview/llgesturemgr.cpp
<http://codereview.secondlife.com/r/231/#comment423>

    stepGesture calls hasLoadingAssets



indra/newview/llgesturemgr.cpp
<http://codereview.secondlife.com/r/231/#comment424>

    hasLoadingAssets calls set::find in a loop (which is logarithmic in the set's size)
    
    so we have N * M * log(L) behavior for update(). This might not be too bad, as the number of queued gestures N, the number of steps per gesture M and the number of loading assets L are probably all small most of the time, and the inner loop is exited via return once the first match has been found. But I still wonder whether it might pay off to e.g. make the gestures observers of their pending assets.
    
    



indra/newview/llgesturemgr.h
<http://codereview.secondlife.com/r/231/#comment421>

    Shouldn't this be a (non-static) method of LLMulitGesture taking no argument, rather than a static member of LLGestureMgr taking a pointer to a gesture?
    
    Also, the gesture should not be changed by that, should it? If possible, make the argument type pointer-to-const (or, if you make it a method of LLMultiGesture as suggested, make it a const method).


- Boroondas


On March 31, 2011, 10:18 a.m., Seth ProductEngine wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/231/
> -----------------------------------------------------------
> 
> (Updated March 31, 2011, 10:18 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Added syncing animations and sounds before the gesture starts playing.
> The actual playing of animations and sounds of a gesture starts only when all needed animations and sound files are loaded into viewer cache. This reduces the delay between animations and sounds meant to be played simultaneously but may increase the delay between the moment a gesture is triggered and the moment it starts playing.
> 
> Fixed calling assets callback to clean up the void pointer in getAssetData() and avoid potential memory leaks.
> 
> 
> This addresses bug STORM-380.
>     http://jira.secondlife.com/browse/STORM-380
> 
> 
> Diffs
> -----
> 
>   indra/llmessage/llassetstorage.cpp d30636c2a83a 
>   indra/newview/llgesturemgr.h d30636c2a83a 
>   indra/newview/llgesturemgr.cpp d30636c2a83a 
> 
> Diff: http://codereview.secondlife.com/r/231/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Seth
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110401/d582589e/attachment-0001.htm