SL-18560 Make local materials save correctly from right-click menu

5 files changed, 133 insertions, 103 deletions

diff --git a/indra/newview/lllocalgltfmaterials.cpp b/indra/newview/lllocalgltfmaterials.cpp
index 03ef12bde1..b07efff827 100644
--- a/indra/newview/lllocalgltfmaterials.cpp
+++ b/indra/newview/lllocalgltfmaterials.cpp
@@ -61,7 +61,6 @@ static const S32 LL_LOCAL_UPDATE_RETRIES    = 5;
 LLLocalGLTFMaterial::LLLocalGLTFMaterial(std::string filename, S32 index)
     : mFilename(filename)
     , mShortName(gDirUtilp->getBaseFileName(filename, true))
-    , mValid(false)
     , mLastModified()
     , mLinkStatus(LS_ON)
     , mUpdateRetries(LL_LOCAL_UPDATE_RETRIES)
@@ -86,17 +85,11 @@ LLLocalGLTFMaterial::LLLocalGLTFMaterial(std::string filename, S32 index)
             << "Filename: " << mFilename << LL_ENDL;
         return; // no valid extension.
     }
-
-    /* next phase of unit creation is nearly the same as an update cycle.
-       we're running updateSelf as a special case with the optional UT_FIRSTUSE
-       which omits the parts associated with removing the outdated texture */
-    mValid = updateSelf();
 }
 
 LLLocalGLTFMaterial::~LLLocalGLTFMaterial()
 {
-    // delete self from material list
-    gGLTFMaterialList.removeMaterial(mWorldID);
+    // gGLTFMaterialList will clean itself
 }
 
 /* accessors */
@@ -125,11 +118,6 @@ S32 LLLocalGLTFMaterial::getIndexInFile()
     return mMaterialIndex;
 }
 
-bool LLLocalGLTFMaterial::getValid()
-{
-    return mValid;
-}
-
 /* update functions */
 bool LLLocalGLTFMaterial::updateSelf()
 {
@@ -163,7 +151,7 @@ bool LLLocalGLTFMaterial::updateSelf()
                     // addMaterial will replace material witha a new
                     // pointer if value already exists but we are
                     // reusing existing pointer, so it should add only.
-                    gGLTFMaterialList.addMaterial(mWorldID, mGLTFMaterial);
+                    gGLTFMaterialList.addMaterial(mWorldID, this);
 
                     mUpdateRetries = LL_LOCAL_UPDATE_RETRIES;
                     updated = true;
@@ -217,17 +205,6 @@ bool LLLocalGLTFMaterial::loadMaterial()
 {
     bool decode_successful = false;
 
-    if (mWorldID.notNull())
-    {
-        // We should already have it, but update mGLTFMaterial just in case
-        // will create a new one if material doesn't exist yet
-        mGLTFMaterial = (LLFetchedGLTFMaterial*)gGLTFMaterialList.getMaterial(mWorldID);
-    }
-    else
-    {
-        mGLTFMaterial = new LLFetchedGLTFMaterial();
-    }
-
     switch (mExtension)
     {
     case ET_MATERIAL_GLTF:
@@ -241,12 +218,8 @@ bool LLLocalGLTFMaterial::loadMaterial()
             decode_successful = LLTinyGLTFHelper::getMaterialFromFile(
                 mFilename,
                 mMaterialIndex,
-                mGLTFMaterial,
-                material_name,
-                mBaseColorFetched,
-                mNormalFetched,
-                mMRFetched,
-                mEmissiveFetched);
+                this,
+                material_name);
 
             if (!material_name.empty())
             {
@@ -315,7 +288,6 @@ LLLocalGLTFMaterialMgr::LLLocalGLTFMaterialMgr()
 
 LLLocalGLTFMaterialMgr::~LLLocalGLTFMaterialMgr()
 {
-    std::for_each(mMaterialList.begin(), mMaterialList.end(), DeletePointer());
     mMaterialList.clear();
 }
 
@@ -386,11 +358,12 @@ S32 LLLocalGLTFMaterialMgr::addUnit(const std::string& filename)
         // Todo: this is rather inefficient, files will be spammed with
         // separate loads and date checks, find a way to improve this.
         // May be doUpdates() should be checking individual files.
-        LLLocalGLTFMaterial* unit = new LLLocalGLTFMaterial(filename, i);
+        LLPointer<LLLocalGLTFMaterial> unit = new LLLocalGLTFMaterial(filename, i);
 
-        if (unit->getValid())
+        // load material from file
+        if (unit->updateSelf())
         {
-            mMaterialList.push_back(unit);
+            mMaterialList.emplace_back(unit);
             loaded_materials++;
         }
         else
@@ -402,7 +375,6 @@ S32 LLLocalGLTFMaterialMgr::addUnit(const std::string& filename)
             notif_args["FNAME"] = filename;
             LLNotificationsUtil::add("LocalGLTFVerifyFail", notif_args);
 
-            delete unit;
             unit = NULL;
         }
     }
@@ -429,7 +401,7 @@ void LLLocalGLTFMaterialMgr::delUnit(LLUUID tracking_id)
         {   /* iterating over a temporary list, hence preserving the iterator validity while deleting. */
             LLLocalGLTFMaterial* unit = *del_iter;
             mMaterialList.remove(unit);
-            delete unit;
+
             unit = NULL;
         }
     }
