SL-18602 Fix for applying material asset not removing overrides on drag-and-drop

4 files changed, 170 insertions, 139 deletions

diff --git a/indra/newview/llgltfmateriallist.cpp b/indra/newview/llgltfmateriallist.cpp
index 8e184c719d..e08a9c2533 100644
--- a/indra/newview/llgltfmateriallist.cpp
+++ b/indra/newview/llgltfmateriallist.cpp
@@ -51,139 +51,141 @@
 LLGLTFMaterialList gGLTFMaterialList;
 
 LLGLTFMaterialList::modify_queue_t LLGLTFMaterialList::sModifyQueue;
+LLGLTFMaterialList::apply_queue_t LLGLTFMaterialList::sApplyQueue;
 
 const LLUUID LLGLTFMaterialList::BLANK_MATERIAL_ASSET_ID("968cbad0-4dad-d64e-71b5-72bf13ad051a");
 
-namespace
+class LLGLTFMaterialOverrideDispatchHandler : public LLDispatchHandler
 {
-    class LLGLTFMaterialOverrideDispatchHandler : public LLDispatchHandler
-    {
-        LOG_CLASS(LLGLTFMaterialOverrideDispatchHandler);
-    public:
-        LLGLTFMaterialOverrideDispatchHandler() = default;
-        ~LLGLTFMaterialOverrideDispatchHandler() override = default;
+    LOG_CLASS(LLGLTFMaterialOverrideDispatchHandler);
+public:
+    LLGLTFMaterialOverrideDispatchHandler() = default;
+    ~LLGLTFMaterialOverrideDispatchHandler() override = default;
 
-        bool operator()(const LLDispatcher* dispatcher, const std::string& key, const LLUUID& invoice, const sparam_t& strings) override
-        {
-            LL_PROFILE_ZONE_SCOPED;
-            // receive override data from simulator via LargeGenericMessage
-            // message should have:
-            //  object_id - UUID of LLViewerObject
-            //  side - S32 index of texture entry
-            //  gltf_json - String of GLTF json for override data
+    bool operator()(const LLDispatcher* dispatcher, const std::string& key, const LLUUID& invoice, const sparam_t& strings) override
+    {
+        LL_PROFILE_ZONE_SCOPED;
+        // receive override data from simulator via LargeGenericMessage
+        // message should have:
+        //  object_id - UUID of LLViewerObject
+        //  side - S32 index of texture entry
+        //  gltf_json - String of GLTF json for override data
 
 
-            LLSD message;
+        LLSD message;
 
-            sparam_t::const_iterator it = strings.begin();
-            if (it != strings.end()) {
-                const std::string& llsdRaw = *it++;
-                std::istringstream llsdData(llsdRaw);
-                if (!LLSDSerialize::deserialize(message, llsdData, llsdRaw.length()))
-                {
-                    LL_WARNS() << "LLGLTFMaterialOverrideDispatchHandler: Attempted to read parameter data into LLSD but failed:" << llsdRaw << LL_ENDL;
-                }
+        sparam_t::const_iterator it = strings.begin();
+        if (it != strings.end()) {
+            const std::string& llsdRaw = *it++;
+            std::istringstream llsdData(llsdRaw);
+            if (!LLSDSerialize::deserialize(message, llsdData, llsdRaw.length()))
+            {
+                LL_WARNS() << "LLGLTFMaterialOverrideDispatchHandler: Attempted to read parameter data into LLSD but failed:" << llsdRaw << LL_ENDL;
             }
+        }
 
-            LLUUID object_id = message["object_id"].asUUID();
+        LLUUID object_id = message["object_id"].asUUID();
             
-            LLViewerObject * obj = gObjectList.findObject(object_id); // NOTE: null object here does NOT mean nothing to do, parse message and queue results for later
-            bool clear_all = true;
+        LLViewerObject * obj = gObjectList.findObject(object_id); // NOTE: null object here does NOT mean nothing to do, parse message and queue results for later
+        bool clear_all = true;
 
-            if (message.has("sides") && message.has("gltf_json"))
-            {
-                LLSD& sides = message["sides"];
-                LLSD& gltf_json = message["gltf_json"];
+        if (message.has("sides") && message.has("gltf_json"))
+        {
+            LLSD& sides = message["sides"];
+            LLSD& gltf_json = message["gltf_json"];
 
-                if (sides.isArray() && gltf_json.isArray() &&
-                    sides.size() != 0 &&
-                    sides.size() == gltf_json.size())
-                {
-                    clear_all = false;
+            if (sides.isArray() && gltf_json.isArray() &&
+                sides.size() != 0 &&
+                sides.size() == gltf_json.size())
+            {
+                clear_all = false;
 
-                    // message should be interpreted thusly:
-                    ///  sides is a list of face indices
-                    //   gltf_json is a list of corresponding json
-                    //   any side not represented in "sides" has no override
+                // message should be interpreted thusly:
+                ///  sides is a list of face indices
+                //   gltf_json is a list of corresponding json
+                //   any side not represented in "sides" has no override
 
-                    // parse json
-                    std::unordered_set<S32> side_set;
+                // parse json
+                std::unordered_set<S32> side_set;
 
-                    for (int i = 0; i < sides.size(); ++i)
-                    {
-                        LLPointer<LLGLTFMaterial> override_data = new LLGLTFMaterial();
+                for (int i = 0; i < sides.size(); ++i)
+                {
+                    LLPointer<LLGLTFMaterial> override_data = new LLGLTFMaterial();
                         
-                        std::string gltf_json = message["gltf_json"][i].asString();
+                    std::string gltf_json = message["gltf_json"][i].asString();
 
-                        std::string warn_msg, error_msg;
+                    std::string warn_msg, error_msg;
                         
-                        bool success = override_data->fromJSON(gltf_json, warn_msg, error_msg);
+                    bool success = override_data->fromJSON(gltf_json, warn_msg, error_msg);
+
+                    if (!success)
+                    {
+                        LL_WARNS() << "failed to parse GLTF override data.  errors: " << error_msg << " | warnings: " << warn_msg << LL_ENDL;
 
-                        if (!success)
+                        // unblock material editor
+                        if (obj && obj->isAnySelected())
                         {
-                            LL_WARNS() << "failed to parse GLTF override data.  errors: " << error_msg << " | warnings: " << warn_msg << LL_ENDL;
+                            LLMaterialEditor::updateLive(object_id, sides[i].asInteger());
+                        }
+                    }
+                    else
+                    {
+                        S32 side = sides[i].asInteger();
+                        // flag this side to not be nulled out later
+                        side_set.insert(sides[i]);
 
-                            // unblock material editor
-                            if (obj && obj->isAnySelected())
-                            {
-                                LLMaterialEditor::updateLive(object_id, sides[i].asInteger());
-                            }
+                        if (!obj || !obj->setTEGLTFMaterialOverride(side, override_data))
+                        {
+                            // object not ready to receive override data, queue for later
+                            gGLTFMaterialList.queueOverrideUpdate(object_id, side, override_data);
                         }
-                        else
+                        else if (obj && obj->isAnySelected())
                         {
-                            S32 side = sides[i].asInteger();
-                            // flag this side to not be nulled out later
-                            side_set.insert(sides[i]);
-
-                            if (!obj || !obj->setTEGLTFMaterialOverride(side, override_data))
-                            {
-                                // object not ready to receive override data, queue for later
-                                gGLTFMaterialList.queueOverrideUpdate(object_id, side, override_data);
-                            }
-                            else if (obj && obj->isAnySelected())
-                            {
-                                LLMaterialEditor::updateLive(object_id, side);
-                            }
+                            LLMaterialEditor::updateLive(object_id, side);
                         }
                     }
+                }
 
-                    if (obj && side_set.size() != obj->getNumTEs())
-                    { // object exists and at least one texture entry needs to have its override data nulled out
-                        bool object_has_selection = obj->isAnySelected();
-                        for (int i = 0; i < obj->getNumTEs(); ++i)
+                if (obj && side_set.size() != obj->getNumTEs())
+                { // object exists and at least one texture entry needs to have its override data nulled out
+                    bool object_has_selection = obj->isAnySelected();
+                    for (int i = 0; i < obj->getNumTEs(); ++i)
+                    {
+                        if (side_set.find(i) == side_set.end())
                         {
-                            if (side_set.find(i) == side_set.end())
+                            obj->setTEGLTFMaterialOverride(i, nullptr);
+                            if (object_has_selection)
                             {
-                                obj->setTEGLTFMaterialOverride(i, nullptr);
-                                if (object_has_selection)
-                                {
-                                    LLMaterialEditor::updateLive(object_id, i);
-                                }
+                                LLMaterialEditor::updateLive(object_id, i);
                             }
                         }
                     }
                 }
-                else
-                {
-                    LL_WARNS() << "Malformed GLTF override message data: " << message << LL_ENDL;
-                }
             }
+            else
+            {
+                LL_WARNS() << "Malformed GLTF override message data: " << message << LL_ENDL;
+            }
+        }
             
-            if (clear_all && obj)
-            { // override list was empty or an error occurred, null out all overrides for this object
-                bool object_has_selection = obj->isAnySelected();
-                for (int i = 0; i < obj->getNumTEs(); ++i)
+        if (clear_all && obj)
+        { // override list was empty or an error occurred, null out all overrides for this object
+            bool object_has_selection = obj->isAnySelected();
+            for (int i = 0; i < obj->getNumTEs(); ++i)
+            {
+                obj->setTEGLTFMaterialOverride(i, nullptr);
+                if (object_has_selection)
                 {
-                    obj->setTEGLTFMaterialOverride(i, nullptr);
-                    if (object_has_selection)
-                    {
-                        LLMaterialEditor::updateLive(obj->getID(), i);
-                    }
+                    LLMaterialEditor::updateLive(obj->getID(), i);
                 }
             }
-            return true;
         }
-    };
+        return true;
+    }
+};
+
+namespace
+{
     LLGLTFMaterialOverrideDispatchHandler handle_gltf_override_message;
 }
 
@@ -229,12 +231,24 @@ void LLGLTFMaterialList::applyQueuedOverrides(LLViewerObject* obj)
     }
 }
 
-void LLGLTFMaterialList::queueModifyMaterial(const LLUUID& id, S32 side, const LLGLTFMaterial& mat)
+void LLGLTFMaterialList::queueModifyMaterial(const LLUUID& id, S32 side, const LLGLTFMaterial* mat)
 {
-    sModifyQueue.push_back({ id, side, mat, LLUUID::null, true, false });
+    if (mat == nullptr)
+    {
+        sModifyQueue.push_back({ id, side, LLGLTFMaterial(), false });
+    }
+    else
+    {
+        sModifyQueue.push_back({ id, side, *mat, true});
+    }
 }
 
-void LLGLTFMaterialList::flushModifyMaterialQueue(void(*done_callback)(bool))
+void LLGLTFMaterialList::queueApplyMaterialAsset(const LLUUID& object_id, S32 side, const LLUUID& asset_id)
+{
+    sApplyQueue.push_back({ object_id, side, asset_id});
+}
+
+void LLGLTFMaterialList::flushUpdates(void(*done_callback)(bool))
 {
     LLSD data = LLSD::emptyArray();
 
@@ -244,11 +258,6 @@ void LLGLTFMaterialList::flushModifyMaterialQueue(void(*done_callback)(bool))
         data[i]["object_id"] = e.object_id;
         data[i]["side"] = e.side;
          
-        if (e.has_asset_id)
-        {
-            data[i]["asset_id"] = e.asset_id;
-        }
-
         if (e.has_override)
         {
             data[i]["gltf_json"] = e.override_data.asJSON();
@@ -256,19 +265,29 @@ void LLGLTFMaterialList::flushModifyMaterialQueue(void(*done_callback)(bool))
 
         ++i;
     }
+    sModifyQueue.clear();
 
-    std::stringstream str;
+    for (auto& e : sApplyQueue)
+    {
+        data[i]["object_id"] = e.object_id;
+        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
+        ++i;
+    }
+    sApplyQueue.clear();
 
+#if 0 // debug output of data being sent to capability
+    std::stringstream str;
     LLSDSerialize::serialize(data, str, LLSDSerialize::LLSD_NOTATION, LLSDFormatter::OPTIONS_PRETTY);
     LL_INFOS() << "\n" << str.str() << LL_ENDL;
+#endif
 
     LLCoros::instance().launch("modifyMaterialCoro",
         std::bind(&LLGLTFMaterialList::modifyMaterialCoro,
             gAgent.getRegionCapability("ModifyMaterialParams"),
             data,
             done_callback));
-
-    sModifyQueue.clear();
 }
 
 LLGLTFMaterial* LLGLTFMaterialList::getMaterial(const LLUUID& id)
diff --git a/indra/newview/llgltfmateriallist.h b/indra/newview/llgltfmateriallist.h
index e035d2108d..9e8b3cf8e3 100644
--- a/indra/newview/llgltfmateriallist.h
+++ b/indra/newview/llgltfmateriallist.h
@@ -51,25 +51,45 @@ public:
 
     static void registerCallbacks();
 
-    // save an override update that we want to send to the simulator for later
-    static void  queueModifyMaterial(const LLUUID& id, S32 side, const LLGLTFMaterial& mat);
+    // Queue an override update 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);
+
+    // 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
+    //  side - TextureEntry index to apply material to, or -1 for all sides
+    //  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);
 
     // flush pending material updates to the simulator
-    static void  flushModifyMaterialQueue(void(*done_callback)(bool));
+    static void  flushUpdates(void(*done_callback)(bool) = nullptr);
     
     // apply given override data via given cap url
     //  cap_url -- should be gAgent.getRegionCapability("ModifyMaterialParams")
-    //  overrides -- LLSD map in the format
-    //    "object_id": LLUUID - object to be modified
-    //    "side": integer - index of face to be modified
-    //    "gltf_json" : string - GLTF compliant json of override data (optional, if omitted any existing override data will be cleared)
+    //  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.
+    // If the queue/flush API is insufficient, extend it.
     static void modifyMaterialCoro(std::string cap_url, LLSD overrides, void(*done_callback)(bool));
-    
-    // save an override update that we got from the simulator for later (for example, if an override arrived for an unknown object)
-    void queueOverrideUpdate(const LLUUID& id, S32 side, LLGLTFMaterial* override_data);
 
     void applyQueuedOverrides(LLViewerObject* obj);
+
 private:
+    friend class LLGLTFMaterialOverrideDispatchHandler;
+    // save an override update that we got from the simulator for later (for example, if an override arrived for an unknown object)
+    // NOTE: this is NOT for applying overrides from the UI, see queueModifyMaterial above
+    void queueOverrideUpdate(const LLUUID& id, S32 side, LLGLTFMaterial* override_data);
+
     typedef std::unordered_map<LLUUID, LLPointer<LLFetchedGLTFMaterial > > uuid_mat_map_t;
     uuid_mat_map_t mList;
 
@@ -84,15 +104,23 @@ private:
         LLUUID object_id;
         S32 side = -1;
         LLGLTFMaterial override_data;
-        LLUUID asset_id;
 
         bool has_override = false;
-        bool has_asset_id = false;
     };
 
     typedef std::list<ModifyMaterialData> modify_queue_t;
     static modify_queue_t sModifyQueue;
 
+    struct ApplyMaterialAssetData
+    {
+        LLUUID object_id;
+        S32 side = -1;
+        LLUUID asset_id;
+    };
+
+    typedef std::list<ApplyMaterialAssetData> apply_queue_t;
+    static apply_queue_t sApplyQueue;
+
 };
 
 extern LLGLTFMaterialList gGLTFMaterialList;
diff --git a/indra/newview/llmaterialeditor.cpp b/indra/newview/llmaterialeditor.cpp
index 675ca610ce..17da7e8853 100644
--- a/indra/newview/llmaterialeditor.cpp
+++ b/indra/newview/llmaterialeditor.cpp
@@ -2526,7 +2526,7 @@ public:
             {
                 mSuccess = true;
             }
-            LLGLTFMaterialList::queueModifyMaterial(objectp->getID(), te, *material);
+            LLGLTFMaterialList::queueModifyMaterial(objectp->getID(), te, material);
 #else
 
             std::string overrides_json = material->asJSON();
@@ -2598,7 +2598,7 @@ void LLMaterialEditor::applyToSelection()
 
             void(*done_callback)(bool) = LLRenderMaterialOverrideFunctor::modifyCallback;
 
-            LLGLTFMaterialList::flushModifyMaterialQueue(done_callback);
+            LLGLTFMaterialList::flushUpdates(done_callback);
 
             if (!override_func.getResult())
             {
diff --git a/indra/newview/llviewerobject.cpp b/indra/newview/llviewerobject.cpp
index 1efc4ea3d5..3cd5b1dbe7 100644
--- a/indra/newview/llviewerobject.cpp
+++ b/indra/newview/llviewerobject.cpp
@@ -7205,19 +7205,15 @@ void LLViewerObject::setRenderMaterialID(S32 te_in, const LLUUID& id, bool updat
 
     if (update_server)
     {
-        // blank out any override data on the ser
+        // update via ModifyMaterialParams cap (server will echo back changes)
         for (S32 te = start_idx; te < end_idx; ++te)
         {
-            LLCoros::instance().launch("modifyMaterialCoro",
-                std::bind(&LLGLTFMaterialList::modifyMaterialCoro,
-                    gAgent.getRegionCapability("ModifyMaterialParams"),
-                    llsd::map(
-                        "object_id", getID(),
-                        "side", te), nullptr));
+            LLGLTFMaterialList::queueApplyMaterialAsset(getID(), te, id);
         }
+        LLGLTFMaterialList::flushUpdates();
     }
 
-    // update and send LLRenderMaterialParams
+    // predictively update LLRenderMaterialParams (don't wait for server)
     LLRenderMaterialParams* param_block = (LLRenderMaterialParams*)getParameterEntry(LLNetworkData::PARAMS_RENDER_MATERIAL);
     if (!param_block && id.notNull())
     { // block doesn't exist, but it will need to
@@ -7230,18 +7226,6 @@ void LLViewerObject::setRenderMaterialID(S32 te_in, const LLUUID& id, bool updat
         {
             param_block->setMaterial(te, id);
         }
-
-        if (update_server)
-        {
-            // 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);
-            }
-        }
     }
 }