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

Mixed data model compute with proxy accessor #594

Merged
merged 4 commits into from
Jun 30, 2020

Conversation

kxl-adsk
Copy link

This PR enables mixed data model compute between DG and USD, i.e. it allows DG connections to read and write to USD attributes. ProxyAccessor it the class responsible for providing this capability, which becomes the foundation to workflows like:

  • parenting (e.g. DAG to Prim)
  • animation (i.e. driving prim attributes with Maya animation curves)
  • attribute level constraints

We decided to put ProxyAccessor capabilities on ProxyShape but the class itself can be used with any MPxNode with ProxyStageProvider interface for the querying stage and time. Centralizing all writes and reads from the USD stage allows deterministic evaluation order and avoids race conditions during parallel execution.

The ProxyAccessor supports CachedPlayback, i.e. accelerated playback, proper cache invalidation when manipulating and keying accessor plugs and background compute.

This PR doesn't enable native workflows with UFE. A set of reference python methods is provided to illustrate how a particular workflow could be implemented.

The best place to start the review of this PR is the ProxyAccessor class.

The best place to learn more about usage and how to use new capabilities provided by ProxyShape is regression test file. You will see that we leverage proxyAccessor.py which is there only for testing & illustration purpose.

Currently known limitations

  • Proxy shapes with in-memory root layer are not supported by Cached Playback background computation (see skipped testOutputForInMemoryRootLayer_Cache test)
  • Data translation for a specific attribute type may not be available. ProxyAccessor is relying on Converter (One converter with examples #432) class, refer to its documentation to learn more about currently supported types.
  • Modifying USD directly without Maya's commands doesn't allow animation (see skipped testKeyframeWithUFE_NoCaching and testKeyframeWithUSD_NoCaching test)
  • Cyclic dependencies between accessor outputs and inputs
  • Nice name for accessor attributes is not currently set for channels of compound attributes. We rely on UsdMayaReadUtil::FindOrCreateMayaAttr and this is where the fix will have to be done.
  • We still have some room to improve the performance by caching more things in acceleration structure hold by ProxyAccessor

ProxyAccessor is only enabled in PreviewReleases (starting with version 115) and currently only adsk plugin registers it.

from maya.debug.emModeManager import emModeManager
from maya.plugin.evaluator.CacheEvaluatorManager import CacheEvaluatorManager, CACHE_STANDARD_MODE_EVAL

class NonCachingScope(object):
Copy link
Author

Choose a reason for hiding this comment

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

All these helper classes and methods will have to be refactored. It's not part of this PR and we have another PR that moves tests out of Pixar plugin into our global test folder. We will coordinate on where test utils for caching and UFE should live after this PR.

@kxl-adsk kxl-adsk added core Related to core library proxy Related to base proxy shape labels Jun 18, 2020
@cfmoore007
Copy link

cfmoore007 commented Jun 19, 2020

@kxl-adsk - I just tried this out and having some issues here.

Unit test fails, as per : testsFailed.txt

I think because of this, nothing seems to be working in Maya :

  1. no feedback from :
from pxr import Tf
Tf.Debug.SetDebugSymbolsByName("USDMAYA_PROXYACCESSOR", 1)
  1. then, attempts at the following did not work either :
  • can't take a ufe object and move it under the DAG locator I have
  • can't use the constrain command with both selected
  • can't key position of ufe prims

update : seeing the example unit test, looks like this will require 'manual wiring' for now.

Testing environment :

  • Windows 10
  • PR 115
  • maya-usd dev @f45f050
  • usd dev @e7ff8e8

@kxl-adsk
Copy link
Author

As per comments in the description, there is no UFE workflow part currently available. Testing with regular parent or key commands are expected to not work. This PR delivers the compute side only.

For failing test, I saw it on our PF (only Windows).

In your log it looks like the failing test is testParentingDagObjectUnderUsdPrim_Caching. Non cached playback version passed...for some reason cache didn’t fill in background.

Please filter out this test locally (use @unittest.skip(“”)) for the time being.

@cfmoore007
Copy link

Running parts of the .py script manually seems to work.

MixedDataModelTest1

MixedDataModelTest2

The proxyShape is driving the rotate and translation of the USD Prims
The locator follows the USD Prims as well.
MixedDataModelTest3

@cfmoore007
Copy link

testParentingDagObjectUnderUsdPrim_Caching

100% tests passed, 0 tests failed out of 27

Label Time Summary:
ufe = 100.28 secproc (19 tests)
usdPreviewSurface = 10.02 sec
proc (2 tests)

Total Test time (real) = 135.85 sec

Skipping that test worked.

@cfmoore007
Copy link

cfmoore007 commented Jun 19, 2020

It appears in order to use the GetOrCreateAccessPlug() method on the proxyAccessor, the 'ufeItemParent' that is passed in must have an xformOp in place for the plug you are creating.

This example works ( modified from the testMayaUsdProxyAccessor.py script )

  1. time sampled xformOp
    MixedDataModel_works.txt

  2. regular xformOp ( you need the rotate and translate both for the commands to work )
    MixedDataModel_doesnot_work2.txt

"
// Confirm and test
ufeItemParent
// Result: <ufe.PyUfe.SceneItem object at 0x000001FF641457D8> #
translatePlugParent = pa.GetOrCreateAccessPlug(ufeItemParent, usdAttrName='xformOp:translate')
"

However, the following does not ( it's basically the same code, except no timeSampled valuse are added to the ufeItemParent )

MixedDataModel_doesnot_work.txt

"
// Test and confirm
ufeItemParent
// Result: <ufe.PyUfe.SceneItem object at 0x0000015D15DCF1F0> #
translatePlugParent = pa.GetOrCreateAccessPlug(ufeItemParent, usdAttrName='xformOp:translate')

E Error: ArgumentError: file F:\APPLICATIONS\MayaUSD\f45f050-mixed_data_model_compute\lib\python\mayaUsd\lib\proxyAccessor.py line 99: Python argument types in
ReadUtil.FindOrCreateMayaAttr(NoneType, Variability, str, str, str)
did not match C++ signature:
FindOrCreateMayaAttr(class pxrInternal_v0_20__pxrReserved__::SdfValueTypeName typeName, enum pxrInternal_v0_20__pxrReserved__::SdfVariability variability, class std::basic_string<char,struct std::char_traits,class std::allocator > nodeName, class std::basic_string<char,struct std::char_traits,class std::allocator > attrName, class std::basic_string<char,struct std::char_traits,class std::allocator > attrNiceName='')
"

@kxl-adsk
Copy link
Author

You won’t be able to create access plugs for attributes that doesn’t exist on a prim. This is what happens in your “doesnot_work” example.
Important to mention that data type for access plug will be taken from usd attribute.

Without providing attribute, you can fetch world matrix. Here is one example of it:

'''
worldMatrixPlugSphere = pa.GetOrCreateAccessPlug(ufeItemSphere, '', Sdf.ValueTypeNames.Matrix4d)
'''

Copy link
Collaborator

@ppt-adsk ppt-adsk left a comment

Choose a reason for hiding this comment

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

Thanks for the very thorough test, and proxyAccessor.py demonstration code. Will need more time to wrap my head completely around the architecture and the responsibilities of the different member functions, but so far every time I had a question on who does what, I found the answer in the code.

I am eager to find out why we can't use the Sdf path string as part of the attribute name, rather than the hash. Of course I suspect it has to do with performance, but the down side is that readability of the resulting graph is impacted.

if segmentCount == 0:
return None, None

dagPath = str(ufeObject.path().segments[0])[6:]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know you're chopping off "|world" here, but I'd rather we omit this kind of implementation detail by using Ufe::PathString, and getting the first segment from that:

dagPath = ufe.PathString.string(ufeObject.path()).split(',')[0]

Not helpful to you now, but it almost feels like a nice addition to the path string namespace would be
std::string Ufe::PathString::pathSegmentString(const Ufe::Path&, int segmentNdx)
so you could more conveniently write the above as

dagPath = ufe.PathString.pathSegmentString(ufeObject.path(), 0)

Or maybe even, considering your line 52,

std::liststd::string Ufe::PathString::pathSegmentStrings(const Ufe::Path&)

Something to consider for the future.

Copy link
Author

Choose a reason for hiding this comment

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

The fact that we have to deal with |world is something I really don't like. I tried your suggestion but it didn't work as a replacement for the existing code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree, I really don't like it either, and that's why I made the suggestion. I wonder what didn't work; do you have a recent Maya? "|world" should not be produced by ufe.PathString.string(). Otherwise we can have a look at this when you're back at work on Monday, but rest assured that we will get rid of [6:]...

return GetDagAndPrimFromUfe(GetUfeSelection())

def GetAccessPlugName(sdfPath):
plugNameValueAttr = 'AccessValue_' + str(sdfPath.__hash__())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is probably a more general comment, but if we're convertir the Sdf path hash to a string, could we consider using the Sdf path itself, rather than its hash, for readability? In the end, if it all boils down to a string, a more readable string is preferable, though probably longer in length. Are there other implications I'm not seeing?

Copy link
Author

Choose a reason for hiding this comment

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

There have been a few iterations around encoding sdf path onto accessor plugs. Many things got improved and we could now get away from using hash...but sdf path will have to be sanitized to remove illegal characters and we need a way to quickly identify accessor plugs (prefix or something else). FWIW. Check what's allowed in attributes names and you will see why we needed to go with nice name to encode the actual path.

It's something we can iterate on in a separate PR...but please share your ideas for the sanitized version we could use as attribute name.

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 pick this up separately.


The end goal is to leverage UFE to provide integration of mixed data model compute with
workflows like parenting (USD prim to Maya DAG object or vice versa), keying, or constraining.
For the time being, this set of helper functions can be used as a reference to understand
Copy link
Contributor

Choose a reason for hiding this comment

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

This file was very useful to understand the entire PR and also for testing. Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Happy to hear it!

Copy link
Contributor

@pilarmolinalopez pilarmolinalopez left a comment

Choose a reason for hiding this comment

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

awesome! looks good to me. thanks!

@@ -211,6 +218,14 @@ class MayaUsdProxyShapeBase : public MPxSurfaceShape,
const MEvaluationNode& evaluationNode,
PostEvaluationType evalType) override;

#if MAYA_API_VERSION >= 20210000
Copy link
Contributor

@pilarmolinalopez pilarmolinalopez Jun 25, 2020

Choose a reason for hiding this comment

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

configCache and getCacheSetup are also available in 2020. why are we allowing only 2021 here?
(I'm guessing this is because we are only enabling ProxyAccessor in 2021)

Copy link
Author

Choose a reason for hiding this comment

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

Exactly, ProxyAccessor is only available in PreviewRelease because it depends on a number of fixes done on the Maya side. For this particular case, you are correct that cache interfaces are available in Maya 2020, but nothing will use them and we would still need to add conditional compilation for older versions of Maya. This is why I decided to keep it simple and turn it all off for released versions of Maya.

Copy link
Contributor

Choose a reason for hiding this comment

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

understood! thanks!

@kxl-adsk
Copy link
Author

Pushed new changes.

The first change is to properly handle context evaluation. We shouldn't be creating new stages or causing from background context (and threads!) changes to listeners or data consumers.

The second part is about randomly failing caching scope tests. I was hoping that context fixes will make the random failure go away, but they didn't. I got correctness back and warnings removed but we still have a random failure when running the test multiple times. I believe it's a timing problem and fix may have to happen on the Maya side. I won't block this PR since tests are stable and passing in non caching scope (run then 100 times in a row without failure).

@kxl-adsk
Copy link
Author

@williamkrick can you have a look at b057b59 (changes I made to proxy shape base to properly support context evaluation).

@williamkrick
Copy link
Contributor

@williamkrick can you have a look at b057b59 (changes I made to proxy shape base to properly support context evaluation).

It looks okay to me. If we start with the assumption that changing time in USD is similar to cache restore in that it is fast to update the data model then switching the USD stage seems like the correct thing to do.

@kxl-adsk
Copy link
Author

@williamkrick we actually have no other choice than sharing the stage for context evaluation - consider changes made in foreground (or even simpler case like in-memory root layer) which have to be available when computing in background. Because of this, we have to make all contexts see the same stage.

return GetDagAndPrimFromUfe(GetUfeSelection())

def GetAccessPlugName(sdfPath):
plugNameValueAttr = 'AccessValue_' + str(sdfPath.__hash__())
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 pick this up separately.

@kxl-adsk kxl-adsk merged commit 2fa69e8 into dev Jun 30, 2020
@kxl-adsk kxl-adsk deleted the kxl-adsk/MAYA-105180/mixed_data_model_compute branch June 30, 2020 17:28
@kxl-adsk kxl-adsk added the workflows Related to in-context workflows label Jul 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core library proxy Related to base proxy shape workflows Related to in-context workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants