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-106486 Handle UV linkage on material export #876

Merged
merged 3 commits into from
Nov 4, 2020

Conversation

JGamache-autodesk
Copy link
Collaborator

@JGamache-autodesk JGamache-autodesk commented Oct 30, 2020

We now explore the UV linkage and make sure the UV stream name in the material matches the UV set name on the shape.
We will create as many material specializations as needed to match the shape streams in case of UV stream names inconsistency in a group of shapes sharing a single material.
We no longer automatically rename "map1" to "st" since the UV streams in the material will now be correctly set. You can use the following pair of environment variables to bring back the renaming behaviour: MAYAUSD_EXPORT_MAP1_AS_PRIMARY_UV_SET and MAYAUSD_IMPORT_PRIMARY_UV_SET_AS_MAP1.

We now explore the UV linkage and make sure the UV stream name in the material matches the UV set name on the shape.
We will create as many material specializations as needed to match the shape streams in case of UV stream names inconsistency in a group of shapes sharing a single material.
We no longer automatically rename "map1" to "st" since the UV streams in the material will now be correctly set.
UsdPrim materialPrim = MakeStandardMaterialPrim(assignmentsToBind, name);
BindStandardMaterialPrim(materialPrim, assignmentsToBind, boundPrimPaths);
return materialPrim;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This function is split in two parts since the useRegistry exporter now wants to have the material exported before the binding is done.


UsdPrim materialPrim = material.GetPrim();

// could use this to determine where we want to export.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The binding code exported in a separate function.

const VtIntArray& faceIndices = iter.faceIndices;
const TfToken& shapeName = iter.shapeName;

const UsdShadeMaterial& materialToBind = uvMappingManager.getMaterial(shapeName);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Main difference in the extracted code for binding materials is here. We ask the mapping manager for the material to bind. In case of inconsistent texcoord naming in the shape set, this function can return specialized materials that fit the needs of the current shape.

uvSetName = UsdMayaMeshPrimvarTokens->DefaultMayaTexcoordName.GetText();
createUVSet = false;
} else if (!hasDefaultUVSet) {
if (!hasDefaultUVSet) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The removes the automatic "st"-> "map1" renaming on import. Should I create an env var for shops that have internal tools that depend on this behavior?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If possible, that would be great. Would allow transitioning over.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please! Export is more of a concern for us than import (more detail on that below), but it would be good to have a TfEnvSetting we could set for each direction.

// The UV Set "map1" is renamed st. This is a Pixar/USD convention.
TfToken setName(uvSetNames[i].asChar());
if (setName == UsdMayaMeshPrimvarTokens->DefaultMayaTexcoordName.GetText()) {
setName = UsdUtilsGetPrimaryUVSetName();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The removes the automatic "map1"-> "st" renaming on export. Should I create an env var for shops that have internal tools that depend on this behavior?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, definitely. Having the Maya USD export portion of our asset builds suddenly writing map1 primvars instead of st primvars would be a non-trivial pipeline change for us, so at least for now, we'll need to be able to opt out of that.

Including a setting for import, maybe a pair of TfEnvSettings like this?:

TF_DEFINE_ENV_SETTING(
    MAYAUSD_IMPORT_PRIMARY_UV_SET_AS_MAP1,
    false,
    "Translates the primary UV set in USD to the map1 UV set in Maya. "
    "When disabled, UV set names are translated directly (st in USD becomes st in Maya).");

TF_DEFINE_ENV_SETTING(
    MAYAUSD_EXPORT_MAP1_AS_PRIMARY_UV_SET,
    false,
    "Translates the map1 UV set in Maya to the primary UV set in USD. "
    "When disabled, UV set names are translated directly (map1 in Maya becomes map1 in USD).");

Thanks! :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes agreed, having the env variable to make a switch would be useful. Thanks

setups results in USD data with material specializations:
'''
expected = [
("/pPlane1", "/blinn1SG_map1_map1_map1", "map1", "map1", "map1"),
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 longer renamed to st

Copy link
Contributor

@mattyjams mattyjams left a comment

Choose a reason for hiding this comment

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

We will indeed need TfEnvSettings to be able to fallback to the map1->st translation, particularly for export. But otherwise, looks good to me!

Thanks @JGamache-autodesk!

lib/mayaUsd/fileio/shading/shadingModeExporterContext.cpp Outdated Show resolved Hide resolved
lib/mayaUsd/fileio/shading/shadingModeExporterContext.cpp Outdated Show resolved Hide resolved
lib/mayaUsd/fileio/shading/shadingModeExporterContext.h Outdated Show resolved Hide resolved
uvSetName = UsdMayaMeshPrimvarTokens->DefaultMayaTexcoordName.GetText();
createUVSet = false;
} else if (!hasDefaultUVSet) {
if (!hasDefaultUVSet) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please! Export is more of a concern for us than import (more detail on that below), but it would be good to have a TfEnvSetting we could set for each direction.

// The UV Set "map1" is renamed st. This is a Pixar/USD convention.
TfToken setName(uvSetNames[i].asChar());
if (setName == UsdMayaMeshPrimvarTokens->DefaultMayaTexcoordName.GetText()) {
setName = UsdUtilsGetPrimaryUVSetName();
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, definitely. Having the Maya USD export portion of our asset builds suddenly writing map1 primvars instead of st primvars would be a non-trivial pipeline change for us, so at least for now, we'll need to be able to opt out of that.

Including a setting for import, maybe a pair of TfEnvSettings like this?:

TF_DEFINE_ENV_SETTING(
    MAYAUSD_IMPORT_PRIMARY_UV_SET_AS_MAP1,
    false,
    "Translates the primary UV set in USD to the map1 UV set in Maya. "
    "When disabled, UV set names are translated directly (st in USD becomes st in Maya).");

TF_DEFINE_ENV_SETTING(
    MAYAUSD_EXPORT_MAP1_AS_PRIMARY_UV_SET,
    false,
    "Translates the map1 UV set in Maya to the primary UV set in USD. "
    "When disabled, UV set names are translated directly (map1 in Maya becomes map1 in USD).");

Thanks! :)

@JGamache-autodesk JGamache-autodesk added the ready-for-merge Development process is finished, PR is ready for merge label Nov 3, 2020
Copy link
Contributor

@HamedSabri-adsk HamedSabri-adsk left a comment

Choose a reason for hiding this comment

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

Looking good! Thanks.

@kxl-adsk kxl-adsk merged commit 6c27f0a into dev Nov 4, 2020
@kxl-adsk kxl-adsk deleted the t_gamaj/MAYA-106486/uv_linkage_export branch November 4, 2020 00:51
@mattyjams
Copy link
Contributor

Just wanted to flag here that after bringing in this change, we're seeing an issue where exported materials specialize themselves. There isn't any UV set re-mapping happening in these cases so I'm not sure what's going on yet, but there looks to be some fallout to this that needs to be addressed.

@JGamache-autodesk
Copy link
Collaborator Author

JGamache-autodesk commented Nov 4, 2020

Might be missing an important early return in:

const UsdShadeMaterial& getMaterial(const TfToken& shapeName)
{
    if (_uvNamesToMaterial.empty()) {
        return _material;
    }

But I can not reproduce the issue. Can you send just one erroneous specialization (as text) so I can have a starting point?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
import-export Related to Import and/or Export 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.

6 participants