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

glDrawElementsBaseVertex is not available in GL 3.1 #2852

Closed
dpilawa opened this issue Oct 17, 2019 · 9 comments
Closed

glDrawElementsBaseVertex is not available in GL 3.1 #2852

dpilawa opened this issue Oct 17, 2019 · 9 comments
Labels

Comments

@dpilawa
Copy link
Contributor

dpilawa commented Oct 17, 2019

Version/Branch of Dear ImGui:

Version: 1.72b
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_glfw.cpp + imgui_impl_opengl3.cpp
Compiler: Clang 8.0.0
Operating System: win10

My Issue/Question:

This piece in imgui_impl_opengl3.cpp is not sufficient to decide on the use of glDrawElementsBaseVertex which is available from GL 3.2 onwards (and not in GL 3.1).

// Desktop GL has glDrawElementsBaseVertex() which GL ES and WebGL don't have. #if defined(IMGUI_IMPL_OPENGL_ES2) || defined(IMGUI_IMPL_OPENGL_ES3) #define IMGUI_IMPL_OPENGL_HAS_DRAW_WITH_BASE_VERTEX 0 #else #define IMGUI_IMPL_OPENGL_HAS_DRAW_WITH_BASE_VERTEX 1 #endif

@ocornut ocornut added the opengl label Oct 17, 2019
@ocornut
Copy link
Owner

ocornut commented Oct 17, 2019

You are correct. We would need a PR that :

  • Rename IMGUI_IMPL_OPENGL_HAS_DRAW_WITH_BASE_VERTEX to IMGUI_IMPL_OPENGL_MAY_HAVE_DRAW_WITH_BASE_VERTEX.

  • In ImGui_ImplOpenGL3_Init() query the context version number using glGetIntegerv(GL_MAJOR_VERSION, ... etc, store in a single value (e.g. g_GlVersion = major * 1000 + minor), make that query outside of the ifdef for good measure.

  • Set ImGuiBackendFlags_RendererHasVtxOffset accordingly or not.

  • In the rendering function select between both functions depending on the version.

@ocornut
Copy link
Owner

ocornut commented Oct 17, 2019

@dpilawa Do you feel like attempting to make this PR?
Thanks!

@dpilawa
Copy link
Contributor Author

dpilawa commented Oct 17, 2019

@ocornut Yes, will do.

@dpilawa
Copy link
Contributor Author

dpilawa commented Oct 17, 2019

@ocornut I will need to find another solution and remove reference to glDrawElementsBaseVertex by compiler directives in contexts below 3.2. If I select between both functions in rendering function based on global variable, I still end up with code which won't compile on a pure 3.1 system (glDrawElementsBaseVertex is undeclared there and
IMGUI_IMPL_OPENGL_MAY_HAVE_DRAW_WITH_BASE_VERTEX precompiler condition is too weak).

If you have a quick idea, drop me a line, please.

@ocornut
Copy link
Owner

ocornut commented Oct 18, 2019

I still end up with code which won't compile on a pure 3.1 system

Which GL loader are you using?

@dpilawa
Copy link
Contributor Author

dpilawa commented Oct 21, 2019

@ocornut I am using GLAD with compatibility profile.

@ocornut
Copy link
Owner

ocornut commented Oct 21, 2019

OK so I guess we could

  • introduce optional #define to enforce overally GL version and compile that out
  • take advantage of standard #defined emitted in glad to automatically select a default at compile time? Can we find any of those?

@dpilawa
Copy link
Contributor Author

dpilawa commented Oct 21, 2019

Right, let's use these which GLAD emits: #define GL_VERSION_3_x 1. On 3.1 GL_VERSION_3_2 will not be defined. I will work on PR later this week.

ocornut pushed a commit that referenced this issue Oct 25, 2019
ocornut added a commit that referenced this issue Oct 25, 2019
… expose glDrawElementsBaseVertex(), using runtime GL version to decide if we set ImGuiBackendFlags_RendererHasVtxOffset. (#2866, #2852) [@dpilawa]
@ocornut
Copy link
Owner

ocornut commented Oct 25, 2019

Fixed via #2866

@ocornut ocornut closed this as completed Oct 25, 2019
ocornut added a commit that referenced this issue Apr 12, 2020
… instead of 3.2+ to enable ImGuiBackendFlags_RendererHasVtxOffset, leaving 3.2 contexts without it. (#3119, #2866, #2852)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants