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

ImGui crashes in nvoglv32.dll during glDrawElements in ImGui_ImplOpenGL2_RenderDrawData #3668

Closed
mijagourlay opened this issue Dec 23, 2020 · 2 comments

Comments

@mijagourlay
Copy link

mijagourlay commented Dec 23, 2020

Version: 1.80
Branch: master

Back-ends: imgui_impl_opengl2.cpp + imgui_impl_glut.cpp (but my theory is this could also occur in other OpenGL backends)
Compiler: Visual Studio 2003 (but unlikely to depend on compiler -- probably depends on OpenGL driver)
Operating System: Windows 10 Pro

[Aside: Why does this project use obsolete toolchain and libraries? This is a retro project intended to build on old (PS2/Xbox-era) consoles and the original XDK requires Visual Studio 2003. The analogous "cross-platform" build for Windows uses OpenGL 2. But most of that ends up not mattering for the root cause, if my theory is correct.]

ImGui crashes in nvoglv32.dll during glDrawElements in ImGui_ImplOpenGL2_RenderDrawData.

Call stack:

nvoglv32.dll!5d77ec03() 	
nvoglv32.dll!5d813974() 	
nvoglv32.dll!5d805f9e() 	
nvoglv32.dll!5cddae96() 	
opengl32.dll!6bcf4339() 	
PeGaSys_OGL.exe!ImGui_ImplOpenGL2_RenderDrawData(ImDrawData * draw_data=0x087705f8)  Line 181 + 0x20	C++
PeGaSys_OGL.exe!example_glut_opengl2_glut_display_func(bool clearWindow=false)  Line 93 + 0xb	C++
PeGaSys_OGL.exe!GlutDisplayCallback()  Line 216 + 0x7	C++
glut32.dll!processWindowWorkList(_GLUTwindow * window=0x027e5e78)  Line 1307 + 0xb	C
glut32.dll!glutMainLoop()  Line 1362 + 0x9	C
PeGaSys_OGL.exe!main(int argc=1, char * * argv=0x027a28f0)  Line 348	C++
PeGaSys_OGL.exe!mainCRTStartup()  Line 259 + 0x19	C
kernel32.dll!76756359() 	
ntdll.dll!771e8944() 	
ntdll.dll!771e8914() 	

This crash is readily reproduced (more than 50% of the time) but not 100%.

The problem appears to be that ImGui_ImplOpenGL2_SetupRenderState does NOT disable normal arrays, so if the last render state just prior to rendering ImGUI happened to have normal arrays enabled, then the call to glDrawElements can lead the nVidia OpenGL driver to crash.

The fix appears to be to add this line inside ImGui_ImplOpenGL2_SetupRenderState:

glDisableClientState(GL_NORMAL_ARRAY); // in case last render happened to have a vertex buffer with normals

Since the issue does not repro 100% of the time, there is a small chance that this addition did not fix the issue, but I have not repro'd the issue after adding that line.

FWIW, two other issues arose due to render state incompatibilities, but the others I found so far would not be something the OpenGL2 backend could reasonably deal with: The project mostly uses OpenGL2 but has the option to use 2 extensions: glBindBuffer and GL_TEXTURE_CUBE_MAP. So, before calling ImGUI routines like ImGui_ImplOpenGL2_RenderDrawData, the code needs to call these:

glBindBuffer( GL_ARRAY_BUFFER , 0 ) ; // in case last render happened to use VBO
glDisable( GL_TEXTURE_CUBE_MAP ) ; // in case last render happened to use a cubemap texture

...Otherwise either no ImGUI geometry appears, or the font glyphs are nonsense.

Again, that latter one is NOT an ImGUI issue since the above 2 calls are not in the canonical OpenGL2 API, but since it is possible to use OpenGL2 in an OpenGL3 project, I thought this info might be useful to anybody else doing something weird like what this project does.

One more thing: I also noticed ImGui_ImplOpenGL2_SetupRenderState does NOT disable the stencil test. In case the project left that enabled, adding this inside ImGui_ImplOpenGL2_SetupRenderState could potentially help (but this is speculation -- not an issue I have encountered yet, even though this project does use the stencil buffer):

glDisable( GL_STENCIL_TEST ) ;
@mijagourlay
Copy link
Author

See also Issue 3671 for other OpenGL state that needs to be set:

glShadeModel( GL_SMOOTH ) ; // to get multisampling (anti-aliasing) to work on ImGUI glyphs / widgets / chrome

ocornut added a commit that referenced this issue Jan 3, 2021
@ocornut
Copy link
Owner

ocornut commented Jan 3, 2021

Pushed various changes in 7d5d571:

  • glShadeModel() : backup, disable and restore in OpenGL2 backend. Being tied to the legacy pipeline I couldn't repro a side-effect with changing the shade modal in the imgui_impl_opengl3 setup/backends (gl3w doesn't even have the symbols, Glad does) so I assuming in later GL context versions this became unsupported, hard to find a definite confirmation but this is what i'm seeing, and either way users of imgui_impl_opengl3.cpp are even less likely to ever rely on this.

  • GL_NORMAL_ARRAY: cleared in OpenGL2 backend.

  • GL_STENCIL_TEST: explicitly backup, disable and restore in both backends. Tested this with:

glEnable(GL_STENCIL_TEST);
glStencilFunc(GL_NEVER, 0, ~0);
glStencilOp(GL_ZERO, GL_ZERO, GL_ZERO);
  • glBindBuffer( GL_ARRAY_BUFFER , 0 ) ;, glDisable( GL_TEXTURE_CUBE_MAP ) as you mentioned there isn't much we can do, but I tweaked some of the existing comments for glUseProgram() to clarify that the list is non-exhaustive and mentioned those two.

I will close this assuming that the crash has been fixed with glDisableClientState(GL_NORMAL_ARRAY) but if you see it again I'd be interested to hear about it.

Good luck and happy new year :)

@ocornut ocornut closed this as completed Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants