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

Add CMake project #1778

Closed
wants to merge 1 commit into from
Closed

Conversation

podsvirov
Copy link
Contributor

Now implemented:

  • Build ImGui library (CMake's package);
  • Optionaly build implementation (ImGui_BUILD_IMPLEMENTATION option);
  • Optionaly build examples (ImGui_BUILD_EXAMPLES option);

Added examples:

  • opengl2_example;
  • sdl_opengl2_example.

CMake's ImGui package contains targets:

  • ImGui::Library;
  • ImGui::Demo;
  • ImGui::ImplOpenGL2;
  • ImGui::ImplGlfw;
  • ImGui::ImplSDL2.

Users can easy link ImGui::Impl libraries and use example binding
implementation in custom projects.

Tested with MinGW-w64 and MSVC toolchains.

Now implemented:
- Build ImGui library (CMake's package);
- Optionaly build implementation (ImGui_BUILD_IMPLEMENTATION option);
- Optionaly build examples (ImGui_BUILD_EXAMPLES option);

Added examples:
- opengl2_example;
- sdl_opengl2_example.

CMake's ImGui package contains targets:
- ImGui::Library;
- ImGui::Demo;
- ImGui::ImplOpenGL2;
- ImGui::ImplGlfw;
- ImGui::ImplSDL2.

Users can easy link ImGui::Impl<Xxx> libraries and use example binding
implementation in custom projects.

Tested with MinGW-w64 and MSVC toolchains.
@podsvirov
Copy link
Contributor Author

Hello @ocornut, please review this PR. Changes like #1713 but for viewport branch.

@ocornut
Copy link
Owner

ocornut commented Apr 27, 2018

Hello @podsvirov, thanks a lot! This looks solid!

My mental roadmap is that I would like 1.62~1.63 to include the refactored example structure (the same as in the viewport branch, but without the multi-viewport support) in master. At the same time we can provide both premake and cmake projects. So this can be part of that effort.

I don't know when I will be able to test and look at this seriously, please bear with me there. Some quick feedback:

  • imgui_demo.cpp should always be packaged in the main library and not as a separate thing (also see the comments at the top of imgui_demo.cpp).

  • Could ImGui::ImplOpenGL2 be renamed to ImGuiImplOpenGL2 etc. or it a standard practice in cmake world to name things this way?

  • Be mindful that neither SDL or GLFW implies OpenGL. Both SDL and GLFW and can be used with Vulkan or other back-ends (potentially SDL can be used with DirectX, etc.). So the dependencies needs to be listed with that in mind. The idea of those refactored examples is that things can be easily combined and reduce code duplication and maintenance.

  • Under Windows the end-goal is to generate .sln .vcproj files that are identical or similar in structure with the existing one. cmake is notorious for (by default) creating projects that looks very different from a default VS project. It is possible to control its output (general project naming organisation, build setting names) finely?

@podsvirov
Copy link
Contributor Author

Answer:

  • For CMake users it's more easy to add/remove demo functionality by link or not the prebuild ImGui::Demo library. The IMGUI_DISABLE_DEMO_WINDOWS will not have any effect after building the imgui_demo.cpp code. I also have idea add ImGui::LibrarySource and ImGui::DemoSource as INTERFACE libraries with sources like in build: cmake support, examples included #255.
  • Prefix with the :: symbols it's mandatory for IMPORTED CMake targets. For example, we use ImplOpenGL2 target name internal, then export it with prefix ImGui:: - result IMPORTED target name for ImGui package users is ImGui::ImplOpenGL2.
  • I start with OpenGL2 examples. I add other examples and apply your notes.
  • Generated .sln identical yours, but contains some libraries like ImplXXX and Demo that links to concrete example. And example projects contains only main.cpp file. We do not need compile imgui_implXXX file int each example.

@ocornut
Copy link
Owner

ocornut commented May 2, 2018

For me the focus of those cmake/premake build files is to build the example apps. It's ok if they have an option to create a .lib but the Demo should be included in there. The Demo is part of imgui and we shouldn't encourage removing it. If users want to configure imgui they can use imconfig.h, they are many other useful options in there that requires compile-time modification and you can't possibly custom-wrap all those options in the cmake file. Let the user pass the IMGUI_DISABLE_DEMO_WINDOWS define if they want but don't encourage it.

ImGui is really not designed to be packaged into a library, nor "installed" in system paths - I think it is often a mistake to do so because it often creates a situation that prevents users from actually configuring the library. We want to encourage people to configure the library to e.g. add the necessary defines to use their maths types and cast them to ImVec2 implicitly, or to modify the ImDrawIdx type, or the ImDrawVert layout.

We do not need compile imgui_implXXX file int each example.

I think it would be perfectly fine compile the imgui_implXXX files into each example, to put an emphasis that they are example code that the user can freely rework and modify, rather than libraries.

@OvermindDL1
Copy link

If users want to configure imgui they can use imconfig.h

Yeah this is still a horror. We'd never ever ever be okay with editing third-party source just to set configs. A configure script or CMakeLists file is definitely what should be done to generate such a file (this is why there needs to be a top-level CMakeLists.txt file who's sole purpose is to include the necessary source files and generate the configuration source based on the options specified, this would allow add_subdirectory'ing imgui into a project).

ImGui is really not designed to be packaged into a library, nor "installed" in system paths - I think it is often a mistake to do so because it often creates a situation that prevents users from actually configuring the library. We want to encourage people to configure the library to e.g. add the necessary defines to use their maths types and cast them to ImVec2 implicitly, or to modify the ImDrawIdx type, or the ImDrawVert layout.

This should all be done via a generated configuration file, all of it.

@ocornut
Copy link
Owner

ocornut commented May 2, 2018

Well you can generate a imconfig file and use -DIMGUI_USER_CONFIG myconfigfile.h.
EDIT I have nothing against a cmakefile that generate ones and compiles with it. I'm merely saying that the demo is part of imgui, people can use a configuration file to remove it, and the nature of the configuration file is somehow incompatible with the idea of "installing" the library in system folders.

We'd never ever ever be okay with editing third-party source just to set configs.

What if the file didn't exist in the repository? Would you consider it third-party to create a file that doesn't exist? The provided file is 100% comments and guaranteed to always be 100% comments. It only exists so people don't need to manually create a file before compiling.

@ocornut
Copy link
Owner

ocornut commented May 2, 2018

If I delete the imconfig.h file from the repo, I slightly reduce friction for people who have an elaborate build system in place (who could perfectly use -DIMGUI_USER_CONFIG, or use a git branch, or delete/overwrite the file from their script), and I largely increase friction for people who want to quickly use imgui in a new/casual project. What do you think is best?

I am happy to hear of other alternatives but I haven't found one yet.

Btw none of that really affects the PR much, I was merely elaborating on the fact that if people want to strip the demo code it should be done in a standard manner of using a configuration file, not by deciding that the cmake script would split imgui in 2 libraries. (I am myself really looking forward to switch to using project generators for the examples so this is super useful.)

@OvermindDL1
Copy link

Keeping the existing imgui config file is fine in the project, but including a default CMakeLists.txt file (and/or make/configure script) to generate one while using the generated file and including the rest of the files as a bindable project in cmake would make add_subdirectory'ing super easy.

@podsvirov podsvirov mentioned this pull request Jul 1, 2018
@ocornut
Copy link
Owner

ocornut commented Jul 3, 2018

Closing this PR as we moved to #1713 with a more complete/recent PR.

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.

3 participants