-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Support older 2.x series of glbinding as loader for OpenGL3 #3061
Conversation
Hello and thanks for this PR.
I totally see the point and logic here and would have probably initially done the same, but the end result seems to be really misleading for the end-user since the expectation is that 2 comes after the unnumbered one. My suggestion would be to backtrack on that specific aspect:
Also note that both main.cpp and Makefile in Thank you! |
5a34b79
to
d3d04f8
Compare
Yeah, you are right that versioned and unversioned is confusing. I am just kinda suspecting I am more or less the only one left with version 2 usage (as version 3 is 20 months old by now) so I want to avoid disturbing others too much… So, how about using the unversioned define for auto-detection and two explicit versioned ones for all those with specific needs or compilers not supporting c++17 yet. The new version of the commit tries to implement this. (Also for SDL… completely overlooked that.) |
Thanks for the update! Looking at this, I feel that removing support for the unnumbered define would clear things out. I was surprised you added it. Note that This way we can clear out the Makefile and the impl_.h file from excess cruft. We can keep the lower ___has_include auto-detection block, the remaining auto-detection block becomes more trivially noise-free. Using Finally you may add a note at the top of imgui_impl_opengl3.cpp and in docs/Changelog.txt but if you are not sure about it I'll just them add over your commit when merging. Thanks for your help and patience! |
For the moment I have only the older 2.x series available on my system which uses slightly different header filenames and initialisation, so as a slight variation of the 3.x series support recently implemented here the glue needed to make 2.x series work as well. This removes the unversioned definition IMGUI_IMPL_OPENGL_LOADER_GLBINDING in favour of two versioned ones to choose explicitly. References: ocornut#2870, 5e2329b
d3d04f8
to
8ef2a83
Compare
My middle name might as well be "No breaking change" so little ifdef trickery and feature detection comes natural to me. No worries, if that isn't house-style I can adapt (after all, breaking house style is a form of breaking change…). Pushed v3 hopefully with all the suggested changes & maybe less copy&paste mistakes. Thanks for your help and patience as well! I haven't add changelog entries here or there though. Please do as needed. |
Merged now, thank you! |
…r OpenGL3 (ocornut#3061) This removes the unversioned definition IMGUI_IMPL_OPENGL_LOADER_GLBINDING in favor of two versioned ones to choose explicitly. References: ocornut#2870, 5e2329b
For the moment I have only the older 2.x series available on my system
which uses slightly different header filenames and initialisation, so
as a slight variation of the 3.x series support recently implemented
here the glue needed to make 2.x series work.
I would hope a potential 4.x will not change filenames and such, so the
existing flag isn't rebranded to 3 which also avoids breaking compat.
References: #2870, 5e2329b
No worries if you conclude that you don't want to support/include this, I am pushing the PR mostly at the off chance of it helping someone. That might already be achieved with the references. 😁