-
Notifications
You must be signed in to change notification settings - Fork 202
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-127483 - Deleting a connection does not delete source and destination properties. #2852
MAYA-127483 - Deleting a connection does not delete source and destination properties. #2852
Conversation
return true; | ||
} | ||
|
||
// Do not remove if there is a connection with a child prim. |
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.
There is a very useful workflow where you drag the first port of the type you want on the boundary, and connect it to the input node as many times as desired, then delete the connections.
In this scenario, the user expects the ports on the input node to remain so he can do further work on them. He can always delete the ones he does not want anymore.
That would mean returning false here even if there are no connections left. Same thing on the reverse function.
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.
Thank you. I didn't know about this workflow.
I've updated the solution also with the test.
I've also reintroduced the condition for the UsdShadeMaterial
which we should keep ( it is also required by some tests).
The test file |
Thank you for having corrected it. |
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.
Looks good from me, except one minor change requested to move helper functions.
@@ -284,6 +284,104 @@ UsdUndoDeleteConnectionCommand::Ptr UsdUndoDeleteConnectionCommand::create( | |||
return std::make_shared<UsdUndoDeleteConnectionCommand>(srcAttr, dstAttr); | |||
} | |||
|
|||
bool canRemoveSrcProperty(const PXR_NS::UsdAttribute& srcAttr) |
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.
I think this helper function (and the one below) should be in the unnamed namespace near the top of this file (with the other helper functions).
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.
Could I move them to mayaUsd/ufe/Utils.h
? I need them also for other issues I'm working on.
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.
Yes that is a good idea.
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.
I moved also the isConnected
function since it is used by canRemoveSrcProperty
and canRemoveDstProperty
, I also need it for the issues I'm working on.
This reverts commit a27a9bc.
d8dd340
|
||
UsdShadeMaterial asMaterial(prim); | ||
if (asMaterial) { | ||
const TfToken baseName = dstAttr.GetBaseName(); |
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.
I reverted the change made on GetBaseName
after your explanation @JGamache-autodesk .
e.g. if the outputs:mtlx:surface
is not authored, it is removed with the connection.
Description
This PR solves the bug reported in Jira ticket MAYA-127483: when we delete a connection, we should check if it is possible also to delete the source and destination properties.
We checked only the destination and not the source.
We can delete a property when:
This last condition should be explored in all its complexity.
If the source/destination attribute is on a Compound, we should check also the connection to/from the inner children and also consider, in this case, that a destination is also a source and vice versa.
Type of change
How Has This Been Tested?
I've added extensive tests in order to cover all the cases of property removal: