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

IMGUI_API in imgui.h and imgui_impl_... #1888

Closed
sonoro1234 opened this issue Jun 18, 2018 · 8 comments
Closed

IMGUI_API in imgui.h and imgui_impl_... #1888

sonoro1234 opened this issue Jun 18, 2018 · 8 comments
Labels

Comments

@sonoro1234
Copy link

Hi,

In imgui_impl_... files IMGUI_API precedes function declaration, If IMGUI_API is defined as extern "C" __declspec(dllexport) they can be compiled as functions accesible by a binding, but IMGUI_API defined this way in imgui.h fails because they are not exportable C function.

Solution would be to have IMGUI_API in imgui.h but IMGUI_APIX (any different name is ok) in the implementations

@ocornut
Copy link
Owner

ocornut commented Jun 20, 2018

Since you are presumably only including the imgui_impl_xxx.h files once, can't you change the IMGUI_API define jusr for them?

@sonoro1234
Copy link
Author

now fighting with cmake for that solution

@sonoro1234
Copy link
Author

after being able to do it with

set_source_files_properties(./cimgui/imgui/examples/imgui_impl_opengl3.cpp PROPERTIES COMPILE_DEFINITIONS "IMGUI_API=extern \"C\" __declspec\(dllexport\)")

compiler complains with

In file included from C:\luaGL\gitsources\luajit-imgui\cimgui\imgui\examples\imgui_impl_opengl3.cpp:28:0:
C:/luaGL/gitsources/luajit-imgui/cimgui/imgui/imgui.h:186:133: error: declaration of C function 'bool ImGui::B
eginChild(ImGuiID, const ImVec2&, bool, ImGuiWindowFlags)' conflicts with
     IMGUI_API bool          BeginChild(ImGuiID id, const ImVec2& size = ImVec2(0,0), bool border = false, ImG
uiWindowFlags flags = 0);

which is caused by:
https://github.com/ocornut/imgui/blob/master/examples/imgui_impl_glfw.cpp#L29

@ocornut
It seems imposible to solve without IMGUI_IMPL_API and IMGUI_API

@ocornut
Copy link
Owner

ocornut commented Jun 20, 2018

I thought imgui_impl_opengl3.cpp wouldn't need that define, but it must affect decorated symbol names so it needs to be consistent.

We could add IMGUI_IMPL_API for a small extra cognitive cost for the user but we need to carry the guarantee that this API will translate to a C-style API. You could also make a wrapper for the 4 functions?

@sonoro1234
Copy link
Author

sonoro1234 commented Jun 20, 2018

Yes I did it and tried in my binding. Althought there is a default argument this was not a stopper.
Only imgui_impl_vulkan.h cant be directly compiled in C,

@thedmd
Copy link
Contributor

thedmd commented Jun 20, 2018

TL;DR: I think IMGUI_IMPL_API is the better way, because it avoid problems with build systems.

For example:

  • imgui is compiled as DLL, therefore IMGUI_API is set to dllimport
  • imgui_impl_xxx are in static library linked to actual application, IMGUI_API should be defined to nothing

Application link to both libraries which cause conflict in IMGUI_API macro and there is no way to solve that without touching imgui_impl_xxx sources. This is no ideal, bacause it is adding maintainance burden and I wish I could just replace old version with a new one without need to modify it. Hence, see TL;DR. : )

Edit: there are reasons why things are organized this way, but they are not relevant to the issue at hand

@sonoro1234
Copy link
Author

Only imgui_impl_vulkan.h cant be directly compiled in C,
I said, but only the structs are not C and the functions can be C-exported.
So the proposal is to precede all function declarations in imgui_impl_ files by IMGUI_IMPL_API

ocornut added a commit that referenced this issue Jun 21, 2018
…_IMPL_API (which defaults to IMGUI_API) to facilitate some uses. (#1888) + Comments in imgui.h
@ocornut
Copy link
Owner

ocornut commented Jun 21, 2018

@sonoro1234, @thedmd: Applied this change now.

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

No branches or pull requests

3 participants