SL-18602 Integrate queueModify/queueApply/queueUpdate into all the places that used to post to ModifyMaterialParams directly.

7 files changed, 142 insertions, 60 deletions

diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index 66316a18d4..f3f512a6f2 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -107,6 +107,7 @@
 #include "llscenemonitor.h"
 #include "llavatarrenderinfoaccountant.h"
 #include "lllocalbitmaps.h"
+#include "llgltfmateriallist.h"
 
 // Linden library includes
 #include "llavatarnamecache.h"
@@ -4705,6 +4706,8 @@ void LLAppViewer::idle()
 	LLDirPickerThread::clearDead();
 	F32 dt_raw = idle_timer.getElapsedTimeAndResetF32();
 
+    LLGLTFMaterialList::flushUpdates();
+
 	// Service the WorkQueue we use for replies from worker threads.
 	// Use function statics for the timeslice setting so we only have to fetch
 	// and convert MainWorkTime once.
diff --git a/indra/newview/llgltfmateriallist.cpp b/indra/newview/llgltfmateriallist.cpp
index 36b87b1a3d..2991d26d41 100644
--- a/indra/newview/llgltfmateriallist.cpp
+++ b/indra/newview/llgltfmateriallist.cpp
@@ -52,9 +52,87 @@ LLGLTFMaterialList gGLTFMaterialList;
 
 LLGLTFMaterialList::modify_queue_t LLGLTFMaterialList::sModifyQueue;
 LLGLTFMaterialList::apply_queue_t LLGLTFMaterialList::sApplyQueue;
+LLSD LLGLTFMaterialList::sUpdates;
 
 const LLUUID LLGLTFMaterialList::BLANK_MATERIAL_ASSET_ID("968cbad0-4dad-d64e-71b5-72bf13ad051a");
 
+// return true if given data is (probably) valid update message for ModifyMaterialParams capability
+static bool is_valid_update(const LLSD& data)
+{
+    llassert(data.isMap());
+
+    U32 count = 0;
+
+    if (data.has("object_id"))
+    {
+        if (!data["object_id"].isUUID())
+        {
+            LL_WARNS() << "object_id is not a UUID" << LL_ENDL;
+            return false;
+        }
+        ++count;
+    }
+    else
+    { 
+        LL_WARNS() << "Missing required parameter: object_id" << LL_ENDL;
+        return false;
+    }
+
+    if (data.has("side"))
+    {
+        if (!data["side"].isInteger())
+        {
+            LL_WARNS() << "side is not an integer" << LL_ENDL;
+            return false;
+        }
+
+        if (data["side"].asInteger() < -1)
+        {
+            LL_WARNS() << "side is invalid" << LL_ENDL;
+        }
+        ++count;
+    }
+    else
+    { 
+        LL_WARNS() << "Missing required parameter: side" << LL_ENDL;
+        return false;
+    }
+
+    if (data.has("gltf_json"))
+    {
+        if (!data["gltf_json"].isString())
+        {
+            LL_WARNS() << "gltf_json is not a string" << LL_ENDL;
+            return false;
+        }
+        ++count;
+    }
+
+    if (data.has("asset_id"))
+    {
+        if (!data["asset_id"].isUUID())
+        {
+            LL_WARNS() << "asset_id is not a UUID" << LL_ENDL;
+            return false;
+        }
+        ++count;
+    }
+
+    if (count < 3)
+    { 
+        LL_WARNS() << "Only specified object_id and side, update won't actually change anything and is just noise" << LL_ENDL;
+        return false;
+    }
+
+    if (data.size() != count)
+    {
+        LL_WARNS() << "update data contains unrecognized parameters" << LL_ENDL;
+        return false;
+    }
+
+    return true;
+}
+
 class LLGLTFMaterialOverrideDispatchHandler : public LLDispatchHandler
 {
     LOG_CLASS(LLGLTFMaterialOverrideDispatchHandler);
@@ -231,7 +309,7 @@ void LLGLTFMaterialList::applyQueuedOverrides(LLViewerObject* obj)
     }
 }
 
-void LLGLTFMaterialList::queueModifyMaterial(const LLUUID& id, S32 side, const LLGLTFMaterial* mat)
+void LLGLTFMaterialList::queueModify(const LLUUID& id, S32 side, const LLGLTFMaterial* mat)
 {
     if (mat == nullptr)
     {
@@ -243,16 +321,29 @@ void LLGLTFMaterialList::queueModifyMaterial(const LLUUID& id, S32 side, const L
     }
 }
 
-void LLGLTFMaterialList::queueApplyMaterialAsset(const LLUUID& object_id, S32 side, const LLUUID& asset_id)
+void LLGLTFMaterialList::queueApply(const LLUUID& object_id, S32 side, const LLUUID& asset_id)
 {
     sApplyQueue.push_back({ object_id, side, asset_id});
 }
 
+void LLGLTFMaterialList::queueUpdate(const LLSD& data)
+{
+    llassert(is_valid_update(data));
+
+    if (!sUpdates.isArray())
+    {
+        sUpdates = LLSD::emptyArray();
+    }
+    
+    sUpdates[sUpdates.size()] = data;
+}
+
 void LLGLTFMaterialList::flushUpdates(void(*done_callback)(bool))
 {
-    LLSD data = LLSD::emptyArray();
+    LLSD& data = sUpdates;
+
+    S32 i = data.size();
 
-    S32 i = 0;
     for (auto& e : sModifyQueue)
     {
         data[i]["object_id"] = e.object_id;
@@ -263,6 +354,7 @@ void LLGLTFMaterialList::flushUpdates(void(*done_callback)(bool))
             data[i]["gltf_json"] = e.override_data.asJSON();
         }
 
+        llassert(is_valid_update(data[i]));
         ++i;
     }
     sModifyQueue.clear();
@@ -273,6 +365,8 @@ void LLGLTFMaterialList::flushUpdates(void(*done_callback)(bool))
         data[i]["side"] = e.side;
         data[i]["asset_id"] = e.asset_id;
         data[i]["gltf_json"] = ""; // null out any existing overrides when applying a material asset
+
+        llassert(is_valid_update(data[i]));
         ++i;
     }
     sApplyQueue.clear();
@@ -283,11 +377,18 @@ void LLGLTFMaterialList::flushUpdates(void(*done_callback)(bool))
     LL_INFOS() << "\n" << str.str() << LL_ENDL;
 #endif
 
-    LLCoros::instance().launch("modifyMaterialCoro",
-        std::bind(&LLGLTFMaterialList::modifyMaterialCoro,
-            gAgent.getRegionCapability("ModifyMaterialParams"),
-            data,
-            done_callback));
+    if (sUpdates.size() > 0)
+    {
+        LLCoros::instance().launch("modifyMaterialCoro",
+            std::bind(&LLGLTFMaterialList::modifyMaterialCoro,
+                gAgent.getRegionCapability("ModifyMaterialParams"),
+                sUpdates,
+                done_callback));
+
+        sUpdates = LLSD::emptyArray();
+    }
+
+    
 }
 
 class AssetLoadUserData
diff --git a/indra/newview/llgltfmateriallist.h b/indra/newview/llgltfmateriallist.h
index 9f64f89961..c4eabc8ef7 100644
--- a/indra/newview/llgltfmateriallist.h
+++ b/indra/newview/llgltfmateriallist.h
@@ -53,13 +53,13 @@ public:
 
     static void registerCallbacks();
 
-    // Queue an override update that we want to send to the simulator.  Call "flushUpdates" to flush pending updates.
+    // Queue an modification of a material that we want to send to the simulator.  Call "flushUpdates" to flush pending updates.
     //  id - ID of object to modify
     //  side - TexureEntry index to modify, or -1 for all sides
     //  mat - material to apply as override, or nullptr to remove existing overrides and revert to asset
     //
     // NOTE: do not use to revert to asset when applying a new asset id, use queueApplyMaterialAsset below
-    static void queueModifyMaterial(const LLUUID& id, S32 side, const LLGLTFMaterial* mat);
+    static void queueModify(const LLUUID& id, S32 side, const LLGLTFMaterial* mat);
 
     // Queue an application of a material asset we want to send to the simulator.  Call "flushUpdates" to flush pending updates.
     //  object_id - ID of object to apply material asset to
@@ -67,23 +67,26 @@ public:
     //  asset_id - ID of material asset to apply, or LLUUID::null to disassociate current material asset
     //
     // NOTE: implicitly removes any override data if present
-    static void queueApplyMaterialAsset(const LLUUID& object_id, S32 side, const LLUUID& asset_id);
+    static void queueApply(const LLUUID& object_id, S32 side, const LLUUID& asset_id);
 
     // flush pending material updates to the simulator
-    static void  flushUpdates(void(*done_callback)(bool) = nullptr);
+    // Automatically called once per frame, but may be called explicitly
+    // for cases that care about the done_callback forwarded to LLCoros::instance().launch
+    static void flushUpdates(void(*done_callback)(bool) = nullptr);
     
-    // apply given override data via given cap url
-    //  cap_url -- should be gAgent.getRegionCapability("ModifyMaterialParams")
+    // Queue an explicit LLSD ModifyMaterialParams update apply given override data
     //  overrides -- LLSD map (or array of maps) in the format:
     //      object_id   UUID(required)      id of object
     //      side        integer(required)   TE index of face to set, or -1 for all faces
     //      gltf_json   string(optional)    override data to set, empty string nulls out override data, omissions of this parameter keeps existing data
     //      asset_id    UUID(optional)      id of material asset to set, omission of this parameter keeps existing material asset id
     //    
-    // NOTE: if you're calling this from outside of flushUpdates, you're probably doing it wrong.  Use the "queue"/"flush" API above.
+    // NOTE: Unless you already have a gltf_json string you want to send, strongly prefer using queueModify
     // If the queue/flush API is insufficient, extend it.
-    static void modifyMaterialCoro(std::string cap_url, LLSD overrides, void(*done_callback)(bool));
+    static void queueUpdate(const LLSD& data);
 
+    // Called by batch builder to give LLGLTMaterialList an opportunity to apply
+    // any override data that arrived before the object was ready to receive it
     void applyQueuedOverrides(LLViewerObject* obj);
 
 private:
@@ -92,6 +95,7 @@ private:
     // NOTE: this is NOT for applying overrides from the UI, see queueModifyMaterial above
     void queueOverrideUpdate(const LLUUID& id, S32 side, LLGLTFMaterial* override_data);
 
+    static void modifyMaterialCoro(std::string cap_url, LLSD overrides, void(*done_callback)(bool));
 
 protected:
     static void onAssetLoadComplete(
@@ -132,6 +136,8 @@ protected:
     typedef std::list<ApplyMaterialAssetData> apply_queue_t;
     static apply_queue_t sApplyQueue;
 
+    // data to be flushed to ModifyMaterialParams capability
+    static LLSD    sUpdates;
 };
 
 extern LLGLTFMaterialList gGLTFMaterialList;
diff --git a/indra/newview/llmaterialeditor.cpp b/indra/newview/llmaterialeditor.cpp
index cd3f122101..a9728e26da 100644
--- a/indra/newview/llmaterialeditor.cpp
+++ b/indra/newview/llmaterialeditor.cpp
@@ -2353,11 +2353,9 @@ class LLRenderMaterialOverrideFunctor : public LLSelectedNodeFunctor
 public:
     LLRenderMaterialOverrideFunctor(
         LLMaterialEditor * me,
-        std::string const & url,
         const LLUUID &report_on_object_id,
         S32 report_on_te)
     : mEditor(me)
-    , mCapUrl(url)
     , mSuccess(false)
     , mObjectId(report_on_object_id)
     , mObjectTE(report_on_te)
@@ -2530,34 +2528,12 @@ public:
                 material->setAlphaCutoff(revert_mat->mAlphaCutoff, false);
             }
 
-#if 1
             if (mObjectTE == te
                 && mObjectId == objectp->getID())
             {
                 mSuccess = true;
             }
-            LLGLTFMaterialList::queueModifyMaterial(objectp->getID(), te, material);
-#else
-
-            std::string overrides_json = material->asJSON();
-
-            LLSD overrides = llsd::map(
-                "object_id", objectp->getID(),
-                "side", te,
-                "gltf_json", overrides_json
-            );
-
-            void(*done_callback)(bool) = nullptr;
-            if (mObjectTE == te
-                && mObjectId == objectp->getID())
-            {
-                mSuccess = true;
-                // We only want callback for face we are displayig material from
-                // even if we are setting all of them
-                done_callback = modifyCallback;
-            }
-            LLCoros::instance().launch("modifyMaterialCoro", std::bind(&LLGLTFMaterialList::modifyMaterialCoro, mCapUrl, overrides, done_callback));
-#endif
+            LLGLTFMaterialList::queueModify(objectp->getID(), te, material);
         }
         return true;
     }
@@ -2576,7 +2552,6 @@ public:
 
 private:
     LLMaterialEditor * mEditor;
-    std::string mCapUrl;
     LLUUID mObjectId;
     S32 mObjectTE;
     bool mSuccess;
