imgui-sfml
imgui-sfml copied to clipboard
SFML
Better naming and folder structure
Right now, there's a bit of inconsistency in naming. The repo is named "imgui-sfml", the files are "imgui-SFML.h" and "imgui-SFML.cpp", the CMake target is "ImGui-SFML" Plus, the repo doesn't follow good C++ lib project structure. What I propose is this layout:
include/
imgui-sfml/
imgui-sfml.h
...
src/
imgui-sfml.cpp
The CMake target will still be "ImGui-SFML".
In your code, you'll need to do this:
#include <imgui-sfml/imgui-sfml.h>
Please share your thoughts in the thread.
I think it's a good idea, this project structure is also followed by SFML, it will be more consistent in the includes.
But we can't have full mandatory consistency, ImGui itself have no special structure. Some CMake can add one to also have a subfolder include (e.g. #include <imgui/imgui.h>) but as imgui-SFML.cpp uses #include <imgui.h>, we still need to have the include without subfolder available.
Good point, that's a pretty difficult thing to figure out...
I can make it so that during install, you'll get this layout (and use <imgui/imgui.h> in imgui-sfml.cpp):
/usr/include/
imgui/
imgui.h
imgui-sfml/
imgui-sfml.h
Another possibility (and use <imgui.h> in imgui-sfml.cpp):
/usr/include/
imgui-sfml/
imgui-sfml.h
imgui.h
And the worst (and use <imgui.h> in imgui-sfml.cpp):
/usr/include/
imgui-sfml/
imgui-sfml.h
imgui.h
What's happening now is this:
/usr/include/
imgui.h
imgui-sfml.h
The problem putting imgui.h in /usr/include/ is that someone installing ImGui or another binding wight override it with a future/past unsupported version.
Putting imgui.h in /usr/include/imgui/ can lead to the same problem, if someone have the same idea.
The better is maybe
/usr/include/
imgui-sfml/
imgui-sfml.h
imgui.h
to control the ImGui version used.
But still if someone install ImGui or another binding with imgui.h in /usr/include/, with #include <imgui.h> there is a file name conflict and it migth resolve to /usr/include/imgui.h or /usr/include/imgui-sfml/imgui.h.
So maybe use #include <imgui-sfml/imgui.h> even if it is in the same folder to solve the problem.
@ocornut
Any thoughts on where imgui.h should be stored when installing ImGui-SFML?
I think imgui shouldn't be installed for variety of reason including some highlighted by @pinam45 above. There you have my answer.
So, I guess ImGui-SFML should install without imgui.h and require users to add it to their build manually? That might make it problematic if ImGui-SFML was built with ImGui v1.XX, but then user attempted to use it with ImGui v1.YY and there was ABI change there.
In scope of this, I think it'll be good to start producing 'libimgui-sfml.so' and 'libimgui-sfml.a', not 'libImGui-SFML.so' and 'libImGui-SFML.a'.
Why? Also, that's an option - the static library is built by default, but there's an option to build a shared one.
Dear ImGui is not ABI forward nor backward compatible. In addition it is a library that typically involve dozen of thousands of function calls every frame, building as DLL is only going to add extra overhead.
I agree, but it's for the user to decide if they want to build it as DLL or not. IMGUI_API are sprinkled around the code for a reason. ;)
ImGui-SFML v3 is a great place to make such a breaking change. I'd love to remove some of those capital letters. They look pretty awkward. I support pretty much everything Elias recommended so long as we don't touch the path to imgui.h since we don't own that header. These changes will be no trouble to implement. I'll likely write this up in the near future.
