Added file format attribute to LayerManager and improved format detection for serialised layers #1792
Conversation
29976fe to
872e1c9
Compare
|
Hi @kxl-adsk, just saw a missing Clang format blocked the preflight builds. It's been formatted now if you're happy to rerun the CI. |
| _, rootLayerPath = tempfile.mkstemp(suffix=".usda") | ||
| _, tmpMayafilePath = tempfile.mkstemp(suffix=".ma") |
There was a problem hiding this comment.
We have moved away from using temp files, because:
- it's harder to debug
- required periodic temp folder cleanup
For this reason, we use explicit names and local to the test area within the build folder. Generated files are easy to discover and the build folder is more often cleared than the temp folder.
Here is an example - https://github.com/Autodesk/maya-usd/blob/dev/test/lib/usd/translators/testUsdExportMaterialX.py#L43
There was a problem hiding this comment.
Hi @kxl-adsk, looks like PF checks are now passing, and I've updated the test to avoid tempfiles (we'll keep this in mind going forward). Let me know if there's anything else needed.
| import unittest | ||
|
|
||
| from maya import cmds | ||
| from AL.usdmaya import ProxyShape | ||
|
|
||
| import fixturesUtils |
There was a problem hiding this comment.
We use fixturesUtils all over the place. What was the reason to remove it from AL tests?
There was a problem hiding this comment.
Internally we have not set up our testing to take advantage of the module, however that transition is underway and a PR will be up for it at some point. I'll also be removing tempfile usage across AL tests.
There was a problem hiding this comment.
That’s great to hear. Looking forward to this PR.
There was a problem hiding this comment.
@J-Mo63 Looks like I didn't notice something in your change. All our tests are writing files to the build folder. I just run the new tests locally and you write the source folder...this means that now I always have changes in the source tree after running tests, e.g.
git status
HEAD detached at origin/dev
Untracked files:
(use "git add <file>..." to include in what will be committed)
plugin/al/plugin/AL_USDMayaTestPlugin/py/testLayerManagerOutput/
Please adjust it to match what other tests are doing.
There was a problem hiding this comment.
Ah yes that's no good at all. The switch to fixturesUtils is on the way.
| @@ -1027,6 +1026,16 @@ MStatus LayerManager::initialize() | |||
| stat = addAttribute(identifier); | |||
| CHECK_MSTATUS_AND_RETURN_IT(stat); | |||
|
|
|||
| fileFormatId | |||
There was a problem hiding this comment.
Since we don't share the same implementation of LayerManager, it would be good to add a test for this new addition to validate the core implementation as well. I won't make it a blocker for this change, but please open a new PR with it.
This PR allows for the extension to now be retrieved using USD's API instead of string parsing, and adds the layer's file format as a serialisation attribute (along with the current identifier and serialised content).
This is to help deserialisation in the case that the identifier does not resolve without a proper context (which is the case with the layer manager).