-
Notifications
You must be signed in to change notification settings - Fork 4
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
HYDRA-833 - Maya MaterialX material bindings on Maya geometry #72
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
# Copyright 2024 Autodesk | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
import maya.cmds as cmds | ||
import mayaUtils | ||
import fixturesUtils | ||
import mtohUtils | ||
import unittest | ||
|
||
class TestMaterialXOnNative(mtohUtils.MtohTestCase): | ||
_file = __file__ | ||
|
||
IMAGEDIFF_FAIL_THRESHOLD = 0.01 | ||
IMAGEDIFF_FAIL_PERCENT = 0.1 | ||
|
||
def verifySnapshot(self, imageName): | ||
cmds.refresh() | ||
self.assertSnapshotClose(imageName, | ||
self.IMAGEDIFF_FAIL_THRESHOLD, | ||
self.IMAGEDIFF_FAIL_PERCENT) | ||
|
||
@unittest.skipUnless(mtohUtils.checkForPlugin('LookdevXMaya'), "Requires LookDevX Plugin.") | ||
def test_MaterialX(self): | ||
mayaUtils.openTestScene("testMaterialX", "RedMtlxSphere.ma") | ||
self.setBasicCam(2) | ||
self.setHdStormRenderer() | ||
self.verifySnapshot("RedMtlxSphere.png") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we missing to pass IMAGEDIFF_FAIL_THRESHOLD and IMAGEDIFF_FAIL_PERCENT here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, they are passed from inside verifySnapshot method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right, sorry, I missed that. :) |
||
|
||
if __name__ == '__main__': | ||
fixturesUtils.runTests(globals()) |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.