-
Notifications
You must be signed in to change notification settings - Fork 43
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
Unflexible include path of imgui (imGuIZMO.quat v3.0) #5
Comments
Nothing to object, this is more flexible.. mostly if you have imgui directory in the INCLUDE search path: I'll the defines also in the vConfig file. Already there is a CMakeFile.txt "internal" variable (not yet exported), I'll export it for user settings. |
I was thinking of making it even more flexible and less invasive, by specifying the path directly with: // without quotes and with final slash
#define IMGUIZMO_IMGUI_FOLDER imgui/ And in the code (but is transparent for users): #define JOIN(S) S
#define HASH_STR(S) #S
#define STR(S) HASH_STR(S)
#define PATH(A, B) STR(JOIN(A)JOIN(B))
#include PATH(IMGUIZMO_IMGUI_FOLDER,imgui.h)
#include PATH(IMGUIZMO_IMGUI_FOLDER,imgui_internal.h) what do you think about it? |
I see no big advantages in this solution. This feels a little bit like reimplementing the compiler include path functionality. So either I need to add Perhaps the only reason I can think of would be, with this solution you can maintain backwards compatibility, when the default remains "imgui". But I have no strong opinion on this. I am happy with any flexible solution that can be configured from the outside within my build system. |
I had actually taken a somewhat winding road... i can get the absolute path simply: #define GET_PATH(B) B
#define INC_PATH(A) <GET_PATH(IMGUIZMO_IMGUI_FOLDER)A>
#include INC_PATH(imgui.h)
#include INC_PATH(imgui_internal.h)
Yes, via compiler parameters it seems a re-implementation of -I flag
The Also with your previous solution the compatibility with previous versions would be maintained, and also easier to implement... just this solution would have even more flexibility. So an acceptable compromise could be: #if !defined(IMGUIZMO_IMGUI_FOLDER)
#define IMGUIZMO_IMGUI_FOLDER imgui/
#endif If you don't want to use path, just use: //------------------------------------------------------------------------------
// imGuiZmo.quat - v3.0 and later - (used only inside it)
//
// used to specify where ImGui include files should be searched
// #define IMGUIZMO_IMGUI_FOLDER
// is equivalent to use:
// #include <imgui.h>
// #include <imgui_internal.h>
// #define IMGUIZMO_IMGUI_FOLDER myLibs/ImGui/
// (final slash is REQUIRED) is equivalent to use:
// #include <myLib/ImGui/imgui.h>
// #include <myLib/ImGui/imgui_internal.h>
// Default: IMGUIZMO_IMGUI_FOLDER commented/undefined
// is equivalent to use:
// #include <imgui/imgui.h>
// #include <imgui/imgui_internal.h>
//
// N.B. Final slash to end of path is REQUIRED!
//------------------------------------------------------------------------------
// #define IMGUIZMO_IMGUI_FOLDER ImGui/ So, also the old |
Unfortunately just defining |
Yes, you are right, my mistake: I wanted to write: Corrected also above. |
This is not a conventional way of doing includes, I'm afraid... Things like this should be handled via include search paths and not via macros. For people who stumble upon this. This is what worked for me in CMake: add_library(imguizmo_quat
"imGuIZMO.quat/imGuIZMOquat.cpp"
)
target_compile_definitions(imguizmo_quat PUBLIC -DIMGUIZMO_IMGUI_FOLDER=) # this "=" is needed for it to work |
This library makes the assumption that the imgui headers are within an
imgui
directory.I think that is a very unflexible assumption. The imgui library itself does not have any default given in how and where to install the header files, also the official examples does not assume an imgui directory (https://github.com/ocornut/imgui/blob/v1.75/examples/example_glfw_opengl3/main.cpp). It would be nice to be able to use this library without assuming this directory structure. And as you can see you already added some kind of define wrapper, because there is no default. I think it would be more clear to let the build system define where the imgui headers are located by defining the correct include path. Then there would be no define needed and you can just do
#include <imgui.h>
.Otherwise if you like to keep the current state it would be at least nice to add an extra define to choose no imgui directory in the include path:
The text was updated successfully, but these errors were encountered: