-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Add CMake project #1713
base: master
Are you sure you want to change the base?
Add CMake project #1713
Conversation
Note that i’ve been refactoring examples as part of the default branch, and will probably end up providing both premake and cmake files for the examples/ bits. |
@franciscod, thanks for links. Now I have some ground to think about. |
956884f
to
57e2612
Compare
Hi @ocornut, I implement idea to share |
@podgorskiy I understand why cmake is used. I personally think it's unnecessary for imgui as the point of the library is that you can register the .cpp file in your application and I encourage you to do so (you can add imgui cpp files from your main app cmake file). Surely registering a few cpp files in your project shouldn't be harder than maintaining and registering a separate cmake file from your project?
It is a little harmful as it prevents people from configuring ImGui for their application via That said, to provide the example applications I will maintain premake/cmake, but I won't tackle that until the examples are refactored in a later version. Your cmake references will be useful here. Thank you! Also note that your PR also mix and matches multiple unrelated changes in a single commit so it's hard to review or pull. |
GLboolean imgui_GL_ENABLE_BIT_was_enabled = glIsEnabled(GL_ENABLE_BIT); | ||
GLboolean imgui_GL_COLOR_BUFFER_BIT_was_enabled = glIsEnabled(GL_COLOR_BUFFER_BIT); | ||
GLboolean imgui_GL_TRANSFORM_BIT_was_enabled = glIsEnabled(GL_TRANSFORM_BIT); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The emscripten changes are unrelated to the title and intended purpose of the PR. They are useful are a reference because I would like to support emscripten more easily (also see #336).
However note that your replacement code for glPushAttrib()
is incorrect.
glPushAttrib(GL_ENABLE_BIT)
alone with old style GL saves a lot more thing.
https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/glPushAttrib.xml
I would recommend using another breed of OpenGL for the Emscripten examples, or just skipping this push/pop for now.
if (glfwGetWindowAttrib(g_Window, GLFW_FOCUSED)) | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be justified more thoroughly, the and code can be written with a single point of interference.
e.g.
#ifndef __EMSCRIPTEN__
const bool focused = glfwGetWindowAttrib(g_Window, GLFW_FOCUSED);
#else
const bool focused = true;
#endif
if (focused)
...
examples/opengl2_example/main.cpp
Outdated
|
||
// Main loop | ||
#ifdef __EMSCRIPTEN__ | ||
emscripten_set_main_loop(step, 0, 1); | ||
#else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a little annoying but I understand this is how emscripten works. I took a note to consider changing the examples to accommodate for this a little more easily.
FYI I made some comments but don't worry about reworking or splitting the PR. Since the examples are being refactored in the viewport branch, I won't be able take this PR soon. But I will pull the cmake/emscripten information from it so it is useful. Thanks again! |
Just to play an advocate, I hate hate hate pulling in dependency code directly into my projects as it pollutes my repositories with code that is not mine to maintain. As for ease of use, with cmake when I want to bring in a dependency then I'll add something like this in my CMake file (I'm using boost compiled libraries as an example as those are generally fairly painful otherwise, this shows the ease of use): hunter_add_package(Boost COMPONENTS system filesystem) At this point all the files needed for the system and filesystem parts of boost are downloaded and compiled at the default version in my linked repo (latest stable, others are available, and I can specify a version on the above line if you want a specific version that is older than the one in the dep lock file). I then use it like any other library: find_package(Boost CONFIG REQUIRED system filesystem)
target_link_libraries(my_project Boost::system Boost::filesystem) And that's it. To specify a lock file I just do: HunterGate(
URL "https://github.com/ruslo/hunter/archive/v0.16.15.tar.gz"
SHA1 "6974c2150fc0d3b09de3ad1efcbf15d360647ffa"
) And I can use multiple lock files if I so wish, which are combined in order as specified (so, just as an example, imgui could have it's own lock files of it's own versions if it didn't want to be listed in the main 'main' hunter dependency repository). If imgui could be built as a bog-standard cmake system then it could be added to such a repository system with trivial ease. There can be many versions of imgui there-of in the dependency system, for different branches, different actual versions, could be broken up into component parts or not, and the user doesn't have to worry about the API ever changing until they actually update HunterGate call's and/or change the version listed, it is very reproducible builds all while keeping third-party code out of your own code repository. |
@@ -129,12 +136,30 @@ void ImGui_ImplGlfwGL2_RenderDrawData(ImDrawData* draw_data) | |||
glDisableClientState(GL_COLOR_ARRAY); | |||
glDisableClientState(GL_TEXTURE_COORD_ARRAY); | |||
glDisableClientState(GL_VERTEX_ARRAY); | |||
#ifdef __EMSCRIPTEN__ | |||
if(last_texture) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ocornut pay attention to this too. Without this check I have problem to draw custom scene.
@ocornut, I review changes in It's okay that my changes can not be applied right now. We live in an era of change and this is normal. I am glad that there is a discussion on an interesting topic for me and that the results can be useful for everyone. I also think that I need to take into account the experience of @tamaskenez. |
By the way, about the changes: what about moving the Demo functions in a separate header (for example, |
Rebase on Dear @ocornut, please review again and merge if possible. Now possible install ImGui like:
Then copy example directory (for example example_glfw_opengl2) and use it as base for custom project. |
Rebase on Dear @ocornut, please review again and merge if possible. Now implemented: Export targets:
Import targets from:
Examples:
|
Sorry I don't know when I can look at this in great details yet, but this looks solid. Thanks!
|
Short ansver for |
Ok. I will rename |
Technically I can implement everything in one file, but it will be difficult to understand. |
36d2c64
to
b6d41a7
Compare
Rebase on v1.87. |
c817acb
to
8d39063
Compare
Rebase on v1.88. |
@podsvirov Given that the owner never accepted (not merged) this PR? Are you considering creating a separate repo for this PR content? |
@JackBoosY It's already done in |
I mean the repo only contains cmake code. |
Kind of sad that after 4 years CMake is still not supported. 😀 |
There is not much to support to be honest. On one hand you have build scripts like mine that try to do everything right in all contexts. As you can see from script length it is far from trivial to cover all the cases, and even after all that it still is incomplete. On the other hand library does not require any special build procedure and 99% of the time all you have to do to include it in your project is this: add_library(imgui STATIC
${imgui_SOURCE_DIR}/imgui.h
${imgui_SOURCE_DIR}/imgui_internal.h
${imgui_SOURCE_DIR}/imgui.cpp
${imgui_SOURCE_DIR}/imgui_demo.cpp
${imgui_SOURCE_DIR}/imgui_draw.cpp
${imgui_SOURCE_DIR}/imgui_tables.cpp
${imgui_SOURCE_DIR}/imgui_widgets.cpp)
target_include_directories(imgui PUBLIC
${imgui_SOURCE_DIR}/
${imgui_SOURCE_DIR}/backends) Or this: target_sources(MyTarget PRIVATE
${imgui_SOURCE_DIR}/imgui.h
${imgui_SOURCE_DIR}/imgui_internal.h
${imgui_SOURCE_DIR}/imgui.cpp
${imgui_SOURCE_DIR}/imgui_demo.cpp
${imgui_SOURCE_DIR}/imgui_draw.cpp
${imgui_SOURCE_DIR}/imgui_tables.cpp
${imgui_SOURCE_DIR}/imgui_widgets.cpp)
target_include_directories(MyTarget PRIVATE
${imgui_SOURCE_DIR}/
${imgui_SOURCE_DIR}/backends) Only real use of in-tree CMake script is for library developers that use IDE which supports CMake. Other than that, adding a CMake build script would just be extra strain on Omar for no practical gain whatsoever. Conclusion: it is most likely not going to happen, because users do not really need it. They may think they need it, but they really do not. |
ImGui with CMake support is available on Conan (I didn't try, I'm using my own script). I understand and agree why Omar doesn't add official CMake support. But I think one can have legitimate reasons to use ImGui via CMake even in IDEs without CMake support (but as an actual, precompiled library, not as an interface library as in @rokups' script). Reasons I use it:
|
From my experience in dealing with lots of dependencies i can say that there is no "one size fits all" solution. If you are using dependencies through some package manager - this task is ultimately on shoulders of package maintainer. If you want to use imgui through In the end you must answer this to yourself: what would change for you, if imgui had a cmake build script in the repo? And answer is - hardly anything. So this boils down to "would be nice", but cost-benefit analysis makes me conclude that it indeed is not worth the effort. |
@rokups That pretty much goes against the goal of programming and automating things in general. 🙂 I ended up creating minimalistic repository with
However, unfortunately I have to agree that in this case it would be very difficult to make something generic because there are too many backends that people can use. Either way it is sad to see @podsvirov 's effort go to waste. |
Hi all! You are all right! It's hard for me to please everyone all at once. I am a supporter of incremental improvements. Let's say the Christmas miracle happens and basic CMake support is added, then the community can improve it. Let's... :-) |
I don’t think those efforta are going to waste. The cmake pr and repos exist for this purpose. I am not against helping to make this more visible somehow. Compiling/Linking with core Dear ImGui (without backends) is trivial. The problem is compiling/linking with dependencies required by backends. In principle if you have your own app running, you should ALREADY have a way to use those dependencies. It is tremendously difficult to provide a cmake that will nicely overlap with the myriad of ways those dependencies can be used and obtained. It is a problem for the Windows ecosystem mostly. If only Linux/osx existed providing those cmakefiles would likely be easier. Third-party cmakefiles have the advantage that if they don’t work with your setup you are more likely to sidestep the problem. I think most people complaining about the lack of an official cmakefile are misunderstanding this difficulty and the constraints they would add. @eliasdaler also wanted to make this happen at some point and ended up thinking it wasn’t easy to solve. |
Yeah, I've tried here but eventually gave up. But for Dear ImGui itself - not really, it just has so many "manual" configuration tweaks that expressing it in CMake build script would be a huge burden to maintain. I had quite a lot of problems maintaining my own ImGui-SFML's CMake build... now imagine maintaining 5+ builds at once. |
FYI in 1b27ac9 i have renamed imgui_impl_sdl.cpp to imgui_impl_sdl2.cpp as SDL3 is down the road (and we have a backend for SDL3 now). I wouldn't suggest bothering with the SDL3 part of building as the library is not going to be out for a while, but applying the renaming of the SDL2 backend will be needed. |
Thanks for mentioning it here. I'll keep that in mind for the next update. |
I have rebased this branch on latest:
I didn't push intermediary names e.g. Again, apologize for leaving that dangling. It's unlikely that I can devote full energy to this soon but one day I will. In the meanwhile it's nice having that cherry pickable commit for some people I suppose. |
Export Dear ImGui as CMake's ImGui package. Options: - ImGui_USER_CONFIG; - ImGui_EXAMPLES; - ImGui_BACKENDS; - ImGui_MISC; - ImGui_3RDPARTY; - ImGui_OPENGL_LOADER; - ImGui_FREETYPE; - ImGui_TOOLS; - ImGui_PACKAGE. Export targets: - ImGui::Core; - ImGui::ImplGLUT; - ImGui::ImplSDL2; - ImGui::ImplGlfw; - ImGui::ImplOpenGL2; - ImGui::ImplOpenGL3; - ImGui::ImplVulkan; - ImGui::FreeType; - ImGui::StdLib; - ImGui::BinaryToCompressedC. Import targets from: - build directory; - installed package. Examples: - example_null; - example_emscripten_opengl3; - example_glut_opengl2 - example_sdl2_opengl2; - example_sdl2_opengl3; - example_sdl2_vulkan; - example_glfw_opengl2; - example_glfw_opengl3; - example_glfw_vulkan.
Add CMake project
Now implemented:
Export Dear ImGui as CMake's ImGui package.
Options:
Export targets:
Import targets from:
Examples:
Users can easy link ImGui::XXX or ImGui::ImplXXX libraries and use example binding
implementation ore other misc features in custom projects.
Tested with MinGW-w64, MSVC, Emscripten.
Live Preview
Based on this PR I port some examples to Emscripten platform.
Now we need path for some binding code.
And my examples port.
WebGL 1.0
WebGL 2.0