From f84fc1a7782572678e64ff3d1e2e2ad228b64d43 Mon Sep 17 00:00:00 2001 From: krickw Date: Fri, 25 Feb 2022 12:03:42 -0500 Subject: [PATCH 1/4] Share the cached HdVP2TextureInfo between all materials so that isSRGB and uvScaleOffset and re-used correctly when hitting a texture in the cache. --- lib/mayaUsd/render/vp2RenderDelegate/material.cpp | 4 ++++ lib/mayaUsd/render/vp2RenderDelegate/material.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/mayaUsd/render/vp2RenderDelegate/material.cpp b/lib/mayaUsd/render/vp2RenderDelegate/material.cpp index 25b9d3de6c..459adf8e0e 100644 --- a/lib/mayaUsd/render/vp2RenderDelegate/material.cpp +++ b/lib/mayaUsd/render/vp2RenderDelegate/material.cpp @@ -1318,6 +1318,7 @@ class HdVP2Material::TextureLoadingTask std::mutex HdVP2Material::_refreshMutex; std::chrono::steady_clock::time_point HdVP2Material::_startTime; std::atomic_size_t HdVP2Material::_runningTasksCounter; +HdVP2TextureMap HdVP2Material::_textureMap; /*! \brief Releases the reference to the texture owned by a smart pointer. */ @@ -2491,6 +2492,7 @@ const HdVP2TextureInfo& HdVP2Material::_AcquireTexture( const std::string& path, const HdMaterialNode& node) { + fprintf(stderr, "Searching for texture %s in the texture map\n", path.c_str()); const auto it = _textureMap.find(path); if (it != _textureMap.end()) { return it->second; @@ -2512,6 +2514,8 @@ const HdVP2TextureInfo& HdVP2Material::_AcquireTexture( MHWRender::MTexture* texture = _LoadTexture(path, hasFallbackColor, fallbackColor, isSRGB, uvScaleOffset); + + fprintf(stderr, "Adding texture %s in the texture map\n", path.c_str()); HdVP2TextureInfo& info = _textureMap[path]; info._texture.reset(texture); info._isColorSpaceSRGB = isSRGB; diff --git a/lib/mayaUsd/render/vp2RenderDelegate/material.h b/lib/mayaUsd/render/vp2RenderDelegate/material.h index ddec248cf2..bca4dc75ea 100644 --- a/lib/mayaUsd/render/vp2RenderDelegate/material.h +++ b/lib/mayaUsd/render/vp2RenderDelegate/material.h @@ -149,7 +149,7 @@ class HdVP2Material final : public HdMaterial HdVP2ShaderUniquePtr _surfaceShader; //!< VP2 surface shader instance SdfPath _surfaceShaderId; //!< Path of the surface shader - HdVP2TextureMap _textureMap; //!< Textures used by this material + static HdVP2TextureMap _textureMap; //!< Textures used by this material TfTokenVector _requiredPrimvars; //!< primvars required by this material std::unordered_map _textureLoadingTasks; From 7e550afef5a7ddfefe09db4d0c5d9456551cce64 Mon Sep 17 00:00:00 2001 From: krickw Date: Fri, 25 Feb 2022 12:07:30 -0500 Subject: [PATCH 2/4] Clang format, remove debugging fprintf --- lib/mayaUsd/render/vp2RenderDelegate/material.cpp | 3 --- lib/mayaUsd/render/vp2RenderDelegate/material.h | 8 ++++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/mayaUsd/render/vp2RenderDelegate/material.cpp b/lib/mayaUsd/render/vp2RenderDelegate/material.cpp index 459adf8e0e..663781a882 100644 --- a/lib/mayaUsd/render/vp2RenderDelegate/material.cpp +++ b/lib/mayaUsd/render/vp2RenderDelegate/material.cpp @@ -2492,7 +2492,6 @@ const HdVP2TextureInfo& HdVP2Material::_AcquireTexture( const std::string& path, const HdMaterialNode& node) { - fprintf(stderr, "Searching for texture %s in the texture map\n", path.c_str()); const auto it = _textureMap.find(path); if (it != _textureMap.end()) { return it->second; @@ -2514,8 +2513,6 @@ const HdVP2TextureInfo& HdVP2Material::_AcquireTexture( MHWRender::MTexture* texture = _LoadTexture(path, hasFallbackColor, fallbackColor, isSRGB, uvScaleOffset); - - fprintf(stderr, "Adding texture %s in the texture map\n", path.c_str()); HdVP2TextureInfo& info = _textureMap[path]; info._texture.reset(texture); info._isColorSpaceSRGB = isSRGB; diff --git a/lib/mayaUsd/render/vp2RenderDelegate/material.h b/lib/mayaUsd/render/vp2RenderDelegate/material.h index bca4dc75ea..0f0bde5585 100644 --- a/lib/mayaUsd/render/vp2RenderDelegate/material.h +++ b/lib/mayaUsd/render/vp2RenderDelegate/material.h @@ -147,10 +147,10 @@ class HdVP2Material final : public HdMaterial TfToken _surfaceNetworkToken; //!< Generated token to uniquely identify a material network - HdVP2ShaderUniquePtr _surfaceShader; //!< VP2 surface shader instance - SdfPath _surfaceShaderId; //!< Path of the surface shader - static HdVP2TextureMap _textureMap; //!< Textures used by this material - TfTokenVector _requiredPrimvars; //!< primvars required by this material + HdVP2ShaderUniquePtr _surfaceShader; //!< VP2 surface shader instance + SdfPath _surfaceShaderId; //!< Path of the surface shader + static HdVP2TextureMap _textureMap; //!< Textures used by this material + TfTokenVector _requiredPrimvars; //!< primvars required by this material std::unordered_map _textureLoadingTasks; From 36685b9625c2d82f827375d43c1b0f7628967487 Mon Sep 17 00:00:00 2001 From: krickw Date: Mon, 28 Feb 2022 13:38:06 -0500 Subject: [PATCH 3/4] Use reference counting to manage the lifetime of HdVP2TextureInfo. This allows textures to be deleted when all materials using that texture are destroyed. --- .../render/vp2RenderDelegate/material.cpp | 62 ++++++++++++------- .../render/vp2RenderDelegate/material.h | 23 +++++-- 2 files changed, 56 insertions(+), 29 deletions(-) diff --git a/lib/mayaUsd/render/vp2RenderDelegate/material.cpp b/lib/mayaUsd/render/vp2RenderDelegate/material.cpp index 663781a882..a443702280 100644 --- a/lib/mayaUsd/render/vp2RenderDelegate/material.cpp +++ b/lib/mayaUsd/render/vp2RenderDelegate/material.cpp @@ -1318,7 +1318,7 @@ class HdVP2Material::TextureLoadingTask std::mutex HdVP2Material::_refreshMutex; std::chrono::steady_clock::time_point HdVP2Material::_startTime; std::atomic_size_t HdVP2Material::_runningTasksCounter; -HdVP2TextureMap HdVP2Material::_textureMap; +HdVP2GlobalTextureMap HdVP2Material::_globalTextureMap; /*! \brief Releases the reference to the texture owned by a smart pointer. */ @@ -2492,9 +2492,16 @@ const HdVP2TextureInfo& HdVP2Material::_AcquireTexture( const std::string& path, const HdMaterialNode& node) { - const auto it = _textureMap.find(path); - if (it != _textureMap.end()) { - return it->second; + // see if we already have the texture loaded. + const auto it = _globalTextureMap.find(path); + if (it != _globalTextureMap.end()) { + HdVP2TextureInfoSharedPtr cacheEntry = it->second.lock(); + if (cacheEntry) { + _localTextureMap[path] = cacheEntry; + return *cacheEntry; + } + // if cacheEntry is nullptr then there is a stale entry in the _globalTextureMap. No need + // to erase it because we're going to re-fill that entry right now. } // Get fallback color if defined @@ -2513,18 +2520,22 @@ const HdVP2TextureInfo& HdVP2Material::_AcquireTexture( MHWRender::MTexture* texture = _LoadTexture(path, hasFallbackColor, fallbackColor, isSRGB, uvScaleOffset); - HdVP2TextureInfo& info = _textureMap[path]; - info._texture.reset(texture); - info._isColorSpaceSRGB = isSRGB; + HdVP2TextureInfoSharedPtr info = std::make_shared(); + _localTextureMap.insert_or_assign( + path, info); // should never have already been in _localTextureMap, because if it was + // we'd have found it in _globalTextureMap + _globalTextureMap.insert_or_assign(path, info); + info->_texture.reset(texture); + info->_isColorSpaceSRGB = isSRGB; if (uvScaleOffset.length() > 0) { TF_VERIFY(uvScaleOffset.length() == 4); - info._stScale.Set( + info->_stScale.Set( uvScaleOffset[0], uvScaleOffset[1]); // The first 2 elements are the scale - info._stOffset.Set( + info->_stOffset.Set( uvScaleOffset[2], uvScaleOffset[3]); // The next two elements are the offset } - return info; + return *info; } auto* task = new TextureLoadingTask(this, sceneDelegate, path, hasFallbackColor, fallbackColor); @@ -2581,20 +2592,23 @@ void HdVP2Material::_UpdateLoadedTexture( // function on idle to delete the task object. _textureLoadingTasks.erase(path); - // Check the local cache again, do not overwrite if same texture has - // been loaded asynchronously - if (_textureMap.find(path) != _textureMap.end()) { - return; - } - - HdVP2TextureInfo& info = _textureMap[path]; - info._texture.reset(texture); - info._isColorSpaceSRGB = isColorSpaceSRGB; - if (uvScaleOffset.length() > 0) { - TF_VERIFY(uvScaleOffset.length() == 4); - info._stScale.Set(uvScaleOffset[0], uvScaleOffset[1]); // The first 2 elements are the scale - info._stOffset.Set( - uvScaleOffset[2], uvScaleOffset[3]); // The next two elements are the offset + // Check the cache again. If the texture is not in the cache + // the add it. + if (_globalTextureMap.find(path) == _globalTextureMap.end()) { + HdVP2TextureInfoSharedPtr info = std::make_shared(); + _localTextureMap.insert_or_assign( + path, info); // should never have already been in _localTextureMap, because if it was + // we'd have found it in _globalTextureMap + _globalTextureMap.insert_or_assign(path, info); + info->_texture.reset(texture); + info->_isColorSpaceSRGB = isColorSpaceSRGB; + if (uvScaleOffset.length() > 0) { + TF_VERIFY(uvScaleOffset.length() == 4); + info->_stScale.Set( + uvScaleOffset[0], uvScaleOffset[1]); // The first 2 elements are the scale + info->_stOffset.Set( + uvScaleOffset[2], uvScaleOffset[3]); // The next two elements are the offset + } } // Mark sprim dirty diff --git a/lib/mayaUsd/render/vp2RenderDelegate/material.h b/lib/mayaUsd/render/vp2RenderDelegate/material.h index 0f0bde5585..e6f26e0480 100644 --- a/lib/mayaUsd/render/vp2RenderDelegate/material.h +++ b/lib/mayaUsd/render/vp2RenderDelegate/material.h @@ -62,9 +62,21 @@ struct HdVP2TextureInfo bool _isColorSpaceSRGB { false }; //!< Whether sRGB linearization is needed }; +using HdVP2TextureInfoSharedPtr = std::shared_ptr; +using HdVP2TextureInfoWeakPtr = std::weak_ptr; + /*! \brief An unordered string-indexed map to cache texture information. + + Maya has a global internal texture map but we can't rely on it here, because we miss out + on the extra information we store, such as _isColorSpaceSRGB. In HdVP2GlobalTextureMap we + have that additional information. + + In order to correctly delete textures when they are no longer in use the global texture map + holds only a weak_ptr to the HdVP2TextureInfo. The individual materials hold shared_ptrs to + the textures they are using, so that when no materials are using a texture it'll be deleted. */ -using HdVP2TextureMap = std::unordered_map; +using HdVP2LocalTextureMap = std::unordered_map; +using HdVP2GlobalTextureMap = std::unordered_map; /*! \brief A VP2-specific implementation for a Hydra material prim. \class HdVP2Material @@ -147,10 +159,11 @@ class HdVP2Material final : public HdMaterial TfToken _surfaceNetworkToken; //!< Generated token to uniquely identify a material network - HdVP2ShaderUniquePtr _surfaceShader; //!< VP2 surface shader instance - SdfPath _surfaceShaderId; //!< Path of the surface shader - static HdVP2TextureMap _textureMap; //!< Textures used by this material - TfTokenVector _requiredPrimvars; //!< primvars required by this material + HdVP2ShaderUniquePtr _surfaceShader; //!< VP2 surface shader instance + SdfPath _surfaceShaderId; //!< Path of the surface shader + static HdVP2GlobalTextureMap _globalTextureMap; //!< Texture in use by all materials in MayaUSD + HdVP2LocalTextureMap _localTextureMap; //!< Textures used by this material + TfTokenVector _requiredPrimvars; //!< primvars required by this material std::unordered_map _textureLoadingTasks; From 7ed337d35771f3b9e7444cb6c382a75ac359204c Mon Sep 17 00:00:00 2001 From: krickw Date: Mon, 28 Feb 2022 14:33:51 -0500 Subject: [PATCH 4/4] Build fix for OSX and Linux, where we don't have full C++17 support. --- .../render/vp2RenderDelegate/material.cpp | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/mayaUsd/render/vp2RenderDelegate/material.cpp b/lib/mayaUsd/render/vp2RenderDelegate/material.cpp index a443702280..936396fca1 100644 --- a/lib/mayaUsd/render/vp2RenderDelegate/material.cpp +++ b/lib/mayaUsd/render/vp2RenderDelegate/material.cpp @@ -2499,9 +2499,11 @@ const HdVP2TextureInfo& HdVP2Material::_AcquireTexture( if (cacheEntry) { _localTextureMap[path] = cacheEntry; return *cacheEntry; + } else { + // if cacheEntry is nullptr then there is a stale entry in the _globalTextureMap. Erase + // it now to simplify adding the new texture to the global cache later. + _globalTextureMap.erase(it); } - // if cacheEntry is nullptr then there is a stale entry in the _globalTextureMap. No need - // to erase it because we're going to re-fill that entry right now. } // Get fallback color if defined @@ -2521,10 +2523,12 @@ const HdVP2TextureInfo& HdVP2Material::_AcquireTexture( = _LoadTexture(path, hasFallbackColor, fallbackColor, isSRGB, uvScaleOffset); HdVP2TextureInfoSharedPtr info = std::make_shared(); - _localTextureMap.insert_or_assign( - path, info); // should never have already been in _localTextureMap, because if it was - // we'd have found it in _globalTextureMap - _globalTextureMap.insert_or_assign(path, info); + // path should never already be in _localTextureMap because if it was + // we'd have found it in _globalTextureMap + _localTextureMap.emplace(path, info); + // path should never already be in _globalTextureMap because if it was present + // and nullptr then we erased it. + _globalTextureMap.emplace(path, info); info->_texture.reset(texture); info->_isColorSpaceSRGB = isSRGB; if (uvScaleOffset.length() > 0) { @@ -2596,10 +2600,12 @@ void HdVP2Material::_UpdateLoadedTexture( // the add it. if (_globalTextureMap.find(path) == _globalTextureMap.end()) { HdVP2TextureInfoSharedPtr info = std::make_shared(); - _localTextureMap.insert_or_assign( - path, info); // should never have already been in _localTextureMap, because if it was - // we'd have found it in _globalTextureMap - _globalTextureMap.insert_or_assign(path, info); + // path should never already be in _localTextureMap because if it was + // we'd have found it in _globalTextureMap + _localTextureMap.emplace(path, info); + // path should never already be in _globalTextureMap because if it was present + // and nullptr then we erased it. + _globalTextureMap.emplace(path, info); info->_texture.reset(texture); info->_isColorSpaceSRGB = isColorSpaceSRGB; if (uvScaleOffset.length() > 0) {