Split imgui backends into separate packages

[Rookfighter]

Hi everybody,

I finally managed to finish a first draft for splitting the backends of imgui. Basically I created one build2 package per backend. Each package is postfixed by its backend-type (either render or platform) and the implementation which it is based on, e.g. metal or opengl.

A few general notes:

• I added GitHub workflows
• The opengl backends depend on the opengl-meta package which I created some while ago
• I am have no clue how to build executables from objective-C code, which also depend on build2 packages, so I would need some help to get the macos examples running
  • @boris-kolpackov gave me some hints / examples on how to do this for libraries, but I do not fully understand what is happening there and I can't figure out how to properly translate that build2 code into an equivalent for executables :D
• The examples use optional dependencies depending on the cxx.target.class
• I had to copy the imconfig.h file to imgui-core and edit it to make dynamic libraries work on windows
  • The original imconfig.h just contains outcommented code and is supposed to be edited by imgui-users

Feedback and suggestions are welcome

Disclaimer: As of this writing the latest CI build on GitHub is still in progress. If it is broken I will fix it ASAP, but I opened the PR already to give you a chance to comment on what I have done so far.

[Klaim]

A first pass of review:

• I didn't try to use/build these packages yet, will do so soon.
• I have no understanding on the objective-c issues so didn't comment on these parts.
• Beware of the end-of-files (with red markers), you probably want to fix that to avoid issues with some tools.

[Klaim]

I cloned and built successfully the example package locally on Windows 11 + msvc (2022 preview), I tried all the current examples (knowing that some are missing) and they all work as expected. Good job!

I'll also try on linux tomorrow.

[Klaim]

I also tried to create a new project and depend on this repo's packages, but it fails at initialization:

$ bdep init
initializing in project E:\projects\build2-packaging\dearimgui\usertest\myapp\
synchronizing:
  update glfw/3.3.4+1 (required by imgui-platform-glfw)
  new imgui-core/1.86.0-a.0.20220601215117 (required by imgui-platform-glfw, imgui-render-opengl3)
  new imgui-platform-glfw/1.86.0-a.0.20220601215117 (required by myapp)
  new opengl-meta/1.0.0-a.0.20220601235057.b9271584abf6 (required by imgui-render-opengl3)
  new imgui-render-opengl3/1.86.0-a.0.20220601215117 (required by myapp)
  new myapp/0.1.0-a.0.19700101000000
checking out imgui-core/1.86.0-a.0.20220601215117
distributing imgui-core/1.86.0-a.0.20220601215117
error: distribution of uncommitted project ..\build-msvc\.bpkg\tmp\63c3ca6500b9\imgui-core\
  info: specify config.dist.uncommitted=true to force
 84% of targets distributed

This is reproducible by just b dist ... from one of the packages sources after cloning.

This will probably be fixed once you remove -a.0.z from version names in manifest.

Using bdep init --build-option config.dist.uncommitted=true instead works around the issue.

