Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAYA-121882 Share the cached HdVP2TextureInfo between all materials so that isSRGB #2145

Merged
merged 4 commits into from
Mar 1, 2022
+62 −28
Merged

MAYA-121882 Share the cached HdVP2TextureInfo between all materials so that isSRGB #2145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f84fc1a
Share the cached HdVP2TextureInfo between all materials so that isSRG…
Feb 25, 2022
7e550af
Clang format, remove debugging fprintf
Feb 25, 2022
36685b9
Use reference counting to manage the lifetime of HdVP2TextureInfo. Th…
Feb 28, 2022
7ed337d
Build fix for OSX and Linux, where we don't have full C++17 support.
Feb 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 44 additions & 23 deletions lib/mayaUsd/render/vp2RenderDelegate/material.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
HdVP2GlobalTextureMap HdVP2Material::_globalTextureMap;

/*! \brief Releases the reference to the texture owned by a smart pointer.
*/
Expand Down Expand Up @@ -2491,9 +2492,18 @@ 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;
} 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);
}
}

// Get fallback color if defined
Expand All @@ -2512,18 +2522,24 @@ 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<HdVP2TextureInfo>();
// 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) {
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);
Expand Down Expand Up @@ -2580,20 +2596,25 @@ 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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The early exit here is wrong. It also stops marking the sprim dirty and triggering a refresh.

}

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<HdVP2TextureInfo>();
// 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) {
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
Expand Down
23 changes: 18 additions & 5 deletions lib/mayaUsd/render/vp2RenderDelegate/material.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,21 @@ struct HdVP2TextureInfo
bool _isColorSpaceSRGB { false }; //!< Whether sRGB linearization is needed
};

using HdVP2TextureInfoSharedPtr = std::shared_ptr<HdVP2TextureInfo>;
using HdVP2TextureInfoWeakPtr = std::weak_ptr<HdVP2TextureInfo>;

/*! \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<std::string, HdVP2TextureInfo>;
using HdVP2LocalTextureMap = std::unordered_map<std::string, HdVP2TextureInfoSharedPtr>;
using HdVP2GlobalTextureMap = std::unordered_map<std::string, HdVP2TextureInfoWeakPtr>;

/*! \brief A VP2-specific implementation for a Hydra material prim.
\class HdVP2Material
Expand Down Expand Up @@ -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
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a single global static structure we'd never delete the textures until the plugin unloads or Maya exits. At that time it is often not safe to try to destroy the texture, and textures were held in the cache beyond file -new.

This way we get the deletion at the right time.

HdVP2LocalTextureMap _localTextureMap; //!< Textures used by this material
TfTokenVector _requiredPrimvars; //!< primvars required by this material

std::unordered_map<std::string, TextureLoadingTask*> _textureLoadingTasks;

Expand Down