[Question]: About the LOAD_GL* macros

[Mathias-Boulay]

Hi once again seb, I hope you're having a great day.

I'm curious about what led you to the current design of the LOAD_GL* macros.
Each use of these macros creates at least:

• 2 static variables, one being useless past the init phase
• 1 always evaluated if statement, useless past the init phase

Example with LOAD_GLES2(glCreateShader);

static glCreateShader_PTR gles_glCreateShader = ((void *) 0);
{
    static _Bool first = 1;
    if (first) {
        first = 0;
        if (gles != ((void *) 0)) { gles_glCreateShader = (glCreateShader_PTR) proc_address(gles, "glCreateShader"); }
    }
}

Wouldn't it be a better idea to centralize OGL ES function pointers in a place where they get initialized at the start of the program ?

This way, you reduce redundant if statements, and the number of allocated variables.

There must be something I'm missing here to get the full picture.

Note: an if statement might seem pretty harmless by itself, but when you have 12 of them in a row on a drawing function, any of them failing to be speculated properly can stall the rendering a bit 🤔

[ptitSeb]

Yes, the current LOAD_GLESx mecanism is not that great, and I wanted to have a global structure with all function pointer for some time now, but never actually started working on that.
It's quite a huge change, not terribly complex, but not super easy either.

One main "issue" is that you need to have a context to get function pointer. So I think current mecanism should be maintain for LOAD_EGL, and a new mecanism based on a mega structure can be used for LOAD_GLESx.
That's a big change to have the structure be created with the exhaustive list of function pointer, but it would certainly be a good evolution.

[Mathias-Boulay]

Hmm, I may as well try to do that. Although given the scale of this change I don't know if this will be easy to bring into mainline once it is done a fork of mine

[Mathias-Boulay]

Oh my, we do have a lot of references to the macros:

{'LOAD_GLES2': 102, 'LOAD_GLES': 383, 'LOAD_GLES_OR_OES': 1, 'LOAD_GLES2_OR_OES': 56, 'LOAD_GLES_OES': 35, 'LOAD_GLES_FPE': 33, 'LOAD_GLES_EXT': 6}

[ptitSeb]

Yep, I told you it was a huge job.

[Mathias-Boulay]

Regarding LOAD_EGL when you say it requires a context, you just mean the library has to be loaded first, right ?
Screenshot from loader.c

[ptitSeb]

Regarding LOAD_EGL when you say it requires a context, you just mean the library has to be loaded first, right ? Screenshot from loader.c

I meant to say for LOAD_GLESx you need a context, and those need to be done after a context is created (so maybe caried over witht the state). I propose to keep LOAD_EGL as is, because it will be much less used (only for context creation kind of things).

[Mathias-Boulay]

While direct usages of LOAD_EGL can be kept as is easily, it becomes tricky for some macros like LOAD_GLES_OR_OES
Example:

static glStencilMaskSeparate_PTR gles_glStencilMaskSeparate = ((void *) 0);
{
    static eglGetProcAddress_PTR egl_eglGetProcAddress = ((void *) 0);
    {
        static _Bool first = 1;
        if (first) {
            first = 0;
            if (egl != ((void *) 0)) {
                egl_eglGetProcAddress = (eglGetProcAddress_PTR) proc_address(egl, "eglGetProcAddress");
            }
            if (egl_eglGetProcAddress == ((void *) 0))
                LogPrintf("warning, %s line %d function %s: " "egl_eglGetProcAddress" " is NULL\n", "_file_name_", 39,
                          "_function_name_");;
        }
    };
    {
        static _Bool first = 1;
        if (first) {
            first = 0;
            if (gles != ((void *) 0)) {
                gles_glStencilMaskSeparate = (glStencilMaskSeparate_PTR) ((hardext.esversion == 1)
                                                                          ? ((void *) egl_eglGetProcAddress(
                                "glStencilMaskSeparate" "OES")) : ((void *) dlsym(gles, "glStencilMaskSeparate")));
            }
        }
    };
}

I have to say, I'm slightly surprised why this method of getting the function address is even needed when using the GL 1 backend

[ptitSeb]

The easiest what to handle this would be to not change the macro (or maybe just rename it if needed), create the mega structure (probably need some change in the yaml thingy) and and use those macro to feed the structure with function pointer. Ah well. Maybe the static _Bool first test could be removed, but that's it.

[Mathias-Boulay]

>proceeds to make demonic french sounds
I'll try something

[Mathias-Boulay]

There is inconsistent loading for the same functions.
Not accounting for variants like LOAD_GLES vs LOAD_GLES2, we have the following:

Inconsistent loading for glTexCoordPointer, current_key: LOAD_GLES, new_key: LOAD_GLES_FPE
Inconsistent loading for glClientActiveTexture, current_key: LOAD_GLES, new_key: LOAD_GLES_FPE
Inconsistent loading for glBlendColor, current_key: LOAD_GLES_OR_OES, new_key: LOAD_GLES2_OR_OES
Inconsistent loading for glClientActiveTexture, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES2
Inconsistent loading for glDrawArrays, current_key: LOAD_GLES, new_key: LOAD_GLES_FPE
Inconsistent loading for glDrawElements, current_key: LOAD_GLES, new_key: LOAD_GLES_FPE
Inconsistent loading for glVertexPointer, current_key: LOAD_GLES, new_key: LOAD_GLES_FPE
Inconsistent loading for glEnable, current_key: LOAD_GLES, new_key: LOAD_GLES_FPE
Inconsistent loading for glDisable, current_key: LOAD_GLES, new_key: LOAD_GLES_FPE
Inconsistent loading for glVertexPointer, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glColorPointer, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glNormalPointer, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glTexCoordPointer, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glAlphaFunc, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glColor4f, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glDisable, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glDrawArrays, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glDrawElements, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glEnable, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glFogfv, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glLightModelf, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glLightModelfv, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glLightfv, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glMaterialf, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glMaterialfv, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glMatrixMode, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glMultiTexCoord4f, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glNormal3f, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glPointParameterfv, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glPointSize, current_key: LOAD_GLES_FPE, new_key: LOAD_GLES
Inconsistent loading for glStencilFuncSeparate, current_key: LOAD_GLES2_OR_OES, new_key: LOAD_GLES
Inconsistent loading for glStencilMaskSeparate, current_key: LOAD_GLES2_OR_OES, new_key: LOAD_GLES
Inconsistent loading for glStencilOpSeparate, current_key: LOAD_GLES2_OR_OES, new_key: LOAD_GLES

Edit: Seems like most of the inconsistencies come from the legacy define guard skip system

[ptitSeb]

Yeah, LOAD_GLES_FPE vs LOAD_GLES should be for new code vs legacy GLES1.1 only code, and are probably normal.

The last 3 with LOAD_GLES2_OR_OES vs LOAD_GLES are a bit strange maybe a mystake to use LOAD_GLES?

[Mathias-Boulay]

After investigation, I confirm the last 3 occurrences are from gles.c with the skip guards

[Mathias-Boulay]

Update on a few things:
1 -

Yeah, LOAD_GLES_FPE vs LOAD_GLES should be for new code vs legacy GLES1.1 only code, and are probably normal.

Yes it was normal. I encountered the case of fpe_glDrawElements which LOAD_GLES to the real function, and other functions using LOAD_GLES_FPE which loads either fpe_glDrawElements or the real function directly depending on the es version.

2 -

I meant to say for LOAD_GLESx you need a context, and those need to be done after a context is created (so maybe caried over witht the state). I propose to keep LOAD_EGL as is, because it will be much less used (only for context creation kind of things).

Wouldn't this be contradictory to what is given by the following documentation about eglGetProcAddress (https://registry.khronos.org/EGL/sdk/docs/man/html/eglGetProcAddress.xhtml) ?

To quote the relevant part:
"Client API function pointers returned by eglGetProcAddress are independent of the display and the currently bound client API context, and may be used by any client API context which supports the function."

[ptitSeb]

To quote the relevant part:
"Client API function pointers returned by eglGetProcAddress are independent of the display and the currently bound client API context, and may be used by any client API context which supports the function."

I would not trust this part. Especialy because GLES 1.1 and GLES2 are different library.
Also, even if it is independant, functin will not be retreivable if using the wrong context creation. Like... you cannot get shader function when using GLES1.1 context.

[Mathias-Boulay]

Even the EGL 1.0 specification has the same quote though: https://registry.khronos.org/EGL/specs/eglspec.1.0.pdf

I guess I can still save the static proc_address to the eglGetProcAddress but keep this function call to when the LOAD_EGL macro is used

[Mathias-Boulay]

Isn't the LOAD_GLES2_OR_OES inconsistent with the rest ?

When the es_version is > 1, it straights up uses dlsym to resolve the function address instead of proc_address(gles... implemented by loader.c

[Mathias-Boulay]

I would not trust this part. Especialy because GLES 1.1 and GLES2 are different library. Also, even if it is independant, functin will not be retreivable if using the wrong context creation. Like... you cannot get shader function when using GLES1.1 context.

Seems like you were right, forcing to load functions beforehand with LIBGL_ES = 1 does indeed crash.

[ptitSeb]

Yeah, I really don't see how that paragraph could be enforced... Makes no sense to me...

[Mathias-Boulay]

I think I managed something more or less functional for testing purposes

[Mathias-Boulay]

Since you merged the PR in a custom branch, feel free to update me on any issue I might have missed, aside from the build systems !