pthom / litgen

litgen: a pybind11 automatic generator for humans who like nice code and API documentation. Also a C++ transformer tool
https://pthom.github.io/litgen
GNU General Public License v3.0
46 stars 6 forks source link

How to support nanobind? #3

Open davidlatwe opened 7 months ago

davidlatwe commented 7 months ago

Hi @pthom ,

If I want to integrate litgen with nanobind, make it able to chose the flavor of binding, what would you suggest?

There are lots of small differences between the two, like:

Although they can be done with str.replace(), regex and some hardwork, but if litgen can add one new flavor that would be awesome.

Thanks. :)

pthom commented 7 months ago

Hello,

If you want to do this, here are some advices:

litgen is not a small library, so I think you may encounter a few subtleties, such as how to make methods overridable, which is very much linked to pybind11 at the moment.

If you take this route, please keep me informed of your progress!

thanks

pthom commented 6 months ago

Hello, could you give me more information about your current status? Did you manage to get something working? If so, I would be interested in having more information. Thanks.

davidlatwe commented 6 months ago

Hey @pthom, sorry for being late!

We did manage to get our nanobind flavor imgui binding, here's what I did:

About this one particular commit https://github.com/davidlatwe/litgen/commit/19b48266b26416408e8845e3e1751ae8d668ad61 in my litgen fork, it was for fixing this:

    # imgui.CalcTextSize("Test")
    ^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: CalcTextSize(): incompatible function arguments. The following argument types are supported:
    1. CalcTextSize(text: str, text_end: Optional[str] = None, hide_text_after_double_hash: bool = False, wrap_width: float = -1.0) -> imgui.ImVec2

Somehow, py::arg("text_end") = py::none() is not enough for nanobind to work with NULL as default:

ImVec2 ImGui::CalcTextSize(const char* text, const char* text_end = NULL, ...)

It has to be like this:

m.def("CalcTextSize", [](const char * text, std::optional<std::string> text_end = std::nullopt, ...)
{
    auto CalcTextSize_adapt = [](const char * text, std::optional<std::string> text_end = std::nullopt, ...)
    {
        const char * text_end_adapt_default_null = nullptr;
        if (text_end.has_value())
            text_end_adapt_default_null = text_end.value().c_str();

        auto lambda_result = ImGui::CalcTextSize(text, text_end_adapt_default_null, ...);
        return lambda_result;
    };

    return CalcTextSize_adapt(text, text_end, ...);
},
py::arg("text"), py::arg("text_end") = py::none(), ...);


