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

MAYA-105827 - Compute extents for modified GPrims #1112

Merged
merged 12 commits into from
Feb 5, 2021
Merged
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
86 changes: 86 additions & 0 deletions lib/mayaUsd/nodes/proxyShapeBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,17 @@
#include <pxr/base/tf/token.h>
#include <pxr/base/trace/trace.h>
#include <pxr/usd/ar/resolver.h>
#include <pxr/usd/sdf/attributeSpec.h>
#include <pxr/usd/sdf/layer.h>
#include <pxr/usd/sdf/path.h>
#include <pxr/usd/usd/editContext.h>
#include <pxr/usd/usd/prim.h>
#include <pxr/usd/usd/stage.h>
#include <pxr/usd/usd/stageCacheContext.h>
#include <pxr/usd/usd/timeCode.h>
#include <pxr/usd/usdGeom/bboxCache.h>
#include <pxr/usd/usdGeom/boundable.h>
#include <pxr/usd/usdGeom/gprim.h>
#include <pxr/usd/usdGeom/imageable.h>
#include <pxr/usd/usdGeom/tokens.h>
#include <pxr/usd/usdUtils/stageCache.h>
Expand Down Expand Up @@ -74,6 +78,7 @@
#include <maya/MPlug.h>
#include <maya/MPlugArray.h>
#include <maya/MPoint.h>
#include <maya/MProfiler.h>
#include <maya/MPxSurfaceShape.h>
#include <maya/MSelectionMask.h>
#include <maya/MStatus.h>
Expand Down Expand Up @@ -134,6 +139,18 @@ MObject MayaUsdProxyShapeBase::outTimeAttr;
MObject MayaUsdProxyShapeBase::outStageDataAttr;
MObject MayaUsdProxyShapeBase::outStageCacheIdAttr;

namespace {
//! Profiler category for proxy accessor events
const int _shapeBaseProfilerCategory = MProfiler::addCategory(
#if MAYA_API_VERSION >= 20190000
"ProxyShapeBase",
"ProxyShapeBase events"
#else
"ProxyShapeBase"
#endif
);
} // namespace

/* static */
void* MayaUsdProxyShapeBase::creator() { return new MayaUsdProxyShapeBase(); }

Expand Down Expand Up @@ -773,6 +790,9 @@ MBoundingBox MayaUsdProxyShapeBase::boundingBox() const
{
TRACE_FUNCTION();

MProfilingScope profilingScope(
_shapeBaseProfilerCategory, MProfiler::kColorB_L1, "Get shape BoundingBox");

MStatus status;

// Make sure outStage is up to date
Expand Down Expand Up @@ -1186,7 +1206,73 @@ void MayaUsdProxyShapeBase::_OnStageContentsChanged(const UsdNotice::StageConten

void MayaUsdProxyShapeBase::_OnStageObjectsChanged(const UsdNotice::ObjectsChanged& notice)
{
MProfilingScope profilingScope(
_shapeBaseProfilerCategory, MProfiler::kColorB_L1, "Process USD objects changed");

ProxyAccessor::stageChanged(_usdAccessor, thisMObject(), notice);

// Recompute the extents of any UsdGeomBoundable that has authored extents
const auto& stage = notice.GetStage();
if (stage != getUsdStage()) {
TF_CODING_ERROR("We shouldn't be receiving notification for other stages than one "
"returned by stage provider");
return;
}

for (const auto& changedPath : notice.GetChangedInfoOnlyPaths()) {
if (!changedPath.IsPrimPropertyPath()) {
continue;
}

const TfToken& changedPropertyToken = changedPath.GetNameToken();
if (changedPropertyToken == UsdGeomTokens->extent) {
continue;
}

SdfPath changedPrimPath = changedPath.GetPrimPath();
const UsdPrim& changedPrim = stage->GetPrimAtPath(changedPrimPath);
UsdGeomBoundable boundableObj = UsdGeomBoundable(changedPrim);
if (!boundableObj) {
continue;
}

// If the attribute is not part of the primitive schema, it does not affect extents
#if USD_VERSION_NUM > 2002
auto attrDefn
= changedPrim.GetPrimDefinition().GetSchemaAttributeSpec(changedPropertyToken);
#else
auto attrDefn = PXR_NS::UsdSchemaRegistry::GetAttributeDefinition(
changedPrim.GetTypeName(), changedPropertyToken);
#endif
if (!attrDefn) {
continue;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Filters out the transform notifications when manipulating USD objects.

}

// Ignore all attributes known to GPrim and its base classes as they
// are guaranteed not to affect extents:
static const std::unordered_set<TfToken, TfToken::HashFunctor> ignoredAttributes(
UsdGeomGprim::GetSchemaAttributeNames(true).cbegin(),
UsdGeomGprim::GetSchemaAttributeNames(true).cend());
if (ignoredAttributes.count(changedPropertyToken) > 0) {
continue;
}

UsdAttribute extentsAttr = boundableObj.GetExtentAttr();
if (extentsAttr.GetNumTimeSamples() > 0) {
TF_CODING_ERROR(
"Can not fix animated extents of %s made dirty by a change on %s.",
changedPrimPath.GetString().c_str(),
changedPropertyToken.GetText());
continue;
}
if (extentsAttr && extentsAttr.HasValue()) {
VtVec3fArray extent(2);
if (UsdGeomBoundable::ComputeExtentFromPlugins(
boundableObj, UsdTimeCode::Default(), &extent)) {

Choose a reason for hiding this comment

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

Here is another challenging place. What if the extent is animated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I must say that reacting by notification is probably not appropriate. There is not enough context to know if the values is/was animated which forces writing a single non-animated extent value. This makes the extent completely wrong if another untouched parameter is kept animated (radius and length of a capsule, for example). There is also no information stored for Undo, and no mechanism to store the undo data of an animated extent (same issue appears in Ufe Undo handler for TypedUsdAttribute::Set).

We could build a set of affected boundables and wait until the Compute function of the shape node is called. At this time we might have a better overall view of the state. We still end up with animation issues: we are expected to compute the extent at the current frame, but might be forced to recompute for the whole clip duration since we have absolutely no idea about the final animated state of the extent, and more specifically, which attributes affect the extent.

extentsAttr.Set(extent);
}
}
}
}

bool MayaUsdProxyShapeBase::closestPoint(
Expand Down
34 changes: 34 additions & 0 deletions test/lib/ufe/testObject3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,40 @@ def testMayaHideAndShowHiddenUndoCommands(self):
self.assertTrue(bool(primSpecCapsule and UsdGeom.Tokens.visibility in primSpecCapsule.attributes))
self.assertTrue(bool(primSpecCylinder and UsdGeom.Tokens.visibility in primSpecCylinder.attributes))

def testMayaGeomExtentsRecomputation(self):
''' Verify the automatic extents computation in when geom attributes change '''

cmds.file(new=True, force=True)

# create a Capsule via contextOps menu
import mayaUsd_createStageWithNewLayer
mayaUsd_createStageWithNewLayer.createStageWithNewLayer()
proxyShapePath = ufe.PathString.path('|stage1|stageShape1')
proxyShapeItem = ufe.Hierarchy.createItem(proxyShapePath)
proxyShapeContextOps = ufe.ContextOps.contextOps(proxyShapeItem)
proxyShapeContextOps.doOp(['Add New Prim', 'Capsule'])

# capsule
capsulePath = ufe.PathString.path('|stage1|stageShape1,/Capsule1')
capsuleItem = ufe.Hierarchy.createItem(capsulePath)
capsulePrim = mayaUsd.ufe.ufePathToPrim(ufe.PathString.string(capsulePath))

# get the height and extent "attributes"
capsuleHeightAttr = capsulePrim.GetAttribute('height')
capsuleExtentAttr = capsulePrim.GetAttribute('extent')

self.assertAlmostEqual(capsuleHeightAttr.Get(), 1.0)
capsuleExtent = capsuleExtentAttr.Get()
self.assertAlmostEqual(capsuleExtent[0][2], -1.0)
self.assertAlmostEqual(capsuleExtent[1][2], 1.0)

capsuleHeightAttr.Set(10.0)

self.assertAlmostEqual(capsuleHeightAttr.Get(), 10.0)
# Extent will have been recomputed:
capsuleExtent = capsuleExtentAttr.Get()
self.assertAlmostEqual(capsuleExtent[0][2], -5.5)
self.assertAlmostEqual(capsuleExtent[1][2], 5.5)

if __name__ == '__main__':
unittest.main(verbosity=2)