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

Configurable OpenGL loader in the example bindings. #2002

Closed
wants to merge 4 commits into from

Conversation

jdumas
Copy link
Contributor

@jdumas jdumas commented Aug 4, 2018

This is not ready to merge yet, but I'm opening this PR so we can discuss changes related to #2001.

For now I've only added the glad backend for glfw+opengl3. I believe sdl+opengl3 will be the only other example needing a custom OpenGL loader.

I'll start with gl3w/glad/glew for now, as they are the more popular. Then we may add other loaders from this list, but honestly I'm not sure which one are popular enough so they should be added outside of this 3.

imgui.h Outdated
&& !defined(IMGUI_IMPL_OPENGL_LOADER_CUSTOM)
#define IMGUI_IMPL_OPENGL_LOADER_GL3W
#endif

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be in imgui_impl_opengl3.h and not imgui.h

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok done. Btw should imgui_impl_opengl3.h also #include "imgui.h"? Since IMGUI_IMPL_API is defined in imgui.h, it would be a compile error if the include order were reversed in client code.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, but since things have been working well without until now so I wouldn’t change it. This is in case in the future we add user-defines that affect the visibility or certain elements of imgui.h (like we do for imgui_internals.h). The implicit order constraint allows us to keep this a possibility without affecting end-user code.

@jdumas
Copy link
Contributor Author

jdumas commented Aug 4, 2018

Ok glfw+opengl3 should work on Linux with glad/gl3w/glew. Let me know if there's anything else you'd like to change.

@jdumas jdumas changed the title WIP on configurable OpenGL loader. Examples with configurable OpenGL loader. Aug 7, 2018
@jdumas jdumas changed the title Examples with configurable OpenGL loader. Configurable OpenGL loader in the example bindings. Aug 7, 2018
ocornut added a commit that referenced this pull request Aug 8, 2018
)

Not modifying the main.cpp yet because we need to test GL ES 3 context creation on iOS (only imgui_impl_opengl3.cpp was tested).
ocornut added a commit that referenced this pull request Aug 9, 2018
… on Android. Default on "#version 300 es" on ES 3. (#2002, #1873)
ocornut pushed a commit that referenced this pull request Aug 28, 2018
ocornut added a commit that referenced this pull request Aug 28, 2018
…of the box. (#2001, #2002). Changelog, tweaks, applied changes to SDL+OpenGL3 example.
@ocornut
Copy link
Owner

ocornut commented Aug 28, 2018

@jdumas This is merged now, thanks! (see 2 commits above, made some minor tweaks + applied to the SDL + OpenGL3). Thanks again!

I removed the glext block at it seemed unused/unfinished but if you could add glext it would be welcome.

@ocornut ocornut closed this Aug 28, 2018
@jdumas
Copy link
Contributor Author

jdumas commented Aug 28, 2018

Hmm not really using glext, so I think I'll leave it to anyone else who needs it =)
Thanks for the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants