Repository: google/filament
File: filament/backend/src/opengl/platforms/PlatformEGL.cpp (C++)

Code before:
    ext.egl.KHR_no_config_context = extensions.has("EGL_KHR_no_config_context");
    ext.egl.KHR_surfaceless_context = extensions.has("EGL_KHR_surfaceless_context");
    ext.egl.EXT_protected_content = extensions.has("EGL_EXT_protected_content");

    eglCreateSyncKHR = PFNEGLCREATESYNCKHRPROC(eglGetProcAddress("eglCreateSyncKHR"));
    eglDestroySyncKHR = PFNEGLDESTROYSYNCKHRPROC(eglGetProcAddress("eglDestroySyncKHR"));
    eglClientWaitSyncKHR = PFNEGLCLIENTWAITSYNCKHRPROC(eglGetProcAddress("eglClientWaitSyncKHR"));

    eglCreateImageKHR = PFNEGLCREATEIMAGEKHRPROC(eglGetProcAddress("eglCreateImageKHR"));
    eglDestroyImageKHR = PFNEGLDESTROYIMAGEKHRPROC(eglGetProcAddress("eglDestroyImageKHR"));

    EGLint const pbufferAttribs[] = {
            EGL_WIDTH,  1,
            EGL_HEIGHT, 1,
            EGL_NONE
    };

#ifdef __ANDROID__
    bool requestES2Context = driverConfig.forceGLES2Context;
    char property[PROP_VALUE_MAX];
    int const length = __system_property_get("debug.filament.es2", property);
    if (length > 0) {
        requestES2Context = bool(atoi(property));
    }
#else
    bool requestES2Context = isOpenGL();
#endif

    Config contextAttribs;

    if (isOpenGL()) {
        // Request a OpenGL 4.1 context
        contextAttribs[EGL_CONTEXT_MAJOR_VERSION] = 4;
        contextAttribs[EGL_CONTEXT_MINOR_VERSION] = 1;
    } else {
        // Request a ES2 context, devices that support ES3 will return an ES3 context
        contextAttribs[EGL_CONTEXT_CLIENT_VERSION] = 2;
    }

    // FOR TESTING ONLY, enforce the ES version we're asking for.
    // FIXME: we should check EGL_ANGLE_create_context_backwards_compatible, however, at least
    //        some versions of ANGLE don't advertise this extension but do support it.
    if (requestES2Context) {
        // TODO: is there a way to request the ANGLE driver if available?
        contextAttribs[EGL_CONTEXT_OPENGL_BACKWARDS_COMPATIBLE_ANGLE] = EGL_FALSE;
    }

#ifdef NDEBUG
    // When we don't have a shared context, and we're in release mode, we always activate the
    // EGL_KHR_create_context_no_error extension.
    if (!sharedContext && extensions.has("EGL_KHR_create_context_no_error")) {

Review comment (suggestion):
I don't understand this logic here. 

1. the only thing `requestES2Context` does is to set `        contextAttribs[EGL_CONTEXT_OPENGL_BACKWARDS_COMPATIBLE_ANGLE] = EGL_FALSE;` which would only work with ANGLE and only if the `EGL_ANGLE_create_context_backwards_compatible` extension was present. I don't understand why you would need that.

2. because it is set only when `isOpenGL()` is true, that means that we're actually requesting an OpenGL 4.1 context. Is this the intended behavior?

So the code means this:

"if we're not on ANDROID and using OpenGL (as opposed to GLES) then request a GL 4.1 context from ANGLE if it supports it".

I don't understand what this buys you.



Code after:
    ext.egl.KHR_no_config_context = extensions.has("EGL_KHR_no_config_context");
    ext.egl.KHR_surfaceless_context = extensions.has("EGL_KHR_surfaceless_context");
    ext.egl.EXT_protected_content = extensions.has("EGL_EXT_protected_content");

    eglCreateSyncKHR = PFNEGLCREATESYNCKHRPROC(eglGetProcAddress("eglCreateSyncKHR"));
    eglDestroySyncKHR = PFNEGLDESTROYSYNCKHRPROC(eglGetProcAddress("eglDestroySyncKHR"));
    eglClientWaitSyncKHR = PFNEGLCLIENTWAITSYNCKHRPROC(eglGetProcAddress("eglClientWaitSyncKHR"));

    eglCreateImageKHR = PFNEGLCREATEIMAGEKHRPROC(eglGetProcAddress("eglCreateImageKHR"));
    eglDestroyImageKHR = PFNEGLDESTROYIMAGEKHRPROC(eglGetProcAddress("eglDestroyImageKHR"));

    EGLint const pbufferAttribs[] = {
            EGL_WIDTH,  1,
            EGL_HEIGHT, 1,
            EGL_NONE
    };

#ifdef __ANDROID__
    bool requestES2Context = driverConfig.forceGLES2Context;
    char property[PROP_VALUE_MAX];
    int const length = __system_property_get("debug.filament.es2", property);
    if (length > 0) {
        requestES2Context = bool(atoi(property));
    }
#else
    constexpr bool requestES2Context = false;
#endif

    Config contextAttribs;

    if (isOpenGL()) {
        // Request a OpenGL 4.1 context
        contextAttribs[EGL_CONTEXT_MAJOR_VERSION] = 4;
        contextAttribs[EGL_CONTEXT_MINOR_VERSION] = 1;
    } else {
        // Request a ES2 context, devices that support ES3 will return an ES3 context
        contextAttribs[EGL_CONTEXT_CLIENT_VERSION] = 2;
    }

    // FOR TESTING ONLY, enforce the ES version we're asking for.
    // FIXME: we should check EGL_ANGLE_create_context_backwards_compatible, however, at least
    //        some versions of ANGLE don't advertise this extension but do support it.
    if (requestES2Context) {
        // TODO: is there a way to request the ANGLE driver if available?
        contextAttribs[EGL_CONTEXT_OPENGL_BACKWARDS_COMPATIBLE_ANGLE] = EGL_FALSE;
    }

#ifdef NDEBUG
    // When we don't have a shared context, and we're in release mode, we always activate the
    // EGL_KHR_create_context_no_error extension.
    if (!sharedContext && extensions.has("EGL_KHR_create_context_no_error")) {