mosra / magnum-integration

Integration libraries for the Magnum C++11 graphics engine
https://magnum.graphics/
Other
99 stars 45 forks source link

ImGuiIntegration: glUseProgram(0) called anywhere in drawEvent causes imgui to not render #64

Closed rune-scape closed 4 years ago

rune-scape commented 4 years ago

I know, another one.

I'm not an openGL expert, so I may just be misunderstanding. but.. glUseProgram(0) should reset the shader program, and is not an error ImGuiIntegration::Context::draw() should call glUseProgram(shader_id) in some way

Calling glUseProgram(0) anywhere in the drawEvent causes ImGuiIntegration::Context to not draw anything because (as I understand it) its not using the correct shader. If I put glUseProgram(_imgui._shader.id()); right before ImGuiIntegration::Context::draw() then everything works.

There is a similar problem calling glBindVertexArray(0)

Am I missing something?

I tested this using https://github.com/mosra/magnum-examples/blob/master/src/imgui/ImGuiExample.cpp With the latest on corrade, magnum, and magnum-integration And with v2019.10 on corrade, magnum, and magnum-integration

mosra commented 4 years ago

Hi!

This is not related to ImGui, but rather to the way how Magnum tracks OpenGL state in general -- in order to avoid redundant OpenGL calls, Magnum remembers what buffers / VAOs / textures ... are bound and what shaders are used. Doing it like this leads to significant perf improvements where the cost of GL API calls is high, especially Emscripten and shitter mobile drivers.

But of course this breaks when you (or some 3rd party lib) accesses GL directly, and in that case Magnum gets confused and assumes a particular state is set while it isn't. To fix that, you need to inform Magnum's state tracker before and after the direct GL access is happening. Docs: https://doc.magnum.graphics/magnum/opengl-wrapping.html#opengl-state-tracking

Hope this helps! :)

rune-scape commented 4 years ago

Yes, thank you!

Is this on the milestone to do better state tracking? or automatic state tracking? or something else?

rune-scape commented 4 years ago

I just realize you may not have wanted me to close this

mosra commented 4 years ago

How do you mean, better? :) There's not really a way to handle this automatically and still be performant enough -- the "safe bet" which works with 3rd party GL code would be that every API assumes no state is set to desired values and always bind / use / set ... everything needed upfront, and then reset all that back so 3rd party GL code isn't affected by what magnum did. That's what I was doing originally back in ~2010 but that made the driver waste a lot of time compared to what it does now. So there's not really much to do now (except for adding similar state tracking for a bunch of features that don't have it yet).

In comparison, with Vulkan (that's on the roadmap) this whole global driver state is gone and this is not a problem there anymore.

Hope that explains it :)

rune-scape commented 4 years ago

I understand now, I was just a bit confused by the automated project management stuff. Thank you.

And good luck with vulkan!