diff --git a/indra/newview/lllocalgltfmaterials.h b/indra/newview/lllocalgltfmaterials.h
index 097f2ea30a..3bdccbbf3d 100644
--- a/indra/newview/lllocalgltfmaterials.h
+++ b/indra/newview/lllocalgltfmaterials.h
@@ -29,18 +29,17 @@
 
 #include "lleventtimer.h"
 #include "llpointer.h"
+#include "llgltfmateriallist.h"
 
 class LLScrollListCtrl;
 class LLGLTFMaterial;
 class LLViewerObject;
-class LLViewerFetchedTexture;
-class LLFetchedGLTFMaterial;
 
-class LLLocalGLTFMaterial
+class LLLocalGLTFMaterial : public LLFetchedGLTFMaterial
 {
 public: /* main */
     LLLocalGLTFMaterial(std::string filename, S32 index);
-    ~LLLocalGLTFMaterial();
+    virtual ~LLLocalGLTFMaterial();
 
 public: /* accessors */
     std::string	getFilename();
@@ -48,7 +47,6 @@ public: /* accessors */
     LLUUID		getTrackingID();
     LLUUID		getWorldID();
     S32			getIndexInFile();
-    bool		getValid();
 
 public:
     bool updateSelf();
@@ -74,20 +72,11 @@ private: /* members */
     std::string mShortName;
     LLUUID      mTrackingID;
     LLUUID      mWorldID;
-    bool        mValid;
     LLSD        mLastModified;
     EExtension  mExtension;
     ELinkStatus mLinkStatus;
     S32         mUpdateRetries;
     S32         mMaterialIndex; // Single file can have more than one
-
-    // Maintain textures and material pointers to
-    // make sure they won't be deleted in any way
-    LLPointer<LLFetchedGLTFMaterial> mGLTFMaterial;
-    LLPointer<LLViewerFetchedTexture> mBaseColorFetched;
-    LLPointer<LLViewerFetchedTexture> mNormalFetched;
-    LLPointer<LLViewerFetchedTexture> mMRFetched;
-    LLPointer<LLViewerFetchedTexture> mEmissiveFetched;
 };
 
 class LLLocalGLTFMaterialTimer : public LLEventTimer
@@ -120,9 +109,9 @@ public:
     void         doUpdates();
 
 private:
-    std::list<LLLocalGLTFMaterial*>    mMaterialList;
+    std::list<LLPointer<LLLocalGLTFMaterial> >    mMaterialList;
     LLLocalGLTFMaterialTimer           mTimer;
