SL-18820: Fix applying material clearing transform overrides. Loosen some asserts to allow non-default transform overrides.

10 files changed, 114 insertions, 44 deletions

diff --git a/indra/llprimitive/llgltfmaterial.cpp b/indra/llprimitive/llgltfmaterial.cpp
index 5442af33d0..a8dad89292 100644
--- a/indra/llprimitive/llgltfmaterial.cpp
+++ b/indra/llprimitive/llgltfmaterial.cpp
@@ -392,11 +392,23 @@ bool LLGLTFMaterial::setBaseMaterial()
 {
     const LLGLTFMaterial old_override = *this;
     *this = sDefault;
-    // Preserve the texture transforms
-    mTextureTransform = old_override.mTextureTransform;
+    setBaseMaterial(old_override);
     return *this != old_override;
 }
 
+bool LLGLTFMaterial::isClearedForBaseMaterial()
+{
+    LLGLTFMaterial cleared_override = sDefault;
+    cleared_override.setBaseMaterial(*this);
+    return *this == cleared_override;
+}
+
+// For material overrides only. Copies transforms from the old override.
+void LLGLTFMaterial::setBaseMaterial(const LLGLTFMaterial& old_override_mat)
+{
+    mTextureTransform = old_override_mat.mTextureTransform;
+}
+
 
 // static
 void LLGLTFMaterial::hackOverrideUUID(LLUUID& id)
diff --git a/indra/llprimitive/llgltfmaterial.h b/indra/llprimitive/llgltfmaterial.h
index 7bfcd3bf66..dec7b696f8 100644
--- a/indra/llprimitive/llgltfmaterial.h
+++ b/indra/llprimitive/llgltfmaterial.h
@@ -185,6 +185,8 @@ public:
     // For material overrides only. Clears most properties to
     // default/fallthrough, but preserves the transforms.
     bool setBaseMaterial();
+    // True if setBaseMaterial() was just called
+    bool isClearedForBaseMaterial();
 
 private:
 
@@ -193,5 +195,7 @@ private:
 
     template<typename T>
     void writeToTexture(tinygltf::Model& model, T& texture_info, TextureInfo texture_info_id, const LLUUID& texture_id) const;
+
+    void setBaseMaterial(const LLGLTFMaterial& old_override_mat);
 };
 
diff --git a/indra/llprimitive/lltextureentry.cpp b/indra/llprimitive/lltextureentry.cpp
index a0d5793dcc..49f67918f8 100644
--- a/indra/llprimitive/lltextureentry.cpp
+++ b/indra/llprimitive/lltextureentry.cpp
@@ -514,15 +514,15 @@ S32 LLTextureEntry::setBumpShiny(U8 bump_shiny)
 	return TEM_CHANGE_NONE;
 }
 
-void LLTextureEntry::setGLTFMaterial(LLGLTFMaterial* material)
+void LLTextureEntry::setGLTFMaterial(LLGLTFMaterial* material, bool local_origin)
 { 
     if (material != getGLTFMaterial())
     {
         // assert on precondtion:
-        // whether or not mGLTFMaterial is null, any existing override should have been nulled out 
+        // whether or not mGLTFMaterial is null, any existing override should have been cleared
         // before calling setGLTFMaterial
         // NOTE: if you're hitting this assert, try to make sure calling code is using LLViewerObject::setRenderMaterialID
-        llassert(getGLTFMaterialOverride() == nullptr);
+        llassert(!local_origin || getGLTFMaterialOverride() == nullptr || getGLTFMaterialOverride()->isClearedForBaseMaterial());
 
         mGLTFMaterial = material;
         if (mGLTFMaterial == nullptr)
@@ -538,6 +538,27 @@ void LLTextureEntry::setGLTFMaterialOverride(LLGLTFMaterial* mat)
     mGLTFMaterialOverrides = mat; 
 }
 
+S32 LLTextureEntry::setBaseMaterial()
+{
+    S32 changed = TEM_CHANGE_NONE;
+
+    if (mGLTFMaterialOverrides)
+    {
+        if (mGLTFMaterialOverrides->setBaseMaterial())
+        {
+            changed = TEM_CHANGE_TEXTURE;
+        }
+
+        if (LLGLTFMaterial::sDefault == *mGLTFMaterialOverrides)
+        {
+            mGLTFMaterialOverrides = nullptr;
+            changed = TEM_CHANGE_TEXTURE;
+        }
+    }
+
+    return changed;
+}
+
 LLGLTFMaterial* LLTextureEntry::getGLTFRenderMaterial() const
 { 
     if (mGLTFRenderMaterial.notNull())
@@ -545,7 +566,7 @@ LLGLTFMaterial* LLTextureEntry::getGLTFRenderMaterial() const
         return mGLTFRenderMaterial;
     }
     
-    llassert(getGLTFMaterialOverride() == nullptr);
+    llassert(getGLTFMaterialOverride() == nullptr || getGLTFMaterialOverride()->isClearedForBaseMaterial());
     return getGLTFMaterial();
 }
 
diff --git a/indra/llprimitive/lltextureentry.h b/indra/llprimitive/lltextureentry.h
index d94e14bd73..9a81181f3a 100644
--- a/indra/llprimitive/lltextureentry.h
+++ b/indra/llprimitive/lltextureentry.h
@@ -195,12 +195,16 @@ public:
 	enum { MF_NONE = 0x0, MF_HAS_MEDIA = 0x1 };
 
     // GLTF asset
-    void setGLTFMaterial(LLGLTFMaterial* material);
+    void setGLTFMaterial(LLGLTFMaterial* material, bool local_origin = true);
     LLGLTFMaterial* getGLTFMaterial() const { return mGLTFMaterial; }
 
     // GLTF override
     LLGLTFMaterial* getGLTFMaterialOverride() const { return mGLTFMaterialOverrides; }
     void setGLTFMaterialOverride(LLGLTFMaterial* mat);
+    // Clear most overrides so the render material better matches the material
+    // ID (preserve transforms). If the overrides become passthrough, set the
+    // overrides to nullptr.
+    S32 setBaseMaterial();
 
     // GLTF render material
     // nuanced behavior here -- if there is no render material, fall back to getGLTFMaterial, but ONLY for the getter, not the setter
diff --git a/indra/newview/llgltfmateriallist.cpp b/indra/newview/llgltfmateriallist.cpp
index d04a674e91..9539ffc700 100644
--- a/indra/newview/llgltfmateriallist.cpp
+++ b/indra/newview/llgltfmateriallist.cpp
@@ -404,9 +404,19 @@ void LLGLTFMaterialList::queueModify(const LLUUID& id, S32 side, const LLGLTFMat
     }
 }
 
-void LLGLTFMaterialList::queueApply(const LLUUID& object_id, S32 side, const LLUUID& asset_id)
+void LLGLTFMaterialList::queueApply(const LLViewerObject* obj, S32 side, const LLUUID& asset_id)
 {
-    sApplyQueue.push_back({ object_id, side, asset_id});
+    const LLGLTFMaterial* material_override = obj->getTE(side)->getGLTFMaterialOverride();
+    if (material_override)
+    {
+        LLGLTFMaterial* cleared_override = new LLGLTFMaterial(*material_override);
+        cleared_override->setBaseMaterial();
+        sApplyQueue.push_back({ obj->getID(), side, asset_id, cleared_override });
+    }
+    else
+    {
+        sApplyQueue.push_back({ obj->getID(), side, asset_id, nullptr });
+    }
 }
 
 void LLGLTFMaterialList::queueUpdate(const LLSD& data)
@@ -436,6 +446,11 @@ void LLGLTFMaterialList::flushUpdates(void(*done_callback)(bool))
         {
             data[i]["gltf_json"] = e.override_data.asJSON();
         }
+        else
+        {
+            // Clear all overrides
+            data[i]["gltf_json"] = "";
+        }
 
         llassert(is_valid_update(data[i]));
         ++i;
@@ -447,7 +462,15 @@ void LLGLTFMaterialList::flushUpdates(void(*done_callback)(bool))
         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
+        if (e.override_data)
+        {
+            data[i]["gltf_json"] = e.override_data->asJSON();
+        }
+        else
+        {
+            // Clear all overrides
+            data[i]["gltf_json"] = "";
+        }
 
         llassert(is_valid_update(data[i]));
         ++i;
diff --git a/indra/newview/llgltfmateriallist.h b/indra/newview/llgltfmateriallist.h
index abbb755599..70540e5e01 100644
--- a/indra/newview/llgltfmateriallist.h
+++ b/indra/newview/llgltfmateriallist.h
@@ -59,7 +59,7 @@ public:
     //  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
+    // NOTE: do not use to revert to asset when applying a new asset id, use queueApply below
     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.
@@ -67,8 +67,8 @@ public:
     //  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 queueApply(const LLUUID& object_id, S32 side, const LLUUID& asset_id);
+    // NOTE: Implicitly clears most override data if present
+    static void queueApply(const LLViewerObject* obj, S32 side, const LLUUID& asset_id);
 
     // flush pending material updates to the simulator
     // Automatically called once per frame, but may be called explicitly
@@ -136,6 +136,7 @@ protected:
         LLUUID object_id;
         S32 side = -1;
         LLUUID asset_id;
+        LLPointer<LLGLTFMaterial> override_data;
     };
 
     typedef std::list<ApplyMaterialAssetData> apply_queue_t;
diff --git a/indra/newview/llpanelface.cpp b/indra/newview/llpanelface.cpp
index 0b18bdc6e6..cf02f3c4e4 100644
--- a/indra/newview/llpanelface.cpp
+++ b/indra/newview/llpanelface.cpp
@@ -4527,8 +4527,8 @@ void LLPanelFace::onPasteTexture(LLViewerObject* objectp, S32 te)
                 tep->setGLTFRenderMaterial(nullptr);
                 tep->setGLTFMaterialOverride(nullptr);
 
-                // blank out any override data on the server
-                LLGLTFMaterialList::queueApply(objectp->getID(), te, LLUUID::null);
+                // blank out most override data on the server
+                LLGLTFMaterialList::queueApply(objectp, te, LLUUID::null);
             }
 
             // Texture map
diff --git a/indra/newview/llselectmgr.cpp b/indra/newview/llselectmgr.cpp
index 4a69eba4d3..cb1e46068e 100644
--- a/indra/newview/llselectmgr.cpp
+++ b/indra/newview/llselectmgr.cpp
@@ -1806,11 +1806,9 @@ void LLObjectSelection::applyNoCopyPbrMaterialToTEs(LLViewerInventoryItem* item)
                 }
 
                 // apply texture for the selected faces
+                // blank out most override data on the server
                 //add(LLStatViewer::EDIT_TEXTURE, 1);
-                object->setRenderMaterialID(te, asset_id, false /*will be sent later*/);
-
-                // blank out any override data on the server
-                LLGLTFMaterialList::queueApply(object->getID(), te, asset_id);
+                object->setRenderMaterialID(te, asset_id);
             }
         }
     }
@@ -1949,10 +1947,8 @@ void LLSelectMgr::selectionSetGLTFMaterial(const LLUUID& mat_id)
                 objectp->setParameterEntryInUse(LLNetworkData::PARAMS_RENDER_MATERIAL, TRUE, false /*prevent an update*/);
             }
 
-            objectp->setRenderMaterialID(te, asset_id, false /*prevent an update to prevent a race condition*/);
-
-            // blank out any override data on the server
-            LLGLTFMaterialList::queueApply(objectp->getID(), te, asset_id);
+            // Blank out most override data on the object and send to server
+            objectp->setRenderMaterialID(te, asset_id);
 
             return true;
         }
@@ -2223,17 +2219,12 @@ void LLSelectMgr::selectionRevertGLTFMaterials()
                     && asset_id.notNull())
                 {
                     // Restore overrides
-                    LLSD overrides;
-                    overrides["object_id"] = objectp->getID();
-                    overrides["side"] = te;
-
-                    overrides["gltf_json"] = nodep->mSavedGLTFOverrideMaterials[te]->asJSON();
-                    LLGLTFMaterialList::queueUpdate(overrides);
+                    LLGLTFMaterialList::queueModify(objectp->getID(), te, nodep->mSavedGLTFOverrideMaterials[te]);
                 } 
                 else
                 {
                     //blank override out
-                    LLGLTFMaterialList::queueApply(objectp->getID(), te, asset_id);
+                    LLGLTFMaterialList::queueApply(objectp, te, asset_id);
                 }
 
             }