@@ -2603,7 +2578,7 @@ void LLMaterialEditor::applyToSelection()
         {
             mOverrideInProgress = true;
             LLObjectSelectionHandle selected_objects = LLSelectMgr::getInstance()->getSelection();
-            LLRenderMaterialOverrideFunctor override_func(this, url, mOverrideObjectId, mOverrideObjectTE);
+            LLRenderMaterialOverrideFunctor override_func(this, mOverrideObjectId, mOverrideObjectTE);
             selected_objects->applyToNodes(&override_func);
 
             void(*done_callback)(bool) = LLRenderMaterialOverrideFunctor::modifyCallback;
diff --git a/indra/newview/llpanelface.cpp b/indra/newview/llpanelface.cpp
index f88735e275..b6af2feee3 100644
--- a/indra/newview/llpanelface.cpp
+++ b/indra/newview/llpanelface.cpp
@@ -4383,7 +4383,7 @@ void LLPanelFace::onPasteTexture(LLViewerObject* objectp, S32 te)
             // PBR/GLTF
             if (te_data["te"].has("pbr"))
             {
-                objectp->setRenderMaterialID(te, te_data["te"]["pbr"].asUUID(), false /*send in bulk later*/);
+                objectp->setRenderMaterialID(te, te_data["te"]["pbr"].asUUID(), false /*managing our own update*/);
                 tep->setGLTFRenderMaterial(nullptr);
                 tep->setGLTFMaterialOverride(nullptr);
 
@@ -4394,12 +4394,14 @@ void LLPanelFace::onPasteTexture(LLViewerObject* objectp, S32 te)
                 {
                     override_data["gltf_json"] = te_data["te"]["pbr_override"];
                 }
+                else
+                {
+                    override_data["gltf_json"] = "";
+                }
+
+                override_data["asset_id"] = te_data["te"]["pbr"].asUUID();
 
-                LLCoros::instance().launch("modifyMaterialCoro",
-                    std::bind(&LLGLTFMaterialList::modifyMaterialCoro,
-                        gAgent.getRegionCapability("ModifyMaterialParams"),
-                        override_data,
-                        nullptr));
+                LLGLTFMaterialList::queueUpdate(override_data);
             }
             else
             {
@@ -4408,7 +4410,7 @@ void LLPanelFace::onPasteTexture(LLViewerObject* objectp, S32 te)
                 tep->setGLTFMaterialOverride(nullptr);
 
                 // blank out any override data on the server
-                LLGLTFMaterialList::queueApplyMaterialAsset(objectp->getID(), te, LLUUID::null);
+                LLGLTFMaterialList::queueApply(objectp->getID(), te, LLUUID::null);
             }
 
             // Texture map
diff --git a/indra/newview/llselectmgr.cpp b/indra/newview/llselectmgr.cpp
index 22dae36255..c89348be60 100644
--- a/indra/newview/llselectmgr.cpp
+++ b/indra/newview/llselectmgr.cpp
@@ -1814,7 +1814,7 @@ void LLObjectSelection::applyNoCopyPbrMaterialToTEs(LLViewerInventoryItem* item)
                 object->setRenderMaterialID(te, asset_id, false /*will be sent later*/);
 
                 // blank out any override data on the server
-                LLGLTFMaterialList::queueApplyMaterialAsset(object->getID(), te, asset_id);
+                LLGLTFMaterialList::queueApply(object->getID(), te, asset_id);
             }
         }
     }
@@ -1954,7 +1954,7 @@ void LLSelectMgr::selectionSetGLTFMaterial(const LLUUID& mat_id)
             objectp->setRenderMaterialID(te, asset_id, false /*prevent an update to prevent a race condition*/);
 
             // blank out any override data on the server
-            LLGLTFMaterialList::queueApplyMaterialAsset(objectp->getID(), te, asset_id);
+            LLGLTFMaterialList::queueApply(objectp->getID(), te, asset_id);
 
             return true;
         }
@@ -2229,11 +2229,7 @@ void LLSelectMgr::selectionRevertGLTFMaterials()
                     overrides["gltf_json"] = nodep->mSavedGLTFOverrideMaterials[te]->asJSON();
                 } // else nothing to blank override out
 
-                LLCoros::instance().launch("modifyMaterialCoro",
-                    std::bind(&LLGLTFMaterialList::modifyMaterialCoro,
-                        gAgent.getRegionCapability("ModifyMaterialParams"),
-                        overrides,
-                        nullptr));
+                LLGLTFMaterialList::queueUpdate(overrides);
             }
             return true;
         }
diff --git a/indra/newview/llviewerobject.cpp b/indra/newview/llviewerobject.cpp
index 3cd5b1dbe7..a6e4b2b849 100644
--- a/indra/newview/llviewerobject.cpp
+++ b/indra/newview/llviewerobject.cpp
@@ -7208,9 +7208,8 @@ void LLViewerObject::setRenderMaterialID(S32 te_in, const LLUUID& id, bool updat
         // update via ModifyMaterialParams cap (server will echo back changes)
         for (S32 te = start_idx; te < end_idx; ++te)
         {
-            LLGLTFMaterialList::queueApplyMaterialAsset(getID(), te, id);
+            LLGLTFMaterialList::queueApply(getID(), te, id);
         }
-        LLGLTFMaterialList::flushUpdates();
     }
 
     // predictively update LLRenderMaterialParams (don't wait for server)