-    typedef std::list<LLLocalGLTFMaterial*>::iterator local_list_iter;
+    typedef std::list<LLPointer<LLLocalGLTFMaterial> >::iterator local_list_iter;
 };
 
 #endif // LL_LOCALGLTFMATERIALS_H
diff --git a/indra/newview/llmaterialeditor.cpp b/indra/newview/llmaterialeditor.cpp
index 397a302069..9173981c01 100644
--- a/indra/newview/llmaterialeditor.cpp
+++ b/indra/newview/llmaterialeditor.cpp
@@ -36,6 +36,7 @@
 #include "llfilesystem.h"
 #include "llgltfmateriallist.h"
 #include "llinventorymodel.h"
+#include "lllocalgltfmaterials.h"
 #include "llnotificationsutil.h"
 #include "lltexturectrl.h"
 #include "lltrans.h"
@@ -236,6 +237,7 @@ struct LLSelectedTEGetMatData : public LLSelectedTEFunctor
     LLUUID mObjectId;
     S32 mObjectTE;
     LLPointer<LLGLTFMaterial> mMaterial;
+    LLPointer<LLLocalGLTFMaterial> mLocalMaterial;
 };
 
 LLSelectedTEGetMatData::LLSelectedTEGetMatData(bool for_override)
@@ -262,48 +264,63 @@ bool LLSelectedTEGetMatData::apply(LLViewerObject* objectp, S32 te_index)
     // or has no base material
     if (can_use && tep && mat_id.notNull())
     {
-        LLPointer<LLGLTFMaterial> mat = tep->getGLTFRenderMaterial();
-        LLUUID tex_color_id;
-        LLUUID tex_metal_id;
-        LLUUID tex_emissive_id;
-        LLUUID tex_normal_id;
-        llassert(mat.notNull()); // by this point shouldn't be null
-        if (mat.notNull())
-        {
-            tex_color_id = mat->mBaseColorId;
-            tex_metal_id = mat->mMetallicRoughnessId;
-            tex_emissive_id = mat->mEmissiveId;
-            tex_normal_id = mat->mNormalId;
-        }
-        if (mFirst)
-        {
-            mMaterial = mat;
-            mTexColorId = tex_color_id;
-            mTexMetalId = tex_metal_id;
-            mTexEmissiveId = tex_emissive_id;
-            mTexNormalId = tex_normal_id;
-            mObjectTE = te_index;
-            mObjectId = objectp->getID();
-            mFirst = false;
-        }
-        else
+        if (mIsOverride)
         {
-            if (mTexColorId != tex_color_id)
+            LLPointer<LLGLTFMaterial> mat = tep->getGLTFRenderMaterial();
+
+            LLUUID tex_color_id;
+            LLUUID tex_metal_id;
+            LLUUID tex_emissive_id;
+            LLUUID tex_normal_id;
+            llassert(mat.notNull()); // by this point shouldn't be null
+            if (mat.notNull())
             {
-                mIdenticalTexColor = false;
+                tex_color_id = mat->mBaseColorId;
+                tex_metal_id = mat->mMetallicRoughnessId;
+                tex_emissive_id = mat->mEmissiveId;
+                tex_normal_id = mat->mNormalId;
             }
-            if (mTexMetalId != tex_metal_id)
+            if (mFirst)
             {
-                mIdenticalTexMetal = false;
+                mMaterial = mat;
+                mTexColorId = tex_color_id;
+                mTexMetalId = tex_metal_id;
+                mTexEmissiveId = tex_emissive_id;
+                mTexNormalId = tex_normal_id;
+                mObjectTE = te_index;
+                mObjectId = objectp->getID();
+                mFirst = false;
             }
-            if (mTexEmissiveId != tex_emissive_id)
+            else
             {
-                mIdenticalTexEmissive = false;
+                if (mTexColorId != tex_color_id)
+                {
+                    mIdenticalTexColor = false;
+                }
+                if (mTexMetalId != tex_metal_id)
+                {
+                    mIdenticalTexMetal = false;
+                }
+                if (mTexEmissiveId != tex_emissive_id)
+                {
+                    mIdenticalTexEmissive = false;
+                }
+                if (mTexNormalId != tex_normal_id)
+                {
+                    mIdenticalTexNormal = false;
+                }
             }
-            if (mTexNormalId != tex_normal_id)
+        }
+        else
+        {
+            LLGLTFMaterial *mat = tep->getGLTFMaterial();
+            LLLocalGLTFMaterial *local_mat = dynamic_cast<LLLocalGLTFMaterial*>(mat);
+
+            if (local_mat)
             {
-                mIdenticalTexNormal = false;
+                mLocalMaterial = local_mat;
             }
+            mMaterial = tep->getGLTFRenderMaterial();
         }
     }
     return true;
