-
Notifications
You must be signed in to change notification settings - Fork 129
Looks: add basic support for Renderman #3190
Looks: add basic support for Renderman #3190
Conversation
Task linked: OP-3256 Maya Looks: support for renderman PxrTexture |
@@ -165,7 +170,7 @@ def get_file_node_path(node): | |||
if any(pattern in lower for pattern in patterns): | |||
return texture_pattern | |||
|
|||
if cmds.nodeType(node) == 'aiImage': | |||
if cmds.nodeType(node) in ['aiImage', 'PxrTexture']: |
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.
Would lookup in set
be better?
if cmds.nodeType(node) in ['aiImage', 'PxrTexture']: | |
if cmds.nodeType(node) in {'aiImage', 'PxrTexture'}: |
if cmds.nodeType(node) not in [ | ||
"file", "aiImage", "RedshiftNormalMap", "PxrTexture"]: |
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.
Also, lookup in set
maybe better?
try: | ||
extension = cmds.getAttr('%s.useFrameExtension' % node) | ||
except ValueError: | ||
extension = None |
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.
This should be extension = False
to still return a boolean.
Also, might be better as a variable named use_frame_extension
?
There is much more Rendeman native nodes having file I/O I know this PR adds basic support, but we should at least add the most used ones. PxrEnvGround |
True but this is really for another PR - for example |
I did not make it clear in my previous message. Exactly for that reason, I marked the ones to be included with bold (and later even added italic) but it is not very clear. My bad, sorry about that. |
Just to mention possible custom tokens used in file strings in RfM which could cause troubles down the line . . . https://rmanwiki.pixar.com/display/RFM23/String+tokens+in+RfM |
Ouch - that could definitely make things more complicated. There is Thanks for mentioning. |
DEFAULT_FILE_NODES = frozenset( | ||
["file"] | ||
) | ||
|
||
ARNOLD_FILE_NODES = frozenset( | ||
["aiImage"] | ||
) | ||
|
||
REDSHIFT_FILE_NODES = frozenset( | ||
["RedshiftNormalMap"] | ||
) | ||
|
||
RENDERMAN_FILE_NODES = frozenset( | ||
[ | ||
"PxrBump", | ||
"PxrNormalMap", | ||
# PxrMultiTexture (need to handle multiple filename0 attrs) | ||
"PxrPtexture", | ||
"PxrTexture", | ||
] | ||
) |
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.
Would it make more sense to store it like this:
file_nodes = {
# Maya
"file": "filename",
# Arnold
"aiImage": "filename",
# Redshift
"RedshiftNormalMap": "tex0",
# Renderman
"PxrBump": "filename",
"PxrNormalMap": "filename",
"PxrPtexture": "filename",
"PxrTexture": "filename",
# PrxMultiTexture is a special case with multiple filename attributes
# So this could maybe be a special function?
"PxrMultiTexture": get_pixar_multi_texture
}
# Example usages
node_type = cmds.nodeType(node)
attr = file_nodes[node_type]
path = cmds.getAttr('{}.{}'.format(node, attr))
Special edge case:
def get_pixar_multi_texture(node):
filepaths = []
for path in path_attributes:
filepaths.append(path)
return filepaths
Somehow feels more manageable overall instead of having the logic spread out as much as it is now?
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.
seems like we are trying to convert ptex textures with maketx
Message:
Command 'D:\REPO\OpenPype\vendor\bin\oiio\windows\maketx.exe -v -u --unpremult --checknan --oiio --filter lanczos3 -sattrib sourceHash hdrenv,ptx|1262811026,0|5962|maketx -o C:\Users\murph\AppData\Local\Temp\pyblish_tmp_0rtzfbjq\resources\hdrenv.tx D:\REPO_LIB\PIXAR\hdrenv.ptx' returned non-zero exit status 1.
Line:
512
Traceback:
Traceback (most recent call last):
File "D:\REPO\OpenPype\.venv\lib\site-packages\pyblish\plugin.py", line 522, in __explicit_process
runner(*args)
File "D:\REPO\OpenPype\openpype\hosts\maya\plugins\publish\extract_look.py", line 226, in process
File "D:\REPO\OpenPype\openpype\hosts\maya\plugins\publish\extract_look.py", line 363, in process_resources
File "D:\REPO\OpenPype\openpype\hosts\maya\plugins\publish\extract_look.py", line 508, in _process_texture
File "D:\REPO\OpenPype\openpype\hosts\maya\plugins\publish\extract_look.py", line 95, in maketx
File "C:\Program Files\Autodesk\Maya2022\Python37\lib\subprocess.py", line 411, in check_output
**kwargs).stdout
File "C:\Program Files\Autodesk\Maya2022\Python37\lib\subprocess.py", line 512, in run
output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command 'D:\REPO\OpenPype\vendor\bin\oiio\windows\maketx.exe -v -u --unpremult --checknan --oiio --filter lanczos3 -sattrib sourceHash hdrenv,ptx|1262811026,0|5962|maketx -o C:\Users\murph\AppData\Local\Temp\pyblish_tmp_0rtzfbjq\resources\hdrenv.tx D:\REPO_LIB\PIXAR\hdrenv.ptx' returned non-zero exit status 1.
That is really for another PR - there is an option to disable maketx on look so you shouldn't publish renderman look with that option turned on - also there is ongoing effort to implement these renderer specific conversion tools (#2971) already and renderman owns should be implemented too |
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.
After manually switching off the maketx option on look instances in Maya the publishing works correctly.
Reapplied the looks in lighting and everything seems correct.
As mentioned already, we should redesign the renderers support to better define each renderer options, texture conversions, color management and other specifics for each individually.
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.
node_path = get_file_node_path(node).lower() | ||
|
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.
Removing this introduced a bug where e.g. <UDIM>
in the path wouldn't be recognized.
Feature
This is adding support for renderman shaders and file nodes in look processing.
Testing:
Create look with PxrDiffuse shader and PxrTexture node. It should be correctly collected and published.
cmds.getAttr()
you'll get correct result, but UI is updated only when you close and re-open Hypershade. This is main concern for testing because when you load such look in the same session of Maya, you'll get the feeling that texture path will still point to its original location.Close #3189