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

Restrict namespace edits to current stage #2977

Merged

Conversation

dj-mcg
Copy link
Collaborator

@dj-mcg dj-mcg commented Mar 28, 2023

This will restrict namespace edits to your current usd stage. This avoids the issue of spanning references and inherits, etc, which will often be illegal operations. This addresses the issue I posted here:

#2953

dj-mcg added 2 commits March 28, 2023 08:58
This is needed for the calls to std::once_flag and std::call_once
When authoring namespace edits across layers, restrict the targets to
layers within your current usd stage. This avoids trying the edit across
references, inherits, payloads, etc, which will often times be illegal.
@dj-mcg dj-mcg changed the title Pr/restrict namespace edits to current stage Restrict namespace edits to current stage Mar 28, 2023
@dj-mcg dj-mcg requested a review from pierrebai-adsk March 28, 2023 21:56
@@ -20,6 +20,8 @@
#include <pxr/usd/usd/tokens.h>
#include <pxr/usd/usdUI/tokens.h>

#include <mutex>
Copy link
Collaborator

Choose a reason for hiding this comment

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

No sure why this file got modified, was this to fix a build issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I couldn't build locally without that change. It's PR'd here:
#2975

pierrebai-adsk
pierrebai-adsk previously approved these changes Mar 29, 2023
Copy link
Collaborator

@pierrebai-adsk pierrebai-adsk left a comment

Choose a reason for hiding this comment

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

It's always hard to imagine all possible scenarios that the change could affect, but the principle seems correct, so this is good to go. If we find cases that need to modify non-local layer stacks in the future, we'll revisit the design and API.

@@ -116,16 +116,33 @@ void enforceMutedLayer(const PXR_NS::UsdPrim& prim, const char* command)
}
}

static SdfPrimSpecHandleVector _GetLocalPrimStack(const UsdPrim& prim)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ON second thought, maybe adding a comments on what this code is trying to avoid, like the description you gave in teh bug would be helpful for future coders.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I too the liberty of adding the comment myself to your branch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you!!

Add a comment to explain the code that only affect local layers.
@seando-adsk seando-adsk added workflows Related to in-context workflows ready-for-merge Development process is finished, PR is ready for merge labels Mar 30, 2023
@seando-adsk seando-adsk merged commit 42fb722 into Autodesk:dev Mar 30, 2023
@dj-mcg dj-mcg deleted the pr/Restrict_Namespace_Edits_to_Current_Stage branch March 31, 2023 19:49
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