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

Add support for OpenGL ES as a shading language #1497

Merged
merged 3 commits into from
Sep 27, 2021

Conversation

amyspark
Copy link
Contributor

This commit adds two new entries to GpuLanguage,
GPU_LANGUAGE_GLSL_ES_1_0 and GPU_LANGUAGE_GLSL_ES_3_0.

The only meaningful differences w.r.t. stock OpenGL are:

  • the 1D texture optimization isn't applied to ES, as they are not
    supported at all;

  • the textureD() calls are replaced in GLSL ES 3 by a call to
    texture().

Fixes #1486

@hodoulp
Copy link
Member

hodoulp commented Sep 20, 2021

@amyspark that's great. Thanks a lot for the help.
Did you test the pull request with Krita ?

@amyspark
Copy link
Contributor Author

@hodoulp Yeah! The legacy GPU pipeline runs perfectly with OpenGL ES 3 under Angle, there is no need anymore for the texture<N>D defines. I'm still to test the new GPU pipeline, though, I got detoured by #1496.

@amyspark
Copy link
Contributor Author

amyspark commented Sep 20, 2021

@hodoulp status update, the new GPU pipeline returns completely scrambled results under OpenGL ES:

Legacy: New (and ba52937 reverted)
Captura de pantalla 2021-09-20 164327 Captura de pantalla 2021-09-20 164607

With and without your 1D LUT optimization it still showed the same glitched appearance, so I'm guessing there's an encoding that's not being properly skipped? Do you know where I could have a deeper look at?

UPD: A note for future reference, 1D textures also use GL_R32F (single-channel) format, which is not available until OpenGL ES 3.2. I'm looking into possible alternatives.

@amyspark
Copy link
Contributor Author

Upon further debugging, I believe the 1D LUT codepath may have a serious bug. I've tested with various input profiles from the ACES configuration and none of them actually trigger the tex1d codepath; however, every profile that does involve a 1D LUT results in those clamp-looking glitches. I've checked with both our OpenGL 4.x pipeline and through Angle, both against current master.

@amyspark amyspark marked this pull request as draft September 21, 2021 01:05
@remia
Copy link
Collaborator

remia commented Sep 21, 2021

Upon further debugging, I believe the 1D LUT codepath may have a serious bug. I've tested with various input profiles from the ACES configuration and none of them actually trigger the tex1d codepath; however, every profile that does involve a 1D LUT results in those clamp-looking glitches. I've checked with both our OpenGL 4.x pipeline and through Angle, both against current master.

Did you made any progress on this @amyspark ? You could try to reproduce the issue in ociodisplay which use the v2 GPU pipeline by default.

@amyspark
Copy link
Contributor Author

@remia, no, I haven't made any further progress on this. I hadn't thought of ociodisplay because it doesn't get built with our current configuration (we don't use OpenImageIO in Krita).

@hodoulp
Copy link
Member

hodoulp commented Sep 22, 2021

Another option is to use ocioconvert (but you still need OpenImageIO) to compare CPU vs GPU legacy vs GPU converted images. Otherwise, you can provide us with the LUT plus the image so we can test on our side for OpenGL GLSL 1.3 at least.

~/dev/ocio_2/build_rls (master) $ ./src/apps/ocioconvert/ocioconvert --help
ocioconvert -- apply colorspace transform to an image 

usage: ocioconvert [options]  inputimage inputcolorspace outputimage outputcolorspace
   or: ocioconvert [options] --lut lutfile inputimage outputimage
   or: ocioconvert [options] --view inputimage inputcolorspace outputimage displayname viewname


Options:
    --lut                  Convert using a LUT rather than a config file
    --view                 Convert to a (display,view) pair rather than to an output color space
    --gpu                  Use GPU color processing instead of CPU (CPU is the default)
    --gpulegacy            Use the legacy (i.e. baked) GPU color processing instead of the CPU one (--gpu is ignored)
    --gpuinfo              Output the OCIO shader program
    --help                 Print help message
    -v                     Display general information

This commit adds two new entries to `GpuLanguage`,
`GPU_LANGUAGE_GLSL_ES_1_0` and `GPU_LANGUAGE_GLSL_ES_3_0`.

The only meaningful differences w.r.t. stock OpenGL are:

- the 1D texture optimization isn't applied to ES, as they are not
  supported at all;

- the texture<N>D() calls are replaced in GLSL ES 3 by a call to
  texture().

Fixes AcademySoftwareFoundation#1486

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
@amyspark
Copy link
Contributor Author

@remia was spot on; it turns out I was using the legacy pipeline's edgelen value for the texture size, and not the one returned by OCIO. (For context: we cannot use the classes in oglapphelpers because Krita uses Qt's OpenGL facilities, so I ported the calls we needed to our shader code.)

For verification:

OpenGL OpenGL ES (Angle)
Captura de pantalla 2021-09-21 222916 Captura de pantalla 2021-09-21 223013

@amyspark amyspark marked this pull request as ready for review September 22, 2021 01:42
Copy link
Member

@hodoulp hodoulp left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.

include/OpenColorIO/OpenColorTypes.h Show resolved Hide resolved
src/libutils/oglapphelpers/glsl.cpp Outdated Show resolved Hide resolved
Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
@amyspark
Copy link
Contributor Author

@hodoulp, should I add support for using the classes in oglapphelpers with GLES?

@hodoulp
Copy link
Member

hodoulp commented Sep 23, 2021

@amyspark Yes, that would be interesting to have oglapphelpers with GL ES support. For long-term, the most important would be to have OpenGL ES unit tests like []/tests/gpu. But this pull request could be merged as-is for short-term because it's already a major improvement for OpenColorIO. Let me know what you plan to do so, I can approve or wait for the next commit.

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
@amyspark
Copy link
Contributor Author

@hodoulp, it seems CMake doesn't provide a Find module for the ES equivalent of FindOpenGL, and adding support proper to the app helpers needs to re-detect from the ground up the needed dependencies.

It'll be certainly interesting! but for another MR in the future. For now, I'll wrap it up here 😃

@hodoulp hodoulp merged commit 83fd833 into AcademySoftwareFoundation:master Sep 27, 2021
hodoulp pushed a commit that referenced this pull request Sep 27, 2021
* Add support for OpenGL ES as a shading language

This commit adds two new entries to `GpuLanguage`,
`GPU_LANGUAGE_GLSL_ES_1_0` and `GPU_LANGUAGE_GLSL_ES_3_0`.

The only meaningful differences w.r.t. stock OpenGL are:

- the 1D texture optimization isn't applied to ES, as they are not
  supported at all;

- the texture<N>D() calls are replaced in GLSL ES 3 by a call to
  texture().

Fixes #1486

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>

* GLSL ES: remove ABI break

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>

* OpenGLBuilder: use switch to check GLSL version

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
hodoulp pushed a commit that referenced this pull request Sep 27, 2021
* Add support for OpenGL ES as a shading language

This commit adds two new entries to `GpuLanguage`,
`GPU_LANGUAGE_GLSL_ES_1_0` and `GPU_LANGUAGE_GLSL_ES_3_0`.

The only meaningful differences w.r.t. stock OpenGL are:

- the 1D texture optimization isn't applied to ES, as they are not
  supported at all;

- the texture<N>D() calls are replaced in GLSL ES 3 by a call to
  texture().

Fixes #1486

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>

* GLSL ES: remove ABI break

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>

* OpenGLBuilder: use switch to check GLSL version

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
hodoulp added a commit that referenced this pull request Oct 6, 2021
* Add support for OpenGL ES as a shading language

This commit adds two new entries to `GpuLanguage`,
`GPU_LANGUAGE_GLSL_ES_1_0` and `GPU_LANGUAGE_GLSL_ES_3_0`.

The only meaningful differences w.r.t. stock OpenGL are:

- the 1D texture optimization isn't applied to ES, as they are not
  supported at all;

- the texture<N>D() calls are replaced in GLSL ES 3 by a call to
  texture().

Fixes #1486

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>

* GLSL ES: remove ABI break

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>

* OpenGLBuilder: use switch to check GLSL version

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Co-authored-by: amyspark <13498015+amyspark@users.noreply.github.com>
@amyspark amyspark deleted the amyspark/opengles branch October 8, 2021 23:08
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.

Support OpenGL ES
3 participants