search

secondlife / jira-archive

2 stars 0 forks source link

[BUG-225832] Animesh causes continuous avatar complexity changes with CATWA Eyes #4539

Open sl-service-account opened 5 years ago

sl-service-account commented 5 years ago

After updating to the new Animesh release viewer, I noticed the avatar complexity is constantly changing while wearing animated rigged eyes that are part of the CATWA Bento mesh heads. This is extremely annoying as the avatar complexity toast will always be visible in this case. See these links for examples:

Original Jira Fields | Field | Value | | ------------- | ------------- | | Issue | BUG-225832 | | Summary | Animesh causes continuous avatar complexity changes with CATWA Eyes | | Type | Bug | | Priority | Unset | | Status | Accepted | | Resolution | Accepted | | Reporter | Ansariel Hiller (ansariel.hiller) | | Created at | 2018-11-17T12:17:40Z | | Updated at | 2018-11-28T14:04:51Z | ``` { 'Build Id': 'unset', 'Business Unit': ['Platform'], 'Date of First Response': '2018-11-21T10:52:18.712-0600', 'ReOpened Count': 0.0, 'Severity': 'Unset', 'System': 'SL Viewer', 'Target Viewer Version': 'viewer-development', 'What just happened?': "After updating to the new Animesh release viewer, I noticed the avatar complexity is constantly changing while wearing animated rigged eyes that are part of the CATWA Bento mesh heads. This is extremely annoying as the avatar complexity toast will always be visible in this case. See this link for an example: https://gyazo.com/3c743eb444f1c05c8b21f0e2a05c2624\r\n\r\nNotice the avatar complexity is changing when the eyes' pupils widen.\r\n\r\nFor a repro, you will need a CATWA Bento head and the additional CATWA mesh eyes pack:\r\n* Wear the rigged eyes coming with the head\r\n* Wear the CATWA HUD Eyes part of the mesh eyes pack\r\n* Toggle the button marked here: https://gyazo.com/18765dbc5cc77f582abc54bec542ec08 (need to turn it off an on again to enable eye animations)\r\n* Watch the eyes animate and the avatar complexity change", 'What were you doing when it happened?': '-', 'What were you expecting to happen instead?': '-', } ```
sl-service-account commented 5 years ago

polysail commented at 2018-11-21T16:52:19Z, updated at 2018-11-21T17:00:30Z

Easier repro of this phenomenon.  No CatWa products needed.  Lay out a grid of about 32 prim cubes.  Link them all together, drop this script into them and wear them.

default
{
    state_entry()
    {
        llSetLinkPrimitiveParamsFast ( LINK_THIS , [ PRIM_NAME , "FlashingBoxBeqWillHate" ] ) ;
        llSetTimerEvent ( 0.01 ) ;
    }    

  timer ( )
    {
        integer numPrims = llGetNumberOfPrims () ;
        if ( numPrims > 5 ) 
        {

            integer i ;
            for ( i = 1 ; i < 5 ; i ++ )
            {
                integer linkNum = (integer) llFrand ( numPrims ) ;
                integer isGlow = llList2Integer ( llGetLinkPrimitiveParams ( linkNum , [ PRIM_GLOW , 0 ] ) , 0 ) ;
                llSetLinkPrimitiveParamsFast ( linkNum , [ PRIM_GLOW , ALL_SIDES ,  !isGlow ] ) ;
            }
        }

    }
}
sl-service-account commented 5 years ago

Beq Janus commented at 2018-11-21T17:00:38Z

Short video showing this, as an observer. https://gyazo.com/35883925f0126696da70550d419820a6

sl-service-account commented 5 years ago

polysail commented at 2018-11-21T17:10:47Z

Here's the annoying behavior on the LL Viewer as well : 

https://i.gyazo.com/7750e96fd9fd9bb6cee50a266be30c55.mp4

https://i.gyazo.com/b91c4e6804a68b8040b94470b35b4dcb.mp4

sl-service-account commented 5 years ago

Beq Janus commented at 2018-11-25T23:29:01Z

I have identified the cause of the problem and isolated it. This is far more than just an annoying display issue that we should turn off. 

Essentially during the Animesh development, a number of methods were tested to deal with Animesh accounting. This was part of one of them. 

The impact of the change is significant. The video shows my avatar, which is of average complexity for a typical resident. I am wearing a single attached Animesh. 

At the start of the video, I am tabbed out so the frame rate rises to stabilise around 100fps. I have placed a debug setting around the problem code and toggling that to re-enable the current default behaviour leads to the fast updating complexity and an immediate 10% drop in my frame rate. 

https://i.gyazo.com/cb7333d3220cedcdd7eef18654e00800.mp4

disabling it again brings an immediate restoration.

The change in question is revision 56204 on SL-808. Most of the code introduced by that change was subsequently removed/refactored so my hope is that this line was left behind in error and we can remove it. It would need to be pretty important to justify keeping it given the apparent performance cost.

https://i.gyazo.com/2407dbe5f729139429df9fcfcf6aa83a.png

For my test, I simply added a boolean enable/disable flag to the if() condition that is highlighted in the preceding image. My preference though is to just remove the three lines completely rather than have yet another conditional branch.

If is it deemed necessary to have complexity recalculated outside of appearance updates as was historically the case, can I suggest that we examine some way to move this to another thread and have it calculate in parallel so as not to stomp on the critical path performance. 

Note: This was tested with a single Animesh attachment. Without an Animesh attachment, the cost is still high but lower than with (https://i.gyazo.com/14185c2b9c4f1fca314697d410620e98.mp4) this is because there is a recursive check on the attached avatar. This would imply that premium users who are wearing two Animesh attachments get a further performance hit. (Theory untested)

I have also made no attempt, so far, to ascertain whether this is resulting in additional load on the region, or to quantify the additional cost to the viewer of potentially updating the complexity count for all the avatars present in a scene. 

 

sl-service-account commented 5 years ago

Vir Linden commented at 2018-11-26T19:45:50Z

The complexity update is happening in rebuildGeom(), which is called when something about the volume changes. Testing with a rigged mesh avatar and an animesh attachment, this normally gets triggered very rarely - mainly when zooming in and out triggers an LOD update. I think the problem is when an LSL script is making high speed updates to prim properties; this will trigger frequent calls to rebuildGeom(). There's quite a lot of expensive work happening in rebuildGeom(), so we really don't want to be triggering it several times a second. Slow calls to updateVisualComplexity() are just one symptom of the general performance problem.

sl-service-account commented 5 years ago

Beq Janus commented at 2018-11-26T20:45:57Z

Is rebuildGeom() being called more than it was previously though? I did look to see if that were the case but I could not find anything to suggest that (I'll try to pop a debugger on pre-Animesh and do some tests later). If not then this was an existing problem which people are used to as the items that they have, have always had this overhead, it would be great to fix this and give us all a speed up, but the complexity updates are making it considerably worse (my tests suggest). As Ansa noted, this is going to be standard behaviour for a large proportion of residents that happen to have that head and for anyone wearing any item that is scripted to flash or animate. I'll check with Ansa as to whether the eyes that are causing this are standard on the heads. That said though, as we have shown, many other attachments, from scripted weapons and collars to jewellery are going to have this effect, unfortunately.

Given that llSetPrimitiveParamsFast was designed to allow rapid script changes it is not all that surprising that this is happening, sad, but not surprising. Unless you can see what I missed, a recent change that increased the frequency to calls to rebuildGeom, I suspect we are stuck with that behaviour as limiting it on the server side will break a lot of content. We perhaps need to start yet another education drive for how to reduce the render cost of your scripted attachments.

What is the conclusion though? Do we need that update call, is it giving us any significant benefits?

sl-service-account commented 5 years ago

Vir Linden commented at 2018-11-27T20:30:51Z

rebuildGeom() is not being called more AFAIK. The change is that we now call updateVisualComplexity() inside rebuildGeom() when we didn't use to. On the viewer side, we are looking at a fix that would do a couple of things:

sl-service-account commented 5 years ago

Whirly Fizzle commented at 2018-11-28T02:31:00Z

Another repro:

sl-service-account commented 5 years ago

Vir Linden commented at 2018-11-28T14:04:51Z

Disassembled the wedge shoes and looked at the individual prims. Several of them are running a script ("Invisibility Prim Refresh") that changes a texture every 30 seconds, lord knows why. This triggers the usual update/rebuild shuffle, and since the two textures have different resolutions, the cost changes.