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

Added cleanup code such that all code coverage run tests pass. #196

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,11 @@ MayaHydraSceneIndex::MayaHydraSceneIndex(
}

MayaHydraSceneIndex::~MayaHydraSceneIndex()
{
_Destroy();
}

void MayaHydraSceneIndex::_Destroy()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add Destroy() method to perform proper cleanup on code coverage builds.

{
//Remove global materials
if (_mayaDefaultMaterialFallback.IsHolding<HdMaterialNetworkMap>()){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,12 @@ class MAYAHYDRALIB_API MayaHydraSceneIndex : public HdRetainedSceneIndex, public
const SdfPath& id,
HdDirtyBits dirtyBits,
DirtyBitsToLocatorsFunc dirtyBitsToLocatorsFunc);

#ifdef CODE_COVERAGE_WORKAROUND
friend class MtohRenderOverride;
#endif
void _Destroy();

private:
// ------------------------------------------------------------------------
// HdSceneIndexBase implementations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ MayaUsdProxyShapeSceneIndex::MayaUsdProxyShapeSceneIndex(
}

MayaUsdProxyShapeSceneIndex::~MayaUsdProxyShapeSceneIndex()
{
_Destroy();
}

void MayaUsdProxyShapeSceneIndex::_Destroy()
{
TfNotice::Revoke(_stageSetNoticeKey);
TfNotice::Revoke(_stageInvalidateNoticeKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ class MayaUsdProxyShapeSceneIndex : public HdSingleInputFilteringSceneIndexBase
_SendPrimsDirtied(entries);
}

#ifndef CODE_COVERAGE_WORKAROUND
private:
#endif
void _Destroy();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add Destroy() method to perform proper cleanup on code coverage builds.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a ticket to track when to remove the CODE_COVERAGE_WORKAROUND?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just created one: HYDRA-1297


private:
void _ObjectsChanged(const MAYAUSDAPI_NS::ProxyStageObjectsChangedNotice& notice);
void _StageSet(const MAYAUSDAPI_NS::ProxyStageSetNotice& notice);
Expand Down
28 changes: 28 additions & 0 deletions lib/mayaHydra/hydraExtensions/sceneIndex/registration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@

#include <optional>

PXR_NAMESPACE_OPEN_SCOPE
struct MayaUsdSceneIndexRegistration;
PXR_NAMESPACE_CLOSE_SCOPE

namespace {

const std::string digits = "0123456789";
Expand Down Expand Up @@ -91,6 +95,9 @@ class SceneObserver : public Observer
}
};

class PathInterfaceSceneIndex;
typedef TfRefPtr<PathInterfaceSceneIndex> PathInterfaceSceneIndexRefPtr;

/// \class PathInterfaceSceneIndex
///
/// Implement the path interface for plugin scene indices.
Expand Down Expand Up @@ -306,6 +313,14 @@ class PathInterfaceSceneIndex : public Fvp::PathInterfaceSceneIndexBase
}

~PathInterfaceSceneIndex() {
Destroy();
}

#ifdef CODE_COVERAGE_WORKAROUND
friend struct PXR_NS::MayaUsdSceneIndexRegistration;
#endif

void Destroy() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add Destroy() method to perform proper cleanup on code coverage builds.

// Unregister our path mapper.
TF_AXIOM(Fvp::PathMapperRegistry::Instance().Unregister(
_sceneIndexAppPath));
Expand Down Expand Up @@ -339,6 +354,18 @@ struct MayaUsdSceneIndexRegistration : public MayaHydraSceneIndexRegistration
auto proxyShapeSceneIndex = TfDynamic_cast<MayaUsdProxyShapeSceneIndexRefPtr>(pluginSceneIndex);
proxyShapeSceneIndex->UpdateTime();
}

#ifdef CODE_COVERAGE_WORKAROUND
void Destroy() override {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Invoke Destroy() methods.

auto proxyShapeSceneIndex = TfDynamic_cast<MayaUsdProxyShapeSceneIndexRefPtr>(pluginSceneIndex);
proxyShapeSceneIndex->_Destroy();

auto pathInterfaceSceneIndex = TfDynamic_cast<PathInterfaceSceneIndexRefPtr>(rootSceneIndex);
if (pathInterfaceSceneIndex) {
pathInterfaceSceneIndex->Destroy();
}
}
#endif
};

// MayaHydraSceneIndexRegistration is used to register a scene index for
Expand Down Expand Up @@ -440,6 +467,7 @@ bool MayaHydraSceneIndexRegistry::_RemoveSceneIndexForNode(const MObject& dagNod
_registrationsByObjectHandle.erase(dagNodeHandle);
_registrations.erase(registration->sceneIndexPathPrefix);
#ifdef CODE_COVERAGE_WORKAROUND
registration->Destroy();
Fvp::leakSceneIndex(registration->rootSceneIndex);
#endif
return true;
Expand Down
3 changes: 3 additions & 0 deletions lib/mayaHydra/hydraExtensions/sceneIndex/registration.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ struct MayaHydraSceneIndexRegistration
MayaHydraInterpretRprimPath interpretRprimPathFn = nullptr;

virtual void Update() = 0;
#ifdef CODE_COVERAGE_WORKAROUND
virtual void Destroy() = 0;
#endif
};

/**
Expand Down
14 changes: 7 additions & 7 deletions lib/mayaHydra/mayaPlugin/renderOverride.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,13 @@ void MtohRenderOverride::_SetRenderPurposeTags(const MayaHydraParams& delegatePa

void MtohRenderOverride::_ClearMayaHydraSceneIndex()
{
#ifdef CODE_COVERAGE_WORKAROUND
// Leak the Maya scene index for code coverage, as its base class
// HdRetainedSceneIndex dtor crashes in Windows clang code coverage build.
_mayaHydraSceneIndex->_Destroy();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Invoke Destroy() method.

#else
_renderIndexProxy->RemoveSceneIndex(_mayaHydraSceneIndex);
#endif
_mayaHydraSceneIndex.Reset();
}

Expand Down Expand Up @@ -1145,13 +1151,7 @@ void MtohRenderOverride::ClearHydraResources(bool fullReset)
// Remove the scene index registry
_sceneIndexRegistry.reset();

#ifdef CODE_COVERAGE_WORKAROUND
// Leak the Maya scene index, as its base class HdRetainedSceneIndex
// destructor crashes under Windows clang code coverage build.
_mayaHydraSceneIndex.Reset();
#else
_ClearMayaHydraSceneIndex();
#endif
_ClearMayaHydraSceneIndex();

_displayStyleSceneIndex = nullptr;
_pruneTexturesSceneIndex = nullptr;
Expand Down