Skip to content
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

Restore session layer properly or create one when root-layer is not editable. #899

Merged
merged 1 commit into from
Nov 19, 2020

Conversation

marsupial
Copy link
Contributor

No description provided.

@@ -109,7 +109,10 @@ MStatus UsdMayaStageNode::compute(const MPlug& plug, MDataBlock& dataBlock)
UsdStageCacheContext ctx(UsdMayaStageCache::Get(loadAll));
usdStage = UsdStage::Open(rootLayer, ArGetResolver().GetCurrentContext());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think similar to what's in proxyShapeBase.cpp, we should probably be checking permissions and possibly creating the sessionLayer first, and then use the Open() overload that takes a sessionLayer if we created one. Otherwise the anonymous layer created below is not part of the stage's layer stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

@kxl-adsk
Copy link

kxl-adsk commented Nov 6, 2020

@marsupial thank you for reporting an issue and opening a pull-request to address it. We have a tricky situation here, since the change of the default edit target was requested internally (and merged with #661). Let's have a conversation on the issue first to make sure we have a solution that works for all stakeholders.

@kxl-adsk kxl-adsk added do-not-merge-yet Development is not finished, PR not ready for merge proxy Related to base proxy shape workflows Related to in-context workflows labels Nov 6, 2020
@marsupial
Copy link
Contributor Author

Update with comments from #898

@kxl-adsk kxl-adsk removed the do-not-merge-yet Development is not finished, PR not ready for merge label Nov 18, 2020
@kxl-adsk kxl-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Nov 19, 2020
Copy link
Contributor

@mattyjams mattyjams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some tiny, mostly cosmetic notes, but otherwise looks good to me!

// limitations under the License.
//

#pragma once
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The coding guidelines would suggest #include-style guards rather than #pragma once.

Comment on lines +36 to +41
// Convenience to convert a TfToken to MString
//
static inline MString toMString(const TfToken& token)
{
return MString(token.data(), token.size());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A helper for this actually exists already as UsdMayaUtil::convert():

MString convert(const TfToken& token);

@kxl-adsk kxl-adsk merged commit 4ebdc83 into Autodesk:dev Nov 19, 2020
@marsupial
Copy link
Contributor Author

Already been merged, so function-removal and guard changes: #935

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proxy Related to base proxy shape ready-for-merge Development process is finished, PR is ready for merge workflows Related to in-context workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants