-
Notifications
You must be signed in to change notification settings - Fork 203
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
Minimize name-based lookup of proxy shape nodes. #1245
Conversation
#include <cassert> | ||
|
||
PXR_NAMESPACE_USING_DIRECTIVE | ||
|
||
namespace { | ||
|
||
MObjectHandle proxyShapeHandle(const Ufe::Path& path) | ||
MObjectHandle nameLookup(const Ufe::Path& path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed function name to make it very clear what's involved.
@@ -68,15 +98,10 @@ UsdStageMap g_StageMap; | |||
// UsdStageMap | |||
//------------------------------------------------------------------------------ | |||
|
|||
void UsdStageMap::addItem(const Ufe::Path& path, UsdStageWeakPtr stage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to pass in the stage, you can simply retrieve it from the proxy shape when populating the cache.
UsdStageWeakPtr UsdStageMap::stage(const Ufe::Path& path) | ||
UsdStageWeakPtr UsdStageMap::stage(const Ufe::Path& path) { return objToStage(proxyShape(path)); } | ||
|
||
MObject UsdStageMap::proxyShape(const Ufe::Path& path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the engine method for the stage() and proxyShapeNode() methods. It indexes into the unordered map of MObjectHandle's to proxy shapes using the Ufe::Path hash. This is how we avoid name-based lookups.
return iter == std::end(fPathToObject) ? MObject() : iter->second.object(); | ||
} | ||
|
||
MayaUsdProxyShapeBase* UsdStageMap::proxyShapeNode(const Ufe::Path& path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New accessor to the cache for the proxy shape node pointer. Allows us to retrieve data from the proxy shape node, such as time, or enabled purposes.
lib/mayaUsd/ufe/UsdStageMap.cpp
Outdated
auto stage = ProxyShapeHandler::dagPathToStage(psn); | ||
addItem(ufePath, stage); | ||
for (const auto& psn : ProxyShapeHandler::getAllNames()) { | ||
addItem( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebuilding is now simpler, name-based lookup now done by addItem.
nothing in the data model prevents it). To generalized access to the | ||
underlying node, we store an MObjectHandle in the maps. | ||
|
||
The cache is refreshed on access to a stage given a path which cannot be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still wish to avoid having the cache observe the data model, to avoid observation cost as well as order of notification problems. Rebuild on access failure is simpler to code, and access failure through incorrect input is most likely not a high-frequency code path.
proxyShapeObj = proxyShapeDagPath.node(); | ||
cache = std::pair<Ufe::Path, MObjectHandle>(proxyShapePath, MObjectHandle(proxyShapeObj)); | ||
// Proxy shape node should not be null. | ||
auto proxyShape = g_StageMap.proxyShapeNode(path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed homebrew, single node caching in favor of using the global proxy shape cache.
Will have QA try this branch out quickly before we merge. |
David Santos tested this branch manually, and ran QATA tests, so good to go. |
The UsdStageMap was performing name-based lookup of proxy shape nodes on every access. This has been replaced with a scheme that uses an unordered map index by Ufe::Path, which are hashable.