HYDRA-833 - Maya MaterialX material bindings on Maya geometry #72
Conversation
vlasovi
commented
Feb 14, 2024
vlasovi
commented
- Support MaterialX on Maya native prims
- Autotest
- Autotest
| mayaUtils.openTestScene("testMaterialX", "RedMtlxSphere.ma") | ||
| self.setBasicCam(2) | ||
| self.setHdStormRenderer() | ||
| self.verifySnapshot("RedMtlxSphere.png") |
There was a problem hiding this comment.
are we missing to pass IMAGEDIFF_FAIL_THRESHOLD and IMAGEDIFF_FAIL_PERCENT here?
There was a problem hiding this comment.
No, they are passed from inside verifySnapshot method
There was a problem hiding this comment.
right, sorry, I missed that. :)
| VtValue GetMaterialResource() override | ||
| { | ||
| TF_DEBUG(MAYAHYDRALIB_ADAPTER_MATERIALS) | ||
| .Msg("MayaHydraShadingEngineAdapter::GetMaterialResource(): %s\n", GetID().GetText()); | ||
|
|
||
| HdMaterialNetworkMap materialXNetworkMap; | ||
| if (PopulateMaterialXNetworkMap(materialXNetworkMap)) { |
There was a problem hiding this comment.
is there any reason not to do the translation inside MayaHydraMaterialNetworkConverter? I feel it's better to move the code into MayaHydraMaterialNetworkConverter as MatX is also one kind of Material.
There was a problem hiding this comment.
The interface of MayaHydraMaterialNetworkConverter seems to be developped specifically for UsdPreviewSurface network. I think we would have hard times adapting and rewriting it for the needs of MaterialX. For example method HdMaterialNode* GetMaterial would need to change its output type as well as all other public methods have no meaning for MaterialX (like GetPreviewShaderParams). I don't see any reason for readapting MayaHydraMaterialNetworkConverter for MaterialX
There was a problem hiding this comment.
ok, sounds like having a base materialConverter and diferent sub materialConverters would make the architecture looks better, as the actual conversion is the responsibility of materialConverter, not materialAdapter.
I'm fine with the current code, just trying to make it easier to maintain, we could also refactor it later when we need more materialConverters in the future.
vlasovi
added
the
ready-for-merge
