Generated backend function/type name issues

[shawnhatori]

Fully acknowledged the backends are "experimental". Hopefully many of the issues I've recently filed will help iron out some of the kinks. dear_bindings has otherwise been great to use!

1. All generated function names are prefixed with c, like cImGui_ImplWin32_Init(). I suspect this is unintentional, since it does not follow the format of the generated imgui.h functions.

2. Win32 backend: ImGui_ImplWin32_WndProcHandler() does not seem to be generated correctly. This would be really great to fix as soon as possible, since it is pretty much a showstopper (i.e. no input) for this backend.

3. DX11 backend (likely an issue in DX12 too): cimgui_impl_dx11.h causes a redefinition error since it typedefs to a known DX11 type.

   typedef struct ID3D11Device_t ID3D11Device;
   typedef struct ID3D11DeviceContext_t ID3D11DeviceContext;

   This gives an error like:

   cimgui_impl_dx11.h(28,31): error C2371: 'ID3D11Device': redefinition; different basic typesC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um\d3d11.h(337,32): note: see declaration of 'ID3D11Device'
   cimgui_impl_dx11.h(29,38): error C2371: 'ID3D11DeviceContext': redefinition; different basic typesC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um\d3d11.h(260,39): note: see declaration of 'ID3D11DeviceContext'

   I can solve this problem by removing the typedefs, but this requires including d3d11.h before cimgui_impl_dx11.h

[ShironekoBen]

Thank you for the great reports! It may take a few days for me to get a chance to look into them properly but I'll let you know when I have some fixes.

[shawnhatori]

Sounds great, thanks. I'd be happy to help test, and once things are working correctly, I can provide some code for examples/. I'm planning on integrating these bindings into a Win32 C application with DX11, DX12, OpenGL, and Vulkan backends so I'll naturally be testing all of those.

I created a build script that generates the libs/headers for each backend. Makes it easy to quickly generate and integrate the different backends. I'll link it below in case it is useful to users/maintainers. https://github.com/shawnhatori/dear-imgui-c-build

[ShironekoBen]

So I've taken a look at these and I think I've got fixes for most of them in.

All generated function names are prefixed with c, like cImGui_ImplWin32_Init(). I suspect this is unintentional, since it does not follow the format of the generated imgui.h functions.

FWIW this is actually intentional. Very non-obvious (to the point that I forgot the reason until I tried turning it off and discovered what goes wrong), but intentional!

Basically something like ImGui_LogText has that name because it was originally ImGui::LogText, and we flatten the ImGui namespace. However ImGui_ImplWin32_Init isn't in a namespace, it just has that name in the C++ header.

The issue with that is that if we allow the C header to also contain ImGui_ImplWin32_Init, then in the case where both are present in the same program (which is pretty much guaranteed in the case of whatever is doing the binding) you hit a link error as a result.

So Dear Bindings has logic which renames any loose functions by adding a c prefix to them to avoid this clash.

It's definitely not ideal in this case, as it makes the bindings function names all a little inconsistent-seeming (since there are so few loose functions in the normal ImGui API that you rarely see it there), but off the top of my head I can't think of a good way around this that doesn't involve the user having to jump through hoops when linking or similar. I'm very much open to ideas if you have any, though!

Win32 backend: ImGui_ImplWin32_WndProcHandler() does not seem to be generated correctly. This would be really great to fix as soon as possible, since it is pretty much a showstopper (i.e. no input) for this backend.

ImGui_ImplWin32_WndProcHandler() is another interesting case. Because it's disabled with a #if 0 the code generator generates binding code that is also disabled in the same way, which is simultaneously completely correct and completely useless.

As a workaround, I added a patch that changes that #if to check IMGUI_BACKEND_HAS_WINDOWS_H, which is then set by the .cpp file after including the windows headers. This means that the stub code gets generated correctly, and also that on the user side of things you can do:

#include <windows.h>
#define IMGUI_BACKEND_HAS_WINDOWS_H
#include "cimgui_impl_win32.h"

...and not have to manually declare the function.

I'm almost inclined to say that this might be a better way for the main ImGui code to handle this, as I would assume that in the majority of cases people are already doing something that looks like the code above (and I can't imagine many codebases include the backend headers in more than a couple of places). What are your thoughts, @ocornut ?

DX11 backend (likely an issue in DX12 too): cimgui_impl_dx11.h causes a redefinition error since it typedefs to a known DX11 type.

Ah, yeah, that's a problem with the autogenerated typedef naming! I've added a workaround that explicitly excludes those types (and a couple of others I found) so that they shouldn't have that problem any more.

Hopefully that fixes at least some of your problems - thanks again for the reports and just give me a shout if any more crop up!

[ShironekoBen]

Oh, and the build batch file looks very cool, too - I have a vaguely similar thing here I use for testing but it's nowhere near as convenient as yours so I may well switch over. Thanks!

[shawnhatori]

I can confirm that all of the issues I reported have been resolved! Especially the WndProc change works very nicely. Thank you for the prompt and reliable fixes. :)

However, there is one change that I believe is a regression. The generated backend C++ file now expects the backend header file to be in a backends/ path.

imgui.cpp
imgui_demo.cpp
imgui_draw.cpp
imgui_impl_dx11.cpp
imgui_impl_win32.cpp
imgui_tables.cpp
imgui_widgets.cpp
cimgui.cpp
cimgui_impl_dx11.cpp
cimgui_impl_dx11.cpp(6): fatal error C1083: Cannot open include file: 'backends/imgui_impl_dx11.h': No such file or directory
cimgui_impl_win32.cpp
cimgui_impl_win32.cpp(6): fatal error C1083: Cannot open include file: 'backends/imgui_impl_win32.h': No such file or directory
Generating Code...

imgui_impl_dx11.cpp has #include "imgui.h" and #include "imgui_impl_dx11.h". These files can either be in the same directory or add backends/ to the include path. This is reasonable, since it doesn't force a directory structure on the user, especially when they just want to throw a bunch of files into a deps/imgui folder in their project and compile everything. The generated files should be consistent with this I think and compile the same way.

[shawnhatori]

However ImGui_ImplWin32_Init isn't in a namespace, it just has that name in the C++ header.

I see, yeah that makes sense. That's unfortunate. And I assume adding a namespace to the original backend files is a breaking change no matter what?

I'm almost inclined to say that this might be a better way for the main ImGui code to handle this

Note that because of the discrepancy now between Dear ImGui and Dear Bindings, the comments in the generated cimgui_impl_win32.h don't match how it actually works.

the build batch file

I'll continue to update it. I don't mind pull requests either. There could always be a:

win32_build.bat
macos_build.sh
linux_build.sh

And each builds by default with the platform's native compiler and with all platform-compatible backends. It would help people easily get over the hurdle of ImGui bindings, since right now it's a little daunting. I originally considered changing my codebase from C to C-like C++ just to avoid the hassle.

[ShironekoBen]

However, there is one change that I believe is a regression. The generated backend C++ file now expects the backend header file to be in a backends/ path.

Ah, yeah. That's a good point. When I did that I was kinda looking at it going "wait, the ImGui distribution has these in a subdirectory, why isn't the generated code looking for them there?", but you're right that building the backend files directly out of their original location probably isn't the most common use-case.

I've reverted the behaviour to the way it was before, and added a new command-line option --backend-include-dir that allows explicitly specifying the backend path for cases where people are building with them in separate locations.

I see, yeah that makes sense. That's unfortunate. And I assume adding a namespace to the original backend files is a breaking change no matter what?

That's a question for @ocornut , I think, but my gut instinct is that it would be a fairly big breaking change that affects a lot of pre-existing code for a relatively small benefit to new code unfortunately.

[ocornut]

That would indeed by definition break near 100% of codebases but breakage would be easy to workaround. It’s something we could consider for 2.0 or if we transition to explicit context apis.

Either way my understanding is that many other loose helper functions in eg imgui_internal.h would be affected by the same issue either way, so the c prefix would look consistent?

[ShironekoBen]

Either way my understanding is that many other loose helper functions in eg imgui_internal.h would be affected by the same issue either way, so the c prefix would look consistent?

Yeah, what we have now is consistent on a mechanical level, it just doesn't look very consistent if you're mostly looking at the imgui.h API since that's virtually all inside the imgui namespace and thus the loose-functions-get-a-prefix rule is hardly ever used.

[shawnhatori]

Okay now that the issues mentioned here have been discussed and addressed, I'll close this issue. Thank you!