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

Replace GLX dependency with EGL dependency #1039

Closed
ChinYing-Li opened this issue Jun 17, 2020 · 12 comments
Closed

Replace GLX dependency with EGL dependency #1039

ChinYing-Li opened this issue Jun 17, 2020 · 12 comments

Comments

@ChinYing-Li
Copy link
Contributor

ChinYing-Li commented Jun 17, 2020

Quoting @jfpanisset :
"" Currently OpenColorIO uses GLUT in tests/gpu/GPUUnitTest.cpp to create a
window to render OpenGL tests, which in turn depends on GLX. I'm assuming
these days it's linking against FreeGLUT, and it is apparently possible to use
EGL instead of GLX for creating the rendering context. The advantage is that it
might be possible to do away with having to stand up an X server for the CI
pipeline, and whereas EGL rendering is supported inside NVIDIA containers
(with built-in support starting in Docker 19.03), GLX is not. "

@ChinYing-Li
Copy link
Contributor Author

In fact, freeglut also seems to support headless rendering but has to go through a X server.

@ChinYing-Li
Copy link
Contributor Author

Hi, a bit of guidance needed. In order to support headless rendering, I refactoredOglApp, which is now a base class, from which derived ScreenApp and HeadlessApp.
This means that wherever OCIO::OglApp & app is passed as a function parameter, I have to substitute it with OCIO::OglAppRcPtr. (typedef OCIO_SHARED_PTR<OglApp> OglAppRcPtr)
So far, I don't see if there would be any drawbacks of this approach, and would like others' input on my approach. Thanks in advance.

@hodoulp
Copy link
Member

hodoulp commented Jun 22, 2020

Note: Discussion started in #547

@hodoulp
Copy link
Member

hodoulp commented Jun 22, 2020

Hi,

The approach seems to be the right one, and most of the code is already using a OCIO::OglAppRcPtr so the transition should be fine.

However, EGL is nVidia specific and OCIO must not stick to a specific vendor. Even if the CI build system is running on nVidia graphic cards that should not be mandatory for our users (e.g. using an app to debug or investigate a color transformation) & contributors (e.g. running GPU unit tests on their machine). How do you plan to handle that?

Ideally the code should detect the graphic card vendor and auto adjust itself to the right 'headless implementation' (i.e. perhaps using glGetString(GL_VENDOR)?).

Here is a list of apps needing oglApp, with a transition proposal:

  • ociochecklut -> HeadlessApp
  • ocioconvert -> HeadlessApp
  • ociodisplay -> ScreenApp
  • ocioperf -> HeadlessApp
  • GPUHelper -> HeadlessApp, that's the use case to handle.

@ChinYing-Li
Copy link
Contributor Author

Thank you for the input. My work-around for the vendor specific problem is to use the EGL implementation of Mesa, which should be available on most if not all Linux machines.

@hodoulp
Copy link
Member

hodoulp commented Jun 22, 2020

@doug-walker
Copy link
Collaborator

@ChinYing-Li, thanks for your work on this!

I'm worried about relying on Mesa for non-Nvidia hardware. We have tried using Mesa several times over the years and it has resulted in large numbers of GPU unit test failures. Perhaps Mesa would work better now, but if not, we would need to find another solution. It is definitely a requirement that the GPU unit tests continue to run on AMD/ATI hardware (which apparently don't have EGL support yet?).

@jfpanisset
Copy link
Contributor

These Blender threads seem to point at recent efforts to bring back EGL headless rendering, as well as comments about needing to request "Core" rather than "Compatibility" profiles from the driver.

https://developer.blender.org/T54638
https://developer.blender.org/D6585

So perhaps some useful info to be mined there?

@ChinYing-Li
Copy link
Contributor Author

ChinYing-Li commented Jun 25, 2020

System requirement: Linux

Steps are as follows:

  1. Install Mesa with EGL backend

  2. Get findEGL.cmake

git clone git://anongit.kde.org/extra-cmake-modules
cd extra-cmake-modules
mkdir build
cd build
cmake .. # or run : cmake -DCMAKE_INSTALL_PREFIX=/usr .. &&
make
sudo make install

This will install the extra-cmake-module module, which contains findEGL.cmake.

  1. Build OpenColorIO with the option OCIO_USE_HEADLESS
    Remember to add the line below at the sudo cmake step.
-D OCIO_USE_HEADLESS=ON

@ChinYing-Li
Copy link
Contributor Author

Thanks for all the pointers! Interestingly, I did not encounter the issue mentioned in the blender posts.

Result of GPU Unit tests

cyli@cyli-MacBookAir:~/repo/OCIO_exp_build/tests/gpu$ ./test_gpu_exec
GPU unit tests used N19OpenColorIO_v2_0dev11HeadlessAppE

GL Vendor:    Intel Open Source Technology Center
GL Renderer:  Mesa DRI Intel(R) HD Graphics 5000 (HSW GT3)
GL Version:   3.0 Mesa 20.0.4
GLSL Version: 1.30

EGL Vendor:    Mesa Project
EGL Version:   1.4

OpenColorIO_Core_GPU_Unit_Tests

# test results ommitted 
#    .
#    .
#    .

[145/147] [RangeOp / arbitrary_1_no_clamp                    ] - PASSED - (MaxDiff: 1.19209e-07 at pix[43709][0])
[146/147] [RangeOp / arbitrary_2                             ] - PASSED - (MaxDiff: 1.19209e-07 at pix[29207][1])
[147/147] [RangeOp / arbitrary_2_no_clamp                    ] - PASSED - (MaxDiff: 4.76837e-07 at pix[58016][2])

0 tests failed

@hodoulp
Copy link
Member

hodoulp commented Jun 25, 2020

extra-cmake-modules

Other pull requests (#1016) are moving cmake version from 3.10 to 3.12, would that be enough for findEGL?

@ChinYing-Li
Copy link
Contributor Author

@hodoulp
Sorry that I have to delete the pull-request since I accidentally signed-off other people's commits and could not revert them.
To answer your first question:
Note 1: You can see build breaks with github actions by clicking on 'details' corresponding to the failing build.
The break on OSX is because glut header is not included.

hodoulp pushed a commit that referenced this issue Jul 9, 2020
* Modified CMakeLists.txt

Signed-off-by: ChinYing-Li <[email protected]>

* Support headless rendering in Linux build with EGL (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Fix CMakeLists bugs
Made OglApp's destructor virtual

Signed-off-by: ChinYing-Li <[email protected]>

* Remove bugs in app's CMakeLists (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Modified CMakeLists and add factory function for OglAppRcPtr (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Use imported target to find EGL (#1039)
Change the CI workflow to build with headless option

Signed-off-by: ChinYing-Li <[email protected]>

* Modify CMakeLists to properly link EGL (#1039)

Remove unused variables (#1039)

Include glext.h and debug print (#1039)

Check GLEW initialization (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Add debug print for HeadlessApp initialization (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Modify CMakeLists to accomodate system that support GLVND (#1039)

Remove unused variables (#1039)

Define GLEW_EGL preprocessor for NVidia implementation (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Fix CMakeLists (#1039)
Add the factory method for creating OglAppRcPtr

Modify CMakeLists (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Rename the factory method OglApp::CreateOglApp (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Change workflow to check the GL vendor of CI Linux build (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Add proper mechanism to detect GLVND support in CmakeLists (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Reformat the code (#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Turn off GPU unit test in CI (#1039)

Signed-off-by: ChinYing-Li <[email protected]>
michdolan pushed a commit to michdolan/OpenColorIO that referenced this issue Jul 13, 2020
* Modified CMakeLists.txt

Signed-off-by: ChinYing-Li <[email protected]>

* Support headless rendering in Linux build with EGL (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Fix CMakeLists bugs
Made OglApp's destructor virtual

Signed-off-by: ChinYing-Li <[email protected]>

* Remove bugs in app's CMakeLists (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Modified CMakeLists and add factory function for OglAppRcPtr (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Use imported target to find EGL (AcademySoftwareFoundation#1039)
Change the CI workflow to build with headless option

Signed-off-by: ChinYing-Li <[email protected]>

* Modify CMakeLists to properly link EGL (AcademySoftwareFoundation#1039)

Remove unused variables (AcademySoftwareFoundation#1039)

Include glext.h and debug print (AcademySoftwareFoundation#1039)

Check GLEW initialization (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Add debug print for HeadlessApp initialization (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Modify CMakeLists to accomodate system that support GLVND (AcademySoftwareFoundation#1039)

Remove unused variables (AcademySoftwareFoundation#1039)

Define GLEW_EGL preprocessor for NVidia implementation (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Fix CMakeLists (AcademySoftwareFoundation#1039)
Add the factory method for creating OglAppRcPtr

Modify CMakeLists (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Rename the factory method OglApp::CreateOglApp (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Change workflow to check the GL vendor of CI Linux build (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Add proper mechanism to detect GLVND support in CmakeLists (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Reformat the code (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>

* Turn off GPU unit test in CI (AcademySoftwareFoundation#1039)

Signed-off-by: ChinYing-Li <[email protected]>
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

4 participants