Make imgui configurations available from build2

[Rookfighter]

imgui uses the file imconfig.h to configure various build time parameters. The original imgui library requires users to copy and modify this specific file, such that the library builds according to their needs. The build2 package should expose these configuration options as build2 config parameters, otherwise package consumers have no "non-hacky" way of configuring the library to their needs.

See also the discussion: https://github.com/build2-packaging/imgui/pull/2#discussion_r887346121

[KarlHedlund]

Any update on this?

[Rookfighter]

Sorry, there hasn't been any work on this topic that I am aware of. Though, if you feel like digging into this I would be happy to review your PR :+1:

@Klaim or @Swat-SomeBug, is anything on-going for this issue?

[Klaim]

Nothing ongoing on my side and I will not be able to help on it much in the coming months, outside maybe reviewing.

[KarlHedlund]

Sorry, there hasn't been any work on this topic that I am aware of. Though, if you feel like digging into this I would be happy to review your PR 👍

@Klaim or @Swat-SomeBug, is anything on-going for this issue?

I've just started out with Build2 so I can't really make an estimate on how much time this will entail. We had a discussion on the slack server yesterday regarding whether this was possible, that's why I revived the issue. I'm guessing it's not a quick fix then?

[Klaim]

I might be able to clarify what needs to be done at least, maybe that can help you jumpstart. There is 2 approaches to the issue, the one you initially asked in the chat was about allowing to define the header where all the options will be set. This can be done quickly I think but it might not be good to do on the long term. The approach initially discussed in the link above is to generate the header from a template and set the values depending on configurations. Here is how this second approach would be done:

1. Familliarize yourself with the in module and target type: it allows to generate files depending on templates and variables in buildfiles. Also familiarize yourself with how to define configuration variables. Both are described in the build2 manual.
2. Look into the config header in the imgui build2 package and list all the options being presented, their "type", meaning and range of values.
3. For each option, in the imgui build2 package, in build/root.build , add the options as configuration variables (as described in section 2 of build2's manual). Make sure the type and values allowed match what is expected in the original header. Set the default values there too.
4. Make a template file from that header by copying it, then editing the copy so that each option gets it's value from the configuration variable previously setup.
5. Remove the original header from the imgui build2 package and instead generate that header using the template you made, using the in target type.

That's basically it. The most difficult part is to step 3 where you need to have some idea of what these options mean, that's the time consuming part. Except if you already know have an idea, in which case it shouldn't be too hard.

If you have other questions about build2 we can probably help here, feel free to ask here.

I'm guessing it's not a quick fix then?

It's not quick but it's not too long either I think, depends on how much you already know about build2 and imgui. There is almost not technical difficulty other than making sure the configuration options allows the right kind of valeus and then making sure they are properly transmitted in the generated config header (and that the header is properly taken into account but that should already work).

[Rookfighter]

Thanks @Klaim for the detailed description! One addition: we should probably add a description in README-DEV how to adjust the template when upstream changes its config parameters in newer versions.

[kamrann]

I needed to adjust one of the config variables in my project so I've forked the package and made a PR in case it's useful.

Note I've added preprocessor options directly in the buildfile. While generating an imconfig.h would match more precisely upstream's (to me questionable) approach, it seems rather convoluted in this instance. All the options are just optional preprocessor defines, so it doesn't seem worth it to take a build tool variable and generate a header file from it, only to then #define a bunch of things in there, when we can just pass them to the compiler command line. Thoughts?

[Klaim]

I would say the main set of issues is confusion from people knowing Dear IMGUI before using build2 and looking for that configuration to read how it's set, although not sure of the importance of the issue. The lack of documentation of the defined, which is in that header, also doesnt help. Maybe moving that documentation in the ReadMe would help but it also means having to maintain it when it changes upstream.

[boris-kolpackov]

All the options are just optional preprocessor defines, so it doesn't seem worth it to take a build tool variable and generate a header file from it, only to then #define a bunch of things in there, when we can just pass them to the compiler command line. Thoughts?

I think it's a reasonable approach provided you don't have a large number of such macros (with a large number the compilation command line will become unwieldy). I think in this case we are ok.

[Klaim]

Yeah I asked in the PR to just add in the REadme a link to the upstream header which contains the documentation for these options. Other than that probably fine.

[kamrann]

That's done.

I noticed that there were already some changes between imconfig.h in upstream and the modified version in the package - mostly comment changes, but a couple of additions too. It looks like the reason it's not symlinked is to handle Windows symbol export macros. It seems unfortunate to have to maintain changes to that file just for that purpose, so perhaps that could also be handled via command line manipulation in the buildfile instead. Otherwise, if that would be getting into unwieldy territory, maybe a generated imconfig.h is the better way to go, to handle all of this together - I hadn't noticed the manual modifications to imconfig.h when implementing my changes.

[boris-kolpackov]

Otherwise, if that would be getting into unwieldy territory, maybe a generated imconfig.h is the better way to go, to handle all of this together.

I took a look at imconfig.h and it doesn't appear to have been made with the auto-generation in mind (like say config.h.in type of files). Rather, the expectation appears to be for the user to edit it manually. We can likely still process it as is with a bunch of sed builtin calls in an ad hoc recipe (all those //#define IMGUI_XXX lines are quite regular) but it won't be as easy as using the in or autoconf module. Still, seeing that they appear to keep adding new defines, sooner or later it will get into the unwieldy territory so it may make sense to just go with this approach from the start. I can definitely help with and review the recipe.

[kamrann]

Yep, it's very much intended to just be a "copy the file and edit it" kind of thing, which obviously doesn't fit very cleanly with build2 or packaging generally. My understanding of @Klaim's intention was not to process the upstream file, but to simply generate it explicitly from within the package buildfile, based on a manually maintained set of config variables.

I guess a question to ask is simply how far is it worth going to support all upstream configuration? I've not used the library before now, but I'd guess that other than a couple of the defines in there, the vast majority are very rarely used. I wonder is there a way with build2 to make a backdoor so consumers can do the legwork themselves if they really need to, and make package maintenance simpler by only covering the basics? Something like an (admittedly horrible) config.libimgui.additional_defines variable - or maybe there's already a built in way to achieve this?

[boris-kolpackov]

My understanding of @Klaim's intention was not to process the upstream file, but to simply generate it explicitly from within the package buildfile, based on a manually maintained set of config variables.

Yes, that would be another way to do it.

I guess a question to ask is simply how far is it worth going to support all upstream configuration?

I would suggest handling what you need plus maybe anything else that looks likely to be needed (by you or someone else). We can always add more later since adding a configuration variable with false default would be backwards compatible with not having it.

Something like an (admittedly horrible) config.libimgui.additional_defines variable - or maybe there's already a built in way to achieve this?

There is no automatic way but you can already do this if you wanted to. But I would advise against it, especially for something widely-used as imgui since this way makes the options non-composable/negotiatable compared to having a bunch of boolean flags. For details, see: https://build2.org/stage/bpkg/doc/build2-package-manager-manual.xhtml#dep-config-negotiation

[Klaim]

I guess a question to ask is simply how far is it worth going to support all upstream configuration? I've not used the library before now, but I'd guess that other than a couple of the defines in there, the vast majority are very rarely used.

My intuition having worked in the game industry is that they are all used in various ways. If I had to set a priority: I think only the DLL/so symbol export ones are rarely used; the disabling ones are definitely mandatory (for being able to build a releasable game without tweaking the project's code), in addition to anything that helps debugging.

Something like an (admittedly horrible) config.libimgui.additional_defines variable - or maybe there's already a built in way to achieve this?

BTW, isnt there a difference of how defines are treated when they come from cli and when they are in source code? Or is it just a headers-unit thing?

---

Note that the easiest way to both support every configuration and have basically no maintenance to do each release even if they add new options is simply to let the user provide that header (or not) and build imgui with it. How about something like:

config.imgui.imconfig = path/to/the/custom/myimguiconfig.hpp

Then imgui's buildfiles would create a symlink to that header before building the library, with the name that imgui is waiting for (letting the user use whatever file name they want). The that config is unique to a build config anyway. And then that header will be exported to all the related libraries. If that option is not provided, we just symlink the original header wich is only comments. That would also allows users to use the last part of the header allowing for introducing custom code in imgui namespace, which is a regular practice in game development when working on consoles for example.

[boris-kolpackov]

How about something like: config.imgui.imconfig = path/to/the/custom/myimguiconfig.hpp

What happens if two applications shared the imgui library and each want to impose its own myimguiconfig.hpp?

[Klaim]

Isnt it the same issue with the discussed config.imgui.specific_option being set? Or am I misunderstanding something? If my understanding is correct, its imgui's sources that's modified by these options, not just user's side, so these options affect all imgui users in the build configuration indeed - if I understand correctly. (there is even a define to not include the config in imgui.h, which is another hidden option) I suspect that's why they "highly" recommend not using imgui as a shared library (in the commends of that specific config header). So I was assuming if anyone would want to use these options, they will build imgui the same way for all the project's exes, and that's I think a normal assumption if you're making games (because you will probably just have 1 exe which will act both as a tool and the game). And if not, they would just use multiple build configs, but maybe that's too much work...

But you are right that it's not working well in the general case and feels hacky. I dont see how any other solution discussed here changes that problem though?

[boris-kolpackov]

Isnt it the same issue with the discussed config.imgui.specific_option being set?

Yes, and that's why I am advising against going with this approach: https://github.com/build2-packaging/imgui/issues/3#issuecomment-1674248134

I dont see how any other solution discussed here changes that problem though?

If you expose the individual options as boolean configuration values then build2 will negotiate (or fail if unable to) a configuration that is suitable to all the dependents. See: https://build2.org/stage/bpkg/doc/build2-package-manager-manual.xhtml#dep-config-negotiation

[Klaim]

Ok got it now. 👍🏽

[kamrann]

BTW, isnt there a difference of how defines are treated when they come from cli and when they are in source code? Or is it just a headers-unit thing?

Setting aside modules, about which I know very little, there are no differences so far as I'm aware other than the fact that the define is visible from the very start of the TU when passed direct to the compiler. I suppose there's also the minor downside that it will be seen by all TUs in all library-level dependents, rather than only those that directly/indirectly #include "imgui.h".

That would also allows users to use the last part of the header allowing for introducing custom code in imgui namespace, which is a regular practice in game development when working on consoles for example.

I saw this and really couldn't work out what the message was. What would be the reason for wanting to inject extension code there rather that just creating a my_imgui_extensions.h and including that in your project instead of directly including imgui.h?

[Klaim]

. What would be the reason for wanting to inject extension code there rather that just creating a my_imgui_extensions.h and including that in your project instead of directly including imgui.h?

I believe it's to be able to also affect imgui's sources as well, that header is included in the library's sources. Keep in mind that this kind of library have been developed in a context (console game dev) where the best way to manage dependencies is to have all the sources of the dependencies gathered in the project, and be able to modify them specifically for that project. I suppose we can ignore this use case as its still possible to use a modified version of any lib with build2.