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.

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

Flush all resource caches on exit #2529

Merged
merged 2 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
48 changes: 37 additions & 11 deletions lib/mayaUsd/render/vp2RenderDelegate/material.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
#include <maya/MFragmentManager.h>
#include <maya/MGlobal.h>
#include <maya/MProfiler.h>
#include <maya/MSceneMessage.h>
#include <maya/MShaderManager.h>
#include <maya/MStatus.h>
#include <maya/MString.h>
Expand Down Expand Up @@ -116,6 +117,16 @@ static const std::size_t kRefreshDuration { 1000 };

namespace {

// USD `UsdImagingDelegate::ApplyPendingUpdates()` would request to
// remove the material then recreate, this is causing texture disappearing
// when user manipulating a prim (while holding mouse buttion).
// We hold a copy of the texture info reference, so that the texture will not
// get released immediately along with material removal.
// If the textures would have been requested to reload in `ApplyPendingUpdates()`,
// we could still reuse the loaded one from cache, otherwise the idle task can
// safely release the texture.
std::unordered_set<HdVP2TextureInfoSharedPtr> pendingRemovalTextures;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved from the destructor so it can be accessed by OnMayaExit.


// clang-format off
TF_DEFINE_PRIVATE_TOKENS(
_tokens,
Expand Down Expand Up @@ -1734,18 +1745,8 @@ HdVP2Material::~HdVP2Material()
// Tell pending tasks or running tasks (if any) to terminate
ClearPendingTasks();

// USD `UsdImagingDelegate::ApplyPendingUpdates()` would request to
// remove the material then recreate, this is causing texture disappearing
// when user manipulating a prim (while holding mouse buttion).
// We hold a copy of the texture info reference, so that the texture will not
// get released immediately along with material removal.
// If the textures would have been requested to reload in `ApplyPendingUpdates()`,
// we could still reuse the loaded one from cache, otherwise the idle task can
// safely release the texture.
static std::unordered_set<HdVP2TextureInfoSharedPtr> pendingRemovalTextures;
static std::mutex removalTaskMutex;

if (!_IsDisabledAsyncTextureLoading() && !_localTextureMap.empty()) {
std::mutex removalTaskMutex;
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought that this mutex needs to be static so that all HdVP2Material will share the same mutex? With this version of the code the lock_guard does nothing and we could have unsafe parallel modification of pendingRemovalTextures.

Can we go back to the original changes and see if we can figure out the workflow that requires the lock?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. The mutex and the structure it protects should ideally be kept together.

// Locking to avoid race condition for insertion to pendingRemovalTextures
std::lock_guard<std::mutex> lock(removalTaskMutex);

Expand Down Expand Up @@ -3027,13 +3028,31 @@ void HdVP2Material::_UpdateShaderInstance(
}
}

namespace {

void exitingCallback(void* /* unusedData */)
{
// Maya does not unload plugins on exit. Make sure we perform an orderly
// cleanup of the texture cache. Otherwise the cache will clear after VP2
// has shut down.
HdVP2Material::OnMayaExit();
}

MCallbackId gExitingCbId = 0;
} // namespace

/*! \brief Acquires a texture for the given image path.
*/
const HdVP2TextureInfo& HdVP2Material::_AcquireTexture(
HdSceneDelegate* sceneDelegate,
const std::string& path,
const HdMaterialNode& node)
{
// Register for the Maya exit message
if (!gExitingCbId) {
gExitingCbId = MSceneMessage::addCallback(MSceneMessage::kMayaExiting, exitingCallback);
}

// see if we already have the texture loaded.
const auto it = _globalTextureMap.find(path);
if (it != _globalTextureMap.end()) {
Expand Down Expand Up @@ -3226,4 +3245,11 @@ void HdVP2Material::_MaterialChanged(HdSceneDelegate* sceneDelegate)

#endif

void HdVP2Material::OnMayaExit()
{
pendingRemovalTextures.clear();
_globalTextureMap.clear();
HdVP2RenderDelegate::OnMayaExit();
}

PXR_NAMESPACE_CLOSE_SCOPE
2 changes: 2 additions & 0 deletions lib/mayaUsd/render/vp2RenderDelegate/material.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ class HdVP2Material final : public HdMaterial
class TextureLoadingTask;
friend class TextureLoadingTask;

static void OnMayaExit();

private:
void _ApplyVP2Fixes(HdMaterialNetwork& outNet, const HdMaterialNetwork& inNet);
#ifdef WANT_MATERIALX_BUILD
Expand Down
23 changes: 23 additions & 0 deletions lib/mayaUsd/render/vp2RenderDelegate/render_delegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,23 @@ class MShaderCache final
}
#endif

void OnMayaExit()
{
if (_isInitialized) {
for (size_t i = 0; i < FallbackShaderTypeCount; ++i) {
_fallbackShaders[i]._map.clear();
_fallbackCPVShaders[i] = nullptr;
}
_3dSolidShaders._map.clear();
_3dFatPointShaders._map.clear();
_userCache._map.clear();
_3dDefaultMaterialShader = nullptr;
_3dCPVSolidShader = nullptr;
_3dCPVFatPointShader = nullptr;
}
_isInitialized = false;
}

private:
bool _isInitialized { false }; //!< Whether the shader cache is initialized

Expand Down Expand Up @@ -588,6 +605,10 @@ HdVP2RenderDelegate::HdVP2RenderDelegate(ProxyRenderDelegate& drawScene)
*/
HdVP2RenderDelegate::~HdVP2RenderDelegate()
{
CleanupMaterials();
// Release Textures and Shader resources:
_materialSprims.clear();

std::lock_guard<std::mutex> guard(_renderDelegateMutex);
if (_renderDelegateCounter.fetch_sub(1) == 1) {
_resourceRegistry.reset();
Expand Down Expand Up @@ -1141,4 +1162,6 @@ void HdVP2RenderDelegate::CleanupMaterials()
}
}

void HdVP2RenderDelegate::OnMayaExit() { sShaderCache.OnMayaExit(); }

PXR_NAMESPACE_CLOSE_SCOPE
2 changes: 2 additions & 0 deletions lib/mayaUsd/render/vp2RenderDelegate/render_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ class HdVP2RenderDelegate final : public HdRenderDelegate

static const int sProfilerCategory; //!< Profiler category

static void OnMayaExit();

private:
HdVP2RenderDelegate(const HdVP2RenderDelegate&) = delete;
HdVP2RenderDelegate& operator=(const HdVP2RenderDelegate&) = delete;
Expand Down