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

maintain UV connectivity across export and re-import round trips #1175

Conversation

mattyjams
Copy link
Contributor

Some of our artists have discovered that the re-indexing and compression that we're doing when we export UV sets is actually messing with the connectivity information that Maya computes from the values and indices arrays. The result is that when they export Maya meshes to USD and then re-import them back into Maya, they're ending up with disconnected and hard to work with UV sets.

With the changes here, we now translate the Maya mesh's UV values and assignment indices more or less directly, and we no longer apply any compression. This offers the best chance of maintaining the UV set's connectivity across round-trips. The net result in USD is the same (i.e. the array of UVs produced by doing primvar.ComputeFlattened() is the same as before), but the array values for the UV set primvar and its accompanying indices attribute may be different.

The real meat of the changes are in 82ae536 while the other commits are from clean-up, simplification, and testing.

We'll no longer be applying any compression to UV sets when exporting from Maya
to USD.
…ets test

Exporting from Maya will no longer compress UV sets, so the mesh name was
tweaked slightly and a comment was added to clarify that Meshes with compressed
UV sets would have to come from somewhere else.
Previously when exporting UV sets to USD, we used the face vertex ordering
imposed by MItMeshFaceVertex to effectively re-arrange all of the Maya mesh's
UVs into face vertex order. We then further de-duplicated the UV values and
compressed the indices array to a coarser interpolation if possible. All of
this manipulation effectively destroyed any UV shell and connectivity
information that was encoded in the Maya indexing, resulting in UV sets that
were disconnected and difficult to work with if a Maya mesh was round-tripped
out to USD and back in.

Instead, we now translate the Maya mesh's UV values and assignment indices more
or less directly, and we no longer apply any compression. This offers the best
chance of maintaining the UV set's connectivity across round-trips. The net
result in USD is the same (i.e. the array of UVs produced by doing
primvar.ComputeFlattened() is the same as before), but the array values for the
UV set primvar and its accompanying indices attribute may be different.
…s are of minimal length

Although the UV sets on the "SharedFacesCubeShape" are stored in the Maya scene
with a minimal number of UV values and UV shells, they seem to be expanded when
the file is opened such that we end up with a UV value per face vertex rather
than smaller arrays of UV values with only the indices being per face vertex.
As a result, we have to reassign the UVs just before we export.
@mattyjams mattyjams added core Related to core library import-export Related to Import and/or Export labels Feb 12, 2021
Comment on lines -439 to +358
setAttr -s 23 ".uvst[1].uvsp[0:22]" -type "float2" 0 0 0.5 0 0.5 0.5
0 0.5 0.5 0.5 1 0.5 1 1 0.5 1 0 0 0.5 0 0.5 0.5 0 0.5 0.5 0.5 1 0.5 1 1 0.5 1 0 0
0.5 0 0.5 0.5 0 0.5 0.5 0.5 1 0.5 1 1;
setAttr -s 7 ".uvst[1].uvsp[0:6]" -type "float2" 0 0 0.5 0 0.5 0.5
0 0.5 1 0.5 1 1 0.5 1;
setAttr ".uvst[2].uvsn" -type "string" "AllFacesSharedSet";
setAttr -s 24 ".uvst[2].uvsp[0:23]" -type "float2" 0 0 1 0 1 1 0 1 0
0 1 0 1 1 0 1 0 0 1 0 1 1 0 1 0 0 1 0 1 1 0 1 0 0 1 0 1 1 0 1 0 0 1 0 1 1 0 1;
setAttr -s 4 ".uvst[2].uvsp[0:3]" -type "float2" 0 0 1 0 1 1 0 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would expect that these changes and the indexing changes just below would result in shorter arrays of UV values, but that doesn't seem to be the case. See more in testUsdExportUVSets.py below.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What I see from experimenting in Maya is that there is one mu row per UV assigned to the face. The first number in a mu row being the UV set index. You might want to edit in Maya, freeze the edits, and then replicate the deleted rows you see in the edited file.

Comment on lines +90 to +95
# XXX: Although the UV sets on the "SharedFacesCubeShape" are stored in
# the Maya scene with a minimal number of UV values and UV shells, they
# seem to be expanded when the file is opened such that we end up with
# a UV value per face vertex rather than these smaller arrays of UV
# values with only the indices being per face vertex. As a result, we
# have to reassign the UVs just before we export.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this ring any bells with anyone at Autodesk?

I attempted to author the UVs in the way I'd like to see them exported in 25e721b, but found that I needed to explicitly re-author them after opening the file in the test.

If you were to open the Maya scene file, run the clear(), setUVs(), and assignUVs() commands below and then save the file, there is no change to the mesh in the Maya scene file.

Any thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be a missing freeze history. Are you seeing extra operators added to the mesh construction history in the save file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @JGamache-autodesk. There were some polyMapDel nodes in the Maya scene file, but those applied to the OneMissingFaceCubeShape and OneAssignedFaceCubeShape. I deleted all history in the scene and that cleaned up those shapes, so I pushed that in b94de4f, but it unfortunately didn't seem to affect SharedFacesCubeShape, so I couldn't take this workaround out of the test script.

Comment on lines -377 to +367
self.assertEqual(stPrimvar[0], Gf.Vec2f(1.0, 1.0))
self.assertEqual(stPrimvar[0], Gf.Vec2f(1.0, 2.0))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This UV set was previously getting compressed to vertex interpolation. With the compression now removed, it remains as faceVarying, and the first UV in the flattened array is now a different value.

Copy link
Collaborator

@JGamache-autodesk JGamache-autodesk left a comment

Choose a reason for hiding this comment

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

Code is good. Might want to revisit the Maya file used in the unit test to see if the contents correspond to what Maya would save for partial UV sets.

@kxl-adsk kxl-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Feb 18, 2021
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.

Please fix the compilation error with older versions of USD (20.02)

@kxl-adsk kxl-adsk removed the ready-for-merge Development process is finished, PR is ready for merge label Feb 19, 2021
…ve VtArray::emplace_back()

VtArray did not gain emplace_back() until USD 20.11, and before that it's
push_back() took a const reference parameter.
@kxl-adsk kxl-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Feb 19, 2021
@kxl-adsk
Copy link

@mattyjams Thank you. PF failures are unrelated to the change, this is now ready for merge.

@kxl-adsk kxl-adsk merged commit 419022f into Autodesk:dev Feb 19, 2021
@mattyjams mattyjams deleted the pr/maintain_UV_connectivity_across_export_and_reimport_round_trips branch February 19, 2021 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core library 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.

4 participants