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

Submodules for cimguizmo, cimnodes, cimplot and CMake file to build corresponding libraries #3

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

TillAlex
Copy link

This is just a first test how to build separate DLLs for cimplot, cimnodes and cimguizmo. Only tested cimplot until now. Not sure how you would integrate it.

@TillAlex
Copy link
Author

TillAlex commented Dec 1, 2020

CI seems to work for Windows, Linux and OSX now.

@sonoro1234
Copy link

Having imgui.cpp compiled in different Dlls is risky because of globals.

@TillAlex
Copy link
Author

Personally I would prefer compiling into a single binary, but I also understand that @mellinoe wants to keep binary size small for users not using all modules.

From the imgui docs I understand that the only globals are context and allocator functions. As long as your are setting those for all DLLs you should be OK. Is this correct?

@sonoro1234
Copy link

I hope so. But another alternative would be having imgui.cpp only in cimgui.dll and let the others Dlls link dynamically to cimgui.dll

@TillAlex
Copy link
Author

Are all the internal imgui symbols needed for those modules exported? Then this is certainly the best solution. Will have a look into this as soon as I have the time.

@sonoro1234
Copy link

sonoro1234 commented Apr 11, 2021

Are all the internal imgui symbols needed for those modules exported?

You should define IMGUI_API as __declspec( dllexport ) in cimgui.dll and then __declspec( dllimport ) in the others.

Althought the notes in: https://github.com/ocornut/imgui/blob/master/imconfig.h#L22 are saying that globals cant be shared across DLL boundaries.

@ThomasFOG
Copy link

ThomasFOG commented Apr 21, 2021

ImGui.NET is a pretty lightweight package in comparison to most other ones (if I refer to .NET game development tools). If I'd have to share my thoughts on the topic, I'd say that the benefits of having all the libraries built into one single library and nuget outweighs by far the value of having individual small nugets.
If it really is an issue, ImGui.NET could be packaged into 2 flavors: one naked, one with all extensions built into cimgui.

Another thing to me might be an issue: cimgui dynamically links to the VC++ runtime. For game development, it's preferred to statically link to it.

EDIT: FYI, I've derived this work to implement this proposal and build a unified binary with the VC runtime statically linked and with macOS backward compatibility: https://github.com/FlyingOakGames/ImGui.NET-nativebuild/tree/widgets

@zaafar
Copy link
Collaborator

zaafar commented Sep 14, 2021

-> If it really is an issue, ImGui.NET could be packaged into 2 flavors: one naked, one with all extensions built into cimgui.

I like this idea, esp if CI pipelines can be created that can release 2 flavors.

@MarioGK
Copy link

MarioGK commented Apr 21, 2022

Is there something missing to merge this PR? I would be glad to help out so this can be merged

@Hi-ImKyle
Copy link

Hi-ImKyle commented Mar 8, 2023

I hope so. But another alternative would be having imgui.cpp only in cimgui.dll and let the others Dlls link dynamically to cimgui.dll

How would one go about doing this? I'd like to avoid problems as much as possible and this sounds like it avoids a big one?

EDIT: Is anyone actually able to use the additional libraries in C#? I can get so far but always end up with AccessViolationExceptions trying to use ImPlot.BeginPlot(), KingFisher#0001 on Discord if you are able to help me out 👍

@sonoro1234
Copy link

How would one go about doing this?

with cmake:

add_library(cimgui SHARED ${CIMGUI_SOURCES})
add_library(cimplot SHARED ${CIMPLOT_SOURCES})
target_link_libraries(cimplot PUBLIC cimgui)

with IMGUI_API defined for exporting functions

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

Successfully merging this pull request may close these issues.

6 participants