Emscripten/ES2 backend

[gabrielcuvillier]

Hi, I don't know what is the current status of Emscripten backend (#336), but here is a PR with how I handled the issue.

I created a specific backend renderer to target "gles2" (~= WebGL 1), and a specific example program "example_sdl_gles2".
You can see a live demo here: http://wasm.continuation-labs.com/imguidemo
As it is targeting WebGL 1, it should work on all browsers, including mobile.

The example program is also working on native Linux (not tested on Mac/Win32)
For convenience, I provided a CMakeFile to build on Emscripten and Linux

To build on emscripten:

• setup emscripten environment (eg. source <emscripten_path>/emsdk_env.sh)
• go to the "example_sdl_gles2" folder
• mkdir build-wasm
• emcmake cmake .. -DCMAKE_BUILD_TYPE=Release
• emmake make
• firefox imgui_example_sdl_gles2.html

To build on linux:

• go to the "example_sdl_gles2" folder
• mkdir build-linux
• cmake .. -DCMAKE_BUILD_TYPE=Release
• make
• ./imgui_example_sdl_gles2

Enjoy!

[ocornut]

(Edit) Fixed typos.

Hello @gabrielcuvillier,

Since the GLES2 code is so close to the imgui_impl_opengl3.cpp code which ALREADY support GLES3, I think the change should be made to imgui_impl_opengl3.cpp.
The same wayl, the existing SDL/GLFW+OpenGL example should be reworked with the necessary ifdef to make it workable on GLES2.

Could you look into this approach?
Comparing imgui_impl_opengl3.cpp and your imgui_impl_gles3.cpp would be a good starting point, but for some reason you have changed all the formatting in there so it is harder to see the changes. If I un-obscufity the change I see very few changes between them. Those would better be be done using ifdef.

The remaining task (which I can do later) will be to rename that file to e.g. imgui_impl_gl and rename imgui_impl_opengl2 to imgui_impl_opengl_legacy.

[gabrielcuvillier]

Ok, I'll take a look.
However, as the "GL3 backend" will in fact handle GL3, GLES3 (WebGL2), and GLES2 (WebGL1), this could be a little bit confusing in the end. Maybe it should be necessary to let the user to specify what is really the target to use, no ?

As for formatting, I think it is simply because my IDE that have different tab settings than the one used in imgui source

[ocornut]

We currently used the GLSL version to select version but I agree that maybe explicit function calls would work out better. I can do some of that work but I mostly need a explanation/reference for all the things you changed: vao, removed ifdef blocks, removed pixelstore call. And we can sort it out within the same file. Thanks a lot, this will be useful to have in the official distribution as everyone has been doing those changes locally until now.

[gabrielcuvillier]

Yep. I'll soon do an update to the PR with a proposal to handle all that GL version mess :) I have an idea to handle this based on things I saw on Regal GL emulation library. They had similar issue.

Then we can discuss and iterate on this

[gabrielcuvillier]

Here's the proposal, along with the updated PR:

Based on "imgui_impl_opengl3", I created the "imgui_impl_gl" backend. It is handling GL 3.x, 4.x, ES 2.0, ES 3.x (and indirectly WebGL 1 and WebGL 2 too)

GL version selection is done at compilation time using the following #defines:

• IMGUI_IMPL_OPENGL_3
• IMGUI_IMPL_OPENGL_4
• IMGUI_IMPL_OPENGL_ES_2
• IMGUI_IMPL_OPENGL_ES_3

If no #define is given at compilation time, the following defaults will be setup depending on compilation target:

• Windows, Mac, Linux => IMGUI_IMPL_OPENGL_3 (also support OPENGL_4, OPENGL_ES_2, OPENGL_ES_3)
• Android, iOS => IMGUI_IMPL_OPENGL_ES_3 (also support OPENGL_ES_2)
• Emscripten => IMGUI_IMPL_OPENGL_ES_2 (also support OPENGL_ES_3, but 2 is better due to wider browser support)

Note that IMGUI_IMPL_OPENGL_3 and IMGUI_IMPL_OPENGL_4 will also let you choose the GL loader (GLEW, GLFW, ...) the same way it is done in the existing gl backends. ES usually does not need loaders.

Fine tuning the GL3 and GL4 versions is possible (eg. 3.0, 3.1, 3.2, 4.1, etc.). This can be achieved using the glsl_version string passed to the Init function. For example, with IMGUI_IMPL_OPENGL_3 target you can pass "#version 130" for GL 3.0, "#version 150" to target GL 3.2 and so on. By default, if NULL is given to the function, Init will automatically configure it for GL 3.0, GL 4.1, ES 2.0 and ES 3.0. For now, no check is done on what the user could give.

I provided the "example_sdl_gl" as a base example, along with a CMake file. This example is using the new gl backend, the GL version setup is done in the CMakeList.txt file ("USE_GL_VERSION"), and SDL+GL configuration (PROFILE_MASK, PROFILE_VERSION, etc.) is done in the main file. Actually, this example is working fine on the following platforms:

• Emscripten with ES 2 and ES 3
• Linux with GL 3 and GL 4 with GLEW wrapper, as well as ES 2 and ES 3.

Note that it should still be tested on Mac and Windows (I think some minor tweaks will be required on Mac, since it is GL 3.2 that must be targeted), as well as Android and iOS. I have no Apple or Windows to test myself (yes, you read it correctly, no Windows :).

For now, the rest is unmodified. I did not update "example_glfw_opengl3" to "example_glfw_gl" as I am not used to glfw, and I think first the PR have to be validated to continue.

If this backend and new example is known to work on every platform, then the opengl3 backend could be definitively removed, "glfw_opengl3" could be updated to "glfw_gl", and "opengl2" renamed to "gl_legacy".

As for the small differences related to ES 2: neither VAOs nor custom PixelTransfer modes are supported, so I simply disabled the code. This seem to work actually, I did not dig deeper on the outcomes ;)

[gabrielcuvillier]

Ah, and normally the whitespace issue should be fixed (I updated it to 4 spaces, except for #ifdef stuff). However, maybe overall code style is not matching exactly the same as Imgui

[codeandroid]

@gabrielcuvillier Thank you for the work!
I am trying out this new backend on Windows with ANGLE and ES2 (within a Qt app) with great results so far.

One small thing: If IMGUI_IMPL_OPENGL_ES_2 is defined then GLES2/gl2.h gets included. Is there a specific reason for ignoring IMGUI_IMPL_OPENGL_LOADER_CUSTOM if IMGUI_IMPL_OPENGL_ES_2 is defined?

[gabrielcuvillier]

Great it works on Windows!

Well, I think loaders are not needed for ES 2 and 3, and that's why I completely ignored the LOADER* #defines if OPENGL_ES_2 or OPENGL_ES_3 are defined. But maybe I'm wrong... At least the option to have a custom loader is indeed something to keep.

[ocornut]

I am afraid it is still extremely tedious to parse and read this patch considering

1. it adds completely unrelated stuff (cmake)
2. has created a new renamed file instead of using the existing one, making diff impossible (+ cardinal sin of copy and paste the GL code which is a large pain to maintain and therefore shouldn't be duplicated)
3. doesn't follow coding guidelines in many places

The underlying ideas are probably extremely valuable but not being able to even look at a clear diff, this creates enormous friction to even look at this PR at the moment. I would appreciate if you could simplify this. PR need to be expressed with the minimum possible diff. Right now this seems like the exact opposite.

Thank you.

[ocornut]

4. The header file seems to be including other GL header files which is quite problematic imho, this should only be done from the .cpp file.
5. Best to not copy the file, rename the file or any function unnecessarily while the important changes are being reviewed. This is just creating noise around an already tricky issue. We can do the renaming a separate commit later.

[gabrielcuvillier]

Well, it looks like the last PR update have created some unexpected anger... ("extremely tedious", "cardinal sin", "enormous friction", "exact opposite", "just creating noise"). That's definitively not the goal.

I can understand not well formatted PR can be disappointing, but please keep cool.

[ocornut]

Apologies if my wording was strong. Please understand I am basically drowning in tasks and requests, it happens to me all the time in one form or another and that makes me feel I am everyone's janitor. Even the core of the work is super useful (it is), if you give the impression that you didn't double-check your patch and throwing 500 lines of noise for 20 lines of signal and leaving me to sort this out, it is going to weight the whole thing down and not be productive for anyone.

With the current PR there's simply no way of easily spotting the change you made (the file was copied, functions were renamed, formatting was changed).

However simple it may appear, this GL code is really tricky to update and maintain (because GL is a mess in term of portability), and changes need to be super careful and explained.

I made a commit beb3062 with a mix of bits adapted from here and from #2393, which only covers the GL ES 2 part. I think there's more to it, which is being implemented in this PR, and we can aim to extract those bits out manually.

[gabrielcuvillier]

Ok, I agree I expected the project maintainer to do some integration work in case that work might have been interesting for the project, as this PR was for me just a way to help by making a suggestion on how that tricky GL issue could be solved (I saw a couple of people struggling with it).

But I didn't knew you were overwhelmed by requests (beware of the FOSS burnout btw, this is a real thing), and in the end, I think doing a PR for this was simply not the right approach.

In fact, the dreaded copy/paste to a new file was even on purpose. This is because I thought it would have been better to first have that new backend as a separate experimental stuff (because not fully tested on all platforms) while still having the existing one around untouched. Too bad it gave you that feeling of low signal/noise ratio + hard time with diff.

I guess we were just not on the same wavelength!

[gabrielcuvillier]

So here a couple of changes to make things move forward:

• 1: I removed the FindSDL2.cmake
  While I agree it might have looked weird, I did this for a specific reason: cmake can't reliably find SDL2 on some distros, and usually projects are providing their own version of FindSDL2. So, let's say it is up to the user to fix this on their systems.

• 3: Did my best to match. Should be better now.
  Honestly I juggle between dozen of different projects, each with their own formatting rules and standards. This is very messy, and mistakes happen (+ the random autoformater tools that make things even worse sometime). I guess Python have the right approach here.

• 4: I moved the GL header inclusion to the cpp. However I think this is arguable: it require the GL header inclusion logic to be in all .cpp files using GL (so here, it is in both the gl.cpp backend and the main.cpp). Maybe that should be handled differently

• 2 & 5: As I explained, not modifying the gl3.cpp file and creating a new one by copy/paste is on purpose, as I don't want to mess with existing code that is already working. But now 3) is better, you should have less issues by diffing gl.cpp and gl3.cpp

The noise would be only related to function renaming. For the rest, this is the signal.

A couple of notes on the signal:

• I added GL loader initialization in the init function. I think it is a good idea to do it here.
• There's unexpected noise in the GLSL shader part, as I grouped vertex and fragment shader definitions by GL platform

Please, consider this work to be "super low" priority in your tasks. This is just to give insights about how the issue can be fixed, and not a proper PR. I guess this is the source of misunderstanding: I should have kept this work in my fork and just notify you about it in case you might find it interesting.

As so, we can close the PR as it is not a real one

[olilarkin]

I really like this approach of only having a single imgui_impl_gl, it's much more convenient. I get an error if I compile using emscripten with -02 "Uncaught RuntimeError: float unrepresentable in integer range" this goes away with -01. adding -s BINARYEN_TRAP_MODE=js like in your cmake file fixed it

[ocornut]

Hello all,

Merged emscripten example from #2494 today. It is pretty much the same as this except without the GL code duplication and it is taking advantage of SDL_opengles2.h so doesn't need a loader.

Current PR was helpful to rework the GL version selected (which was done earlier).

There will probably be things to iterate/tweaks with the Emscripten build options, now that it is all merged this will be easier for everyone to collaborate.

Thanks!