SL-20523 Local textures not updating on PBR Materials #5

10 files changed, 158 insertions, 110 deletions

diff --git a/indra/llprimitive/llgltfmaterial.cpp b/indra/llprimitive/llgltfmaterial.cpp
index 3945be3254..390110b0ec 100644
--- a/indra/llprimitive/llgltfmaterial.cpp
+++ b/indra/llprimitive/llgltfmaterial.cpp
@@ -607,6 +607,8 @@ void LLGLTFMaterial::applyOverride(const LLGLTFMaterial& override_mat)
     }
 
     mTrackingIdToLocalTexture.insert(override_mat.mTrackingIdToLocalTexture.begin(), override_mat.mTrackingIdToLocalTexture.begin());
+
+    updateTextureTracking();
 }
 
 void LLGLTFMaterial::getOverrideLLSD(const LLGLTFMaterial& override_mat, LLSD& data)
@@ -771,15 +773,6 @@ LLUUID LLGLTFMaterial::getHash() const
     return hash;
 }
 
-void LLGLTFMaterial::addTextureEntry(LLTextureEntry* te)
-{
-    mTextureEntires.insert(te);
-}
-void LLGLTFMaterial::removeTextureEntry(LLTextureEntry* te)
-{
-    mTextureEntires.erase(te);
-}
-
 void LLGLTFMaterial::addLocalTextureTracking(const LLUUID& tracking_id, const LLUUID& tex_id)
 {
     mTrackingIdToLocalTexture[tracking_id] = tex_id;
@@ -818,4 +811,5 @@ bool LLGLTFMaterial::replaceLocalTexture(const LLUUID& tracking_id, const LLUUID
 void LLGLTFMaterial::updateTextureTracking()
 {
     // setTEGLTFMaterialOverride is responsible for tracking
+    // for material overrides editor will set it
 }
diff --git a/indra/llprimitive/llgltfmaterial.h b/indra/llprimitive/llgltfmaterial.h
index 7b38663307..02f62fb08c 100644
--- a/indra/llprimitive/llgltfmaterial.h
+++ b/indra/llprimitive/llgltfmaterial.h
@@ -219,8 +219,8 @@ public:
 
     // For local materials, they have to keep track of where
     // they are assigned to for full updates
-    virtual void addTextureEntry(LLTextureEntry* te);
-    virtual void removeTextureEntry(LLTextureEntry* te);
+    virtual void addTextureEntry(LLTextureEntry* te) {};
+    virtual void removeTextureEntry(LLTextureEntry* te) {};
 
     // For local textures so that editor will know to track changes
     void addLocalTextureTracking(const LLUUID& tracking_id, const LLUUID &tex_id);
@@ -229,9 +229,9 @@ public:
     virtual bool replaceLocalTexture(const LLUUID& tracking_id, const LLUUID &old_id, const LLUUID& new_id);
     virtual void updateTextureTracking();
 
+    // These fields are local to viewer and are a part of local bitmap support
     typedef std::map<LLUUID, LLUUID> local_tex_map_t;
     local_tex_map_t mTrackingIdToLocalTexture;
-    std::set<LLTextureEntry*> mTextureEntires;
 
 protected:
     static LLVector2 vec2FromJson(const std::map<std::string, tinygltf::Value>& object, const char* key, const LLVector2& default_value);
diff --git a/indra/newview/llfetchedgltfmaterial.cpp b/indra/newview/llfetchedgltfmaterial.cpp
index c61b25f0a2..a47b52fddc 100644
--- a/indra/newview/llfetchedgltfmaterial.cpp
+++ b/indra/newview/llfetchedgltfmaterial.cpp
@@ -155,13 +155,6 @@ LLViewerFetchedTexture* fetch_texture(const LLUUID& id)
     return img;
 };
 
-void LLFetchedGLTFMaterial::applyOverride(const LLGLTFMaterial& override_mat)
-{
-    LLGLTFMaterial::applyOverride(override_mat);
-
-    updateTextureTracking();
-}
-
 bool LLFetchedGLTFMaterial::replaceLocalTexture(const LLUUID& tracking_id, const LLUUID& old_id, const LLUUID& new_id)
 {
     bool res = false;
@@ -202,9 +195,19 @@ bool LLFetchedGLTFMaterial::replaceLocalTexture(const LLUUID& tracking_id, const
     return res;
 }
 
+void LLFetchedGLTFMaterial::addTextureEntry(LLTextureEntry* te)
+{
+    mTextureEntires.insert(te);
+}
+
+void LLFetchedGLTFMaterial::removeTextureEntry(LLTextureEntry* te)
+{
+    mTextureEntires.erase(te);
+}
+
 void LLFetchedGLTFMaterial::updateTextureTracking()
 {
-    for (local_tex_map_t::value_type val : mTrackingIdToLocalTexture)
+    for (local_tex_map_t::value_type &val : mTrackingIdToLocalTexture)
     {
         LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(val.first, this);
     }
diff --git a/indra/newview/llfetchedgltfmaterial.h b/indra/newview/llfetchedgltfmaterial.h
index d778966c22..2559aa46cc 100644
--- a/indra/newview/llfetchedgltfmaterial.h
+++ b/indra/newview/llfetchedgltfmaterial.h
@@ -50,7 +50,8 @@ public:
 
     bool isFetching() const { return mFetching; }
 
-    void applyOverride(const LLGLTFMaterial& override_mat) override;
+    void addTextureEntry(LLTextureEntry* te) override;
+    void removeTextureEntry(LLTextureEntry* te) override;
     virtual bool replaceLocalTexture(const LLUUID& tracking_id, const LLUUID& old_id, const LLUUID& new_id) override;
     virtual void updateTextureTracking() override;
 
@@ -60,6 +61,8 @@ public:
     LLPointer<LLViewerFetchedTexture> mMetallicRoughnessTexture;
     LLPointer<LLViewerFetchedTexture> mEmissiveTexture;
 
+    std::set<LLTextureEntry*> mTextureEntires;
+
 protected:
     // Lifetime management
     
diff --git a/indra/newview/lllocalbitmaps.cpp b/indra/newview/lllocalbitmaps.cpp
index cd5b2e262b..4121df4bac 100644
--- a/indra/newview/lllocalbitmaps.cpp
+++ b/indra/newview/lllocalbitmaps.cpp
@@ -293,14 +293,28 @@ void LLLocalBitmap::addGLTFMaterial(LLGLTFMaterial* mat)
     {
         return;
     }
-    for (mat_list_t::value_type ptr : mGLTFMaterialWithLocalTextures)
+
+    mat_list_t::iterator end = mGLTFMaterialWithLocalTextures.end();
+    for (mat_list_t::iterator it = mGLTFMaterialWithLocalTextures.begin(); it != end;)
     {
-        if (ptr.get() != mat)
+        if (it->get() == mat)
+        {
+            return;
+        }
+
+        if ((*it)->getNumRefs() == 1)
+        {
+            it = mGLTFMaterialWithLocalTextures.erase(it);
+            end = mGLTFMaterialWithLocalTextures.end();
+        }
+        else
         {
-            mat->addLocalTextureTracking(getTrackingID(), getWorldID());
-            mGLTFMaterialWithLocalTextures.push_back(mat);
+            it++;
         }
     }
+
+    mat->addLocalTextureTracking(getTrackingID(), getWorldID());
+    mGLTFMaterialWithLocalTextures.push_back(mat);
 }
 
 bool LLLocalBitmap::decodeBitmap(LLPointer<LLImageRaw> rawimg)
@@ -615,31 +629,41 @@ void LLLocalBitmap::updateUserLayers(LLUUID old_id, LLUUID new_id, LLWearableTyp
 void LLLocalBitmap::updateGLTFMaterials(LLUUID old_id, LLUUID new_id)
 {
     // Might be a better idea to hold this in LLGLTFMaterialList
-    for (mat_list_t::iterator it = mGLTFMaterialWithLocalTextures.begin(); it != mGLTFMaterialWithLocalTextures.end();)
+    mat_list_t::iterator end = mGLTFMaterialWithLocalTextures.end();
+    for (mat_list_t::iterator it = mGLTFMaterialWithLocalTextures.begin(); it != end;)
     {
-        if ((*it)->replaceLocalTexture(mTrackingID, old_id, new_id))
+        if ((*it)->getNumRefs() == 1)
+        {
+            // render and override materials are often recreated,
+            // clean up any remains
+            it = mGLTFMaterialWithLocalTextures.erase(it);
+            end = mGLTFMaterialWithLocalTextures.end();
+        }
+        else if ((*it)->replaceLocalTexture(mTrackingID, old_id, new_id))
         {
-            for (LLTextureEntry* entry : (*it)->mTextureEntires)
+            LLFetchedGLTFMaterial* fetched_mat = dynamic_cast<LLFetchedGLTFMaterial*>((*it).get());
+            if (fetched_mat)
             {
-                // Normally a change in applied material id is supposed to
-                // drop overrides thus reset material, but local materials
-                // currently reuse their existing asset id, and purpose is
-                // to preview how material will work in-world, overrides
-                // included, so do an override to render update instead.
-                LLGLTFMaterial* override_mat = entry->getGLTFMaterialOverride();
-                if (override_mat)
+                for (LLTextureEntry* entry : fetched_mat->mTextureEntires)
                 {
-                    // do not create a new material, reuse existing pointer
-                    LLFetchedGLTFMaterial* render_mat = (LLFetchedGLTFMaterial*)entry->getGLTFRenderMaterial();
-                    if (render_mat)
+                    // Normally a change in applied material id is supposed to
+                    // drop overrides thus reset material, but local materials
+                    // currently reuse their existing asset id, and purpose is
+                    // to preview how material will work in-world, overrides
+                    // included, so do an override to render update instead.
+                    LLGLTFMaterial* override_mat = entry->getGLTFMaterialOverride();
+                    if (override_mat)
                     {
-                        llassert(dynamic_cast<LLFetchedGLTFMaterial*>(entry->getGLTFRenderMaterial()) != nullptr);
-                        LLFetchedGLTFMaterial *fetched_mat = dynamic_cast<LLFetchedGLTFMaterial*>((*it).get());
-                        if (fetched_mat)
+                        // do not create a new material, reuse existing pointer
+                        LLFetchedGLTFMaterial* render_mat = (LLFetchedGLTFMaterial*)entry->getGLTFRenderMaterial();
+                        if (render_mat)
                         {
-                            *render_mat = *fetched_mat;
+                            llassert(dynamic_cast<LLFetchedGLTFMaterial*>(entry->getGLTFRenderMaterial()) != nullptr);
+                            {
+                                *render_mat = *fetched_mat;
+                            }
+                            render_mat->applyOverride(*override_mat);
                         }
-                        render_mat->applyOverride(*override_mat);
                     }
                 }
             }
@@ -650,6 +674,7 @@ void LLLocalBitmap::updateGLTFMaterials(LLUUID old_id, LLUUID new_id)
             // Matching id not found, no longer in use
             // material would clean itself, remove from the list
             it = mGLTFMaterialWithLocalTextures.erase(it);
+            end = mGLTFMaterialWithLocalTextures.end();
         }
     }
 }
diff --git a/indra/newview/lllocalgltfmaterials.cpp b/indra/newview/lllocalgltfmaterials.cpp
index b7fdead3f9..61e0163798 100644
--- a/indra/newview/lllocalgltfmaterials.cpp
+++ b/indra/newview/lllocalgltfmaterials.cpp
@@ -119,15 +119,6 @@ S32 LLLocalGLTFMaterial::getIndexInFile() const
     return mMaterialIndex;
 }
 
-void LLLocalGLTFMaterial::addTextureEntry(LLTextureEntry* te)
-{
-    mTextureEntires.insert(te);
-}
-void LLLocalGLTFMaterial::removeTextureEntry(LLTextureEntry* te)
-{
-    mTextureEntires.erase(te);
-}
-
 /* update functions */
 bool LLLocalGLTFMaterial::updateSelf()
 {
diff --git a/indra/newview/lllocalgltfmaterials.h b/indra/newview/lllocalgltfmaterials.h
index 1442b83a40..13b7577e96 100644
--- a/indra/newview/lllocalgltfmaterials.h
+++ b/indra/newview/lllocalgltfmaterials.h
@@ -49,9 +49,6 @@ public: /* accessors */
     LLUUID		getWorldID() const;
     S32			getIndexInFile() const;
 
-    void addTextureEntry(LLTextureEntry* te) override;
-    void removeTextureEntry(LLTextureEntry* te) override;
-
 public:
     bool updateSelf();
 
@@ -81,7 +78,6 @@ private: /* members */
     ELinkStatus mLinkStatus;
     S32         mUpdateRetries;
     S32         mMaterialIndex; // Single file can have more than one
-    std::set<LLTextureEntry*> mTextureEntires;
 };
 
 class LLLocalGLTFMaterialTimer : public LLEventTimer
diff --git a/indra/newview/llmaterialeditor.cpp b/indra/newview/llmaterialeditor.cpp
index 1b606b0311..7d1ac32282 100644
--- a/indra/newview/llmaterialeditor.cpp
+++ b/indra/newview/llmaterialeditor.cpp
@@ -343,6 +343,39 @@ bool LLSelectedTEGetMatData::apply(LLViewerObject* objectp, S32 te_index)
     return false;
 }
 
+class LLSelectedTEUpdateOverrides: public LLSelectedNodeFunctor
+{
+public:
+    LLSelectedTEUpdateOverrides(LLMaterialEditor* me) : mEditor(me) {}
+
+    virtual bool apply(LLSelectNode* nodep);
+
+    LLMaterialEditor* mEditor;
+};
+
+bool LLSelectedTEUpdateOverrides::apply(LLSelectNode* nodep)
+{
+    LLViewerObject* objectp = nodep->getObject();
+    if (!objectp)
+    {
+        return false;
+    }
+    S32 num_tes = llmin((S32)objectp->getNumTEs(), (S32)objectp->getNumFaces()); // avatars have TEs but no faces
+    for (S32 te_index = 0; te_index < num_tes; ++te_index)
+    {
+
+        LLTextureEntry* tep = objectp->getTE(te_index);
+        LLGLTFMaterial* override_mat = tep->getGLTFMaterialOverride();
+        if (mEditor->updateMaterialLocalSubscription(override_mat))
+        {
+            LLGLTFMaterial* render_mat = tep->getGLTFRenderMaterial();
+            mEditor->updateMaterialLocalSubscription(render_mat);
+        }
+    }
+
+    return true;
+}
+
 ///----------------------------------------------------------------------------
 /// Class LLMaterialEditor
 ///----------------------------------------------------------------------------
@@ -535,7 +568,7 @@ void LLMaterialEditor::onClose(bool app_quitting)
     {
         mSelectionUpdateSlot.disconnect();
     }
-    for (mat_connection_map_t::value_type cn : mTextureChangesUpdates)
+    for (mat_connection_map_t::value_type &cn : mTextureChangesUpdates)
     {
         cn.second.mConnection.disconnect();
     }
@@ -902,6 +935,45 @@ LLUUID LLMaterialEditor::getLocalTextureTrackingIdFromFlag(U32 flag)
     return LLUUID();
 }
 
+bool LLMaterialEditor::updateMaterialLocalSubscription(LLGLTFMaterial* mat)
+{
+    if (!mat)
+    {
+        return false;
+    }
+
+    bool res = false;
+    for (mat_connection_map_t::value_type& cn : mTextureChangesUpdates)
+    {
+        LLUUID world_id = LLLocalBitmapMgr::getInstance()->getWorldID(cn.second.mTrackingId);
+        if (world_id == mat->mTextureId[LLGLTFMaterial::GLTF_TEXTURE_INFO_BASE_COLOR])
+        {
+            LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(cn.second.mTrackingId, mat);
+            res = true;
+            continue;
+        }
+        if (world_id == mat->mTextureId[LLGLTFMaterial::GLTF_TEXTURE_INFO_METALLIC_ROUGHNESS])
+        {
+            LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(cn.second.mTrackingId, mat);
+            res = true;
+            continue;
+        }
+        if (world_id == mat->mTextureId[LLGLTFMaterial::GLTF_TEXTURE_INFO_EMISSIVE])
+        {
+            LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(cn.second.mTrackingId, mat);
+            res = true;
+            continue;
+        }
+        if (world_id == mat->mTextureId[LLGLTFMaterial::GLTF_TEXTURE_INFO_NORMAL])
+        {
+            LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(cn.second.mTrackingId, mat);
+            res = true;
+            continue;
+        }
+    }
+    return res;
+}
+
 void LLMaterialEditor::replaceLocalTexture(const LLUUID& old_id, const LLUUID& new_id)
 {
     // todo: might be a good idea to set mBaseColorTextureUploadId here
@@ -1509,7 +1581,7 @@ void LLMaterialEditor::finishInventoryUpload(LLUUID itemId, LLUUID newAssetId, L
             {
                 // local materials were assigned, force load material and init tracking
                 LLGLTFMaterial* mat = gGLTFMaterialList.getMaterial(item->getAssetUUID());
-                for (mat_connection_map_t::value_type val : me->mTextureChangesUpdates)
+                for (mat_connection_map_t::value_type &val : me->mTextureChangesUpdates)
                 {
                     LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(val.second.mTrackingId, mat);
                 }
@@ -1534,7 +1606,7 @@ void LLMaterialEditor::finishTaskUpload(LLUUID itemId, LLUUID newAssetId, LLUUID
         {
             // local materials were assigned, force load material and init tracking
             LLGLTFMaterial* mat = gGLTFMaterialList.getMaterial(newAssetId);
-            for (mat_connection_map_t::value_type val : me->mTextureChangesUpdates)
+            for (mat_connection_map_t::value_type &val : me->mTextureChangesUpdates)
             {
                 LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(val.second.mTrackingId, mat);
             }
@@ -1578,7 +1650,7 @@ void LLMaterialEditor::finishSaveAs(
                 {
                     // local materials were assigned, force load material and init tracking
                     LLGLTFMaterial* mat = gGLTFMaterialList.getMaterial(item->getAssetUUID());
-                    for (mat_connection_map_t::value_type val : me->mTextureChangesUpdates)
+                    for (mat_connection_map_t::value_type &val : me->mTextureChangesUpdates)
                     {
                         LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(val.second.mTrackingId, mat);
                     }
@@ -3087,22 +3159,9 @@ void LLMaterialEditor::setFromGLTFMaterial(LLGLTFMaterial* mat)
     setAlphaMode(mat->getAlphaMode());
     setAlphaCutoff(mat->mAlphaCutoff);
 
-
-    for (mat_connection_map_t::value_type cn : mTextureChangesUpdates)
-    {
-        cn.second.mConnection.disconnect();
-    }
-    mTextureChangesUpdates.clear();
-
     if (mat->hasLocalTextures())
     {
-        for (mat_connection_map_t::value_type cn : mTextureChangesUpdates)
-        {
-            cn.second.mConnection.disconnect();
-        }
-        mTextureChangesUpdates.clear();
-
-        for (LLGLTFMaterial::local_tex_map_t::value_type val : mat->mTrackingIdToLocalTexture)
+        for (LLGLTFMaterial::local_tex_map_t::value_type &val : mat->mTrackingIdToLocalTexture)
         {
             LLUUID world_id = LLLocalBitmapMgr::getInstance()->getWorldID(val.first);
             if (val.second != world_id)
@@ -3127,34 +3186,6 @@ void LLMaterialEditor::setFromGLTFMaterial(LLGLTFMaterial* mat)
             }
         }
     }
-    else
-    {
-        for (mat_connection_map_t::value_type cn : mTextureChangesUpdates)
-        {
-            LLUUID world_id = LLLocalBitmapMgr::getInstance()->getWorldID(cn.second.mTrackingId);
-            if (world_id == mat->mTextureId[LLGLTFMaterial::GLTF_TEXTURE_INFO_BASE_COLOR])
-            {
-                LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(cn.second.mTrackingId, mat);
-                continue;
-            }
-            if (world_id == mat->mTextureId[LLGLTFMaterial::GLTF_TEXTURE_INFO_METALLIC_ROUGHNESS])
-            {
-                LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(cn.second.mTrackingId, mat);
-                continue;
-            }
-            if (world_id == mat->mTextureId[LLGLTFMaterial::GLTF_TEXTURE_INFO_EMISSIVE])
-            {
-                LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(cn.second.mTrackingId, mat);
-                continue;
-            }
-            if (world_id == mat->mTextureId[LLGLTFMaterial::GLTF_TEXTURE_INFO_NORMAL])
-            {
-                LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(cn.second.mTrackingId, mat);
-                continue;
-            }
-            cn.second.mConnection.disconnect();
-        }
-    }
 }
 
 bool LLMaterialEditor::setFromSelection()
@@ -3173,6 +3204,8 @@ bool LLMaterialEditor::setFromSelection()
         const LLViewerInventoryItem* item = selected_object->getInventoryItemByAsset(func.mMaterialId);
         const bool allow_modify = !item || canModify(selected_object, item);
         setEnableEditing(allow_modify);
+
+        // todo: apply local texture data to all materials in selection
     }
     else
     {
@@ -3195,6 +3228,11 @@ bool LLMaterialEditor::setFromSelection()
         // Memorize selection data for filtering further updates
         mOverrideObjectId = func.mObjectId;
         mOverrideObjectTE = func.mObjectTE;
+
+        // Ovverdired might have been updated,
+        // refresh state of local textures in overrides
+        LLSelectedTEUpdateOverrides local_tex_func(this);
+        selected_objects->applyToNodes(&local_tex_func);
     }
 
     return func.mMaterial.notNull();
diff --git a/indra/newview/llmaterialeditor.h b/indra/newview/llmaterialeditor.h
index e6939098e0..95a4c4572d 100644
--- a/indra/newview/llmaterialeditor.h
+++ b/indra/newview/llmaterialeditor.h
@@ -232,6 +232,7 @@ class LLMaterialEditor : public LLPreview, public LLVOInventoryListener
     U32 getUnsavedChangesFlags() { return mUnsavedChanges; }
     U32 getRevertedChangesFlags() { return mRevertedChanges; }
     LLUUID getLocalTextureTrackingIdFromFlag(U32 flag);
+    bool updateMaterialLocalSubscription(LLGLTFMaterial* mat);
 
     static bool capabilitiesAvailable();
 
diff --git a/indra/newview/llviewerobject.cpp b/indra/newview/llviewerobject.cpp
index 8228775596..2bcd0858d3 100644
--- a/indra/newview/llviewerobject.cpp
+++ b/indra/newview/llviewerobject.cpp
@@ -5475,6 +5475,11 @@ S32 LLViewerObject::setTEGLTFMaterialOverride(U8 te, LLGLTFMaterial* override_ma
             tep->setGLTFRenderMaterial(render_mat);
             retval = TEM_CHANGE_TEXTURE;
 
+            for (LLGLTFMaterial::local_tex_map_t::value_type &val : override_mat->mTrackingIdToLocalTexture)
+            {
+                LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(val.first, override_mat);
+            }
+
         }
         else if (tep->setGLTFRenderMaterial(nullptr))
         {
@@ -5482,14 +5487,6 @@ S32 LLViewerObject::setTEGLTFMaterialOverride(U8 te, LLGLTFMaterial* override_ma
         }
     }
 
-    if (retval == TEM_CHANGE_TEXTURE)
-    {
-        for (LLGLTFMaterial::local_tex_map_t::value_type val : override_mat->mTrackingIdToLocalTexture)
-        {
-            LLLocalBitmapMgr::getInstance()->associateGLTFMaterial(val.first, override_mat);
-        }
-    }
-
     return retval;
 }