-
Notifications
You must be signed in to change notification settings - Fork 202
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
MAYA-129798 - Use experimental OCIO hooks for MaterialX shading #3171
Conversation
MATERIALX_NAMESPACE_BEGIN | ||
|
||
namespace { | ||
// Internal OCIO strings: |
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.
Familiarity with the internal structure of an OGS fragment is recommended. You can use lib\mayaUsd\render\vp2ShaderFragments\Float3ToFloatX.xml
as reference.
} | ||
|
||
void addOCIONodeDef(DocumentPtr lib, const pugi::xml_document& doc, const std::string& output_type) | ||
{ |
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.
Add the NodeDef and Implementation in the library, allowing us to use them in our internal graphs.
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 file has UsdPreviewSurface and MaterialX textures in 7 different color spaces. We want all squares to be color managed identically.
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.
Necessary to split this reference image because of a small but important difference between the old color correction and the OCIO code. The OCIO code will clamp the colors between 0 and 1 before doing any computations.
The triplanar projection used on the rightmost sphere of the top row can generate colors with out of range components. This causes visible differences when using OCIO.
A fix for incorrect triplanar blending has been submitted to MaterialX by Chris Rydalch, so we will need to revisit these reference images in future MaterialX updates.
@@ -87,16 +87,32 @@ def testMayaSurfaces(self): | |||
cmds.file(force=True, new=True) | |||
cmds.move(6, -6, 6, 'persp') | |||
cmds.rotate(60, 0, 45, 'persp') | |||
self._StartTest('MayaSurfaces') |
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.
Copied code from _StartTest to help keep this test clean.
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.
Config copied from ACES, but with additional file rules that catches the naming convention used below.
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.
Standard color paletted used for calibration. Contains multiple midtones that help make sure the color management provides correct response.
All images below were generated in sRGB and converted to a different source color space using OCIO-aware OIIO tools.
for (auto&& property : ocioDoc.child(TAG_FRAGMENT).child(TAG_PROPERTIES).children()) { | ||
if (std::strncmp(property.name(), TAG_TEXTURE2, TAG_TEXTURE2_LEN) == 0) { | ||
std::string samplerName = property.attribute(ATTR_NAME).as_string(); | ||
samplerName += "Sampler"; |
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.
Preserving the name to make sure Maya can load the LUTs for the more complex color management shaders.
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.
Looks good
@seando-adsk ready for merge. |
No description provided.