I am sure there are still some bugs in our imgui nanobind binding, but it is working great so far for our goals. But util now though, because I found that I have to also try supporting older versions of Python which are not supported by nanobind, so I am going back to pybind11 at the moment. (Our library runs in another application which has its own Python embedded from 2.7-3.11+ depends on its version. And we have Python 3.11+ embedded in our library and ImGui will be bound with it for internal GUI drawing. We'd like to use only one binding library through out entire project. So although ImGui is aimed to Python 3.11+, our other bindings are aimed to the host application for public use. Therefore we now have to go with pybind11 v2.9.2 to see if that works.)

Oh and, about this one:

you should add an option bind_library + enum BindLibraryType inside options.py

I have not done that yet ๐Ÿ˜…

I installed my litgen fork in a venv with pip install -e, and here's the script that we used to run the generator:

import os
import shutil
# ./external/imgui_bundle/external/imgui/bindings/generate_imgui.py
import generate_imgui

ROOT = os.path.dirname(__file__)

@generate_imgui.my_time_it
def generate():
    # Note: Don't need imgui_test_engine, so not using `generate_imgui.main()`
    # Note: Don't need, but also cannot bind imgui_internal because `GImGui`
    #   symbol is unresolvable by linker.
    generate_imgui.autogenerate_imgui()

    stub_dir = generate_imgui.STUB_DIR
    pydef_dir = generate_imgui.PYDEF_DIR
    wrapper_dir = os.path.join(os.path.dirname(pydef_dir), "imgui_pywrappers")
    files = [
        # cpp
        os.path.join(pydef_dir, "nanobind_imgui.cpp"),
        # pyi
        os.path.join(stub_dir, "imgui/__init__.pyi"),
        # wrapper
        os.path.join(wrapper_dir, "imgui_pywrappers.cpp"),
        os.path.join(wrapper_dir, "imgui_pywrappers.h"),
    ]

    return files

def to_nanobind(file):
    with open(file, "r") as fp:
        lines = fp.readlines()

    in_text_buffer_class = False
    in_text_buffer_delete = False

    new_lines = []
    for line in lines:
        # error LNK2001: unresolved external symbol "public:
        #   static char * ImGuiTextBuffer::EmptyString"
        if "auto pyClassImGuiTextBuffer =" in line:
            in_text_buffer_class = True
        if in_text_buffer_class:
            if '.def("begin",' in line or '.def("c_str",' in line:
                in_text_buffer_delete = True
            if '.def("size",' in line:
                in_text_buffer_delete = False
            if '.def("append",' in line:
                in_text_buffer_delete = False
                in_text_buffer_class = False
            if in_text_buffer_delete:
                continue

        line = line.replace(" = NULL", " = nullptr")
        new_lines.append(line)

    with open(file, "w") as fp:
        fp.writelines(new_lines)

def run():
    # Generate binding
    files = generate()

    # Copy generated file
    out_dir = os.path.join(ROOT, "src")
    cpp_files = []
    for src in files:
        fname = os.path.basename(src)
        if fname.endswith(".cpp") or fname.endswith(".h"):
            cpp_files.append(fname)
        shutil.copy2(src, os.path.join(out_dir, fname))

    # Final nanobind related cleanup
    for fname in cpp_files:
        to_nanobind(os.path.join(out_dir, fname))

if __name__ == "__main__":
    run()
pthom commented 6 months ago

Hello,

Thanks for the details info!

You went far into the customization, since I see you added a specific adapter (which is deep inside litgen internals). Well done! Did you find it easy to find your way in the code?

Note: your to_nanobind(file) function could potentially be a postprocess

you should add an option bind_library + enum BindLibraryType inside options.py

I have not done that yet ๐Ÿ˜…

There is no rush. Adding this to litgen would require additional effort (integration tests, polishing, etc.); all of which is time consuming, I know. If you have time to work on it later, I would appreciate it; but it depends on ๏ปฟwhether you have time to invest on it or not.

And we have Python 3.11+ embedded in our library and ImGui will be bound with it for internal GUI drawing

May I ask what kind of project you are working on?

davidlatwe commented 6 months ago

๐Ÿ˜Š

Well done! Did you find it easy to find your way in the code?

Thanks! Yeah, it was easy enough to get my way. With some simple text search, I can learn where I need to go. And the variable/function naming is quite clear.

This is us ๐Ÿ‘‹๐Ÿผ : https://ragdolldynamics.com/

We use ImGui to draw our user interface on the 3D viewport in another application like Maya/Blender. You can find the trace of ImGui in here: https://learn.ragdolldynamics.com/tutorials/manikin/#shapes

At the moment, our user interface was written in C++, but in the futrue we may enable our clients the ability to make their own UI with our embedded Python and ImGui Python binding. To do so, there can have only one ImGuiContext instance, which means we need to build our own Python binding and link to our shared library for accessing ImGui.

By the way, here is one interesting thing I found. When I build the binding with nanobind from our original ImGui source code, the build works without any problem. But when I switch back to pybind11, I got error like error C2027: use of undefined type 'ImGuiDockNodeSettings'. Then I realized imgui_bundle has this tweak for it to build. Somehow nanobind does not need such change.

mattparks commented 1 month ago

It has been a few months since there has been activity on this issue. Is there a path forward for litgen to support automatic nanobind code generation? If that's a feature that can be supported, I'd love to use this tool in one of my projects.

Thanks!

davidlatwe commented 1 month ago

Hey @mattparks , nanobind support currently only available from my forks:

And I only did the necessary changes for my own project so not fully tested. If possible, you can give my forks a shot. In the mean time, I will try submit a PR.