Skip to content
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

Broken OSL texture colorspace in 1.38.10 #2127

Open
AdrienHerubel opened this issue Dec 2, 2024 · 1 comment
Open

Broken OSL texture colorspace in 1.38.10 #2127

AdrienHerubel opened this issue Dec 2, 2024 · 1 comment

Comments

@AdrienHerubel
Copy link

It seems that after #1757 the color space of a file is not passed to the shadernode input anymore. This breaks OSL renderers relying on having the color space attribute passed to the texture call for fully-featured OCIO transforms and filtering aware color conversion.

The following simple patch restores the functionality but might cause a double conversion if a user is using emitted transforms on top of the OSL texture color transform. Maybe it would be better to not optionally not emit color transform for files when using the OSL generator by default ?

diff --git a/source/MaterialXGenShader/ShaderGraph.cpp b/source/MaterialXGenShader/ShaderGraph.cpp
index b95a9471..c2d73755 100644
--- a/source/MaterialXGenShader/ShaderGraph.cpp
+++ b/source/MaterialXGenShader/ShaderGraph.cpp
@@ -654,6 +654,8 @@ void ShaderGraph::applyInputTransforms(ConstNodePtr node, ShaderNodePtr shaderNo
                     }
                 }
 
+                ShaderInput* shaderInput = shaderNode->getInput(input->getName());
+                shaderInput->setColorSpace(sourceColorSpace);
                 ShaderOutput* shaderOutput = shaderNode->getOutput();
                 populateColorTransformMap(colorManagementSystem, shaderOutput, sourceColorSpace, targetColorSpace, false);
                 populateUnitTransformMap(unitSystem, shaderOutput, input, targetDistanceUnit, false);
@kwokcb
Copy link
Contributor

kwokcb commented Dec 4, 2024

From what I recall it is either the transform is inserted into the shader code or the "colorspace" metadata is added to the texture call.

If it's possible to get double transforms than I agree that by default only one should be allowed with "colorspace" being emitted in the texture call by default.

I think when this was added OSL had not added support for this and this only worked for Arnold, thus the case may have not been testable with testrender as part of unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants