Respect prim writer ShouldPruneChildren when shading mode is useRegistry

[sshklifov]

Currently, there is no way to register a shader writer that handles all
connected plugs to the shader. For example, the writer might handle file
nodes connected to the shader, but useRegistry shading mode will invoke
FileTextureWriter unconditionally. It will write out a UsdUVTexture
shader, which useRegistry won't be able to connect to the shading network.

[seando-adsk]

@sshklifov Thank you for the PR. In order to proceed we will need a signed CLA from you. Please see Contributing

[sshklifov]

Hi @seando-adsk, sorry for the inconvenience. The CLA has been signed and sent to the appropriate destination.

[seando-adsk]

@sshklifov No problem. We got the signed CLA and we can now proceed with your pull-request. I've assigned it to one of our devs for code review.

[JGamache-autodesk]

The code itself is OK, but I suspect it will act more as a workaround than a fix.

It is perfectly OK to challenge returning ContextSupport::Fallback in PxrUsdTranslators_FileTextureWriter::CanExport() when not exporting to UsdPreviewSurface materials. I do not feel that this code is doing the right thing anymore if it forces you to add explicit pruning to fight against it.
Maybe adding a "no-fallback" flag when you register a new useRegistry conversion mode would be better?

[sshklifov]

Hi @JGamache-autodesk. Thanks for the feedback.

Removing ContextSupport::Fallback from PxrUsdTranslators_FileTextureWriter::CanExport() would solve the issue, yes. But I feel that would be a workaround as other prim writers are free to return ContextSupport::Fallback and the same problem will occur.

I am not sure what you mean by adding a "no-fallback" flag. Do you mean adding this flag as an argument to UsdMayaShadingModeRegistry::RegisterExportConversion? I think I prefer this solution, should I update the PR?

[JGamache-autodesk]

Hi @sshklifov, adding a no-fallback argument to RegisterExportConversion looks like the best solution. If you could update the PR that would be really nice. Thanks!

[sshklifov]

Hi, sorry for the delay, I updated the PR. I do have a question regarding ABI though. These changes will require recompilation of plugins that link to mayaUsd (struct ConversionInfo size has changed). It is however possible to maintain ABI compatibility if ConversionInfo is not modified.

Which approach is desired? As someone who is developing a plugin that uses mayaUsd, I think that breaking the ABI in general is undesirable. In this particular instance, we will have to recompile anyway in order to see the new symbol with the noFallback flag.

[JGamache-autodesk]

Looks good to me. Thanks for the update!

I am not worried with new members at the end of a struct that has no virtual members. That is allowed according to the very thorough KDE binary compatibility guidelines. Changing the signatures of existing functions could be an issue though if we want to preserve binary compatibility.

I think our guidelines allow binary incompatible changes between released versions of MayaUSD so I don't think this will be an issue. I will let @seando-adsk confirm.

[seando-adsk]

There is no problem with binary breaking changes. That is the reason we are major version 0.

[seando-adsk]

@sshklifov I ran the preflight build for you and unfortunately it failed during the initial clang-format step. Please access the checks tab and you'll see the clang-format errors along with the fixes. Or if you have access to clang-format you can run it locally. Please see our coding guideslines for info about clang-format.

[sshklifov]

I ran clang-format exact version 10.0.0. The diff seems to be matching the one in the checks tab.