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.

Sign up for GitHub

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-128503 - Improve roundtripping of GeomSubsets #3135

Merged
merged 6 commits into from
Jun 6, 2023
Merged

MAYA-128503 - Improve roundtripping of GeomSubsets #3135

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1bc228d
MAYA-128503 - Improve roundtripping of GeomSubsets
JGamache-autodesk Jun 2, 2023
0b62059
Merge branch 'dev' into gamaj/MAYA-128503/improve_subset_roundtripping
JGamache-autodesk Jun 2, 2023
6a22de1
MAYA-128503 - Responding to review comments
JGamache-autodesk Jun 2, 2023
9f83051
MAYA-128503 - Error reporting rework
JGamache-autodesk Jun 5, 2023
e2764fd
Merge remote-tracking branch 'github/dev' into gamaj/MAYA-128503/impr…
JGamache-autodesk Jun 5, 2023
74dc1b1
Fix formatting
JGamache-autodesk Jun 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/mayaUsd/fileio/jobs/meshDataReadJob.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,11 @@ void getComponentTags(MFnMeshData& dataCreator, const UsdGeomMesh& mesh)
{
#if MAYA_API_VERSION >= 20220000

// Unclear at this time if this mesh requires roundtripping info
// for the component tags.
std::vector<UsdMayaMeshReadUtils::ComponentTagData> componentTags;
JsValue meshRoundtrippingData;
UsdMayaMeshReadUtils::getComponentTags(mesh, componentTags, meshRoundtrippingData);
JsValue unused;
UsdMayaMeshReadUtils::getComponentTags(mesh, componentTags, unused);

for (auto& tag : componentTags) {
auto& name = tag.first;
Expand Down
14 changes: 12 additions & 2 deletions lib/mayaUsd/fileio/shading/shadingModeExporterContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,8 @@ VtIntArray _AssignComponentTags(
const UsdShadeMaterial& materialToBind,
const MObjectHandle& geomHandle,
const std::vector<UsdGeomSubset>& currentGeomSubsets,
const VtIntArray& faceIndices)
const VtIntArray& faceIndices,
SdfPathSet* const boundPrimPaths)
{
if (currentGeomSubsets.empty() || faceIndices.empty() || !geomHandle.isValid()) {
return faceIndices;
Expand Down Expand Up @@ -780,6 +781,10 @@ VtIntArray _AssignComponentTags(
subsetBindingAPI.Bind(materialToBind);
}
JGamache-autodesk marked this conversation as resolved.
Show resolved Hide resolved

if (boundPrimPaths) {
boundPrimPaths->insert(geomSubset.GetPath());
}

// Mark those faces as handled
handledSet.insert(subsetIndices.cbegin(), subsetIndices.cend());
}
Expand Down Expand Up @@ -860,7 +865,12 @@ void UsdMayaShadingModeExportContext::BindStandardMaterialPrim(

const auto currentGeomSubsets = bindingAPI.GetMaterialBindSubsets();
VtIntArray unhandledFaceIndices = _AssignComponentTags(
this, materialToBind, iter.shapeObj, currentGeomSubsets, faceIndices);
this,
materialToBind,
iter.shapeObj,
currentGeomSubsets,
faceIndices,
boundPrimPaths);

// If we assigned all faces to component tags, then we are done for this
// material.
Expand Down
6 changes: 6 additions & 0 deletions lib/mayaUsd/fileio/translators/translatorMaterial.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,12 @@ bool UsdMayaTranslatorMaterial::AssignMaterial(
// identifier is the UUID.
subsetRoundtrippingInfo[UsdMayaGeomSubsetTokens->MaterialUuidKey]
= JsValue(depNodeFn.uuid().asString().asChar());
} else {
TF_RUNTIME_ERROR(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be necessary to also have an error for the iterator check above? IDK what are the semantics for the find to fail.

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, I will revisit the error handling today as this seem a bit fragile.

"Unable to resolve path <%s> on imported GeomSubset <%s> to a shading "
"group.",
materialPath.GetText(),
ssInfoIt.first.c_str());
}
subsetRoundtrippingInfo.erase(materialIt);
updatedInfo[ssInfoIt.first] = subsetRoundtrippingInfo;
Expand Down
4 changes: 3 additions & 1 deletion lib/mayaUsd/fileio/utils/meshReadUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,8 @@ MStatus UsdMayaMeshReadUtils::getComponentTags(
JsObject allRoundtripData;

for (auto& ss : subsets) {
// GeomSubsets that are created to represent faces with a material assigned are marked as
// Maya generated and should not be merged into the mesh componentTags.
if (UsdMayaRoundTripUtil::IsPrimMayaGenerated(ss.GetPrim())) {
JGamache-autodesk marked this conversation as resolved.
Show resolved Hide resolved
continue;
}
Expand Down Expand Up @@ -985,7 +987,7 @@ MStatus UsdMayaMeshReadUtils::getComponentTags(
= JsValue(familyName.GetString());
}

// Could be any applicable API, but we mostly care about materials:
// We are interested in materials bound to faces.
if (ss.GetPrim().HasAPI<UsdShadeMaterialBindingAPI>()) {
const auto ssBindingAPI = UsdShadeMaterialBindingAPI(ss.GetPrim());
const auto boundMaterial = ssBindingAPI.ComputeBoundMaterial();
Expand Down
55 changes: 42 additions & 13 deletions test/lib/usd/translators/testUsdExportImportRoundtripPreviewSurface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,8 @@ def testUVReaderMerging(self):
(connect_api, out_name, _) = src_input.GetConnectedSource()
self.assertEqual(connect_api.GetPath(), mat_path + dst_name)

def testGeomSubsetRoundtrip(self):
"""Test that GeomSubsets numbers stay under control when rountripping"""
def CommonGeomSubsetRoundtrip(self, useCollections):
"""Common test for geom subset export"""
cmds.file(f=True, new=True)

# Take a cube since it already has 6 subsets defined:
Expand All @@ -467,11 +467,19 @@ def createMaterial(name, R, G, B, subset):
createMaterial("blueFace", .0, .0, .95, ".f[4]")

# Export:
usd_path = os.path.abspath('CubeWithAssignedFaces.usda')
cmds.usdExport(mergeTransformAndShape=True,
file=usd_path,
shadingMode='useRegistry',
exportDisplayColor=False)
if useCollections:
usd_path = os.path.abspath('CubeWithAssignedFaces_CB.usda')
cmds.usdExport(mergeTransformAndShape=True,
file=usd_path,
shadingMode='useRegistry',
exportDisplayColor=False,
exportCollectionBasedBindings=True)
else:
usd_path = os.path.abspath('CubeWithAssignedFaces.usda')
cmds.usdExport(mergeTransformAndShape=True,
file=usd_path,
shadingMode='useRegistry',
exportDisplayColor=False)

stage = Usd.Stage.Open(usd_path)

Expand Down Expand Up @@ -575,7 +583,10 @@ def ValidateUSDStage(self, stage, allSubsets):
geomSubset.GetIndicesAttr().Set([3,])

# Save the modified stage under a new name:
modified_usd_path = os.path.abspath('CubeWithModifiedFaces.usda')
if useCollections:
modified_usd_path = os.path.abspath('CubeWithModifiedFaces_CB.usda')
else:
modified_usd_path = os.path.abspath('CubeWithModifiedFaces.usda')
stage.GetRootLayer().Export(modified_usd_path)

# Update our expected results:
Expand Down Expand Up @@ -630,17 +641,35 @@ def ValidateUSDStage(self, stage, allSubsets):

# We export without changes to make sure we get back the modified cube without
# any extra subsets.
reexported_usd_path = os.path.abspath('CubeWithAssignedFaces_reexported.usda')
cmds.usdExport(mergeTransformAndShape=True,
file=reexported_usd_path,
shadingMode='useRegistry',
exportDisplayColor=False)
if useCollections:
reexported_usd_path = os.path.abspath('CubeWithAssignedFaces_reexported_CB.usda')
cmds.usdExport(mergeTransformAndShape=True,
file=reexported_usd_path,
shadingMode='useRegistry',
exportDisplayColor=False,
exportCollectionBasedBindings=True)
else:
reexported_usd_path = os.path.abspath('CubeWithAssignedFaces_reexported.usda')
cmds.usdExport(mergeTransformAndShape=True,
file=reexported_usd_path,
shadingMode='useRegistry',
exportDisplayColor=False)

stage = Usd.Stage.Open(reexported_usd_path)

# Test that the re-exported stage has the exact same GeomsSubsets as the one we imported:
ValidateUSDStage(self, stage, allSubsets)

def testGeomSubsetRoundtrip(self):
"""Test that GeomSubsets numbers stay under control when rountripping
with direct material assignment"""
self.CommonGeomSubsetRoundtrip(False)

def testGeomSubsetRoundtripCollectionBased(self):
"""Test that GeomSubsets numbers stay under control when rountripping
with collection based material assignemnt"""
self.CommonGeomSubsetRoundtrip(True)
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 way the material checks are done with ComputeBoundMaterial work with both assignment methods. The contents of the USDA files will be slightly different since one will use collections based assignment.


@unittest.skipUnless("mayaUtils" in globals() and mayaUtils.mayaMajorVersion() >= 2020, 'Requires standardSurface node which appeared in 2020.')
def testOpacityRoundtrip(self):
"""
Expand Down