dearimgui / dear_bindings

C header (and language binding metadata) generator for Dear ImGui
MIT License
221 stars 12 forks source link

Backend conversion feedback #35

Closed tiawl closed 11 months ago

tiawl commented 11 months ago

Hi,

Here a list of minor changes/bugs I would like to submit for the backend_conversion branch:

  1. Currently your tool generates a cimgui.h header file with these prototypes inside:
CIMGUI_API void ImGui_TextV(const char* fmt, va_list args) IM_FMTLIST(1)
CIMGUI_API bool ImGui_InputTextMultiline(const char* label, char* buf, size_t buf_size);

You are using va_list and size_t types. Unfortunately, you provide header files to allow the C compiler to find them into the cimgui.cpp generated file. However the header file needs also these headers for the prototypes. Would you consider to move these lines from cimgui.cpp into the cimgui.h ?

#include <stdio.h>
#include <stdarg.h>
  1. Generating binding for these backends:
    • sdl2
    • sdl3
    • sdlrenderer2
    • sdlrenderer3

is failing with this output for sdlrenderer2 and sdlrenderer3:

$ python3 dear_bindings.py --backend -o cimgui_impl_sdlrenderer3 ../imgui/backends/imgui_impl_sdlrenderer3.h
Dear Bindings: parse Dear ImGui headers, convert to C and output metadata.
Parsing ../imgui/backends/imgui_impl_sdlrenderer3.h
Storing unmodified DOM
Applying modifiers
Writing output to ../tmp/cimgui_impl_sdlrenderer3[.h/.cpp/.json]
Template file /home/user/Workspace/im-zig-gui/wrapper/dear_bindings/src/templates/imgui_impl_sdlrenderer3-header-template.h could not be found (note that template file names are expected to match source file names, so if you have renamed imgui.h you will need to rename the template as well). The common template file is included regardless of source file name.
Exception during conversion:
Traceback (most recent call last):
  File "dear_bindings.py", line 465, in <module>
    convert_header(args.src, args.output, args.templatedir, args.nopassingstructsbyvalue, args.backend)
  File "dear_bindings.py", line 405, in convert_header
    insert_header_templates(file, template_dir, src_file_name_only, ".h", expansions)
  File "dear_bindings.py", line 54, in insert_header_templates
    insert_single_template(dest_file,
  File "dear_bindings.py", line 37, in insert_single_template
    sys.exit(2)
SystemExit: 2

this output for sdl2:

$ python3 dear_bindings.py --backend -o cimgui_impl_sdl2 ../imgui/backends/imgui_impl_sdl2.h
Dear Bindings: parse Dear ImGui headers, convert to C and output metadata.
Parsing ../imgui/backends/imgui_impl_sdl2.h
Unrecognised element LexToken(TYPEDEF,'typedef',24,1545)
Storing unmodified DOM
Applying modifiers
Writing output to cimgui_impl_sdl3[.h/.cpp/.json]
Template file /home/user/Workspace/im-zig-gui/wrapper/dear_bindings/src/templates/imgui_impl_sdl2-header-template.h could not be found (note that template file names are expected to match source file names, so if you have renamed imgui.h you will need to rename the template as well). The common template file is included regardless of source file name.
Exception during conversion:
Traceback (most recent call last):
  File "dear_bindings.py", line 465, in <module>
    convert_header(args.src, args.output, args.templatedir, args.nopassingstructsbyvalue, args.backend)
  File "dear_bindings.py", line 405, in convert_header
    insert_header_templates(file, template_dir, src_file_name_only, ".h", expansions)
  File "dear_bindings.py", line 54, in insert_header_templates
    insert_single_template(dest_file,
  File "dear_bindings.py", line 37, in insert_single_template
    sys.exit(2)
SystemExit: 2

ths output for sdl3:

$ python3 dear_bindings.py --backend -o cimgui_impl_sdl3 ../imgui/backends/imgui_impl_sdl3.h
Dear Bindings: parse Dear ImGui headers, convert to C and output metadata.
Parsing ../imgui/backends/imgui_impl_sdl3.h
Unrecognised element LexToken(TYPEDEF,'typedef',26,1725)
Storing unmodified DOM
Applying modifiers
Writing output to cimgui_impl_sdl3[.h/.cpp/.json]
Template file /home/user/Workspace/im-zig-gui/wrapper/dear_bindings/src/templates/imgui_impl_sdl3-header-template.h could not be found (note that template file names are expected to match source file names, so if you have renamed imgui.h you will need to rename the template as well). The common template file is included regardless of source file name.
Exception during conversion:
Traceback (most recent call last):
  File "dear_bindings.py", line 465, in <module>
    convert_header(args.src, args.output, args.templatedir, args.nopassingstructsbyvalue, args.backend)
  File "dear_bindings.py", line 405, in convert_header
    insert_header_templates(file, template_dir, src_file_name_only, ".h", expansions)
  File "dear_bindings.py", line 54, in insert_header_templates
    insert_single_template(dest_file,
  File "dear_bindings.py", line 37, in insert_single_template
    sys.exit(2)
SystemExit: 2

I ran your generator on Ubuntu 20.04. Let me know if I forgot to specify crucial details.

  1. For those backends:
    • glut
    • vulkan

into the generated header files, it is missing { after the:

extern "C"

and at the end of the generated file, this matching part is also missing:

#ifdef __cplusplus
}
#endif

Thank you.

ShironekoBen commented 11 months ago

Oh, awesome, thanks for the very detailed bug reports!

Having taken a look at those:

1) Yep... that's actually kinda weird, in that I apparently deliberately removed stdarg.h from the header at some point (with a comment saying "removing unnecessary headers"). Clearly it's not unnecessary, though, so I've added it back along with stddef.h (since stdio.h felt a little like overkill just to get size_t - shout if there's some reason you thing stdio.h would be preferable in this situation, though).

2) Ack, sorry - I was testing against an old version of the ImGui code prior to the SDL2/3 backend split. I've adjusted the templates so that those should generate cleanly now.

3) Ooh, that's a fun bug you found! Turns out there was an issue in the way that extern "C" statements were generated where if the thing immediately following one was a #ifdef or similar, it would see that as a single logical "child" statement and assume it was safe to generate a single-line extern statement rather than a multi-line one with brackets. I've fixed that now so it shouldn't be so trigger-happy about trying to reduce things to a single line.

I've put in fixes that I think should resolve all of these in c6d4f5ac8d0cc0199bdf131863e061f49dcf962d - let me know if those work OK for you!

Thanks again!

tiawl commented 11 months ago

Well ! Thank you for the bug fix, now it is OK from my side.

For the size_t issue, I think you are right: stdio.h is overkill for the situation. I suggested stdio.h because it was already used but there is not any other reason to use it instead of stddef.h (for my use case).

I am closing this issue.