@@ -349,7 +366,7 @@ void LLMaterialEditor::setAuxItem(const LLInventoryItem* item)
 BOOL LLMaterialEditor::postBuild()
 {
     // if this is a 'live editor' instance, it is also
-    // single instacne and uses live overrides
+    // single instance and uses live overrides
     mIsOverride = getIsSingleInstance();
 
     mBaseColorTextureCtrl = getChild<LLTextureCtrl>("base_color_texture");
@@ -1691,17 +1708,69 @@ void LLMaterialEditor::loadLive()
 
 void LLMaterialEditor::saveObjectsMaterialAs()
 {
-    LLSD args;
-    args["DESC"] = LLTrans::getString("New Material");
-
-    LLSD payload;
 
     // Find an applicable material.
     // Do this before showing message, because
     // message is going to drop selection.
     LLSelectedTEGetMatData func(false);
-    LLSelectMgr::getInstance()->getSelection()->applyToTEs(&func);
+    LLSelectMgr::getInstance()->getSelection()->applyToTEs(&func, true /*first applicable*/);
 
+    if (func.mLocalMaterial.notNull())
+    {
+        // This is a local material, reload it from file
+        // so that user won't end up with grey textures
+        // on next login.
+        LLMaterialEditor::loadMaterialFromFile(func.mLocalMaterial->getFilename(), func.mLocalMaterial->getIndexInFile());
+
+        LLMaterialEditor* me = (LLMaterialEditor*)LLFloaterReg::getInstance("material_editor");
+        if (me)
+        {
+            // apply differences on top
+            LLGLTFMaterial* local_mat = func.mLocalMaterial.get();
+            // don't use override mat here, it has 'hacked ids'
+            // and values, use end result.
+            LLGLTFMaterial* cmp_mat = func.mMaterial.get();
+
+            me->setBaseColor(cmp_mat->mBaseColor);
+            me->setMetalnessFactor(cmp_mat->mMetallicFactor);
+            me->setRoughnessFactor(cmp_mat->mRoughnessFactor);
+            me->setEmissiveColor(cmp_mat->mEmissiveColor);
+            me->setDoubleSided(cmp_mat->mDoubleSided);
+            me->setAlphaMode(cmp_mat->getAlphaMode());
+            me->setAlphaCutoff(cmp_mat->mAlphaCutoff);
+
+            // most things like colors we can apply without verifying
+            // but texture ids are going to be different from both, base and override
+            // so only apply override id if there is actually a difference
+            if (local_mat->mBaseColorId != cmp_mat->mBaseColorId)
+            {
+                me->setBaseColorId(cmp_mat->mBaseColorId);
+                me->childSetValue("base_color_upload_fee", me->getString("no_upload_fee_string"));
+            }
+            if (local_mat->mNormalId != cmp_mat->mNormalId)
+            {
+                me->setNormalId(cmp_mat->mNormalId);
+                me->childSetValue("normal_upload_fee", me->getString("no_upload_fee_string"));
+            }
+            if (local_mat->mMetallicRoughnessId != cmp_mat->mMetallicRoughnessId)
+            {
+                me->setMetallicRoughnessId(cmp_mat->mMetallicRoughnessId);
+                me->childSetValue("metallic_upload_fee", me->getString("no_upload_fee_string"));
+            }
+            if (local_mat->mEmissiveId != cmp_mat->mEmissiveId)
+            {
+                me->setEmissiveId(cmp_mat->mEmissiveId);
+                me->childSetValue("emissive_upload_fee", me->getString("no_upload_fee_string"));
+            }
+
+            // recalculate upload prices
+            me->markChangesUnsaved(0);
+        }
+
+        return;
+    }
+
+    LLSD payload;
     if (func.mMaterial.notNull())
     {
         payload["data"] = func.mMaterial->asJSON();
@@ -1715,6 +1784,9 @@ void LLMaterialEditor::saveObjectsMaterialAs()
         LL_WARNS() << "Got no material when trying to save material" << LL_ENDL;
     }
 
+    LLSD args;
+    args["DESC"] = LLTrans::getString("New Material");
+
     LLNotificationsUtil::add("SaveMaterialAs", args, payload, boost::bind(&LLMaterialEditor::onSaveObjectsMaterialAsMsgCallback, _1, _2));
 }
 
diff --git a/indra/newview/lltinygltfhelper.cpp b/indra/newview/lltinygltfhelper.cpp
index cff26ea51f..58031d4204 100644
--- a/indra/newview/lltinygltfhelper.cpp
+++ b/indra/newview/lltinygltfhelper.cpp
@@ -186,11 +186,7 @@ bool LLTinyGLTFHelper::getMaterialFromFile(
     const std::string& filename,
     S32 mat_index,
     LLPointer < LLFetchedGLTFMaterial> material,
-    std::string& material_name,
-    LLPointer<LLViewerFetchedTexture>& base_color_tex,
-    LLPointer<LLViewerFetchedTexture>& normal_tex,
-    LLPointer<LLViewerFetchedTexture>& mr_tex,
-    LLPointer<LLViewerFetchedTexture>& emissive_tex)
+    std::string& material_name)
 {
     tinygltf::TinyGLTF loader;
     std::string        error_msg;
@@ -249,6 +245,11 @@ bool LLTinyGLTFHelper::getMaterialFromFile(
         occlusion_img = LLTinyGLTFHelper::getTexture(folder, model_in, material_in.occlusionTexture.index);
     }
 
+    LLPointer<LLViewerFetchedTexture> base_color_tex;
+    LLPointer<LLViewerFetchedTexture> normal_tex;
+    LLPointer<LLViewerFetchedTexture> mr_tex;
+    LLPointer<LLViewerFetchedTexture> emissive_tex;
+
     // todo: pass it into local bitmaps?
     LLTinyGLTFHelper::initFetchedTextures(material_in,
         base_img, normal_img, mr_img, emissive_img, occlusion_img,
@@ -258,7 +259,7 @@ bool LLTinyGLTFHelper::getMaterialFromFile(
     {
         base_color_tex->addTextureStats(64.f * 64.f, TRUE);
         material->mBaseColorId = base_color_tex->getID();
-        material->mBaseColorTexture = base_color_tex;
+        material->mBaseColorTexture;
     }
     else
     {
diff --git a/indra/newview/lltinygltfhelper.h b/indra/newview/lltinygltfhelper.h
index 48a6985641..250a4b0b9a 100644
--- a/indra/newview/lltinygltfhelper.h
+++ b/indra/newview/lltinygltfhelper.h
@@ -47,11 +47,7 @@ namespace LLTinyGLTFHelper
         const std::string& filename,
         S32 mat_index,
         LLPointer < LLFetchedGLTFMaterial> material,
-        std::string& material_name,
-        LLPointer<LLViewerFetchedTexture>& base_color_tex,
-        LLPointer<LLViewerFetchedTexture>& normal_tex,
-        LLPointer<LLViewerFetchedTexture>& mr_tex,
-        LLPointer<LLViewerFetchedTexture>& emissive_tex);
+        std::string& material_name);
 
     void initFetchedTextures(tinygltf::Material& material,
         LLPointer<LLImageRaw>& base_color_img,