#2991: Fix PBR terrain sometimes not loading textures

Author: cosmic-linden

indra/newview/llfetchedgltfmaterial.h

@@ -31,10 +31,14 @@
 #include "llviewertexture.h"
 
 class LLGLSLShader;
+class LLGLTFMaterialList;
+class LLTerrainMaterials;
 
 class LLFetchedGLTFMaterial: public LLGLTFMaterial
 {
-    friend class LLGLTFMaterialList; // for lifetime management
+    // for lifetime management
+    friend class LLGLTFMaterialList;
+    friend class LLTerrainMaterials;
 public:
     LLFetchedGLTFMaterial();
     virtual ~LLFetchedGLTFMaterial();

indra/newview/llvlcomposition.cpp

@@ -102,14 +102,6 @@ namespace
     }
 };
 
-LLTerrainMaterials::LLTerrainMaterials()
-{
-    for (S32 i = 0; i < ASSET_COUNT; ++i)
-    {
-        mMaterialTexturesSet[i] = false;
-    }
-}
-
 LLTerrainMaterials::~LLTerrainMaterials()
 {
     unboost();
@@ -199,7 +191,6 @@ void LLTerrainMaterials::setDetailAssetID(S32 asset, const LLUUID& id)
     LLPointer<LLFetchedGLTFMaterial>& mat = mDetailMaterials[asset];
     mat = id.isNull() ? nullptr : gGLTFMaterialList.getMaterial(id);
     mDetailRenderMaterials[asset] = nullptr;
-    mMaterialTexturesSet[asset] = false;
 }
 
 const LLGLTFMaterial* LLTerrainMaterials::getMaterialOverride(S32 asset) const
@@ -262,11 +253,17 @@ bool LLTerrainMaterials::makeMaterialsReady(bool boost, bool strict)
         if (!material_asset_ready(mat)) { continue; }
 
         LLPointer<LLFetchedGLTFMaterial>& render_mat = mDetailRenderMaterials[i];
+        // This will be mutated by materialTexturesReady, due to the way that
+        // function is implemented.
+        bool render_material_textures_set = bool(render_mat);
         if (!render_mat)
         {
             render_mat = new LLFetchedGLTFMaterial();
             *render_mat = *mat;
             // This render_mat is effectively already loaded, because it gets its data from mat.
+            // However, its textures may not be loaded yet.
+            render_mat->materialBegin();
+            render_mat->materialComplete(true);
 
             LLPointer<LLGLTFMaterial>& override_mat = mDetailMaterialOverrides[i];
             if (override_mat)
@@ -275,7 +272,8 @@ bool LLTerrainMaterials::makeMaterialsReady(bool boost, bool strict)
             }
         }
 
-        ready[i] = materialTexturesReady(render_mat, mMaterialTexturesSet[i], boost, strict);
+        ready[i] = materialTexturesReady(render_mat, render_material_textures_set, boost, strict);
+        llassert(render_material_textures_set);
     }
 
 #if 1
@@ -414,16 +412,6 @@ bool LLTerrainMaterials::materialTexturesReady(LLPointer<LLFetchedGLTFMaterial>&
     return true;
 }
 
-// Boost the loading priority of every known texture in the material
-// Return true when ready to use
-// static
-bool LLTerrainMaterials::makeMaterialReady(LLPointer<LLFetchedGLTFMaterial> &mat, bool &textures_set, bool boost, bool strict)
-{
-    if (!material_asset_ready(mat)) { return false; }
-
-    return materialTexturesReady(mat, textures_set, boost, strict);
-}
-
 // static
 const LLUUID (&LLVLComposition::getDefaultTextures())[ASSET_COUNT]
 {

indra/newview/llvlcomposition.h

@@ -27,18 +27,17 @@
 #ifndef LL_LLVLCOMPOSITION_H
 #define LL_LLVLCOMPOSITION_H
 
+#include "llfetchedgltfmaterial.h"
+#include "llimage.h"
+#include "llpointer.h"
+#include "llterrainpaintmap.h"
 #include "llviewerlayer.h"
 #include "llviewershadermgr.h"
 #include "llviewertexture.h"
-#include "llpointer.h"
-
-#include "llimage.h"
 
 class LLSurface;
 
 class LLViewerFetchedTexture;
-class LLGLTFMaterial;
-class LLFetchedGLTFMaterial;
 
 class LLModifyRegion
 {
@@ -52,7 +51,7 @@ class LLTerrainMaterials : public LLModifyRegion
 public:
     friend class LLDrawPoolTerrain;
 
-    LLTerrainMaterials();
+    LLTerrainMaterials() {}
     virtual ~LLTerrainMaterials();
 
     void apply(const LLModifyRegion& other);
@@ -93,15 +92,15 @@ class LLTerrainMaterials : public LLModifyRegion
     static bool makeTextureReady(LLPointer<LLViewerFetchedTexture>& tex, bool boost);
     // strict = true -> all materials must be sufficiently loaded
     // strict = false -> at least one material must be loaded
-    static bool makeMaterialReady(LLPointer<LLFetchedGLTFMaterial>& mat, bool& textures_set, bool boost, bool strict);
-    // *NOTE: Prefer calling makeMaterialReady if mat is known to be LLFetchedGLTFMaterial
     static bool materialTexturesReady(LLPointer<LLFetchedGLTFMaterial>& mat, bool& textures_set, bool boost, bool strict);
 
     LLPointer<LLViewerFetchedTexture> mDetailTextures[ASSET_COUNT];
+    // *NOTE: Unlike mDetailRenderMaterials, the textures in this are not
+    // guaranteed to be set or loaded after a true return from
+    // makeMaterialsReady.
     LLPointer<LLFetchedGLTFMaterial> mDetailMaterials[ASSET_COUNT];
     LLPointer<LLGLTFMaterial> mDetailMaterialOverrides[ASSET_COUNT];
     LLPointer<LLFetchedGLTFMaterial> mDetailRenderMaterials[ASSET_COUNT];
-    bool mMaterialTexturesSet[ASSET_COUNT];
 
     U32 mPaintType = TERRAIN_PAINT_TYPE_HEIGHTMAP_WITH_NOISE;
     LLPointer<LLViewerTexture> mPaintMap;