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

Remove imconfig.h include. #41

Closed
wants to merge 1 commit into from
Closed

Remove imconfig.h include. #41

wants to merge 1 commit into from

Conversation

ryanmjacobs
Copy link

I think that imconfig.h should not be included globaly, but included on the project level.

When it's global it's difficult to create shared libraries because now you have two header files to deal with. And, no one wants an extra header files that they don't need hanging around in /usr/lib/.

It would be better for the user to include this at the top of their main file before including imgui.h on as-needed basis.

I created an AUR package and it's kinda hack-ish to remove this line with sed. I'd rather it not be the default.

@ocornut
Copy link
Owner

ocornut commented Sep 3, 2014

Ryan,

You cannot really "package" ImGui (in the sense of building binaries or storing it in /usr/lib as opposed to application side). The reason imconfig.h exists and appears to be intrusive is to allow the user to replace things like the ImVector implementation, adding conversation operators to the math types, potentially changing the vertices layouts. For those things Imgui.cpp needs to be compiled in with imconfig.h and in this context I believe the current system is correct.

I appreciate the intent but I don't have a solution to it and I think imgui is best not "packaged". The files can be copy & pasted into projects rather than attempting to make it a shared library. This is also why imgui is designed to be so few files and I think it also empower the individual developer to just hack in the files at ease to add what they need instead of feeling constrained.

@ryanmjacobs
Copy link
Author

Okay, that sounds about right. I didn't realize that imconfig.h needed to be available at compile time for imgui.cpp.

However I have one more question. Would it be okay for imconfig.h be included in imgui.cpp, and not imgui.h by default?

@ocornut
Copy link
Owner

ocornut commented Sep 4, 2014

What would be the purpose / benefit of doing that ?
imconfig.h has potential effect on data structures exposed in imgui.h so it would be expected that the user include it.

Part of the design is that I want the end-user to use imgui in a random file using 1 include to minimise "setup", because imgui is as much a debug tool than it is a ui library - you can use it to quickly log stuff or edit stuff. So for example you can use Text function to trace an algorithm in a file that has nothing to do with UI.

#include <imgui.h>
...
ImGui::Text("Log some stuff");

So the overhead is reduced to 1 include, and this is why there is an implicit Begin/End pair in ImGui so you can use the widgets and they go in a default window, saving 2 extra lines of overhead.

@ryanmjacobs
Copy link
Author

Oh okay, now I understand. This library is primarily for quick debuging and logging; and not really GUI-focussed (other than to give developers quick access to logging info).

Anways, thank you for your time. And, thank you for explaining it to me.

@ryanmjacobs ryanmjacobs closed this Sep 4, 2014
@ryanmjacobs ryanmjacobs deleted the remove_imconfig.h_include branch September 4, 2014 23:05
@ocornut
Copy link
Owner

ocornut commented Sep 8, 2014

"as much a debug tool than it is a ui library" doing both.
No worry, I realised it is a bit of an oddity compared to more standardized library. Avoiding the need for binary release is an important part of the design because they require maintenance and often not compatible between os or compiler versions (Windows+Visual Studio in particular are terrible with that). This create an adoption wall - lots of otherwise interesting code fitting niches are ignored because trying them is a hassle. So the focus is on making it trivial to just compile Imgui yourself.

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

Successfully merging this pull request may close these issues.

2 participants