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

Seth ProductEngine slitovchuk at productengine.com
Thu Apr 21 15:07:32 PDT 2011



> On April 1, 2011, 4:22 a.m., Boroondas Gupte wrote:
> > indra/newview/llgesturemgr.cpp, lines 1215-1229
> > <http://codereview.secondlife.com/r/231/diff/2-3/?file=1347#file1347line1215>
> >
> >     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.
> >     
> >

Looks like making gestures observers of their pending assets will not allow to remove the nested loops completely. When an asset is loaded we still have to iterate through all the gestures for which this asset had been requested and remove it from each gesture's pending asset list. This also increases the memory usage because we store separate lists of assets which may have duplicates.

Also making gesture an observer adds the need in tracking its lifetime. Deriving LLMultiGesture from boost::signals2::trackable for that purpose will require LLAssetStorage refactoring to use boost::signals2 for connecting the callbacks.


> On April 1, 2011, 4:22 a.m., Boroondas Gupte wrote:
> > indra/newview/llgesturemgr.h, lines 168-170
> > <http://codereview.secondlife.com/r/231/diff/3/?file=1373#file1373line168>
> >
> >     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).

It could be a non-static method of LLMulitGesture if a gesture would be an observer of its own pending assets, or if a gesture would check pending assets from the list stored at LLGestureMgr. In this case the LLMulitGesture class becomes more complicated without any gain in performance because this is the same check that LLGestureMgr::hasLoadingAssets() does.


- Seth


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


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/20110421/89f63ffb/attachment-0001.htm 


More information about the opensource-dev mailing list