(USDU-9) Left handed orientation not handled correctly

[vkovec]

Purpose of this PR

Ticket/Jira #: USDU-9

Edit: After investigation this is an error with this specific USD file and not the import code. Changes reverted and added a unit test

If a mesh is lefthanded orientation in the USD file, then it will be partially converted despite being lefthanded already. Fixed to check if mesh is lefthanded earlier and convert accordingly.

Before fix (compared to FBX version exported from Maya, on right):

After fix (compared to FBX version exported from Maya, on right):

Testing

Functional Testing status:

Added an automated test to compare left vs right handed import.

Manual Test:

1. Create a left handed mesh in USD (or use the one linked to the issue)
2. In Unity, run USD > Import as GameObjects
3. In the Unity Asset, under "Advanced", change the "Change Handedness" settings, refresh and check that result is as expected.

Performance Testing status:

Should not affect performance.

Overall Product Risks

Complexity:

Low

Halo Effect:

Low

Additional information

Note to reviewers:

Reminder:

• Add entry in CHANGELOG.md (if applicable)

[judubu]

Damn, I looked at this code so many times before, and still I missed it!
Good job.

[vkovec]

there is probably a better way to test this, adding this for now to check that the same cube is handled differently whether it is right or left handed.

[jcowles]

This should also fix issue #76 which has a repro.

At first glance what you have seems correct, but I want to read over it more carefully later today

This is awesome!

[jcowles]

This is awesome, thanks for the fix!

[thomas-tu]

Is it enough to compare the data of two import results? Shouldn't we also test the generated mesh against a baked version to ensure the result we get is correct?

[jcowles]

This is actually a really good point -- especially with a symmetrical mesh like a Cube, you might even be getting identical results, but the floating point error alone might make them look different.

Does Is.Not.EqualTo use a fuzzy/epsilon comparison? If so, is the epsilon small enough to work here?

[jcowles]

I do like NOT using a baked/golden result though, because it makes the test more robust to other changes and avoids needing to ratchet a baseline.

[vkovec]

It should call Equals(), however, looking at the Vector3 documentation it looks like == is safer than Equals for approximate comparison, so I'll change this to be more explicit.

[jcowles]

Nice, LGTM!

[jcowles]

I approved this, leaving it up to you how to handle the conversation about testing

[jcowles]

I just noticed, no copyright disclaimer was added to this file

[jcowles]

Thanks for incorporating all the feedback on this one!
And nice work :)

[cguer]

I'm always a bit confused about what the "truth" is when reviewing handedness changes but I noticed one possibly weird behavior testing the assets from the case.

Note: The images below are using the FBX importer in 2019.4, NOT with the new Bake Axis Conversion in 2020.1+. (should I?)

USD version 1.0.3 (last release)
When the Change Handedness parameter is set to Slow And Safe As FBX, the USD geometry matches exactly the FBX geometry

USD version 1.0.4 (current PR)
There appears to be no difference between Slow And Safe As FBX and Slow And Safe.

Is this the expected behavior?

[vkovec]

Did some testing with the following software:

• usdview
• Maya
• Cinema4D (same axis system as Unity)

Cinema4D and Maya both seem to export right handed USD files.

I had a hard time finding software to generate left handed USD files, so instead tried making a right handed version of the original hull brush file.
To generate the right handed version, I simply edited all the orientation values in the file to "rightHanded".
Then I did some tests to see how it would be imported compared to the leftHanded version in different software.

All three software listed above imported both the left handed and right handed version in the same way.

In the Maya USD importer, it appears that the orientation is only considered if an object is not doubleSided.
Therefore, I set doubleSided = 0 in both right and left handed files. This time they imported slightly different in usdview and Maya.
(right handed file on left, left handed file on right)

In Unity, the right handed version matches the FBX version. See the following two images:

In the below images, the following 4 files were imported into a scene:

1. an FBX version of the model created by importing the USD into Maya, then exporting as FBX, imported with bake axis conversion disabled.
2. the original left handed USD. Imported with "Slow and Safe" conversion.
3. the original USD, edited to change all instances of uniform token orientation = "leftHanded" to uniform token orientation = "rightHanded". Imported with "Slow and Safe" conversion.
4. Same as 3) but imported with "Slow and Safe as FBX" conversion.

With USD package using fix from this PR:

with USD package version 1.0.3 from package manager:

• Note: the FBX imported with bake axis conversion enabled matches 3) in the images.

Based on this information, I may have misunderstood left vs right handed orientation in USD, and the issue seems to be with the file rather than the importer. The file appears to actually be a right handed file.

[jcowles]

Perhaps this is more helpful, here is a simple USD file with a right and left handed quad:

#usda 1.0

def Xform "World" {

    def Camera "MainCam" {
        double3 xformOp:translate = (0, 0, 10)
        uniform token[] xformOpOrder = [ "xformOp:translate" ] 
        float2 clippingRange = (1, 20)
    }

    def Mesh "RightHanded" {
        token orientation = "rightHanded"
        int[] faceVertexCounts = [4]
        int[] faceVertexIndices = [0, 1, 2, 3]
        point3f[] points = [(0,0,0),
                            (1,0,0),
                            (1,1,0),
                            (0,1,0)]
    }

    def Mesh "LeftHanded" {
        token orientation = "leftHanded"
        int[] faceVertexCounts = [4]
        int[] faceVertexIndices = [0, 3, 2, 1 ]
        point3f[] points = [(-2,0,0),
                            (-1,0,0),
                            (-1,1,0),
                            (-2,1,0)]
    }
}

And this is what it looks like in usdview:

And through the camera:

[jcowles]

In the current version of the USD Unity SDK, I get:

Which looks correct to me.

I also noticed that the latest revision of this change is effectively a no-op now, right? There is no functional change from stable.

I think Houdini will export left-handed USD files, IIRC.

[jcowles]

So maybe there was never a bug? facepalm

[vkovec]

I agree it looks like this was never actually a bug. Here is another example with a left handed example file from Houdini:

In Houdini:

In usdview:

In Maya (not sure why it imports black, otherwise looks correct):

In Unity with USD 1.0.3 (package manager):

In Unity with this branch:

In the last one the normals look off in some areas, in addition to facing the wrong way.

[judubu]

So it seems the conclusion is there never was a bug, it's the tilt brush asset that was wrong. Sorry @vkovec for the time wasted, and thanks to everybody that spent time looking into the issue.
If nobody comment I'll close the ticket.

[jcowles]

It might be nice to keep the Unit test from this PR, but revert the functional changes.

[vkovec]

@judubu @jcowles sorry for the confusion, I have reverted the changes and fixed up the unit tests to match the correct functionality.

[judubu]

Awesome. Thanks @vkovec Ill merge as soon as Yamato turns green.