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

MAYA-123361 fix unshared stage toggling and loading #2410

Merged
merged 3 commits into from
Jun 17, 2022

Conversation

pierrebai-adsk
Copy link
Collaborator

  • Add two new stage caches for unshared stages.
  • These are necessary to be able to find unshared stages when toggling between shared and unshared.
  • Do not pass an explicit sesssion layer when opening a stage unless unshared stage is explicitly desired.
  • That is because the cache takes into consideration the session layer if given as an argument, which would unshare layers unintetionally.
  • In particular, choosing to target the session layer (which is UI consideration) would indirectly make the stage unshared.
  • Properly reload unshare stage and toggle between shared and unshared stage by transferring the content of the session layer.
  • Clarify with comments the design and behaviour of the stage proxy shape.
  • Adapted AL code to conform to the new API.
  • Add unit test for session preservation

- Add two new stage caches for unshared stages.
- These are necessary to be able to find unshared stages when toggling between shared and unshared.
- Do *not* pass an explicit sesssion layer when opening a stage unless unshared stage is explicitly desired.
- That is because the cache takes into consideration the session layer if given as an argument, which would unshare layers unintetionally.
- In particular, choosing to target the session layer (which is UI consideration) would indirectly make the stage unshared.
- Properly reload unshare stage and toggle between shared and unshared stage by transferring the content of the session layer.
- Clarify with comments the design and behaviour of the stage proxy shape.
- Adapted AL code to conform to the new API.
- Add unit test for session preservation
@@ -798,34 +805,49 @@ MStatus MayaUsdProxyShapeBase::computeInStageDataCached(MDataBlock& dataBlock)
= MGlobal::optionVarIntValue(kSessionLayerOptionVarName) == 1;
targetSession = targetSession || !rootLayer->PermissionToEdit();

if (sessionLayer || targetSession) {
if (!sessionLayer)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When targeting the session layer without a named session layer loaded from Maya scene, it would create a new session layer. This would prevent reusing a previously shared stage. Target session vs stage sharing are orthogonal. I looked at the original github issue and PR and the goal was to be able to target the session, with no discussion about affecting stage sharing.

I think the code was unwittingly written to have that side effect, so I fixed it.

const auto shareMode = isShareableStage() ? ShareMode::Shared : ShareMode::Unshared;
if (shareMode == _previousShareMode)
return;

Copy link
Collaborator Author

@pierrebai-adsk pierrebai-adsk Jun 10, 2022

Choose a reason for hiding this comment

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

On thing I was not sure of: do I need to reset the previous share mode in some cases? For example, if the user edits the filename, would we need to reset the flag? Where and when, through what mechanism would that be done?

I did not see that any other member variable get reset in these cases, so I did not do anything about it for this flag.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Resetting the share mode on file (i.e. root layer) change is an interesting question. I think you'll need to follow up with Design here so that we can schedule some time for this.

Copy link
Collaborator

@ppt-adsk ppt-adsk left a comment

Choose a reason for hiding this comment

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

A few nit-picks, but nothing major. Because you've changed Pixar plugin code (even though the changes are minor), I would feel more comfortable in having Pixar review this pull request, if possible.

lib/mayaUsd/nodes/proxyShapeBase.h Outdated Show resolved Hide resolved
lib/mayaUsd/nodes/proxyShapeBase.h Outdated Show resolved Hide resolved
lib/mayaUsd/nodes/proxyShapeBase.h Outdated Show resolved Hide resolved
lib/mayaUsd/utils/stageCache.h Outdated Show resolved Hide resolved
test/lib/mayaUsd/nodes/testProxyShapeBase.py Outdated Show resolved Hide resolved
lib/mayaUsd/nodes/proxyShapeBase.cpp Outdated Show resolved Hide resolved
lib/mayaUsd/nodes/proxyShapeBase.cpp Outdated Show resolved Hide resolved
lib/mayaUsd/nodes/proxyShapeBase.cpp Outdated Show resolved Hide resolved
const auto shareMode = isShareableStage() ? ShareMode::Shared : ShareMode::Unshared;
if (shareMode == _previousShareMode)
return;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Resetting the share mode on file (i.e. root layer) change is an interesting question. I think you'll need to follow up with Design here so that we can schedule some time for this.

test/lib/mayaUsd/nodes/testProxyShapeBase.py Outdated Show resolved Hide resolved
- Fix typos.
- Simplify code.
- More efficient ref-counted pointer passing.
@pierrebai-adsk
Copy link
Collaborator Author

PF passed except the know testProxyShapeDrawUsdChangeProcessing in 2022 Linux interactive.

@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Jun 17, 2022
@seando-adsk seando-adsk added the workflows Related to in-context workflows label Jun 17, 2022
@seando-adsk seando-adsk merged commit 4ab425b into dev Jun 17, 2022
@seando-adsk seando-adsk deleted the t_bailp/MAYA-123361/unshared-stage-session branch June 17, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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