AnimalLogic / AL_USDMaya

This repo is no longer updated. Please see https://github.com/Autodesk/maya-usd
Other
266 stars 69 forks source link

Layers not initialising properly on file load #26

Open alekamca opened 6 years ago

alekamca commented 6 years ago

This assert (and a matching one in setInternalValueInContext) is failing when loading a maya file that has Layers which deserialized (as part of Global.cpp::postFileOpen). https://github.com/AnimalLogic/AL_USDMaya/blob/develop/lib/AL_USDMaya/AL/usdmaya/nodes/Layer.cpp#L578

This can easily be reproduced by running the supplied endToEndMaya Layout tutorial and saving the maya file, reopening the file, and doing anything that calls get/setInternalValue (e.g., looking at the node in the attribute editor, resaving the file).

The failure appears to be because Layer::setLayerAndClearAttribute is called (which sets m_handle), but m_shape has not been set (this can only be set via Layer::init).

I have what appears to be a fix, and I'm in the process of getting the CLA through legal (I am at Walt Disney Animation Studios).

renaudll commented 6 years ago

Hey alekamca, we have the same issue on our end. Did you had the chance to get it through legal? If you are not able to publish your fix any cues are appreciated. Thanks!

alekamca commented 6 years ago

I am still pushing on getting the CLA. From what I understand, it is coming; it just takes time.

In the mean time, I made 2 changes. One is necessary for fixing the assert. The second fixes a related issue where layers can be orphaned and unable to become edit targets after file save-load.

In Global.cpp::postFileOpen When looping over the proxy shapes, and calling setLayerAndClearAttribute on the session layer, also call layer->init since you know the proxy and SdfLayer.

Rather than looping over the Layer nodes and deserializing them in the next block, instead recursively deserialize and init them starting with the session layer and following its ChildLayers and SubLayers.

This means you will never deserialize "orphan" Layer nodes in your scene that cannot be connected to a proxy node, but they will at least be in a valid "empty" state rather than an invalid state. You could go through and find these orphans and do something with them?

Secondly, in order to init a Layer node, you need to know its SdfLayer. The easiest way to do this is to always set the nameOnLoad attribute when calling Layer::populateSerialisationAttributes, even it has not been an editTarget. This has a lot of nice side effects, since it means these layers can become valid edit targets in the future (previously, they would just silently remain empty and not track any edits).

murphyeoin commented 6 years ago

Hi aleka, renaud - one of our external contributors has also made some related fixes in the layer handling part of the code which they have notified us of but not submitted for PR yet - when they do (hopefully in next few days?) we can probably have a discussion about it, and decide what the best solution is - we're happy to hold off merging until we discuss with you...

alekamca commented 6 years ago

Thanks for the update. It would be good to see what the changes are. I'm not particularly attached my to changes as long as the functionality I need is preserved. I imagine someone closer to the project would have better insight into how best to resolve these issues.

renaudll commented 6 years ago

Hi murphyeoin, same thing on our side. As long a we reach a more usable state we are happy!

murphyeoin commented 6 years ago

Hi @alekamca @renaudll the related fixes I mentioned previously may not be coming as soon as we'd hoped.. so whatever similar code you want to put up for PR - go ahead! Thanks Eoin

alekamca commented 6 years ago

I put it up for PR just now! https://github.com/AnimalLogic/AL_USDMaya/pull/28