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

[usdMaya] add filterTypes option to avoid exporting certain node types #475

Closed

Conversation

pmolodo
Copy link
Contributor

@pmolodo pmolodo commented Apr 25, 2018

Description of Change(s)

Adds a new flag, "filterTypes", which allows you to avoid exporting nodes of certain maya node types.
This is handy for, ie, constraints, which are exported as transforms, but generally have no xform data of their own. Particularly nice if you use mergeTransformAndShape, and want to avoid having your camera renamed just because it has a constraint on it. (I would like to set filterTypes="constraint" by default, but this would technically break backwards compatibility. It seems unlikely anyone is relying on exporting the constraint nodes themselves, since all their useful data is NOT exported, but I suppose anything is possible...)
Likely also useful for other scenarios, as well (ie, maybe you don't want to export cameras, or place3dTexture nodes, etc)

Fixes Issue(s)

  • If you have mergeTransformAndShape on, adding a constraint to a camera will change the USD camera node's name - ie, from "/persp" to "/persp/perspShape"; setting filterTypes="constraint" will avoid this

@jtran56
Copy link

jtran56 commented May 7, 2018

Filed as internal issue #160429.

Copy link
Contributor

@sunyab sunyab left a comment

Choose a reason for hiding this comment

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

Passing along notes from our sets team, primarily minor style and doc changes. Thanks!

// Unfortunately, the returned list will often include weird garbage, like
// "THconstraint" for "constraint", which cannot be converted to a MNodeClass,
// so just ignore these...
// MGlobal::displayError(MString("Given inherited excluded node type '") + inheritedTypes[i]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is keeping this commented-out displayError call intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was - I liked that it was sort of descriptive of what was going on, but it's not really necessary. I'll get rid of it.

@@ -168,12 +171,30 @@ struct JobExportArgs
PXRUSDMAYA_API
static const VtDictionary& GetDefaultDictionary();

PXRUSDMAYA_API
Copy link
Contributor

Choose a reason for hiding this comment

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

For style consistency with the rest of the API, could these be "AddFilteredTypeName", "GetFilteredTypeIds", and "ClearFilteredTypeIds"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, could we add docs to AddFilteredTypeName about the inherited type behavior, e.g:

/// Adds type name to filter out during export. This will also add all 
/// inherited types (so if you exclude "constraint", it will also exclude 
/// "parentConstraint")


// Maya type ids to avoid exporting; these are
// EXACT types, though the only exposed way to modify this,
// addFilteredTypeName, will also add all inherited types
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to fix up this reference to addFilteredTypeName to AddFilteredTypeName per above

// addFilteredTypeName, will also add all inherited types
// (so if you exclude "constraint", it will also exclude
// "parentConstraint")
std::set<unsigned int> filteredTypeIds;
Copy link
Contributor

Choose a reason for hiding this comment

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

Our coding style uses "_" as a prefix for private members, so could we rename this to _filteredTypeIds?

@@ -142,6 +142,11 @@ global proc int usdTranslatorExport (string $parent,
checkBox -l "Export Visibility" exportVisibilityCheckBox;

checkBox -l "Renderable Only" renderableOnlyCheckBox;

textFieldGrp -l "Filtered Node Types"
-annotation ("Do not filter nodes of this type, or their children;\n" +
Copy link
Contributor

Choose a reason for hiding this comment

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

This annotation seems incorrect, should the description be something like "Filter out nodes of this type and their children" or "Do not import nodes of this type or their children"?

@sunyab sunyab added the blocked Issue fix or pull request blocked until questions are answered or pending notes are addressed label Jun 19, 2018
@pmolodo
Copy link
Contributor Author

pmolodo commented Jun 20, 2018

Pushed updated commit addressing code review notes.

Incremental changes are here:
LumaPictures@c9e8d68

@sunyab sunyab added pending push and removed blocked Issue fix or pull request blocked until questions are answered or pending notes are addressed labels Jun 24, 2018
@sirpalee
Copy link
Contributor

Updated the PR to make it work with the latest usdMaya changes.

pixar-oss added a commit that referenced this pull request Jun 26, 2018
[usdMaya] add filterTypes option to avoid exporting certain node types

(Internal change: 1867883)
@sunyab
Copy link
Contributor

sunyab commented Jun 26, 2018

Merged this PR, closing!

@sunyab sunyab closed this Jun 26, 2018
@pmolodo pmolodo deleted the pr/filterTypes branch March 12, 2019 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants