[Question]: About the LOAD_GL* macros #440

Mathias-Boulay · 2023-08-27T17:19:23Z

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 🤔

The text was updated successfully, but these errors were encountered:

ptitSeb · 2023-08-27T17:27:23Z

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 · 2023-08-27T17:44:31Z

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 · 2023-08-28T14:34:33Z

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 · 2023-08-28T15:42:59Z

Yep, I told you it was a huge job.

Mathias-Boulay · 2023-08-28T16:37:27Z

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 · 2023-08-28T16:45:46Z

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 · 2023-08-28T17:34:05Z

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 · 2023-08-28T17:45:49Z

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 · 2023-08-28T18:07:19Z

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

Mathias-Boulay · 2023-08-29T06:48:23Z

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 · 2023-08-29T07:28:40Z

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 · 2023-08-29T09:26:09Z

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

Mathias-Boulay · 2023-08-30T21:51:29Z

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 · 2023-08-31T06:14:23Z

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 · 2023-08-31T09:05:13Z

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 · 2023-08-31T11:45:53Z

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 · 2023-08-31T13:15:08Z

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 · 2023-08-31T14:00:52Z

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

Mathias-Boulay · 2023-08-31T20:49:03Z

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

Mathias-Boulay · 2023-09-04T14:22:33Z

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 !

Mathias-Boulay mentioned this issue Aug 31, 2023

Refactor: preload most function addresses #442

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Question]: About the LOAD_GL* macros #440

[Question]: About the LOAD_GL* macros #440

Mathias-Boulay commented Aug 27, 2023 •

edited

Loading

ptitSeb commented Aug 27, 2023

Mathias-Boulay commented Aug 27, 2023

Mathias-Boulay commented Aug 28, 2023 •

edited

Loading

ptitSeb commented Aug 28, 2023

Mathias-Boulay commented Aug 28, 2023

ptitSeb commented Aug 28, 2023

Mathias-Boulay commented Aug 28, 2023 •

edited

Loading

ptitSeb commented Aug 28, 2023

Mathias-Boulay commented Aug 28, 2023

Mathias-Boulay commented Aug 29, 2023 •

edited

Loading

ptitSeb commented Aug 29, 2023

Mathias-Boulay commented Aug 29, 2023

Mathias-Boulay commented Aug 30, 2023

ptitSeb commented Aug 31, 2023

Mathias-Boulay commented Aug 31, 2023

Mathias-Boulay commented Aug 31, 2023 •

edited

Loading

Mathias-Boulay commented Aug 31, 2023

ptitSeb commented Aug 31, 2023

Mathias-Boulay commented Aug 31, 2023

Mathias-Boulay commented Sep 4, 2023

[Question]: About the LOAD_GL* macros #440

[Question]: About the LOAD_GL* macros #440

Comments

Mathias-Boulay commented Aug 27, 2023 • edited Loading

ptitSeb commented Aug 27, 2023

Mathias-Boulay commented Aug 27, 2023

Mathias-Boulay commented Aug 28, 2023 • edited Loading

ptitSeb commented Aug 28, 2023

Mathias-Boulay commented Aug 28, 2023

ptitSeb commented Aug 28, 2023

Mathias-Boulay commented Aug 28, 2023 • edited Loading

ptitSeb commented Aug 28, 2023

Mathias-Boulay commented Aug 28, 2023

Mathias-Boulay commented Aug 29, 2023 • edited Loading

ptitSeb commented Aug 29, 2023

Mathias-Boulay commented Aug 29, 2023

Mathias-Boulay commented Aug 30, 2023

ptitSeb commented Aug 31, 2023

Mathias-Boulay commented Aug 31, 2023

Mathias-Boulay commented Aug 31, 2023 • edited Loading

Mathias-Boulay commented Aug 31, 2023

ptitSeb commented Aug 31, 2023

Mathias-Boulay commented Aug 31, 2023

Mathias-Boulay commented Sep 4, 2023

Mathias-Boulay commented Aug 27, 2023 •

edited

Loading

Mathias-Boulay commented Aug 28, 2023 •

edited

Loading

Mathias-Boulay commented Aug 28, 2023 •

edited

Loading

Mathias-Boulay commented Aug 29, 2023 •

edited

Loading

Mathias-Boulay commented Aug 31, 2023 •

edited

Loading