Then it leads to linknig errors (when using the code copy pasted from example_glfw_opengl3 into the main source of a bdep new -t exe project. The link errors are because some symbols are imported/exported while I used the static targets. This shows that something is wrong with the symbol import/export dance. Note that most imgui users will use the static versions only (and the doc recommend to not use the shared library version, see imguiconfig.h comments for example) so this scenario needs to work as a priority. Once I switched to shared libraries for imgui, the issue was fixed and the program works as expected 👍🏽

Here is the app in case you want to test (it's quick to reproduce but it's handy to keep it around in a repo): https://github.com/Klaim/build2-test-usage-imgui

[Rookfighter]

@boris-kolpackov Concering the lib prefix of the examples package: True, I was wondering what would be the correct variant. Since the examples package actually contains examples for the libimgui package (and not any utility applications) I thought libimgui-examples would be the correct name. Anyway, will fix it.

[Rookfighter]

@boris-kolpackov Thanks for fixing the bdep ci issue! Works now also on my side.

I think I will leave the mingw support for a follow-up PR, as I can't get it to work straight away and mingw is also not really my priority, right now.

@Klaim I added config files as you suggested (without the common config file for now). However the options are not applied during build time. For example when I use bdep init -C @gcc --options-file config/gcc.release.build I get the following output

$ bdep init -C @gcc --options-file config/gcc.release.build 
initializing in project /home/fabian/develop/imgui/
../imgui-gcc/build/config.build: warning: dropping no longer used variable config.cc.coptions
  info: variable value: '-O3 -DNDEBUG'

When I use b --options-file config/gcc.release.build I do not see any warnings / errors, but the config parameters still do not get applied. Any ideas?

[Klaim]

Any ideas?

Weird, I dont remember ever seeing this. I'll take a look ASAP.

[Klaim]

I managed to reproduce the warning, you used:

$ bdep init -C @gcc --options-file config/gcc.release.build

In that example you did not specify the kind of build configuration, which should be cc I guess, that should appear after the configuration name/directory and before the options. I get that warning only when I don't set this field, probably because build2 assume the next field should be something else.

$ bdep init -C @gcc cc config.cxx=g++ --options-file config/gcc.release.build

Works for me without the warning.

BTW I would have preferred the compiler name set in the config files too, so that the user only have to specify the option file (and the CI only need to use the right file). Also, for msvc debug you need the config.cxx=cl /MDd otherwise it will not work as expected. For example here is the content of one of my projects using SFML for msvc debug:

config.cxx=cl /MDd
config.cc.coptions += /DEBUG /Od /Zi
config.cc.loptions += /DEBUG

However if you have a reason to keep the compiler name given by the CI, then nevermind. In that project using SFML I setup the CI to use these files so I think it should work.

Other than that I just tried with msvc debug and release and it initializes without warnings and builds some examples as expected. Same for init with g++ on linux/ubuntu (I couldnt build because of missing X11 dependencies or something like that)

[boris-kolpackov]

$ bdep init -C @gcc --options-file config/gcc.release.build 

The better way to load extra configuration files is with the config.config.load variable:

$ bdep init -C @gcc cc config.config.load=config/gcc.release.build

[boris-kolpackov]

Quick question: Is comparing cxx.target.system to 'mingw32' the correct way to check for mingw? I took this from
here, but I also see the possibility to check via cxx.id == gcc. Which one is recommended way?

The *.target.system is the correct way: you can also use Clang to build for MinGW. And in parallel universe GCC might target a different runtime on Windows (say, MSVC).

[Rookfighter]

@Klaim I added the compiler specification to the options files.
@boris-kolpackov I am still using the --options-file flag as I got the following error with config.config.load:

$ bdep init -V -C @gcc cc config.config.load=build-config/gcc.release.build
mkdir /tmp/bdep-12013-0/
initializing in project /home/fabian/develop/imgui/
mkdir /home/fabian/develop/imgui/.bdep/
bpkg --verbose 3 create -d /home/fabian/develop/imgui-gcc --name gcc --type target --no-host-config --no-build2-config cc config.config.load=build-config/gcc.release.build
mkdir -p /home/fabian/develop/imgui-gcc/
b --verbose 3 config.config.load=build-config/gcc.release.build "create('/home/fabian/develop/imgui-gcc/', cc)"
mkdir /home/fabian/develop/imgui-gcc/build/
cat >/home/fabian/develop/imgui-gcc/build/bootstrap.build
cat >/home/fabian/develop/imgui-gcc/build/root.build
cat >/home/fabian/develop/imgui-gcc/buildfile
build-config/gcc.release.build: error: incompatible config file build-config/gcc.release.build
  info: config file version   0 (missing)
  info: config module version 1
  info: consider reconfiguring @/home/fabian/develop/imgui-gcc/
rmdir -r /tmp/bdep-12013-0/

Nevertheless, I think this PR might (finally) be nearing its end. One last question:
After this PR is merged, should we rename the repo to libimgui, too? Or leave it as imgui? I guess renaming it would invalidate all existing references to this repo in outher packages / projects. But renaming would also be more consistent with other package repositories like libfreetype. Any thoughts?

[boris-kolpackov]

After this PR is merged, should we rename the repo to libimgui, too? Or leave it as imgui? I guess renaming it would invalidate all existing references to this repo in outher packages / projects. But renaming would also be more consistent with other package repositories like libfreetype. Any thoughts?

I think for the repository itself using upstream name is a good idea, especially if it contains (or may contain in the future) non-library packages.

I got the following error with config.config.load [...]

This mechanism expects a real configuration file (like config.build; it's often created with config.config.save) and which must include a version. Try adding the following as a first non-comment line in your gcc.release.build file:

config.version = 1

[boris-kolpackov]

@Rookfighter BTW, what is the status of CI (as submitted with bdep ci to ci.stage.build2.org)? Ideally, we would at least want successful builds for the main platform/compiler combinations. Maybe add a link to the latest CI submission here?

[Rookfighter]

@boris-kolpackov Thanks, I switched to using config.config.load. The version was what was missing.

Here' a bdep ci run with the latest commit: https://ci.stage.build2.org/@161ea517-24de-4c17-bb06-cce35a4f7657

[Swat-SomeBug]

Hi @Rookfighter awesome job with the PR.
I noticed one point which may not be directly related to this PR, but in general when a glue buildfile is used to the selectively include packages (based on cxx.target.class as in this case), bdep and b behave differently.
During bdep init, the top level glue buildfile seems to be ignored (Not sure, just guessing?) and all the packages are initialized even when they are not meant to be built. As I understand it, b would build the default config but considers the glue buildfile and builds everything correctly.
bdep update on the other hand, builds all the initialized packages which then causes build errors (building libimgui-render-metal on linux for example).
@boris-kolpackov is this observation inline with the current implementation? Is there a way to selectively initialize package using the glue buidlfile?

Tested on:

$ b --version
build2 0.15.0-a.0.2f29c7fbe758
libbutl 0.15.0-a.0.0ae2c768fc8b
$ bdep --version
bdep 0.15.0-a.0.ed05ed4c87df
libbpkg 0.15.0-a.0.9c476d4b12ba
libbutl 0.15.0-a.0.0ae2c768fc8b

[boris-kolpackov]

[...] is this observation inline with the current implementation?

Yes, that's pretty accurate. A glue buildfile is just a convenience to be able to build packages in the default configuration from the multi-package repository root when using the build system directly. This works fairly well for simple cases where there are no platform-specific packages or similar complications. For the complex cases (platform-specific packages, etc) I think it's best to just remove the glue buildfile since the extra complexity to make it work is not worth the trouble in most cases.

Is there a way to selectively initialize package using the glue buidlfile?

No, bdep doesn't know anything about glue buildfiles. It could theoretically be possible to do it the other way around: for the glue buildfile to only build packages that have been initialized by bdep.

[boris-kolpackov]

@Rookfighter

Here' a bdep ci run with the latest commit [...]

Thanks! So there are 792 builds of which 554 are failures. Looking at the first few it's clear the cause of most of the failures is an attempt to build on platforms that do not apply (like building libimgui-platform-osx on Windows). To reduce the noise it would be helpful to exclude irrelevant platforms as described here: https://build2.org/bpkg/doc/build2-package-manager-manual.xhtml#manifest-package-builds

[Swat-SomeBug]

No, bdep doesn't know anything about glue buildfiles. It could theoretically be possible to do it the other way around: for the glue buildfile to only build packages that have been initialized by bdep.

Would it makes sense to add compilation conditions at the package level?
Example:

imgui-platform-osx/imgui/buildfile:
-----------------------------------
./: lib{imgui-platform-osx}: include = ($cxx.target.class == 'macos')

The idea is to disable building the library itself. So even if bdep initialises the package, building itself is prevented at the package level.

[boris-kolpackov]

Would it makes sense to add compilation conditions at the package level?

To me this looks like trying to fix the issue in the wrong place. The correct place is to not bdep-initialize the packages that you don't need. Currently this can be achieved manually (just specify the desired subset with -d). Maybe in the future we can come up with an automatic way, but at this stage this doesn't feel like the best use of our resources.

[Klaim]

In some of my multi-package projects I ended up with a init_configs.sh scripts that would create configs and initialize specific packages depending on the config (for example usually only a few packages can exist in a WASM config).
I used that in CI too, so that the CI script ended up being something like

./init_configs.sh
bdep update --all
bdep test --all

(assuming git-bash on windows)

I'm not saying to do this here, just stating my experience. I wonder if it could made easier by some future bdep features, but that's out of scope here.

[Swat-SomeBug]

@Rookfighter I suggest addition of builds clause for packages that don't build on all platforms.

[Swat-SomeBug]

Suggested change

	depends: libimgui == $
	depends: libimgui == $
	builds: &( +windows -gcc )

[Swat-SomeBug]

Suggested change

	depends: libimgui == $
	depends: libimgui == $
	builds: &( +macos -gcc )

[Swat-SomeBug]

Suggested change

	depends: libimgui == $
	depends: libimgui == $
	builds: &( +windows -gcc )

[Swat-SomeBug]

Suggested change

	depends: libimgui == $
	depends: libimgui == $
	builds: &( +windows -gcc )

[Swat-SomeBug]

Suggested change

	depends: libimgui == $
	depends: libimgui == $
	builds: &( +windows -gcc )

[Swat-SomeBug]

Suggested change

	depends: libimgui == $
	depends: libimgui == $
	builds: &( +windows -gcc )

[Swat-SomeBug]

Suggested change

	depends: libimgui == $
	depends: libimgui == $
	builds: &( +macos -gcc )