Added class to provide an API to work with maya transform nodes types #158
Conversation
|
This is a core functionality that would be great to have in the core library. If I'm not mistaken, this is as well where you will need it in order to integrate with the UFE plugin. Any reasons to keep it in the AL plugin only? |
|
It's not actually a part of the AL plugin, it's in the lower level usdutils library. We put it there because it's code that doesn't have any dependencies on Maya. Having it available to other parts of our pipeline (without needing to pull in the maya libs) seemed like the way to go. |
|
@robthebloke Exactly, this is why for example having it in https://github.com/Autodesk/maya-usd/tree/dev/lib/utils would make it available to all plugins. |
|
@kxl-adsk I think you're misunderstanding me. Look here: https://github.com/Autodesk/maya-usd/blob/dev/lib/utils/utilFileSystem.cpp#L5 Those utils clearly depend on the Maya API. We can't really be in a situation where our Nuke plugin has to link against the Maya API to use a single class that has no dependencies on the Maya API. That's not workable for us. |
|
So, to muddy the waters slightly - when Eoin first mentioned that this PR might be coming, I said we also had a similar PR, which sounded like it covered very similar ground. On a quick glance over this, it seems like it indeed does. It was first proposed here, and actually merged, before it was backed out due to some merge conflicts, at which point this PR was made. It got held up over concerns over linking against a library in the Pixar plugin... and there wasn't a clear place to put "common" components at the time. The current incarnation, against maya_usd, can be found here. @kxl-adsk you mentioned you'd like to move this functionality to a higher level, since the general concept is fairly generic. In some ways,UsdMayaXformStack is even more generic - it defines the concept of an "xform stack", which is simply a defined set of xformops in a certain order. Maya's canonical xform stack is one example, the XformCommonAPI is another. It then defines methods for checking whether a given set of xform ops is compatible with a stack, etc. (The concepts behind UsdMayaXformStack are general enough that it's not even Maya-specific, and I've considered trying to make it into a PR against core USD... however, it seems the other main plugin apps (houdini + katana) don't have a fixed xform stack in the same way that Maya does, so I wasn't sure how much utility there would be for it outside of Maya.) Regardless - if we're attempting to unify stuff, then at some point we'll need to unify this code with UsdMayaXformStack, already in use within the Pixar plugin. I'll try to do a more in-depth comparison of these two branches - where they overlap, what unique functionality they have, and ideally, how they might be merged - but likely won't be able to get to until after addressing the outstanding Maya-to-Hydra work. |
|
One feature that I think UsdMayaXformStack handles nicely, that a quick look through this PR doesn't show, is dealing with split / non-split pivots. ie, stacks like the XformCommonAPI define a single pivot, used for both rotate and scale. Maya's API defines separate rotate + scale pivots. A simple attempt to translate an XformCommonAPI stack into a Maya stack might then conclude that they're incompatible; however, an XformCommonAPI stack can be translated to a Maya stack by simply setting the rotate + scale pivots to be the same. Also, since the AL plugin maintains a "live" link to the USD xform op list, it doesn't split the single pivot op into two until required (ie, the rotate + scale pivots are set to different values), ensuring that any USD-level changes to the single pivot will still get reflected in the maya object. |
|
Thanks for bringing this up @elrond79. It sounds like a topic for the TSC meeting. I wasn't aware of UsdMayaXformStack and it does sound like we should dig into it more and understand the potential for reconciliation (having comparison will help). One thing I will add to this, we also have to make sure this helps us solve issues we are seeing reported around UFE manipulation limitations (caused by restricting it to XformCommonAPI). @cfmoore007 would be great to hear from you regarding that. Going back to the requirement to build a higher level, common components that all plugins can build on top of. I see your point @robthebloke about having the possibility to build components that are reusable in the pipeline and not dependent on Maya, but this doesn't mean we can't create a new lib in /lib folder. |
To be sure, are you referring to #95 and #76 - or other cases? |
|
We are currently investigating a double scaling issue with multipleXform ops (outside the UFE context) and pushToPrim. Our setup is as follows : With UFE, #95 is definitely in the mix. We are using opSuffixes quite a bit ( as you can see above ). Actually, looking at this closer (with UFE), it's a slightly different error : |
|
Hi，
How to add renderman Object Statements to pxr.UsdGeom.Mesh?
I added the render man Object Statements in to mesh for example* Trace Auto
Bias, Trace Bais and Trace Displacements* on to the mesh and assign the the
renderman shader to that respective mesh in maya, while export the usd I
would like to export render man Object Statements ( *Trace Auto Bias*) as
well, How to do that?.
I want to bring this USD in Katana
In Maya render man Object Statements show as a custom attributes, If I
export custom attribute
with USD, The U
Any Examples or help would be greatly appreciated!
Regads,
Subin
|
|
So, I just had a chat with the Animal Logic folks... and we decided that for now we should just move ahead with their implementation (this PR). I'd like to circle back and unify both our efforts at some point... but I don't think I have time to do that now, and in the meantime, they apparently have a bunch of other PRs which rely on this. Also, our work was done in an attempt to unify the AL and Pixar code, and eliminate some bugs we were seeing... we haven't really built much extra infrastructure on top of it. So we can probably use AL's work until we can get around to merging everything again |
|
Sounds good. Thx for the update. |
|
Thanks @elrond79! @kxl-adsk as discussed we will move the code to the core utilities lib, and split out later if we have enough functionality that doesn't depend on Maya |
…todesk#158) MAYA-100926 - Write UFE attribute tests + document new interfaces * Removed Ufe::Attribute::Type() override as a final implementation was coded in UFE. * Added simple mayaUsdProxyShape node AE template to fix weird missing scroll layout problem. * New python test files for UsdAttribute and UsdAttributes * Code review comments.
kxl-adsk
added
the
do-not-merge-yet
robthebloke
force-pushed
the
AL_PR_989_2
branch
from
eb37b92
to
c776ca7
Compare
|
Now with #288 you should have a place to put this in. |
|
@murphyeoin any update on this PR? |
robthebloke
force-pushed
the
AL_PR_989_2
branch
from
c776ca7
to
7cec54f
Compare
lib/CMakeLists.txt
Outdated
| @@ -87,6 +87,7 @@ target_link_libraries(${PROJECT_NAME} | |||
| usdSkel | |||
| usdUtils | |||
| vt | |||
| mayaUsdUtils | |||
There was a problem hiding this comment.
I can't find why we need this dependency?
There was a problem hiding this comment.
Nice work! Only minor comments sprinkled throughout, but the intent and the interface is sound. There are a couple of puzzling things in the interface and implementation of MayaTransformAPI that I'd like your comments on. I must say I'm a bit uncomfortable with some of the silent failure behavior of MayaTransformAPI. Still, overall, looking good!
|
|
||
| protected: | ||
| void convertMatrixOpToComponentOps(const UsdGeomXformOp& op); | ||
| bool matchesMayaTrasformProfile(const std::vector<UsdGeomXformOp>& orderedOps); |
There was a problem hiding this comment.
Typo: "matchesMayaTrasformProfile" --> "matchesMayaTransformProfile".
|
|
||
| namespace MayaUsdUtils { | ||
|
|
||
| #ifndef AL_SUPPORT_LEGACY_NAMES |
There was a problem hiding this comment.
I can't find any use of this definition in this header file. Is this legacy code? Is this needed by an implementation file? If so, it feels like it should go into the implementation file, and in client files that require access to the functionality, presumably AL clients only.
| /// \param convertMatrixOpToComponentOps if true, and if the prim contains a single matrix transform, then | ||
| /// the transform op will be converted to TRS data. | ||
| MAYA_USD_UTILS_PUBLIC | ||
| MayaTransformAPI(UsdPrim prim, bool convertMatrixOpToComponentOps = false); |
There was a problem hiding this comment.
Nit-pick: for these kinds of use cases I like to use a tag rather than a boolean, to improve client code readability, like so:
enum ConvertMatrixOpToComponentOpsTag{ DontConvertMatrixOpToComponentOps, ConvertMatrixOpToComponentOps };
MayaTransformAPI(UsdPrim prim, ConvertMatrixOpToComponentOpsTag convert = DontConvertMatrixOpToComponentOps);
Or similar.
lib/usd/utils/MayaTransformAPI.h
Outdated
| MAYA_USD_UTILS_PUBLIC | ||
| void rotatePivotTranslate(const GfVec3f& value, const UsdTimeCode& time = UsdTimeCode::Default()); | ||
|
|
||
| /// \brief sets thewhether parent transforms affect this UsdGeomXform. |
lib/usd/utils/MayaTransformAPI.cpp
Outdated
| } | ||
|
|
||
| //---------------------------------------------------------------------------------------------------------------------- | ||
| bool MayaTransformAPI::_matchesMayaTrasformProfile(const std::vector<UsdGeomXformOp>& orderedOps) |
There was a problem hiding this comment.
Seems like this could be const.
lib/usd/utils/MayaTransformAPI.cpp
Outdated
| } | ||
|
|
||
| //---------------------------------------------------------------------------------------------------------------------- | ||
| bool MayaTransformAPI::matchesMayaTrasformProfile(const std::vector<UsdGeomXformOp>& orderedOps) |
There was a problem hiding this comment.
Awkward that this isn't const. Could m_api be mutable, or does this pose a problem for object invariants and semantics?
lib/usd/utils/MayaTransformAPI.cpp
Outdated
| m_prim = prim; | ||
| } | ||
|
|
||
| // if the prim is not valid, check to see if we have a transform Op. |
There was a problem hiding this comment.
Why only if the prim is invalid?
| //---------------------------------------------------------------------------------------------------------------------- | ||
| void MayaTransformAPI::inheritsTransform(const bool inherit) | ||
| { | ||
| if(m_prim) |
There was a problem hiding this comment.
In some sense you've chosen silent failure for invalid prims (here and elsewhere in your implementation). Have you thought about raising an exception?
| //---------------------------------------------------------------------------------------------------------------------- | ||
| void MayaTransformAPI::translate(const GfVec3d& value, const UsdTimeCode& time) | ||
| { | ||
| if(m_api != TransformAPI::kFallback && m_prim) |
There was a problem hiding this comment.
Again, we'll get silent failure if m_api != TransformAPI::kFallback.
This reverts commit 6faec56.
| @@ -129,6 +130,7 @@ finally: | |||
| MAYA_PLUG_IN_PATH | |||
| MAYA_SCRIPT_PATH | |||
| PXR_PLUGINPATH_NAME | |||
| LD_LIBRARY_PATH | |||
There was a problem hiding this comment.
Changes from #464 have leaked into this PR. Please remove.
There was a problem hiding this comment.
@robthebloke I looked at all the cmake changes in this PR and they make sense to me but as it was mentioned before, there was an agreement around handling LD_LIBRARY_PATH inside AL's code base rather than test.cmake
Please see Eoin's comment:
#464 (comment)
| @@ -178,6 +180,14 @@ finally: | |||
| "${CMAKE_INSTALL_PREFIX}/plugin/al/lib/usd") | |||
| list(APPEND mayaUsd_varname_PXR_PLUGINPATH_NAME | |||
| "${CMAKE_INSTALL_PREFIX}/plugin/al/plugin") | |||
| if (IS_LINUX AND CMAKE_SKIP_RPATH) | |||
There was a problem hiding this comment.
Same here - part of #464 - please remove.
| @@ -176,4 +176,5 @@ add_subdirectory(utils) | |||
|
|
|||
| if(BUILD_HDMAYA) | |||
| add_subdirectory(render/mayaToHydra) | |||
| add_dependencies(mtoh hdMaya) | |||
There was a problem hiding this comment.
Why? How is that connected to the new API you are adding?
There was a problem hiding this comment.
It isn't. It's connected to the api you've added, and a fix I've had to add to fix it?
There was a problem hiding this comment.
It isn't. It's connected to the api you've added, and a fix I've had to add to fix it?
There was a problem hiding this comment.
@robthebloke add_dependencies(mtoh hdMaya) is not really needed. Once you sync your branch with latest dev, you should get this change automatically:
| /// \param pushToPrimState enable/disable pushToPrim | ||
| /// \return true if the node is valid | ||
| AL_USDMAYA_PUBLIC | ||
| bool enablePushToPrim(const SdfPath& usdPath, bool pushToPrimState) const; |
| /// \param usdPrim a prim that has been brought into maya | ||
| /// \return a dag path to the maya object | ||
| AL_USDMAYA_PUBLIC | ||
| SdfPathVector getTranslatedPrimPaths() const; |
| auto status = AL::usdmaya::registerPlugin(plugin); | ||
| // make sure lockPrims are enabled prior to running tests. | ||
| // Store the value as a global so it can be restored when the plugin unloads. | ||
| g_AL_usdmaya_ignoreLockPrims = MGlobal::optionVarIntValue("AL_usdmaya_ignoreLockPrims"); |
There was a problem hiding this comment.
Again, please explain why it's part of this PR
There was a problem hiding this comment.
I will let @HamedSabri-adsk and @ppt-adsk do the proper review of the logic and cmake part. From my side, I would like to understand the connection to #496 - maybe @marsupial can comment?
As well, please remove changes that are not part of what this PR is proposing, i.e. new API to work with transforms.
|
I give up. Let's fork as a way out of the impasse. |
Within AL_USDUtils, there is a new MayaTransformAPI class that will allow you to easily work with maya transforms in USD. Includes Python bindings
@kxl-adsk this is Rob's API that we discussed. He is currently integrating with UFE
This is the first PR I'm submitting since we got our tests working, so I can fairly much guarantee that this passes the AL test suite on a VFX Platform 2019 compliant CentOS 7.4/USD 0.19.11/maya-2019 configuration