-
Notifications
You must be signed in to change notification settings - Fork 202
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-125897 support duplicate xform ops #2695
Conversation
When a complex rig controls geometry, it is possible that the USD exporter will author a given transform operation (for example, translate) multiple times on a single USD trasformable geometry. The old exporter code would try to create the same USD transform operation, which would fail. Instead, we now detect that a transform operation already has an instance and create new ones with a suffix for each extra duplicate op. A limit of 1000 instances of a given transform op has been set. It seems unlikely that any single geometry will have more than 1000 direct operations applied to it at the same times. (This number does not count indirect transform from parents.)
@@ -260,6 +260,34 @@ bool UsdMayaTransformWriter::_GatherAnimChannel( | |||
return false; | |||
} | |||
|
|||
void UsdMayaTransformWriter::_MakeAnimChannelsUnique(const UsdGeomXformable& usdXformable) | |||
{ |
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.
Can you move the uniqueName()
function from lib\mayaUsd\ufe\Utils.h
to lib\mayaUsd\utils\util.h
and use that instead? You can call it as you go by keeping the existingOps
set up to date after each rename.
This will make sure we have a unique way of making names unique in the app, and properly deal with names that already have a numeric suffix.
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.
Ah, I just checked uniqueName... unfortunately, the API is not quite compatible. uniqueName generates the "full name", but what we need to change is the piece used to generate the full name. (The suffix, to be precise.)
I will add the newly generated xform op to the existingOps set though as suggested, in case the newly added op collides with another one later on while iterating over all ops.
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.
Perhaps a simple-minded question: why do we append a separate translation? Do we need to recover it separately? Why not combine it with the existing translation?
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.
Well, the translation comes from different pieces of the rig, as far as I could understand it. One came from a "foot" rig control, the other from a "foot-roll" rig-control. Given that they can control things is arbitrary ways, it seems hard to know how to merge them since there could be rotation, scale etc between two translations.
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.
Those are all good points. I do wonder how this round-trips, if at all.
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.
Once the rig has been cached to USD, there is no longer a rig when edited-as-Maya, it is pure mesh/transform Maya nodes, so I expect they round-trip as part of our general support for generic xformop:order. If not, that code will need to be fixed, but we have no reason to think it should fail.
If we ever somehow support rig round-trip throughUSD (via maybe a future more complex USD skeleton or somesuch) then we will have to revisit the issue, but that is not even on the horizon.
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.
Requires a unit test so we can see the actual results of your algoritm.
About the unit test: I had thought about it but IDK how to setup a complex rig that would reproduce the problem. The scene / rig used here is of unknown origin for me and is somewhat heavy (7MB). The other problem is that the result was not erroneous: I had sur-imposed the original Maya rig on top of the cache USD one and they were doing exactly the same movements. The error was merely that an error was printed in the log. |
I asked QA if the scene file can be used in unit tests. Will query the expected additional translation on the prim to detect that it got added, I suppose. |
PF all passed except one on Windows where the network drive got disconnected, but it had passed in a previous preflight, this is just a glitch. |
When a complex rig controls geometry, it is possible that the USD exporter will author a given transform operation (for example, translate) multiple times on a single USD trasformable geometry. The old exporter code would try to create the same USD transform operation, which would fail. Instead, we now detect that a transform operation already has an instance and create new ones with a suffix for each extra duplicate op.
A limit of 1000 instances of a given transform op has been set. It seems unlikely that any single geometry will have more than 1000 direct operations applied to it at the same times. (This number does not count indirect transform from parents.)