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

Adsk Contrib - Add preliminary OSL support #1462

Conversation

hodoulp
Copy link
Member

@hodoulp hodoulp commented Aug 19, 2021

Signed-off-by: Patrick Hodoul Patrick.Hodoul@autodesk.com

The pull request provides the first step of the native implementation for the OSL translation. At the high level the translation is implemented like any other translations in OCIO in order to keep the single GPU implementation even if some GPU code changes were still needed to accommodate the OSL vs. HLSL/GLSL differences/limitations.

Refer to #1435 for some details / discussions.

That's a major milestone for OpenColorIO as it enhances potential integrations like in MaterialX environment for example (as discussed in the issue). On the other hand, the OSL library could also benefit from this work by improving color management support in some of the transform methods for example.

The OSL translation works for all the existing ops without too many changes compare to the previous writing of the GPU algorithm. So, contributors should not be lost when jumping in the GPU code. The limitations are that the Lut 1D and 3D ops which are still not supported because of the Lut data exchange using OSL (i.e. no easy technical solution for now), and the dynamic properties which are only 'local' variables for short-term. But OSL supports input parameters to any shader types so it can be implemented at some point.

There is now a new unit test framework to support writing unit tests for the OSL translation with two basic unit tests. This framework only supports the OSL shader compilation and lake of 'easy' unit test writing for now. The idea is to slowly enhance the framework and the number of unit tests.

Lastly, the findOpenShadingLanguage() method only works with the OpenShadingLanguage_ROOT cmake variable (i.e. no auto discovery). Once again, that will be improve over the time to match the other find method capabilities.

This PR also does some clean-up of inconsistent usage of the GPUShaderUtils.

Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
@hodoulp
Copy link
Member Author

hodoulp commented Aug 19, 2021

The generated OSL shader currently processes pixel per pixel using the color4 OSL type for input and output parameters. For now, the only validation is a successfully compilation (using oslcomp library).

Note: The implementation can contain global variables and methods so, I've encapsulated them in the shader method itself (hoping they will be private).

Below is the template of the OSL shader generated by OpenColorIO.

/* All the includes */
  
#include "vector4.h"
#include "color4.h"

/* All the generic helper methods */

vector4 __operator__mul__(vector4 v, matrix m)
{
  return vector4(v.x * m[0][0] + v.y * m[1][0] + v.z * m[2][0] + v.w * m[3][0], v.x * m[0][1] + v.y * m[1][1] + v.z * m[2][1] + v.w * m[3][1], v.x * m[0][2] + v.y * m[1][2] + v.z * m[2][2] + v.w * m[3][2], v.x * m[0][3] + v.y * m[1][3] + v.z * m[2][3] + v.w * m[3][3]);
}

vector4 __operator__mul__(color4 c, vector4 v)
{
  return vector4(c.rgb.r, c.rgb.g, c.rgb.b, c.a) * v;
}

vector4 __operator__mul__(vector4 v, color4 c)
{
  return v * vector4(c.rgb.r, c.rgb.g, c.rgb.b, c.a);
}

vector4 __operator__sub__(color4 c, vector4 v)
{
  return vector4(c.rgb.r, c.rgb.g, c.rgb.b, c.a) - v;
}

vector4 __operator__add__(vector4 v, color4 c) {
  return v + vector4(c.rgb.r, c.rgb.g, c.rgb.b, c.a);
}

vector4 __operator__add__(color4 c, vector4 v) {
  return vector4(c.rgb.r, c.rgb.g, c.rgb.b, c.a) + v;
}

vector4 pow(color4 c, vector4 v) {
  return pow(vector4(c.rgb.r, c.rgb.g, c.rgb.b, c.a), v);
}

vector4 max(vector4 v, color4 c) {
  return max(v, vector4(c.rgb.r, c.rgb.g, c.rgb.b, c.a));
}

/* The shader implementation */

shader OSL_OCIOMain(color4 inColor = {color(0), 1}, output color4 outColor = {color(0), 1})
{
    // Declaration of all helper methods

    <..>

    // Declaration of the OCIO shader function

    color4 OCIOMain(color4 inPixel)
    {
        color4 myColor = inPixel;

        <.. some color transformation code here ..>

        return myColor;
    }

    outColor = OCIOMain(inColor);
}


@hodoulp
Copy link
Member Author

hodoulp commented Aug 20, 2021

During the 2021-08-19 OSL TSC meeting, I mentioned the OSL translation status and expressed the challenges to write the unit test framework. The only examples I found were complex, and contain many useless capabilities for this context. So, @lgritz proposed some 'easier' examples to improve/complete the OSL unit test framework.

Patrick has submitted a PR to OCIO where it can generate OSL source as a string to implement any color transformation supported by OCIO.
For unit tests, LG suggests using either the shade_image() (see end of oslexec.h) or modeling a simple test harness off of the code in testsuite/example-deformer as examples. Of course, testshade is also a possibility, but doing in C++ may end up with faster unit tests than constantly shelling out to testshade hundreds of times.

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable to me.

I would love to see you not use FindOpenShadingLanguage.cmake, and instead rely on our exported OSLConfig.cmake.

I'm a little concerned about what will happen when we eventually have built-in types for color4/vector4, and I wonder if all the notation will 100% work when we switch to built-in times (for example, I would expect c4.r to work, whereas now you have to awkwardly say c4.rgb.r). Maybe we shouldn't be too concerned, because it's not going to happen especially soon. Maybe I'll just have to ensure that this notation still works. But maybe it's worth building in a little bit of logic to designate this as OSL 1.x, or at least to somehow distinguish the before/after we add the new types?

src/OpenColorIO/GpuShaderDesc.cpp Outdated Show resolved Hide resolved
@hodoulp
Copy link
Member Author

hodoulp commented Aug 25, 2021

I'm a little concerned about what will happen when we eventually have built-in types for color4/vector4, and I wonder if all the notation will 100% work when we switch to built-in times (for example, I would expect c4.r to work, whereas now you have to awkwardly say c4.rgb.r). Maybe we shouldn't be too concerned, because it's not going to happen especially soon. Maybe I'll just have to ensure that this notation still works. But maybe it's worth building in a little bit of logic to designate this as OSL 1.x, or at least to somehow distinguish the before/after we add the new types?

I changed the OSL enum to be LANGUAGE_OSL_1 so any later OSL syntax (not backward compatible) changes can be handled correctly.

Concerning the OSL syntax, I suggest to first make c4.r working to ease the readability of any OSL code. But it would be great if the syntax can be backward compatible (i.e. supporting both accesses c4.rgb.r & c4.r).

@hodoulp
Copy link
Member Author

hodoulp commented Aug 25, 2021

Below is the command to 'enable' the OSL translation in OCIO (using OSL from master branch):
cmake -GNinja -DOpenShadingLanguage_ROOT=/Users/hodoulp/dev/osl_1/install_rls -DCMAKE_CXX_STANDARD=14 ../.

Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
@michdolan
Copy link
Collaborator

Looks great @hodoulp !

hodoulp and others added 2 commits August 25, 2021 17:30
@hodoulp
Copy link
Member Author

hodoulp commented Aug 25, 2021

The last commit greatly improves the OSL unit test framework by adding extra error information in case of compilation problem and greatly simplifying how to write an OSL unit test.

~/dev/ocio_publ_1/build_rls (adsk_contrib/add_osl_support) $ ctest -V -R test_osl
UpdateCTestConfiguration  from :/Users/hodoulp/dev/ocio_publ_1/build_rls/DartConfiguration.tcl
UpdateCTestConfiguration  from :/Users/hodoulp/dev/ocio_publ_1/build_rls/DartConfiguration.tcl
Test project /Users/hodoulp/dev/ocio_publ_1/build_rls
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 4
    Start 4: test_osl

4: Test command: /Users/hodoulp/dev/ocio_publ_1/build_rls/tests/osl/test_osl_exec
4: Environment variables: 
4:  SHADERS_DIR=/Users/hodoulp/dev/osl_1/install_rls/share/OSL/shaders
4: Test timeout computed to be: 10000000
4: 
4:  OpenColorIO_Core_OSL_Unit_Tests
4: 
4: [  1/7] [Exponent / value                                            ] - PASSED
4: [  2/7] [FixedFunction / aces_red_mod_03                             ] - PASSED
4: [  3/7] [Matrix / offset_only                                        ] - PASSED
4: [  4/7] [Matrix / diagonal                                           ] - PASSED
4: [  5/7] [Range / identity                                            ] - PASSED
4: [  6/7] [Range / scale                                               ] - PASSED
4: [  7/7] [Range / scale_and_clamp                                     ] - PASSED
4: 
4: 0 tests failed
4: 
1/1 Test #4: test_osl .........................   Passed    0.16 sec

The following tests passed:
	test_osl

100% tests passed, 0 tests failed out of 1

Total Test time (real) =   0.16 sec

@hodoulp
Copy link
Member Author

hodoulp commented Aug 26, 2021

The last commit adds more OSL unit tests and changes the cmake variable names.

Below is now the command to 'enable' the OSL translation in OCIO (using OSL from master branch):
cmake -GNinja -DOSL_ROOT=/Users/hodoulp/dev/osl_1/install_rls -DCMAKE_CXX_STANDARD=14 ../.

Below is the test coverage.

~/dev/ocio_publ_1/build_rls (adsk_contrib/add_osl_support) $ ctest -V -R test_osl
UpdateCTestConfiguration  from :/Users/hodoulp/dev/ocio_publ_1/build_rls/DartConfiguration.tcl
UpdateCTestConfiguration  from :/Users/hodoulp/dev/ocio_publ_1/build_rls/DartConfiguration.tcl
Test project /Users/hodoulp/dev/ocio_publ_1/build_rls
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 4
    Start 4: test_osl

4: Test command: /Users/hodoulp/dev/ocio_publ_1/build_rls/tests/osl/test_osl_exec
4: Environment variables: 
4:  SHADERS_DIR=/Users/hodoulp/dev/osl_1/install_rls/share/OSL/shaders
4: Test timeout computed to be: 10000000
4: 
4:  OpenColorIO_Core_OSL_Unit_Tests
4: 
4: [  1/20] [Exponent / value                                            ] - PASSED
4: [  2/20] [FixedFunction / aces_red_mod_03                             ] - PASSED
4: [  3/20] [FixedFunction / aces_red_mod_03_inv                         ] - PASSED
4: [  4/20] [FixedFunction / aces_red_mod_10                             ] - PASSED
4: [  5/20] [FixedFunction / aces_red_mod_10_inv                         ] - PASSED
4: [  6/20] [FixedFunction / aces_glow_03                                ] - PASSED
4: [  7/20] [FixedFunction / aces_glow_03_inv                            ] - PASSED
4: [  8/20] [FixedFunction / aces_glow_10                                ] - PASSED
4: [  9/20] [FixedFunction / aces_glow_10_inv                            ] - PASSED
4: [ 10/20] [FixedFunction / aces_dark_to_dim_10                         ] - PASSED
4: [ 11/20] [FixedFunction / aces_dark_to_dim_10_inv                     ] - PASSED
4: [ 12/20] [FixedFunction / aces_gamut_map_13                           ] - PASSED
4: [ 13/20] [FixedFunction / aces_gamut_map_13_inv                       ] - PASSED
4: [ 14/20] [FixedFunction / rec2100_surround                            ] - PASSED
4: [ 15/20] [FixedFunction / rec2100_surround_inv                        ] - PASSED
4: [ 16/20] [Matrix / offset_only                                        ] - PASSED
4: [ 17/20] [Matrix / diagonal                                           ] - PASSED
4: [ 18/20] [Range / identity                                            ] - PASSED
4: [ 19/20] [Range / scale                                               ] - PASSED
4: [ 20/20] [Range / scale_and_clamp                                     ] - PASSED
4: 
4: 0 tests failed
4: 
1/1 Test #4: test_osl .........................   Passed    1.27 sec

The following tests passed:
	test_osl

100% tests passed, 0 tests failed out of 1

Total Test time (real) =   1.28 sec

@michdolan
Copy link
Collaborator

michdolan commented Aug 30, 2021

We need to merge this today to release OCIO 2.1.0 in time for VFX Reference Platform 2022. Technically our project process rules require waiting an additional 3 days to merge without a second approval from outside the contributor's organization, but under the circumstances we need to forge ahead sooner. If there are any objections, please comment by today at 5pm EST. Thank you.

Copy link
Collaborator

@doug-walker doug-walker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per TSC and Slack discussion, approving now.

@michdolan michdolan merged commit c84e164 into AcademySoftwareFoundation:master Aug 30, 2021
@hodoulp hodoulp deleted the adsk_contrib/add_osl_support branch October 14, 2021 17:37
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

Successfully merging this pull request may close these issues.

None yet

4 participants