GL version define + runtime checks

[dpilawa]

• new optional define IMGUI_IMPL_OPENGL_VERSION used in imgui_impl_opengl3.cpp
• if undefined attempt to automatically set based on GLAD defines
• improved IMGUI_IMPL_OPENGL_HAS_DRAW_WITH_BASE_VERTEX define (fixing glDrawElementsBaseVertex is not available in GL 3.1 #2852)

[ocornut]

Hello @dpilawa, thanks for looking into this!

(MY INTENDED FIRST ANSWER, FEEL FREE TO IGNORE :)

Some feedback:

• I might be misreading the code but it seems that the PR would now disable IMGUI_IMPL_OPENGL_HAS_DRAW_WITH_BASE_VERTEX for any loader other than Glad. Most loaders are not auto-generated per GL version and wouldn't be subject to the issue discussed in glDrawElementsBaseVertex is not available in GL 3.1 #2852, so afaik we have a regression here.
• I would suggest to default the define to 3.2.
• I am not sure I understand why you #undef IMGUI_IMPL_OPENGL_VERSION at the top of the .cpp file maybe the user unable to override it.

I think the flow should be;

• Let user define it.
• Try to auto-detect based on GL_VERSION_ defines.
• Otherwise default to 3.2
• And later we can improve this for other loaders if we find similar issues like the one you spotted and they don't use standard GL_VERSION_XX defines, but this sounds unlikely?

Additionally:

• In order to make the code look as simple as it could be, I would suggest to avoid caring about the versions above 3.2 since we are not testing for them.
• Would also suggest to clarify the #define IMGUI_IMPL_OPENGL_VERSION XX lines with something like // Compatible with GL X.X and above.

---

(10 MINUTES LATER - ACTUAL ANSWER)

Both glew.h and gl3w.h use the same set of GL_VERSION_ defines.
I think the better action should be much simpler:

• We test for #ifdef GL_VERSION_3_2 + implement the runtime test discussed in glDrawElementsBaseVertex is not available in GL 3.1 #2852 (comment)
• None of that needs to be loader specific. We don't include a new IMGUI_IMPL_OPENGL define either.

So GL_VERSION_3_2 will lead to IMGUI_IMPL_OPENGL_MAY_HAVE_DRAW_WITH_BASE_VERTEX and we still query actual runtime version in ImGui_ImplOpenGL3_Init().

What do you think?

[dpilawa]

Thanks for your feedback, @ocornut !

Regarding regression - the IMGUI_IMPL_OPENGL_HAS_DRAW_WITH_BASE_VERTEX would actually be unset when neither User nor GLAD (indirectly by GL_VERSION_) defined IMGUI_IMPL_OPENGL_VERSION. So this is not a bug per se, but would effectively disable the use of glDrawElementsBaseVertex for anyone who does not set the new optional define explicitly, or uses GLAD. So yes, that's regression.

Now that we know that GLEW and GL3W use the same GL_VERSION_ defines the problem would not be that big, but there is no guarantee anyway on the existence of these in the future. And there are still custom loaders.

So bottom line is - I agree with your suggestion to simplify and still use runtime safety-net in ImGui_ImplOpenGL3_Init() and in ImGui_ImplOpenGL3_RenderDrawData(). Let me get back to you with updated PR.

[ocornut]

Hi @dpilawa was just merging that now at the same time you updated the changelog :)
Will be merged in a minute, thanks again!