Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Glfw + Emscripten + OpenGLES 3.0/WebGL 2.0 #1941

Merged
merged 8 commits into from Jul 30, 2018
Merged

Enable Glfw + Emscripten + OpenGLES 3.0/WebGL 2.0 #1941

merged 8 commits into from Jul 30, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jul 10, 2018

This commit will allow ImGui to be built using Emscripten and ported to WebGL2 OpenGLES3

So generally one can build the same source to a normal desktop app and be able to have the same source built to WebGL2 and have it working inside chrome.

Here is a demo project https://github.com/o-micron/OpenWebGL
See the Makefile, I use -D EMSCRIPTEN_BUILD when porting to WebGL2 using Emscripten

I added vertex and fragment shaders version 300 es
I added vertex and fragment shaders version 410 core
I added EMSCRIPTEN_BUILD ifdefs to allow/block some options because not everything is supported etc ...

1 issue yet to be solved is the input callbacks in browser, I cannot for example expand the ImGui TrreeNode in the WebGL2 app. I have to investigate more on that, maybe it is a MacOs bug. I have no idea. According to emscripten docs glfw has a limited input support https://kripken.github.io/emscripten-site/docs/porting/emscripten-runtime-environment.html#input-output

Please review and correct me if there is a design issue or something I am not aware of

@ghost ghost changed the title Add some ifdefs to add emscripten specific params and includes Enable Glfw + Emscripten + OpenGLES 3.0/WebGL 2.0 Jul 13, 2018
@ocornut
Copy link
Owner

ocornut commented Jul 13, 2018

Thank you Omar :) this is super useful and detailed.

A few things below:

  • Is __EMSCRIPTEN_BUILD__ a standard define? Is it automatically set by em++ or it is a commonly used name for a define that has to be set by the user?

According to https://kripken.github.io/emscripten-site/docs/compiling/Building-Projects.html section "Detecting Emscripten in Preprocessor" we could use __EMSCRIPTEN__ here.

  • Your change to imgui_impl_opengl3.cpp are creating a dependency on GLFW. It would normally be a big no-no but since this is under emscripten ifdef-wrap and considering how emscripten works it may be acceptable. However if I dig in glfw3.h it seems like all that does is, e.g.
 #elif defined(GLFW_INCLUDE_ES3)
  #include <GLES3/gl3.h>
  #if defined(GLFW_INCLUDE_GLEXT)
   #include <GLES2/gl2ext.h>
  #endif

Therefore those header could probably be included directly if they are standard?

  • In the various setup/loader you used, would the glPolygonMode exist as a NULL function pointer? How about the GL_POLYGON_MODE and GL_FILL define? The reason is I wonder if you could safely replace the ifdef with something like if (glPolygonMode) glPolygonMode(GL_FRONT_AND_BACK, GL_FILL); like how we are doing with glBindSampler.

(Also note the approach in https://github.com/ocornut/imgui/pull/1945/files were gl3w.cpp has been modified to support GL ES functions.)

@ghost
Copy link
Author

ghost commented Jul 13, 2018

Thanks Omar for the review :)
I see things got a lot better now 🔨

  • __EMSCRIPTEN_BUILD__ is now replaced by standard __EMSCRIPTEN__
  • glfw macros are totally removed and replaced directly with standard headers
  • call to glPolygonMode() is optional now as well

@podsvirov
Copy link
Contributor

Hello. I'l porting some examples to Emscripten too. See my patch for bindings. Looks like we have problems with input mose events for current window - this is Emscripten bug.
Try example_glfw_opengl3 and example_sdl_opengl3 now from your browser.

@ghost
Copy link
Author

ghost commented Jul 29, 2018

Yep, currently emscripten has marked that as a bug specifically related to glfw, the SDL version has the input working fine. I think once the whole application is going to run in browser, there is always a solution. My solution was to use the browser callback functions and call my own C++ code passing the current information of the mouse, keyboard, window size ... etc
I have an example which I used to call the window resize callback myself from javascript. See https://github.com/o-micron/OpenWebGL/blob/master/src/main.cpp#L40

@ocornut
Copy link
Owner

ocornut commented Jul 30, 2018

@ziecleto Moved from #1984

Yes sorry I've noticed that PR just now, I can confirm on our side that the proposed fix with
#ifndef EMSCRIPTEN
We've fixed it exactly like that before reading #1941 ;)
I'm not sure about the "classic" opengl include maze but I think the ifndef is quite solid IE it won't break > what's not already broken ;)
The only thing about that ifndef is that it's just referring to a particular build pipeline and not a real API > fix, IE we should have:
#ifdef OPENGL_ES
So maybe the correct solution is decoupling imgui_impl_opengl3.cpp but I understand that's a very big job for just 3 ifdefs so might not be worth it in the short run, surely in the long run though.

It is #ifdef EMSCRIPTEN or #ifdef __EMSCRIPTEN__ ?

Do you know if there are standard defines that could allow us to detect OpenGL ES ?
Is #ifdef OPENGL_ES a thing? Could we instead just simply test for Emscripten OR Android OR iOS if that's possible? That would effectively.

@ziocleto
Copy link
Contributor

ziocleto commented Jul 30, 2018

The official preprocessor define is__EMSCRIPTEN__
https://kripken.github.io/emscripten-site/docs/compiling/Building-Projects.html?highlight=__emscripten__

I am not aware unfortunately of any OPENGL_ES defined, but I've found a nicer fix, more API oriented and I think it's the correct fix:

#ifdef GL_POLYGON_MODE

So openGL ES does not define GL_POLYGON_MODE so instead of EMSCRIPTEN using GL_POLYGON_MODE on those 3 functions that are using glPolyMode features will achieve the correct implementation.

Cheers,
Dado.

@ocornut
Copy link
Owner

ocornut commented Jul 30, 2018

That's a good point, we can test for #ifdef GL_POLYGON_MODE which seems much safer than testing for #ifdef glPolygonMode (there are various GL OpenGL loaders and even if gl3w does, they are absolutely not guaranteed to use #define for the functions).

Similar the if (glBindSampler) test could maybe be replaced by #ifdef GL_SAMPLER_BINDING .

@ocornut ocornut merged commit fc737d2 into ocornut:master Jul 30, 2018
@ocornut
Copy link
Owner

ocornut commented Jul 30, 2018

I merged this with minor tweaks.
gl2.h/gl3.h inside bgfx and emscripten seems to define those:

#ifndef GL_ES_VERSION_2_0
#define GL_ES_VERSION_2_0   1

and

#ifndef GL_ES_VERSION_3_0
#define GL_ES_VERSION_3_0   1

Seems like we could rely on those for further testing?

@ziocleto
Copy link
Contributor

Yes I think that's the right approach, in fact I have fixed also few other compiler errors, in particular:

if (glBindSampler) glBindSampler(0, 0);

Fails on Clang5.0+ (C++17) with warning as errors:

error: address of function 'glBindSampler' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]

And equally the "fix":

if (&glBindSampler) glBindSampler(0, 0);

Will fail on GCC 8.1.0+ (and probably earlier)

error: the address of '__glewBindSampler' will never be NULL [-Werror=address]

Using #ifdef GL_SAMPLER_BINDING fixes those errors.

Also there are a couple of other warning as errors on C++17 which are relatively easy to fix, I can prepare a PR with all of this if you prefer.

Cheers,
Dado.

@ocornut
Copy link
Owner

ocornut commented Jul 30, 2018

@ziocleto Yes, I would appreciate a PR, thank you.

@ziocleto
Copy link
Contributor

I'll create che other "warnings as errors" c++17 PR fixes later as I think it's better to have 1 fix per PR to keep a good atomic repository!

ocornut added a commit that referenced this pull request Aug 1, 2018
@ocornut
Copy link
Owner

ocornut commented Aug 1, 2018

@podgorskiy I have pushed your focus workaround too.
Also linking to emscripten-core/emscripten#6409 for reference.
I may later introduce a ImGuiConfigFlags_ to easily disable focus check on the back-ends so this would become the way for Emscripten.

@o-micron
Copy link

Sorry, I had to create a new github account, I would be glad to help maintain that pull request if any changes need to be made in the future.

@ocornut
Copy link
Owner

ocornut commented Apr 15, 2019

Thank you @osheriff ! Btw we now have a Emscripten example and GL code has been massaged enough (with your help above and other PR) into supporting ES2 and ES3.

When I tried to setup Emscripten with ES3/WebGL it wouldn't work on my Firefox. I switched the demo to ES2 thinking it would maximize compatability anyway? Feedback/corrections welcome.

@o-micron
Copy link

o-micron commented Apr 15, 2019

Yeah sure, I have a couple of demos that I've made before one for WebGL2 GLSL v3.0ES and the other for WebGL with GLSL v1.0, I will get back to the code, clean it and will provide you with demos for each.
The problem is the shader version, WebGL 1.0 supports GLSL v1.0 not GLSL v3.0ES ...

@ocornut
Copy link
Owner

ocornut commented Apr 15, 2019

@osheriff You shouldn't need to speficially work on your old demo (as there's enough in the current repo and the GL code itself support now WebGL 1.0 and WebGL 2.0. I'm just stating we are now including an Emscripten exemple in master, so anyone interested in Emscripten tech will be more likely to help improve the demo further, and you might have feedback if you happen to use it later.
Thanks again!

@o-micron
Copy link

o-micron commented Apr 15, 2019

@ocornut Ok great, thanks Omar for your work :)
I will check the emscripten demo and will try to improve it or maybe add some cool stuff to it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants