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-113077: componentTag IO to files #1755

Merged
merged 11 commits into from
Oct 27, 2021

Conversation

pierrebai-adsk
Copy link
Collaborator

Code for component tag support on behalf of Hans Rijpmeka.

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.

There is no test for component tag support in this pull request, which is worrisome. Otherwise, looks good to me, considering this is not my area of expertise.

plugin/adsk/plugin/exportTranslator.cpp Outdated Show resolved Hide resolved
lib/usd/translators/meshWriter.cpp Outdated Show resolved Hide resolved
@pierrebai-adsk
Copy link
Collaborator Author

@rijpkeh I don't know enough about component tags to write unit tests that would read and write them. Could you provide an example of a Python script that would exercise this code so that I could write a unit test in the MayaUSD project?

@ppt-adsk ppt-adsk added the do-not-merge-yet Development is not finished, PR not ready for merge label Oct 19, 2021
@rijpkeh
Copy link
Contributor

rijpkeh commented Oct 19, 2021 via email

- Add exportComponentTags export option in the command and UI.
- Removed useless call to base file translator writer.
- Added a unit test that verifies comonent tags survive USD export / import.
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.

Left some comments I would like you/Hans to answer/address prior to approval.

@@ -1369,4 +1372,53 @@ bool UsdMayaMeshWriteUtils::getMeshColorSetData(
return true;
}

MStatus UsdMayaMeshWriteUtils::exportComponentTags(UsdGeomMesh& primSchema, MObject obj)

Choose a reason for hiding this comment

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

Why to even make this method available for older versions of Maya?

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 wanted to change Hans code as little as possible, but I can move the #ifdef out and #ifdef atthe caller site too instead of having a do-nothing function in older versions of Maya.

TfToken componentTagFamilyName("componentTag");
MFnGeometryData fnGeomData(geomObj);
MStringArray keys;
status = fnGeomData.componentTags(keys);

Choose a reason for hiding this comment

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

This status doesn't seem to be checked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, and it is not the only one, there are others a bit below. In all cases, the code seems to rely on the returned data (for example the set of keys) to be empty. Of course, that could cause silent failures. I'll add more early returns.

@@ -882,4 +885,71 @@ MStatus UsdMayaMeshReadUtils::assignSubDivTagsToMesh(
return MS::kSuccess;
}

MStatus UsdMayaMeshReadUtils::createComponentTags(const UsdGeomMesh& mesh, const MObject& meshObj)

Choose a reason for hiding this comment

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

Why to make this method available in older versions of Maya?

@@ -226,6 +226,7 @@ global proc int mayaUsdTranslatorExport (string $parent,
string $eulerFilterAnn = "Exports the euler angle filtering that was performed in Maya.";
string $staticSingleSampleAnn = "Converts animated values with a single time sample to be static instead.";
string $visibilityAnn = "Exports Maya visibility attributes as USD metadata.";
string $componentTagsAnn = "Exports Maya component tags as USD metadata.";

Choose a reason for hiding this comment

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

Did you run this by design? FWIW. We are writing out component tags as UsdGeomSubset, not as metadata.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, you're right I did not ask design.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the new UI.

- Move ifdef for Maya 2022 outdide of the affected functions and their callers.
- Remove the export component tag UI that was added with designer input.
@pierrebai-adsk pierrebai-adsk added ready-for-merge Development process is finished, PR is ready for merge and removed do-not-merge-yet Development is not finished, PR not ready for merge labels Oct 27, 2021
@kxl-adsk kxl-adsk merged commit 211287f into dev Oct 27, 2021
@kxl-adsk kxl-adsk deleted the rijpkeh/MAYA-113077/component_tags_io branch October 27, 2021 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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