Merge pull request #85 from secondlife/SL-19002

SL-19002: Handle edge case in client networking which could cause material ID and overrides to become out-of-sync

3 files changed, 22 insertions, 110 deletions

diff --git a/indra/newview/llpanelface.cpp b/indra/newview/llpanelface.cpp
index ce086f2520..cb7bc7b5df 100644
--- a/indra/newview/llpanelface.cpp
+++ b/indra/newview/llpanelface.cpp
@@ -3825,34 +3825,12 @@ private:
 
 struct LLPanelFaceUpdateFunctor : public LLSelectedObjectFunctor
 {
-    LLPanelFaceUpdateFunctor(bool update_media, bool update_pbr)
+    LLPanelFaceUpdateFunctor(bool update_media)
         : mUpdateMedia(update_media)
-        , mUpdatePbr(update_pbr)
     {}
 
     virtual bool apply(LLViewerObject* object)
     {
-        if (mUpdatePbr)
-        {
-            // setRenderMaterialId is supposed to create it
-            LLRenderMaterialParams* param_block = (LLRenderMaterialParams*)object->getParameterEntry(LLNetworkData::PARAMS_RENDER_MATERIAL);
-            if (param_block)
-            {
-                if (param_block->isEmpty())
-                {
-                    object->setHasRenderMaterialParams(false);
-                }
-                else if (object->hasRenderMaterialParams())
-                {
-                    object->parameterChanged(LLNetworkData::PARAMS_RENDER_MATERIAL, true);
-                }
-                else
-                {
-                    object->setHasRenderMaterialParams(true);
-                }
-            }
-        }
-
         object->sendTEUpdate();
 
         if (mUpdateMedia)
@@ -3867,7 +3845,6 @@ struct LLPanelFaceUpdateFunctor : public LLSelectedObjectFunctor
     }
 private:
     bool mUpdateMedia;
-    bool mUpdatePbr;
 };
 
 struct LLPanelFaceNavigateHomeFunctor : public LLSelectedTEFunctor
@@ -4003,7 +3980,7 @@ void LLPanelFace::onPasteColor()
     LLPanelFacePasteTexFunctor paste_func(this, PASTE_COLOR);
     selected_objects->applyToTEs(&paste_func);
 
-    LLPanelFaceUpdateFunctor sendfunc(false, false);
+    LLPanelFaceUpdateFunctor sendfunc(false);
     selected_objects->applyToObjects(&sendfunc);
 }
 
@@ -4364,7 +4341,7 @@ void LLPanelFace::onPasteTexture()
     LLPanelFacePasteTexFunctor paste_func(this, PASTE_TEXTURE);
     selected_objects->applyToTEs(&paste_func);
 
-    LLPanelFaceUpdateFunctor sendfunc(true, true);
+    LLPanelFaceUpdateFunctor sendfunc(true);
     selected_objects->applyToObjects(&sendfunc);
 
     LLGLTFMaterialList::flushUpdates();
diff --git a/indra/newview/llselectmgr.cpp b/indra/newview/llselectmgr.cpp
index 02e85d7470..55cf6795fe 100644
--- a/indra/newview/llselectmgr.cpp
+++ b/indra/newview/llselectmgr.cpp
@@ -1941,12 +1941,6 @@ void LLSelectMgr::selectionSetGLTFMaterial(const LLUUID& mat_id)
                 asset_id = mItem->getAssetUUID();
             }
 
-            if (asset_id.notNull() && !objectp->hasRenderMaterialParams())
-            {
-                // make sure param section exists
-                objectp->setParameterEntryInUse(LLNetworkData::PARAMS_RENDER_MATERIAL, TRUE, false /*prevent an update*/);
-            }
-
             // Blank out most override data on the object and send to server
             objectp->setRenderMaterialID(te, asset_id);
 
@@ -1975,25 +1969,6 @@ void LLSelectMgr::selectionSetGLTFMaterial(const LLUUID& mat_id)
                 return false;
             }
 
-            LLRenderMaterialParams* param_block = (LLRenderMaterialParams*)object->getParameterEntry(LLNetworkData::PARAMS_RENDER_MATERIAL);
-            if (param_block)
-            {
-                // To not cause multiple competing request that modify
-                // same param field send update only once per object
-                if (param_block->isEmpty())
-                {
-                    object->setHasRenderMaterialParams(false);
-                }
-                else if (object->hasRenderMaterialParams())
-                {
-                    object->parameterChanged(LLNetworkData::PARAMS_RENDER_MATERIAL, true);
-                }
-                else
-                {
-                    object->setHasRenderMaterialParams(true);
-                }
-            }
-
             if (!mItem)
             {
                 // 1 particle effect per object				
@@ -2209,14 +2184,13 @@ void LLSelectMgr::selectionRevertGLTFMaterials()
             {
                 // Restore base material
                 LLUUID asset_id = nodep->mSavedGLTFMaterialIds[te];
-                objectp->setRenderMaterialID(te, asset_id, false /*wait for bulk update*/);
 
+                // Update material locally
+                objectp->setRenderMaterialID(te, asset_id, false /*wait for LLGLTFMaterialList update*/);
+                objectp->setTEGLTFMaterialOverride(te, nodep->mSavedGLTFOverrideMaterials[te]);
 
-                // todo: make sure this does not cause race condition with setRenderMaterialID
-                // when we are reverting from null id to non null plus override
-                if (te < (S32)nodep->mSavedGLTFOverrideMaterials.size()
-                    && nodep->mSavedGLTFOverrideMaterials[te].notNull()
-                    && asset_id.notNull())
+                // Enqueue update to server
+                if (asset_id.notNull())
                 {
                     // Restore overrides
                     LLGLTFMaterialList::queueModify(objectp, te, nodep->mSavedGLTFOverrideMaterials[te]);
@@ -2232,38 +2206,6 @@ void LLSelectMgr::selectionRevertGLTFMaterials()
         }
     } setfunc(mSelectedObjects);
     getSelection()->applyToTEs(&setfunc);
-
-    struct g : public LLSelectedObjectFunctor
-    {
-        virtual bool apply(LLViewerObject* object)
-        {
-            if (object && !object->permModify())
-            {
-                return false;
-            }
-
-            LLRenderMaterialParams* param_block = (LLRenderMaterialParams*)object->getParameterEntry(LLNetworkData::PARAMS_RENDER_MATERIAL);
-            if (param_block)
-            {
-                if (param_block->isEmpty())
-                {
-                    object->setHasRenderMaterialParams(false);
-                }
-                else if (object->hasRenderMaterialParams())
-                {
-                    object->parameterChanged(LLNetworkData::PARAMS_RENDER_MATERIAL, true);
-                }
-                else
-                {
-                    object->setHasRenderMaterialParams(true);
-                }
-            }
-
-            object->sendTEUpdate();
-            return true;
-        }
-    } sendfunc;
-    getSelection()->applyToObjects(&sendfunc);
 }
 
 void LLSelectMgr::selectionSetBumpmap(U8 bumpmap, const LLUUID &image_id)
diff --git a/indra/newview/llviewerobject.cpp b/indra/newview/llviewerobject.cpp
index 2d9614d5c3..e217ede067 100644
--- a/indra/newview/llviewerobject.cpp
+++ b/indra/newview/llviewerobject.cpp
@@ -6289,6 +6289,11 @@ void LLViewerObject::parameterChanged(U16 param_type, LLNetworkData* data, BOOL
 {
 	if (local_origin)
 	{
+        // *NOTE: Do not send the render material ID in this way as it will get
+        // out-of-sync with other sent client data.
+        // See LLViewerObject::setRenderMaterialID and LLGLTFMaterialList
+        llassert(param_type != LLNetworkData::PARAMS_RENDER_MATERIAL);
+
 		LLViewerRegion* regionp = getRegion();
 		if(!regionp) return;
 
@@ -7245,18 +7250,6 @@ void LLViewerObject::setRenderMaterialID(S32 te_in, const LLUUID& id, bool updat
         });
     }
 
-    if (update_server)
-    {
-        // update via ModifyMaterialParams cap (server will echo back changes)
-        for (S32 te = start_idx; te < end_idx; ++te)
-        {
-            // This sends a cleared version of this object's current material
-            // override, but the override should already be cleared due to
-            // calling setBaseMaterial above.
-            LLGLTFMaterialList::queueApply(this, te, id);
-        }
-    }
-
     // predictively update LLRenderMaterialParams (don't wait for server)
     if (param_block)
     { // update existing parameter block
@@ -7264,17 +7257,17 @@ void LLViewerObject::setRenderMaterialID(S32 te_in, const LLUUID& id, bool updat
         {
             param_block->setMaterial(te, id);
         }
+    }
 
-        if (update_server)
+    if (update_server)
+    {
+        // update via ModifyMaterialParams cap (server will echo back changes)
+        for (S32 te = start_idx; te < end_idx; ++te)
         {
-            // If 'in use' changes, it will send an update itself.
-            bool in_use_changed = setParameterEntryInUse(LLNetworkData::PARAMS_RENDER_MATERIAL, !param_block->isEmpty(), true);
-
-            if (!in_use_changed)
-            {
-                // In use didn't change, but the parameter did, send an update
-                parameterChanged(LLNetworkData::PARAMS_RENDER_MATERIAL, param_block, !param_block->isEmpty(), true);
-            }
+            // This sends a cleared version of this object's current material
+            // override, but the override should already be cleared due to
+            // calling setBaseMaterial above.
+            LLGLTFMaterialList::queueApply(this, te, id);
         }
     }
 }