-
Notifications
You must be signed in to change notification settings - Fork 203
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
MAYA-123013 obey file-format choice in layer editor #2334
Conversation
When re-saving a modified layer, follow the current ASCII / Binary format selected in the layer editor. Notes: - USD SdfLayer does not have a Save() function taking the desired format. The only option we have is to use Export(). - The choice of binary / ASCII is in a sub-menu and is global to all layers and all stages. That may not be convenient for the user: changing it will affect all stages and all layers. Beware!
layer()->Save(); | ||
PXR_NS::SdfFileFormat::FileFormatArguments args; | ||
args["format"] = MayaUsd::utils::usdFormatArgOption(); | ||
layer()->Export(layer()->GetRealPath(), "", args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems okay to me, but I'm wondering what are the implications of calling Export() vs Save()? @ppt-adsk Do you have any comments about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use Export in many other places instead of Save when we want to control the output format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per the docs, Save() saves to the previously-remembered file name, and fails if there is none. Export() saves a copy of the layer, and does not affect the previously-remembered file name. There is one important point: Save() will clear the layer's dirty state (i.e. SdfLayer::IsDirty() will return false), whereas Export() will not, so this change is introducing an important difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Export() calls _WriteToFile() which calls _MarkCurrentStateAsClean() if the filename did not change, which is our case since we're exporting to the same file (we pass GetRelPath()).
The differences I can see are:
- Save() does nothing if the layer is not dirty. We could add a call to IsDirty().
- Set modification time, which I don't think we care about.
Things that look different but are the same in the end:
- Save() check IsMuted() and IsAnonymous() directly, but Export() ends-up doing the same check because _WriteToFile() calls PermissionToSave(0 when the filename is the same and that does the same checks.
- Save(0 clears hints, but the hints are not used once the dirty state is reset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... unfortunately, there are two other places in the code where we call Save() directly on the layer... so they will not be aware of the changed file format. I'm taking a deeper look but we will either have to modify those too or abandon the idea of switching file formats for already-saved layers.
Avoid calling Save() and always use Export() so that the output file format respect the one selected by the user. Provide a function to enforce that and avoid code duplication.
Verify if the file name and format are the same as the ones kept in the layer. If they both match, then call Save() instead of Export(). For this to work, the initial anonymous layer needs to be created with the correct format.
- Forcing the format when creating anonynous layer in memory prevented loading from a different format. - This happened in one test. - Don't affect maya reference updater not AL command for teh same reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thanks.
When re-saving a modified layer, follow the current ASCII / Binary format selected in the layer editor.
Notes: