dearimgui / dear_bindings

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

Contributing files #34

Closed tiawl closed 11 months ago

tiawl commented 11 months ago

Hi,

I am currently writing an Imgui binding generator for Zig programing language. It is currently based on the cimgui/cimgui repository to generate C files. Because I am stucked in an uncomfortable situation I recently opened this issue to request new generated content. However, for personal and reasonable reasons he exposed, the cimgui/cimgui maintainer is not enthusiastic about a potential evolution of its tool. So I am thinking to move on and use your tool instead to generate C files.

As specified, in the README of this repository, your project is not in a mature state. And moving from cimgui/cimgui to your tool will remove a bunch of Imgui features I was adding to the binding generator I am working on. But I am thinking to the long-term and I strongly believe that this choice could improve the quality of the code I am currently writing.

I want to contribute and help your project moving forward. But at a first glance I did not find any:

Would you consider adding these files to explain how the next volunteers could help you ?

Thank you.

ocornut commented 11 months ago

This project is only being worked on a per-need basis and with limited resources, I don’t think any of those suggestions, although nice to have, are enough of a priority to be the best investment of anyone’s time at this point.

I believe the best course of action is simply to try using the emitted metadata to emit your Zig bindings and if you think anything specific can be fixed or improved feel free to report and we can decide what to do.

tiawl commented 11 months ago

This project is only being worked on a per-need basis and with limited resources

So maybe can I suggest you to specify this intent in a dedicated section of your README ?

Adding something like this:

#Development Philosophy

**dear_bindings** is developed incrementally. That means that functions and other things will be included
on-demand and not just for the sake of completeness.

Before closing this issue, I want to make a small digression: I have the feeling that imgui/backends/* files are not currently supported by your generator. Did I miss something ?

Thank you.

ShironekoBen commented 11 months ago

As @ocornut said, for the most part things are getting added to Dear Bindings on an as-needed basis. I'm very happy to talk over any specific requests you have and try to either get them added when I have time to do so or suggest the best route for an implementation, though. In terms of contributions there honestly isn't enough traffic here for it to be worth setting out a formal procedure for PRs right now beyond "throw up a request or an outline of what you're planning on doing and I'll let you know if it makes sense".

I'll definitely give some thought to a note in the readme explaining the situation, though. Thanks!

Looking at your cimgui issue, I do think that one thing which is on the (er, non-existent) roadmap might be of use to you - the experimental "type comprehension" branch generates descriptive data for all the types emitted in the JSON file, and one of the things in there is a "is_nullable" annotation on pointers that indicates if the pointer is allowed to be null or not.

It isn't perfect, as it's generated purely from the C type information (and thus doesn't know anything about ImGui's internal null checks), but it conservative (i.e. it will never indicate that a pointer cannot be null when it actually can, only the other way around), and it should be correct with regards to self pointers and anything that was originally a reference in the C++ header.

As the notes indicate, that branch is experimental but it seems to have been reasonably stable for a while so I'm thinking of merging it into mainline in the not-too-distant future. If there's anything you spot there that looks broken or could be improved to make your use-case easier then please just let me know!

As for the backends/ files, I'm afraid they're not supported. The convertor only works on imgui.h at the moment (some work has been done on imgui_internal.h but it's incomplete right now). The backends are not something I'd really considered generating bindings for as in most cases it seems to make more sense to use a native implementation that can work directly with whatever (presumably also native) graphics library you are using... if there's a specific use-case you have in mind I'd be interested to hear it, but I'm afraid I suspect that in the general case trying to support compiling the backends in a form that would allow binding from other languages in a meaningfully useful way would be quite a challenge.

[Edit] I'd momentarily forgotten about the many-item pointer part of your other post, but I think that may be partially covered by the type annotation metadata as well? Variable-length array pointers aren't really identifiable from the C type information alone but at least fixed-length arrays retain their identity and are marked both as arrays and with their bounds.

tiawl commented 11 months ago

I do think that one thing which [...] might be of use to you - the experimental "type comprehension" branch generates descriptive data for all the types emitted in the JSON file [...] If there's anything you spot there that looks broken or could be improved to make your use-case easier then please just let me know!

Thank you for pointing this. I am going to try it and make you a feedback if something goes wrong or does not fit my needs.

if there's a specific use-case you have in mind I'd be interested to hear it, but I'm afraid I suspect that in the general case trying to support compiling the backends in a form that would allow binding from other languages in a meaningfully useful way would be quite a challenge.

Currently with Zig it is easy to achieve. Zig provides a strong build system that lets the user have a full control over its compilation process. As I already stated, I am not writing a binding but a binding generator. This subtlety allows me to generate and compile what the user needs during the compilation stage (and skip generating/compiling unnecessary stuff). Concretely what I want to implement is:

A generic potential use-case would be:

If I can not rely on your generator to generate bindings for Imgui backends, I think I am going to use the cimgui/cimgui generator for this part of the tool I am working on. But I really do not want to take this path unless if necessary. Relying on 2 different generators will make maintenance harder.

I'd momentarily forgotten about the many-item pointer part of your other post, but I think that may be partially covered by the type annotation metadata as well?

How do you think you can achieve this ?

ShironekoBen commented 11 months ago

So with regards to the backends, I'll try and take a look at how they behave in the generator - it's not something I've tested before so it may be that only minimal tweaking is needed to get them working. It'll probably be the weekend before I get a chance to try that, though, I'm afraid.

As for the many-item pointers, what I meant is that array declarations generate metadata like this in the type comprehension:

"type": {
    "declaration": "float[4]",
    "description": {
        "kind": "Array",
        "bounds": "4",
        "inner_type": {
            "kind": "Builtin",
            "builtin_type": "float"
        }
    }
},

...so you at least have the information necessary to generate suitable bounded types for that case (I'm not at all familiar with Zig so I don't know if this case falls under the many-item pointers situation or if there is a separate array type that can be passed to C functions, but either way it's hopefully possible).

tiawl commented 11 months ago

It'll probably be the weekend before I get a chance to try that, though, I'm afraid.

Wow, thank you for considering my request ! Even if I can't wait to try it, I do not have a deadline so please take the time you need to achieve this.

I want to add a thing that changed since my last reply was written (maybe it does not change things for you but I think it is better to mention it):

generating a C binding file for each Imgui backend with your generator (again: I already can do that with cimgui/cimgui)

cimgui/cimgui does not do the job here: I was thinking that cimgui/cimgui generates a C++ pair of binding files for backends but it only provides a header file. However I can generate the missing C++ source code file from the JSON file it generates.

I'm not at all familiar with Zig so I don't know if this case falls under the many-item pointers situation or if there is a separate array type that can be passed to C functions, but either way it's hopefully possible

In Zig when you are interfacing with C you can use an array type than can be passed to C function. So if the metadata you generate looks like what you shown in your last reply I will use a Zig array to represent the variable it is specifying. Not a many-item pointer.

So if I do not miss your point: for the following function, representing its parameter as a many-item pointer won't be possible:

template<typename T>
struct ImChunkStream
{
    /* ... */
    int     chunk_size(const T* p)      { return ((const int*)p)[-1]; }
    /* ... */
}

Am I right ?

ShironekoBen commented 11 months ago

OK, I've had a look at this and the basic stuff doesn't seem to be too hard, so I did a quick attempt at implementing support for it. It's in this branch - there are some instructions in the readme file there but basically you just have to specify --backend on the command-line when converting a backend file and it should produce a reasonably sensible .h/.cpp file pair.

If that looks like what you need then please give it a go and let me know if you have any thoughts!

As for many-item pointers - yes, I think we're on the same page then. Examples like the one you've given aren't really distinguishable from regular single-item pointers without some sort of information about the usage in the implementation, so there's nothing in the metadata that will help with that, I'm afraid.

tiawl commented 11 months ago

Nice, thank you for your work. I am going to try it in the next few days. I am going to close this issue with a feedback in my next message. If something is missing I will open a new dedicated issue.

tiawl commented 11 months ago

Closing this one in favor of a dedicated issue: #35