Registry should declare filenames as assets in GetTypeAsSdfType #1756
Conversation
| // these attributes always default to empty strings, it's better not to set the | ||
| // VtValue at all, so that we don't get errors about invalid types. | ||
| if (!isAsset) | ||
| attr.Get(&v); |
There was a problem hiding this comment.
I am wondering if the VtValue shouldn't be initialized before passed as defaultValue, because at this stage it doesn't have a defined type
There was a problem hiding this comment.
that's the problem. These attributes are designed in a strange way in USD, being strings in some way but assets in others. If I set it as an empty SdfAssetPath attribute, we have an undesirable behaviour in the lookdevx UI (there's a "@@" value by default) which doesn't happen with other shader libraries like materialx. If I set it as an empty string, I get error messages about a type mismatch. The only way to get the expected behaviour is to not set the value at all, and then we get the same behaviour as all the other shader filenames
Changes proposed in this pull request
Asset attributes of arnold shaders (e.g. image.filename) need to be declared as strings, as it was discussed here . However, the function
GetTypeAsSdfTypestill needs to declare the attributes sdf type as being an asset, otherwise they get saved as string through mayaUSD / Lookdevx.But by just changing this, I had other usd errors because the default value was a string, whereas the sdf type was an asset. Making the default value an empty SdfAssetPath worked, but then in Lookdevx the default filename showed as
@@in the UI, which is confusing and doesn't match what other texture nodes look like. Since these asset attributes only exist for 2 attributes and in practice those filenames always default to empty values, the best solution seemed to declare an empty VtValue for default value. In this case, everything works as expectedIssues fixed in this pull request
Fixes #1755