igl | android | Support sRGB framebuffer on OpenGLES.

[vinsentli]

[francoiscoulombe]

Hi, while I believe you that you get different results, why do you feel this is the right fix?

can you provide screenshots of the differences just to make sure that we’re talking about the same thing?

the sRGB frame buffer might not be very well tested on Android phones

[vinsentli]

@francoiscoulombe You can check out these screenshots

Header	Header	Header
OpenGL BGRA_SRGB & RGBA_UNorm8	Vulkan before : BGRA_SRGB	Vulkan now :RGBA_UNorm8

[francoiscoulombe]

thanks for the screenshots.

for what it's worth, the center one is the correct colors

I don't know why sending a BGRA_SRGB to OpenGL ES isn't resulting in an image that is more like the center one.

I'm more worried about the swapchain color space changes which are separate from this

[francoiscoulombe]

to verify set your display into srgb profile and sample the pixel to compare with the value written on the image.

[vinsentli]

Indeed, the color in the middle image is correct.

[vinsentli]

I will continue researching the reasons behind the incorrect colors in OpenGL.

[vinsentli]

@francoiscoulombe I have resolved it.

[francoiscoulombe]

What was it?

[vinsentli]

We need create a sRGB surface in GLSurfaceView.

The prerequisite is that the device needs to support the EGL_KHR_gl_colorspace extension.

setEGLWindowSurfaceFactory(new SurfaceFactory(getHolder()));

private static class SurfaceFactory implements GLSurfaceView.EGLWindowSurfaceFactory{
    private SurfaceHolder surfaceHolder_;

    public SurfaceFactory(SurfaceHolder surfaceHolder){
      surfaceHolder_ = surfaceHolder;
    }

    final int EGL_GL_COLORSPACE_KHR = 0x309D;
    final int EGL_GL_COLORSPACE_SRGB_KHR = 0x3089;
    final int EGL_GL_COLORSPACE_LINEAR_KHR        =    0x308A;

    final int[] configAttribs = {
            EGL_GL_COLORSPACE_KHR ,EGL_GL_COLORSPACE_SRGB_KHR,
            EGL10.EGL_NONE
    };

    @Override
    public EGLSurface createWindowSurface(EGL10 egl10, EGLDisplay eglDisplay, EGLConfig eglConfig, Object o) {
      return egl10.eglCreateWindowSurface(eglDisplay, eglConfig, surfaceHolder_, configAttribs);
    }

    @Override
    public void destroySurface(EGL10 egl10, EGLDisplay eglDisplay, EGLSurface eglSurface) {
      egl10.eglDestroySurface(eglDisplay, eglSurface);
    }
  }

[francoiscoulombe]

Sweet!
on Vulkan, you only need the colorspace extension to do a color space that is not nonlinear srgb. (Like pass through)

nonlinear srgb “should” work pretty much everywhere nowadays.

[vinsentli]

That's correct.

[vinsentli]

@francoiscoulombe @corporateshark
After the pull request #199 is merged, I will create a new pull request based on the latest OpenGL SampleView to fix the colorspace issue in OpenGL.

[francoiscoulombe]

I’ll take a look first thing on monday

thanks for doing this

[vinsentli]

@francoiscoulombe
I have already updated the code.

[facebook-github-bot]

@corporateshark has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[francoiscoulombe]

sorry for the delay, I had some fire to extinguish

promise I'll try this out today

[francoiscoulombe]

using this code, opengl is still dark.

am i missing something?

[vinsentli]

The prerequisite is that the device needs to support the EGL_KHR_gl_colorspace extension.

[francoiscoulombe]

you're right... surprisingly, my s22 doesn't seem to have that extension.

gonna try on a different phone

[francoiscoulombe]

my pixel definitely has the extension and it still shows dark

[vinsentli]

Please check the format of the swapchain is SRGB.

[francoiscoulombe]

i even hardcoded srgb color format in a few places where it was set to unorm8 but unfortunately i'm still getting dark colors

[vinsentli]

add a breakpoint in java createWindowSurface,check the final congfig attribute is EGL_GL_COLORSPACE_SRGB_KHR

[francoiscoulombe]

i'm not setup with a java debugger but i'm adding logs and they're not coming out

either something's wrong with my setup or something is missing.

I'm also not building this using cmake. maybe i should try to replicate your setup?
how are you running this?

[corporateshark]

@francoiscoulombe The files inbuild/android/app/src/main/java/com/facebook/igl/sample are CMake-only. It can be you are using a completely different code path.

[francoiscoulombe]

i just saw this. your changes were only done to the files inside the build folder and that's the not the files that i'm using lol

i couldn't understand why my C++ log was coming out but not the java one

[francoiscoulombe]

alright, got it. it works now that i use the right java code. sorry for all the confusion, i didn't know we had this duplication

i'll take care of propagating the changes

[facebook-github-bot]

@corporateshark merged this pull request in b36a432.