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

Move stb files into separate directory #1718

Closed
jamesdolan opened this issue Apr 1, 2018 · 9 comments
Closed

Move stb files into separate directory #1718

jamesdolan opened this issue Apr 1, 2018 · 9 comments

Comments

@jamesdolan
Copy link

It would be convenient to include dear imgui in a project via a git submodule, but currently that is problematic because it "pollutes" my header search paths with different versions of stb headers. Keeping externals in a separate directory would avoid this issue.

@ocornut
Copy link
Owner

ocornut commented Apr 1, 2018

How about branching the git repo and deleting the stb_ files in your version of it?

Also note the various stb_ related compile time directive in imconfig.h. Modifying imconfig.h usually imply forking locally.

@jamesdolan
Copy link
Author

jamesdolan commented Apr 6, 2018

Thats what I do now, and its not a huge deal. But not being able to simply do "git submodule update --remote" to grab latest of all dependencies is mildly annoying. And moving external deps into another directory shouldn't really have any negative consequences for any users.

@ocornut
Copy link
Owner

ocornut commented Apr 9, 2018

And moving external deps into another directory shouldn't really have any negative consequences for any users.

Well, it's a library that is meant to be fully contained and having all files in the same folder does make a difference in term of perception.

I reckon the better solution would be to rename those files? (e.g. imgui_stb_truetype.h). It's not a zero friction solution because many users are copying the file manually and it'll be a little confusing, but certainly not impossible.

In the meanwhile, a workaround on your side would be to NOT add the imgui/ folder inside your include path, and have another include file (either also called imgui.h either a different name) that pulled the imgui/imgui.h include. OR to include it via e.g. #include "imgui/imgui.h" which should remove the conflict with the stb files.

@eliasdaler
Copy link
Contributor

Moving stb_* files from the root will indeed be pretty nice.

Is it possible to move all stb files to "extlibs/stb" or something and then require user to add this to their search paths if they want to use the versions which ImGui has in the repo? If the user can add ImGui to include search paths, I'm sure they can add one more folder... (And when we finally make a CMake, it'll be fully automatic)

Yes, it's an additional step for the user to make, but this is what some people do and it works for them. It's good to separate external libraries from the main project, especially when they're header only - makes it much easier.

@ocornut
Copy link
Owner

ocornut commented Sep 5, 2018

@eliasdaler You are ignoring my proposal above of e.g. renaming them, which would solve the same issue without adding extra complication. Right now people are copying files from the root folder, which is a gesture associated to having no external dependency.

EDIT Linking to 1.64 refactor #2036 perhaps this is a good timing to rename them.

@eliasdaler
Copy link
Contributor

eliasdaler commented Sep 5, 2018

@ocornut won't this cause problems with multiply defined symbols if users use both ImGui and another versions of stb_{rect_pack, textedit, truetype.h}?

If not, then just renaming sounds okay.

UPD: looks like include guards will prevent that... but then you'll get a problem when imgui.h and users might use different versions of stb headers and it'll be very confusing...

@ocornut
Copy link
Owner

ocornut commented Sep 5, 2018

The symbols are in a ImGuiStb namespace.

imgui.h and users might use different versions of stb headers and it'll be very confusing...

How it is confusing? (particularly as it would be using different filename)
Regardless, if the user needs to use their version of stb library, imgui still need theirs (the library are frequently patched).

@eliasdaler
Copy link
Contributor

Oh, didn't know about ImGuiStb namespace. It'll work totally fine then. :)

ocornut added a commit that referenced this issue Sep 6, 2018
…extedit.h, and stb_rect_pack.h to imstb_rectpack.h. (#1718, #2036)

If you were conveniently using the imgui copy of those STB headers in your project, you will have to update your include paths.
The reason for this change is to avoid conflicts for projects that may also be importing their own copy of the STB libraries. Note that imgui's copy of stb_textedit.h is modified.
@ocornut
Copy link
Owner

ocornut commented Sep 6, 2018

@jamesdolan
The files are now renamed in 1.65
https://github.com/ocornut/imgui/releases/tag/v1.65
Which should solve your conflict problem.

@ocornut ocornut closed this as completed Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants