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-107276 - As a user, on the stage AE template I'd like to use Pri… #1189

Merged
merged 10 commits into from
Mar 12, 2021

Conversation

spinell-adsk
Copy link
Contributor

…m Path

-Add Prim Path field in attribute editor
-Use the prime path to render with VP2RenderDelegate
-Change prim path tooltip

…m Path

-Add Prim Path field in attribute editor
-Use the prime path to render with VP2RenderDelegate
-Change prim path tooltip
@@ -39,7 +39,7 @@ global proc AEmayaUsdProxyShapeTemplate( string $nodeName )
editorTemplate -suppress "outStageCacheId";

// MAYA-109438 - temp suppress these non-functional attributes.
editorTemplate -suppress "primPath";
//editorTemplate -suppress "primPath";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's just remove this line.

@spinell-adsk
Copy link
Contributor Author

Please do not merge yet. I found 2 issues that I need to fix.
The first one is the primPath feature does not work as expected when the stage is create in memory.
The second one is when using primPath, the transform matrix use for rendering is wrong.

@seando-adsk seando-adsk added the do-not-merge-yet Development is not finished, PR not ready for merge label Feb 18, 2021
…m Path

-Fix transform issue when using primPath
-Move primPathAttr dependacy with inStageDataCachedAttr from ProxyShapeBase to ProxyShape to avoid issue when
 when primPath is dirty (stage in memory get cleared)
# Conflicts:
#	plugin/adsk/scripts/AEmayaUsdProxyShapeTemplate.mel
# Conflicts:
#	plugin/adsk/scripts/AEmayaUsdProxyShapeBaseTemplate.mel
Copy link
Contributor Author

@spinell-adsk spinell-adsk left a comment

Choose a reason for hiding this comment

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

@mattyjams I moved the dependency between primPathAttr and inStageDataCachedAttr from ProxyShapeBase to ProxyShape
because we are currently have issue when the primPath changed. When we are re-computing the stage cache, if the stage is not found in a file (stage created in memory) we are creating a new in memory stage which override the current one.

Do you have any concern about this change ?

@@ -387,8 +387,6 @@ MStatus MayaUsdProxyShapeBase::initialize()
retValue = attributeAffects(filePathAttr, outStageCacheIdAttr);
CHECK_MSTATUS_AND_RETURN_IT(retValue);

retValue = attributeAffects(primPathAttr, inStageDataCachedAttr);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the dependency in subclass ProxyShape.
Having this dependency in proxyShapeBase cause issue when using a primPath with a stage that is created in memory.
When computing the inStageDataCachedAttr, when the stage is create in memory, a new stage is created and everything is lost.

Choose a reason for hiding this comment

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

It's worth mentioning why this dependency doesn't belong to the base class - when computing inStageDataCachedAttr, there is nothing that reads primPathAttr unless we are calling into computeSessionLayer implementation of pxr proxy shape - https://github.com/Autodesk/maya-usd/blob/dev/plugin/pxr/maya/lib/usdMaya/proxyShape.cpp#L189

There is still the unwanted effect of clearing the stage on a change to the prim path with pxr proxy shape, but assembly workflows were not working with in-memory stages.

Copy link

@kxl-adsk kxl-adsk left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me. I think it would be worth saving on matrix multiplication when prim path is not the default.
Additionally, this is something we would like to have a regression test for and make sure we don't regress - please check https://github.com/Autodesk/maya-usd/blob/dev/test/lib/mayaUsd/render/vp2RenderDelegate/testVP2RenderDelegatePointInstanceSelection.py for an example of such a test.

Comment on lines 616 to 620
const UsdTimeCode timeCode = _proxyShapeData->ProxyShape()->getTime();
UsdGeomXformCache xformCache(timeCode);
GfMatrix4d m = xformCache.GetLocalToWorldTransform(_proxyShapeData->ProxyShape()->usdPrim());
transform = m * transform;

Choose a reason for hiding this comment

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

You don't have to do this when the prim path is the default pseudo root.

Choose a reason for hiding this comment

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

And same above in usdProxyShapeAdaptor

@mattyjams
Copy link
Contributor

@mattyjams I moved the dependency between primPathAttr and inStageDataCachedAttr from ProxyShapeBase to ProxyShape
because we are currently have issue when the primPath changed. When we are re-computing the stage cache, if the stage is not found in a file (stage created in memory) we are creating a new in memory stage which override the current one.

Do you have any concern about this change ?

I think we should be fine with this change, since the dependency is still intact from the point of the pxrUsdProxyShape.

I'm hoping that the performance impact of computing the local transform in _Sync() in pxrUsdMayaGL will be relatively minimal, since our typical use case is to fill the primPath attribute with a root prim (e.g. /Asset) that has no transform.

Copy link
Contributor

@mattyjams mattyjams left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, though I'd agree with @kxl-adsk that the transform computation can be avoided when usdPrim() yields the psuedoroot or an invalid prim.

lib/mayaUsd/render/pxrUsdMayaGL/usdProxyShapeAdapter.cpp Outdated Show resolved Hide resolved
…m Path

-Only compute prim transform is the prim is not the root
- Fix typos
-Add VP2 test
@spinell-adsk spinell-adsk removed the do-not-merge-yet Development is not finished, PR not ready for merge label Feb 26, 2021
@kxl-adsk kxl-adsk added the proxy Related to base proxy shape label Mar 3, 2021
@kxl-adsk kxl-adsk added ready-for-merge Development process is finished, PR is ready for merge and removed ready-for-merge Development process is finished, PR is ready for merge labels Mar 3, 2021
Copy link

@kxl-adsk kxl-adsk left a comment

Choose a reason for hiding this comment

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

We have to support older versions of USD. Please see my comments below with suggestions on how to fix compilation errors for all supported versions of USD.

…m Path

-Fix build for minimum supported version of USD, i.e. 20.02.
@kxl-adsk kxl-adsk added ready-for-merge Development process is finished, PR is ready for merge and removed ready-for-merge Development process is finished, PR is ready for merge labels Mar 3, 2021
…m Path

-Fix maya scene fix test (remove required maya version and use relative path to the usd fle)
…m Path

-Update the usd test file to add color on prim to avoid weird image generation on diffirent computer (bug to investigate).
- Update the images test
@kxl-adsk kxl-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Mar 12, 2021
@kxl-adsk kxl-adsk merged commit 67f73fb into dev Mar 12, 2021
@kxl-adsk kxl-adsk deleted the spinell/MAYA-107276/ae_primpath branch March 12, 2021 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proxy Related to base proxy shape ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants