Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Maya: Deformer node ids validation plugin #2826

Merged
merged 15 commits into from
Mar 17, 2022
Merged

Maya: Deformer node ids validation plugin #2826

merged 15 commits into from
Mar 17, 2022

Conversation

2-REC
Copy link
Contributor

@2-REC 2-REC commented Mar 2, 2022

Brief description

In Maya, deformers can create copies of shape nodes without keeping the cbId from the original nodes, breaking subsequent references.

Description

In Maya, when a deformer is added to a mesh, a new shape node is created, copy of the original shape node.
The "deformed" shape node will hold the result of the deformation applied to the original shape node.
The new node doesn't get the cbId of the original node.

Depending on the deformer, the new node will be used as the original node or as the deformed node. Additionally, if applying more deformers, more nodes can be created.
In many cases, the deformed node will not have a cbId and will get a newly generated one, unmatching the original node cbId.

This causes problems downstream in the pipeline, as any reference to the original node's cbId will not match with the deformed shape node's cbId.

Additional info

Typically, this issue will occur when assigning a published look to a rig.

To reproduce the problem, the following steps can be executed:

  • Create and publish a model.
  • Import the published model in a new file ("Load" as a reference), create a look for that model and publish it.
  • Import the published model in a new file ("Load" as a reference), create a rig for that model (with a skin deformer) and publish it.
  • In a new scene, import the published rig, and using the "Look Assigner" tool, assign the published look to the rig.

As a result, the look is not assigned to the mesh, and the following message is displayed in the "History" panel:

# Warning: None of the items can be added to the set # 

The issue is that the process is trying to assign the shaders only to the original shape nodes (referred to by their cbId), which are marked as "intermediate objects" (because of the deformer applied to them).
Shaders cannot be assigned to these nodes, and no other nodes match the cbIds, hence the shaders aren't assigned to anything.

Solution

To fix this issue, and to allow downstream tasks to refer to the deformed nodes as the original nodes without distinction, the solution is to get the cbId from the original shape nodes and "transfer" them to the new deformed shape nodes.

In general, the original shape node can be retrieved from the deformed shape node's history.

In some cases however, it is not possible to retrieve the original shape node this way, as another copy of the original node has been made and is now used as the original for the deformer.

For example, consider the following node graph.
deformer_graph

The shape node "MeshShapeDeformedOrig" is used as the original node for the deformer, but its relation with the original "MeshShape" has been lost.

In this case, it is not possible to get the original shape node from the deformed shape node's history (only the new node which is a copy of the original node).

The nodes do however share the same transform node parent, and are related in that way.

Following is a suggested solution:
The original shape node can be found as a sibling of the deformed shape node.
As mentioned earlier, the original node should have the attribute "intermediate object" set.
Additionally, the node should also have a cbId assigned to it, as opposed to the new node.

A node satisfying these conditions should be the original shape node.
The cbId of this node can then be transferred to the deformed node.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 2, 2022

Thanks for the very thorough write-up!

This logic already exists elsewhere and I think could be re-used.

It's this get_id_from_history function that is also used in some validators:

I believe you might even get by with only adding rig family to validate_node_ids_deformed_shapes.


EDIT:

The shape node "MeshShapeDeformedOrig" is used as the original node for the deformer, but its relation with the original "MeshShape" has been lost.

In this case, it is not possible to get the original shape node from the deformed shape node's history (only the new node which is a copy of the original node).

Ah, I see now that this is a different check. I'd actually propose to maybe refactor some existing logic and just add an argument to get_id_from_history (and maybe refactor it to get_id_from_sibling and add an argument: history_only=True by default but set it to False for this validation).

Re-using more of the logic means if we fix one in the future, then all will get fixed.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 2, 2022

Also to add some more details:

In Maya, when a deformer is added to a mesh, a new shape node is created, copy of the original shape node.
The "deformed" shape node will hold the result of the deformation applied to the original shape node.
The new node doesn't get the cbId of the original node.

I believe this only happens to referenced meshes.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 2, 2022

Another note: It's also technically possible to have multiple mesh shapes under a single transform, which will 'invalidate' with this current logic. I have that seen being used very rarely but I have seen it. It's something to keep in mind at least.

@2-REC
Copy link
Contributor Author

2-REC commented Mar 2, 2022

I saw your other comments after posting, so had to delete my previous comment ;)

As you suggest, a mix between the 2 solutions would probably be ideal (in case the history isn't enough, look at the siblings).
A flag "history_only" seems perfect.

As for the referenced meshes, yes, it seems to happen in that case (I think it might be related to foster parents nodes when trying to modify the referenced nodes), though I feel like I had cases where the problem also occurred with imported meshes.
I will try again and confirm (or not).

@2-REC
Copy link
Contributor Author

2-REC commented Mar 2, 2022

Another note: It's also technically possible to have multiple mesh shapes under a single transform, which will 'invalidate' with this current logic. I have that seen being used very rarely but I have seen it. It's something to keep in mind at least.

Indeed.
A rare case, but possible...
Not sure how to solve this case, maybe by name similarities?

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 2, 2022

I thought that as well, but the issue is that the original shape node might be in the deformed shape node's history anymore. I am not sure how this happened (I didn't do the rig), but the deformed shape node's history refers to a new "shapeOrig" node, and "lost" the connection with the original node.
This is why in my case, looking at the history didn't work.

This can happen when the rigger did for example a delete history in-between. (which I believe would disconnect) But Maya being Maya there's likely a lot more reasons that could happen.

Indeed.
A rare case, but possible...
Not sure how to solve this case, maybe by name similarities?

I'd say for now we might want to add this Validator in Admin Settings so that it can be enabled/disabled. Then if someone ever has the case where they need to have multiple shapes under a transform like that, they can disable it for themselves. I wouldn't say it's directly problematic but it's what makes this a potentially difficult case to "always solve correctly".

@2-REC
Copy link
Contributor Author

2-REC commented Mar 2, 2022

OK, so I can refactor the lib function 'get_id_from_history' as you suggested, and use it in this plugin with the added parameter 'history_only' to False.
If it seems ok for you, I will make the changes now and commit asap.

About the admin settings, I could have a look, but it might be a bit complicated as I haven't touched that (yet).

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 2, 2022

OK, so I can refactor the lib function 'get_id_from_history' as you suggested, and use it in this plugin with the added parameter 'history_only' to False.
If it seems ok for you, I will make the changes now and commit asap.

I think you could remove your Validator and apply your tweak in this Validate Rig Out Set Node Ids so that it does get_id_from_siblings(history_only=self.allow_history_only). Then at the top of the class below actions = [] add self.allow_history_only = False.

Then we can expose that particular True/False settings in Admin Settings - the plug-in would need to be added in this .json file. Likely we'll want to add it inside the Rig validators entry where you'd want to expose the allow_history_only key similar to how it's done here for another validator. Let me know if you can figure it out like that or need more input.

Once you have it added in the settings all you'd need to do is set the default settings values. You can do that by running tools/run_settings.ps1, then Save with "Modify Defaults" enabled on the Project tab (since this is for project settings). That will regenerate the relevant defaults .json file.

@2-REC
Copy link
Contributor Author

2-REC commented Mar 2, 2022

OK, perfect.
Thank you for all the details.

I should be able to do it, I think I have enough info.
I won't have time to finish today, but will definitely have it done tomorrow.

@2-REC 2-REC marked this pull request as draft March 2, 2022 10:25
@2-REC 2-REC marked this pull request as ready for review March 3, 2022 08:02
@BigRoy
Copy link
Collaborator

BigRoy commented Mar 3, 2022

Also don't forget to add the default settings and make sure to commit them:

Once you have it added in the settings all you'd need to do is set the default settings values. You can do that by running tools/run_settings.ps1, then Save with "Modify Defaults" enabled on the Project tab (since this is for project settings). That will regenerate the relevant defaults .json file.

They currently appear to be missing.

@2-REC 2-REC changed the title Deformer node ids validation plugin for Maya Maya: Deformer node ids validation plugin Mar 11, 2022
@mkolar
Copy link
Member

mkolar commented Mar 11, 2022

So I feel like the default should be set to True, even if it can break backwards compatibility...

I actually agree that in this case, it's more a bugfix, than a pure change. If someone already has a scene prepped, it should be validated against this in the new publish, to prevent potential issues.

@mkolar mkolar added the type: bug Something isn't working label Mar 15, 2022
@mkolar
Copy link
Member

mkolar commented Mar 15, 2022

Just a note that this has now been tested on a studio side with one of our clients as well and works as expected

@mkolar mkolar added this to the next milestone Mar 15, 2022
@BigRoy
Copy link
Collaborator

BigRoy commented Mar 16, 2022

@2-REC Great work. Testing this now!

  • Can you quickly also fix up the logged error message? It's not an issue introduced by you. But as I started testing I noticed it's off. Something like this?
            raise RuntimeError("Nodes found with mismatching "
                               "IDs: {0}".format(invalid))

Other than that I can confirm it does take the id from the sibling if no history connection is present. ✅


As a TD my preference would be that it'd log clearly where it's taking the ID from but I think most artists would likely say it confuses them more so I'm fine with current behavior. Let's leave it as is for now.

@mkolar
Copy link
Member

mkolar commented Mar 16, 2022

Very nice. Thank you. @BigRoy happy on your end? we can included in 3.9.1 coming out today with a swarm of other fixes

Copy link
Collaborator

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

Great work @2-REC - sorry for the so much back and forth on this. Really appreciate you sticking through it.

@mkolar mkolar merged commit 0d876cc into ynput:develop Mar 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
host: Maya type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants