mosra / magnum

Lightweight and modular C++11 graphics middleware for games and data visualization
https://magnum.graphics/
Other
4.75k stars 439 forks source link

Move GLSL '#version' directives translation to the Magnum::GL namespace #546

Closed WildRackoon closed 2 years ago

WildRackoon commented 2 years ago

Translation from enum class Magnum::GL::Version to a GLSL #version xyz directive compatible format is implemented in the Magnum::GL::Shader::Shader constructor See: https://github.com/mosra/magnum/blob/master/src/Magnum/GL/Shader.cpp#L641

It would be nice to have a Function by itself and possibly in the Magnum::GL namespace, next to the following ones:

auto version(Int major,Int minor) -> Version constexpr
auto version(Version version) -> std::pair<Int, Int>

This is a tiny nice-to-have feature obviously, only use-case I have found so far is the ability to supply this string to ImGui in a "plain ImGui integration" setup to ImGui_ImplOpenGL3_Init("#version xyz");. (Without relying on the extra Magnum-integration repo just like the plain GLFW example)

I could probably make a PR if this is a desirable refactor.

( This is also not supplied in a concise manner by Magnum::GL::Context::shadingLanguageVersionString() method that relies on glGetString(GL_SHADING_LANGUAGE_VERSION) )

EDIT: Also just realised the magnum-integration repository does not rely on the ImGui OpenGL3 backend if available.

mosra commented 2 years ago

Hi, I don't think this is what you want, in fact.

Magnum's GL wrapper has a state tracker to minimize redundant state changes in the driver and improve performance. This has mainly a big advantage in WebGL contexts, where each call into GL involves a lot of layers and validation, but to some extent it helps in native scenarios as well. The usage of a state tracker however means that you can't just freely combine Magnum's GL wrapper and raw GL calls, because the raw GL calls would cause the tracked state getting outdated. This can be mitigated by explicit calls to GL::Context::resetState(), but it's extra burden and if done too often it would cancel out the advantages of tracking the state in the first place.

Thus, I don't recommend you to use the "plain ImGui integration" -- the integration repository is there mainly to have Magnum and ImGui work well together. And in addition make use of features that Magnum provides on top of plain GL3 (such as DSA code paths), and which would be too burdensome to implement directly in ImGui.

In general, because of the global nature of GL, you can't combine Magnum's GL::Shader APIs with 3rd party code that calls directly into GL and expecting them to work together. This should also answer your other question about the integration not relying on ImGui's own GL backend. (On the other hand, with Vulkan this would be completely fine and a supported use case, as there's no global state to fight over.) If you want to use the ImGui integration without the *Application support and instead use it with, say, plain GLFW, you can -- simply propagate the events yourself.