Skip to content

Conversation

@johnhaddon
Copy link
Member

  • primvars:normals takes precedence over UsdGeomPointBased normals when reading.
  • Writing always creates primvars:normals, because it can round-trip indexed normals while UsdGeomPointBased normals can not.

Note that I haven't updated the code for loading skinned normals. I don't have test data for this, and USD doesn't expose skinning of facevarying or indexed primvars in UsdSkelSkinningQuery, so fixing that is a bigger task.

- `primvars:normals` takes precedence over UsdGeomPointBased `normals` when reading.
- Writing always creates `primvars:normals`, because it can round-trip indexed normals while UsdGeomPointBased `normals` can not.

Note that I haven't updated the code for loading skinned normals. I don't have test data for this, and USD doesn't expose skinning of facevarying or indexed primvars in UsdSkelSkinningQuery, so fixing that is a bigger task.
@johnhaddon johnhaddon self-assigned this Mar 27, 2023
@danieldresser-ie
Copy link
Contributor

LGTM.

Would be nice to fix the test tolerance that flagged this check as failed, I've noticed this one a few times on my own PRs as well:

FAIL: testCancel (AlembicSceneTest.AlembicSceneTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/cortex/cortex/contrib/IECoreAlembic/test/IECoreAlembic/AlembicSceneTest.py", line 1937, in testCancel
    self.assertLess( time.time() - startTime, 0.06 )
AssertionError: 0.06280231475830078 not less than 0.06```

@johnhaddon johnhaddon merged commit 81339b8 into ImageEngine:RB-10.4 Mar 28, 2023
@johnhaddon
Copy link
Member Author

Would be nice to fix the test tolerance that flagged this check as failed

PR welcome :) I think we can just skip that test for debug builds - we can't expect time-sensitive stuff to behave there.

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.

2 participants