mosra / magnum-integration

Integration libraries for the Magnum C++11 graphics engine
https://magnum.graphics/
Other
99 stars 44 forks source link

imgui_stdlib is not being detected #36

Closed paladium closed 5 years ago

paladium commented 5 years ago

I found myself using imgui_stdlib.h file which contains some overrides for using std libraries with ImGUI. However Magnum does not include those when finding components. A quick fix that worked for me was:

File FindImGui.cmake
Line 101: foreach(_file imgui imgui_widgets imgui_draw imgui_demo **imgui_stdlib**)

Next a new entry will appear in cmake to point to that file. Hope that helps someone.

mosra commented 5 years ago

Hi, thanks for the report!

I tried to do this in a way that doesn't make the whole ImGuiIntegration library depend on the imgui_stdlib.cpp file, since this particular STL compatibility is not used there. I came up with the following, currently only in the next branch -- first take the updated FindImGui.cmake from here, then in your CMakeLists add this:

find_package(ImGui REQUIRED SourcesMiscCpp)

...

target_link_libraries(your-app PRIVATE ImGui::SourcesMiscCpp)

After that, you should be able to

#include <misc/cpp/imgui_stdlib.h>

and everything should work as expected. Though I'm not sure if the misc/cpp prefix makes sense, maybe I'll make it so only #include <imgui_stdlib.h> is needed. What do you say? Can you test on your side and confirm it works? Also, suggestions welcome :)

paladium commented 5 years ago

Yes that works, thanks for the fix. The stdlib for imgui is still in development and has little useful functions at this point, however if it becomes larger, it will make sense to make ImGUIIntegration dependent on that file. At this point, this would work.

mosra commented 5 years ago

Thanks for the confirmation! :+1: 3a2cb984d2dc68a3430eeb9e6119cbbc2104eae8 is now in master.

I'm not sure if @ocornut wants to depend on STL too much, so I assume it will stay like this for quite a while. But who knows. If it will later make sense to have it directly, then I can make that change, of course :)

melix99 commented 5 years ago

Unfortunately I'm having problems even using the find_package(ImGui REQUIRED SourcesMiscCpp) inside my CMakeLists: CMake says that it could not find the component SourcesMiscCpp. I'm using the latest ImGui release, and the path of the misc folder is the same as the one specified in the FindImGui.cmake so... idk what's going on.

I also want to specify that I have magnum-integration as my CMake's subdirectory, and that if I add the SourcesMiscCpp to the find_package inside the CMakeLists of the library, the find_package inside my CMakeLists can find the component for some reason (but it's not the best solution obv).

Any suggestions?

mosra commented 5 years ago

That's weird ... I just tried to reproduce here (adding the find_package() to the imgui example), but everything works as expected.

melix99 commented 5 years ago

Are you sure your copy of FindImGui.cmake is the same as the one in the integration repo?

Yep, I literally copied the modules directory from magnum-integration inside mine.

I just tried to reproduce here (adding the find_package() to the imgui example), but everything works as expected.

I just tried that too and it worked for me either, so I analyzed the differences between the example and my project and found that in my project I have the magnum-integration and imgui subprojects inside a ThirdParty directory with a CMakeLists, but, in the imgui example, I putted magnum-integration and imgui directly in a subdirectory inside the main CMakeLists. In fact I did the same thing in my project and it worked fine. To clarify:

My project: Main CMakeLists -> ThirdParty subdir -> magnum-integration subdir (with imgui)

Imgui example: Main CMakeLists -> magnum-integration subdir (with imgui)

This is the CMakeLists inside the ThirdParty dir:

set(WITH_IMGUI ON CACHE BOOL "" FORCE)
set(IMGUI_DIR "${CMAKE_CURRENT_SOURCE_DIR}/imgui/")

add_subdirectory(magnum-integration)

Maybe I did something wrong here?

mosra commented 5 years ago

I see the problem now -- find_package(ImGui REQUIRED SourcesMiscCpp) needs the IMGUI_DIR. Because you're setting it in a subdirectory, the parent won't see it, and then FindImGui.cmake will fail to find the sources because it doesn't know where to look. One way to fix this would be to put it into the cache, so parent dir can pick it up from there:

set(IMGUI_DIR "${CMAKE_CURRENT_SOURCE_DIR}/imgui" CACHE STRING "" FORCE)
melix99 commented 5 years ago

Oh ok, that worked. Thanks!