stricter_gl_extension_headers

[juj]

Remove from #include <GLES3/gl2ext.h> references to all GLES2&GLES3 extensions that are not supported by either WebGL 1 or 2.

Our gl2ext.h file has been historically pulled in as-is from the Khronos reference header file. This header file contains declarations of all extensions that OpenGL ES 2.0, 3.0, 3.1 and 3.2 collectively recognize. Naturally for Emscripten & WebGL, we only support a tiny subset of those, but our header has had the full list, which would lead to builds passing but giving a warning of an undeclared symbol at link time, like seen in #4408.

It is better to have Emscripten not even recognize any GL extensions that it doesn't support, since that will allow build errors to occur directly at compile time instead of link time (currently in link time those are warnings, but after the long standing #2714 is implemented, those would also be errors).

Also, I think it will be good to start adding declarations and #define symbols of WebGL specific extensions. In this PR I've done this for a couple that we support, more can be added as support is added. Not sure though, perhaps we'll like to have those in #include <emscripten/webgl.h> and #include <emscripten/webglext.h> instead.

[kripken]

What do other platforms do - do they customize this header as you did here? Or do they rely on runtime errors?

[juj]

It depends a bit. Other platforms have the difficulty that they don't know which GPU one is targeting at compilation time, e.g. if you are building on Windows, Linux, or Android, the gl2ext.h header comes from the system's generic vendor.

An example from Android NDK: https://android.googlesource.com/platform/development/+/18fc718/ndk/platforms/android-5/include/GLES2/gl2ext.h . That one carries a subset of all extensions in GLES 2.0, but not the "full" header. However when compiling against that, one doesn't know which GPU is being targeted, which might have different extensions.

Khronos has an implementer's guide at https://www.khronos.org/registry/implementers_guide.html, which says that for "vendor controlled platforms", one should use header files provided by the platform vendor, but that using the Khronos-provided "stock" gl2ext.h as a drop-in should also work, as long as one doesn't reference any functions that don't exist. Shipping the full set of extensions in gl2ext.h is certainly not wrong.

Googling random files on the web, https://github.com/collinjackson/gles2-bc-nacl/blob/master/OpenGLES/ES2/glext.h has a version of nacl GLES2 extension headers, which contains custom extensions as add-ons that don't exist officially: (GL_CHROMIUM_resize, GL_CHROMIUM_request_extension, ...)

Pros for having the stock full extension header file with all extensions:

• one that one is able to compile codebases that reference stuff that don't exist because our current linker behavior is to just warn, and as long as one doesn't reach those code paths at runtime, since they would abort.
  • easy to update to latest, just copy the new gl2ext.h from Khronos

Cons for having the full extension header:
- compilation error messages turn into linking error messages, (undefined symbol instead of undeclared identifier), so not following the "fail early" idea
- users might be confused as to why it compiled but did not link ("if this compiled, is this supported? Was I just missing a linker flag then? Or is this a bug?"), which can lead to discussion like #4408
- compiler error explicitly tells the file:line where it occurred, but link error does not, so hunting down a compile error is easier
- some unsupported features can actually be missed at link time even, and only detected at runtime. This occurs for extensions that don't introduce any new functions but only add new enums, e.g. texture formats. This means that one will need to find out at runtime that the added enum is an unsupported one. (good-behaving GL code should always check the extensions before using, but some codebases tend to be lax with these)

Pros for having customized list of extensions:
- get errors early at compilation time instead of link or runtime for stuff that we possibly can't support, so fails as early as possible
- get file:line info of where unsupported stuff is used

Cons for having customized list of extensions:
- need to maintain it up-to-date, and add new extensions if they come available (if those extensions were 1:1 copies of native GLES2 extensions, that is). Although if new extensions do come available, src/library_gl.js will likely need to be updated anyways, so presumably not automatic either way.

So there's certainly a viewpoint for both perspectives. I am currently leaning towards slimming this down and maintaining only the headers of things that we do support. I think early compiler errors could help catch bugs that otherwise might go unseen. What do you think?

[kripken]

Thanks for the writeup. I actually lean the other way - it seems like less work to use the upstream header as-is. Otherwise as webgl and webgl2 evolve we'll need to maintain our changes.

The main downside is people see errors at runtime instead of compile time. For something like GL that doesn't seem like a huge downside to me, this isn't stuff that would hit a production build.

But, I don't feel very strongly here. Curious to hear more opinions.

[erunehtar]

glext headers are not changing constantly afaik, I don't see this as a huge maintenance issue. Instead of removing code from the khronos header, you could use #ifdefs to ease maintenance.

For us this problem showed up when we started linking with the Scaleform library, and it might actually start happening much more often if more and more libs get ported over to WebGL.

This is actually a problem with production builds unless we customize Emscripten headers, which we'd prefer to avoid since we don't want to start distributing Emscripten ourselves.

[kripken]

Thanks for the input @gouletr. Ok, sounds good to me, @juj if you think this is a good path forward and won't be too much to maintain, then let's do it.

[juj]

I'll take this on. I like having errors as early as possible. I think we could have a #define that enables the full header if someone insists on linking to nonexisting functions.

It should be reiterated though that having the stock headers is not a root cause of any GL problems, but the only issue it can cause is that existing GL problems are hidden until link time, or in rare corner cases, at runtime.

[erunehtar]

@juj mostly true, but it turns out the GL stock header will define some preprocessor values, and in code developers can check for those values to know if a specific extensions are available, which is wrong for many of them.

Example: #define GL_OES_get_program_binary 1

If a developer check for this, it might wrongly assume the function is available, that's the main issue with using these stock headers.

[juj]

No, it would be incorrect to use those #define GL_xxx 1 preprocessor macros in that fashion, their presence does not tell that the given extension would be available and client code should not assume that functions related to that extension would be available either. This is stated in https://www.khronos.org/registry/implementers_guide.html#uncontrolled with

For most APIs, functions and enumerants for extensions registered with the Khronos
Extension Registry are declared and defined in a Khronos-provided <api>ext.h file,
e.g, glext.h. Exceptions are noted below. These header files can be used even if the
implementation doesn't provide a particular extension since applications must query
the presence of extensions at run time.

On vendor controlled platforms (which Emscripten's GL support is not) in https://www.khronos.org/registry/implementers_guide.html#controlled, which advises that

Platform SDKs typically come with 2 OpenGL-related header files pre-installed: gl.h,
which defines the OpenGL interfaces, and a header file that defines the interfaces to
that platform's window abstraction layer (e.g. glx.h) . These files are commonly
outdated, often having only OpenGL 1.2 interfaces. As these files are outside the control
of Khronos, Khronos provides *ext.h files which define newly added functions and enums,
such as those specified by OpenGL 4.x. Some platform SDKs may also include *ext.h files.
These can also be outdated. Implementers are advised to use the Khronos provided
*ext.h files and supply these with their SDKs.

This hints that Khronos desires that the stock unmodified glext.h headers should always be functional drop-ins.

[bmuzzin]

The Khronos advice essentially relies on a dynamic binding system. When it comes to the usage of GL_GLEXT_PROTOTYPES (static binding), it won't work in practice to download the Khronos header and drop it in. If you do use the prototypes, then you must decide at compile time whether the function is available or not. There really is no other way to do this, than to rely on the #defines in the extension header. However, these functions can be simple stubs - the user still needs to check the extension availability at runtime before calling the extension functions. Calling an extension function not reported as an extension at runtime is certainly bad.

Vendors have supported static binding by providing their own extension headers (either stock, or customized). For example, iOS and OSX both have customized extension headers, which only provide #defines for the extensions in their ABI. They also do not have a dynamic binding, so you essentially must use GL_GLEXT_PROTOTYPES. Linux and Android provide an (at least mostly) stock version of the Khronos extension headers, but also provide ABI linkage for every extension function in the header. Windows is the only platform (I know of) that does not supply the extension header itself, and requires you to obtain it from Khronos. In this case, you essentially cannot use GL_GLEXT_PROTOTYPES and must use a dynamic binding system (wgl). Not providing the header is a hint that this is the case, as the SDK itself is not advertising support for anything.

To my knowledge, no platform SDK (except Emscripten) both provides an extension header, and does not have linkage for all the extensions within its GL library. Since Emscripten doesn't fail on unresolved functions, this is less of an issue than it is in other environments. I think to resolve this issue, the extension header can either be removed from the SDK, which indicates to users they need to use dynamic binding (EGL), or, linkage stubs for every function within the extension header that is provided in the SDK should be included.

[juj]

Thanks for the comments @bmuzzin.

The topic of linkage is a bit orthogonal, but Emscripten indeed does support static linking to the extensions that it supports. Dynamic binding is also supported, but we recommend users to do static binding instead, since it has better performance.

Anyway, the choice to resolve here was decided to customize the header for Emscripten purposes, so all should be good here. We certainly would not want to remove the extension headers altogether.

[MarkCallow]

To my knowledge, no platform SDK (except Emscripten) both provides an extension header, and does not have linkage for all the extensions within its GL library.

This is not true in spirit though possibly true literally. On Windows you have to use GetProcAddr for anything not in the core OpenGL 1.2. Nevertheless Windows users use the standard Khronos glext.h. In common with all gl*ext.h files this does not by default define prototypes.

It is true that some SDKs provide static linkage for some extensions. However it is always necessary to check at runtime if an extension is supported as support can vary across context configs within an implementation.

Since Emscripten doesn't fail on unresolved functions, this is less of an issue than it is in other environments. I think to resolve this issue, the extension header can either be removed from the SDK, which indicates to users they need to use dynamic binding (EGL), or, linkage stubs for every function within the extension header that is provided in the SDK should be included.

Since gl2ext.h does not define any prototypes - I've just checked - unless GL_GLEXT_PROTOTYPES is defined, which it is not, I do not know why people are having linking errors with Emscripten. Something elsewhere must be defining prototypes or something is defining GL_GLEXT_PROTOTYPES.

This PR should not be included in Emscripten. It is unnecessary to modify the standard gl2ext.h. It only defines tokens and function pointers. The true problem needs to be identified and fixed.

[bmuzzin]

This is not true in spirit though possibly true literally. On Windows you have to use GetProcAddr for anything not in the core OpenGL 1.2. Nevertheless Windows users use the standard Khronos glext.h. In common with all gl*ext.h files this does not by default define prototypes.

Well, I see you wrote the Khronos implementer's guide, so, arguing the spirit might be an uphill battle for me - but I'm going to try anyhow :). In the implementation guide, it speaks of "uncontrolled" and "vendor controlled" platforms. Uncontrolled platforms essentially require you to use the Khronos version of gl_ext.h, because they do not provide it in their SDKs (eg. Windows). By not providing a custom gl_ext.h header, they are advertising that their ABI does not necessarily contain any extensions, thus forcing you to use a dynamic binding system (eg wglGetProcAddress). Conversely, controlled platforms provide the gl*ext.h headers (under that section it even says: "Use the header files, (e.g., for OpenGL ES, gl.h, glext.h & egl.h) provided by the platform vendor."). On those platforms, it's assumed that they provide ABI bindings for any entry points in the headers, since they have (potentially) customized it, and are in a position to know what is in their libraries. So, to me it seems both true in spirit, and in practice?

Since gl2ext.h does not define any prototypes - I've just checked - unless GL_GLEXT_PROTOTYPES is defined, which it is not, I do not know why people are having linking errors with Emscripten. Something elsewhere must be defining prototypes or something is defining GL_GLEXT_PROTOTYPES.

Yes, I am defining it before including gl2ext.h. As I said, this is required on platforms where there is no dynamic binding system (eg. OSX and iOS). If it isn't a valid use case on an "uncontrolled" platform, perhaps Khronos should remove it from their headers, and just have Apple/whoever provide custom gl[2]ext.h files which include it.

is true that some SDKs provide static linkage for some extensions. However it is always necessary to check at runtime if an extension is supported as support can vary across context configs within an implementation.

Yes, it is always necessary to check at runtime. The ABI function might just be a stub, if extension support is dynamic (eg. dependent on a driver whose capabilities may vary, such on OSX). However, you cannot you link to a function which is not in the ABI. Thus, any functions defined in the custom extension header should be in the ABI (as at least a stub). Otherwise, linking statically would be difficult.

I think my original reasoning stands - to put it in the semantics of the implementer's guide: Emscripten needs to decide whether it's an uncontrolled or vendor controlled platform. If it's the former, remove gl2ext.h from the SDK, which will enforce the usage of EGL. If it's vendor controlled, customize gl2ext.h, and ensure that any extension entry points in the header are defined in the ABI. Personally, I like the customized glext.h better, because it doesn't require users to go download the header from Khronos, everything just comes with the SDK. However, I'm also fine if it goes the other way. But, for consistency, I think it should go one way or the other.

[MarkCallow]

@MBuzzin I wrote "in spirit" because I know it is possible to argue, as you are, that what is on Windows is not an SDK.

The trend is toward using unmodified Khronos headers. Both Android and Mesa are on the way, if not already there. I'm pretty sure ARM, Imagination and Qualcomm use the Khronos headers in their SDKs. I think Apple is (will be) the only outlier. Most applications use GetProcAddr for extensions, which is the generally recommended way, partly because it avoids attempts to use extensions based on what is declared in gl*ext.h, and those are what will be ported via Emscripten.

The only reason that I can see for Emscripten to provide static linking for its supported extensions, and therefore modified Khronos headers is if GetProcAddr + call via pointer is hugely slower than a direct function call. If so developers will have to be made aware of this so they change their code.

[erunehtar]

@juj @msc- Emscripten PR #4665 is proposing to change the default -s ERROR_ON_UNDEFINED_SYMBOLS to 1 so that undefined symbols becomes errors instead of warnings. While I strongly agree that -s ERROR_ON_UNDEFINED_SYMBOLS should default to 1 to match other platforms, this issue needs to be resolved in order to comply and clean-up these link issues.

I don't mind if instead of modifying the header file, Emscripten implement some stubs with asserts. Whatever make sense.

[juj]

Closing this as old - may be better to use unmodified Khronos headers even if it means getting late link errors instead of early compile errors.