diff --git a/indra/newview/llviewerobject.cpp b/indra/newview/llviewerobject.cpp
index dd7f679846..2bd0de4d6c 100644
--- a/indra/newview/llviewerobject.cpp
+++ b/indra/newview/llviewerobject.cpp
@@ -7163,11 +7163,11 @@ void LLViewerObject::setRenderMaterialID(S32 te_in, const LLUUID& id, bool updat
 {
     // implementation is delicate
 
-    // if update is bound for server, should always null out GLTFRenderMaterial and GLTFMaterialOverride even if ids haven't changed
+    // if update is bound for server, should always null out GLTFRenderMaterial and clear GLTFMaterialOverride even if ids haven't changed
     //  (the case where ids haven't changed indicates the user has reapplied the original material, in which case overrides should be dropped)
-    // otherwise, should only null out where ids have changed 
+    // otherwise, should only null out the render material where ids or overrides have changed
     //  (the case where ids have changed but overrides are still present is from unsynchronized updates from the simulator)
-    
+
     S32 start_idx = 0;
     S32 end_idx = getNumTEs();
 
@@ -7180,6 +7180,13 @@ void LLViewerObject::setRenderMaterialID(S32 te_in, const LLUUID& id, bool updat
     start_idx = llmax(start_idx, 0);
     end_idx = llmin(end_idx, (S32) getNumTEs());
 
+    LLRenderMaterialParams* param_block = (LLRenderMaterialParams*)getParameterEntry(LLNetworkData::PARAMS_RENDER_MATERIAL);
+    if (!param_block && id.notNull())
+    { // block doesn't exist, but it will need to
+        param_block = (LLRenderMaterialParams*)createNewParameterEntry(LLNetworkData::PARAMS_RENDER_MATERIAL)->data;
+    }
+
+
     // update local state
     for (S32 te = start_idx; te < end_idx; ++te)
     {
@@ -7192,17 +7199,27 @@ void LLViewerObject::setRenderMaterialID(S32 te_in, const LLUUID& id, bool updat
             new_material = gGLTFMaterialList.getMaterial(id);
         }
         
-        bool material_changed = tep->getGLTFMaterial() != new_material;
+        bool material_changed = !param_block || id != param_block->getMaterial(te);
+
+        if (update_server)
+        { 
+            // Clear most overrides so the render material better matches the material
+            // ID (preserve transforms). If overrides become passthrough, set the overrides
+            // to nullptr.
+            if (tep->setBaseMaterial())
+            {
+                material_changed = true;
+            }
+        }
 
         if (update_server || material_changed)
         { 
             tep->setGLTFRenderMaterial(nullptr);
-            tep->setGLTFMaterialOverride(nullptr);
         }
 
         if (new_material != tep->getGLTFMaterial())
         {
-            tep->setGLTFMaterial(new_material);
+            tep->setGLTFMaterial(new_material, !update_server);
         }
     }
 
@@ -7215,17 +7232,14 @@ 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::queueApply(getID(), te, id);
+            // 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)
-    LLRenderMaterialParams* param_block = (LLRenderMaterialParams*)getParameterEntry(LLNetworkData::PARAMS_RENDER_MATERIAL);
-    if (!param_block && id.notNull())
-    { // block doesn't exist, but it will need to
-        param_block = (LLRenderMaterialParams*)createNewParameterEntry(LLNetworkData::PARAMS_RENDER_MATERIAL)->data;
-    }
-
     if (param_block)
     { // update existing parameter block
         for (S32 te = start_idx; te < end_idx; ++te)
diff --git a/indra/newview/llviewerobject.h b/indra/newview/llviewerobject.h
index 8c4afdcba4..71d7a7ebbb 100644
--- a/indra/newview/llviewerobject.h
+++ b/indra/newview/llviewerobject.h
@@ -188,7 +188,7 @@ public:
     // set the RenderMaterialID for the given TextureEntry
     // te - TextureEntry index to set, or -1 for all TEs
     // id - asset id of material asset
-    // update_server - if true, will send updates to server
+    // update_server - if true, will send updates to server and clear most overrides
     void setRenderMaterialID(S32 te, const LLUUID& id, bool update_server = true);
     void setRenderMaterialIDs(const LLUUID& id);