Compile error when using the features/string_view branch

[emoon]

When using the features/string_view branch of the private dear_imgui GH I'm running into a bunch of issues such as this one

cpp/cimgui.cpp:2908:83: error: invalid cast from type ‘cimgui::ImStr’ {aka ‘cimgui::ImStr_t’} to type ‘ImStr’
  return ImGui::SliderFloat2(reinterpret_cast<::ImStr>(label), v, v_min, v_max, reinterpret_cast<::ImStr>(format), power);

when I try to use the generated cpp code from the generator.

[ShironekoBen]

Ooh, interesting. I haven't tried features/string_view so I'll have to give that a shot. Looks like ImStr has got misidentified as being by-reference when it's actually by-value or something similar.

[emoon]

Yeah. ImStr is by value (it's a wrapper around two pointers Begin and End) for start/end of a string. It's useful in my case for Rust as Rust doesn't use zero byte termination.

[ocornut]

Supporting ImStrv (1.85? 1.86? 1.87?) was one of the leading motivation for rewriting cimgui (well, among with many other leading motivations :) but indeed it hasn't been tested yet.

One of the interesting aspect which I am not 100% sure how we can going to handle is that we might need to provide a different cimgui API for raw C users vs for binding users.

• A. High-level languages via bindings will likely call through the ImStrv API
• B. For raw C users are likely to want to expose single char* version of the functions, where the strlen would be done transparently.
• C. Bonus if the strlen can be done in user's compilation unit because for literals it's going to be folded by the compiler..
• D. Bonus if raw C users can still call the ImStr functions...

Meaning maybe we need two API (two prefixes? two suffixes?)

CIMGUI_API bool cImGui_Button(const char* str_id);
CIMGUI_API bool sImGui_Button(ImStrv str_id);

CIMGUI_API bool ImGui_Button(const char* str_id);
CIMGUI_API bool ImGui_Button_Strv(ImStrv str_id);

CIMGUI_API bool ImGui_Button(const char* str_id);
CIMGUI_API bool ImGuiStrv_Button(ImStrv str_id);

Possibly we'll just promote building "either" version via a flag to the python script (if we decide we don't support point D).

One thing to take into account is how high-level language bindings are likely to be generated. If they are generated using our metadata, it's going to be easy for people to reroute names (e.g. C# ImGui.Button calls C ImGuiStrv_Button). If they are generated using off the shelves auto-binding solutions the "Strv" part may be undesirably dragged into the high-level name.

[emoon]

My guess is that most will use the metadata. I see no reason not to do it that way.

[emoon]

As for the above question I think it would make sense to have different modes for the generator. In the Rust case I would only want the ImStrv but someone that wants to use the C API as is likely only care about the const char* version, but allowing both to be generated can be useful.

[emoon]

Just FYI I for now manually patched up the cimgui.cpp file by doing a

static inline ::ImStr ConvertToCPP_ImStr(const cimgui::ImStr& src) {
    ::ImStr dest = { src.Begin, src.End };
    return dest;
}

Then did a search/replace with the above so the generated code looks like this

CIMGUI_API bool cimgui::ImGui_Begin(cimgui::ImStr name)
{
    return ImGui::Begin(ConvertToCPP_ImStr(name));
}

Now (after fixing a minor issue in the cimgui.h file which is related to having static on some functions that shouldn't have) the code builds fine.

[emoon]

Doing some code-generation test I now (for my Rust wrapper) ends up with this

dear_imgui_test::main:
 lea     rdi, [rip, +, .L__unnamed_2]
 lea     rsi, [rip, +, .L__unnamed_2+4]
 jmp     qword, ptr, [rip, +, ImGui_Begin@GOTPCREL]

For a test code that looks like this

fn main() {
    imgui::begin("test");
}

So that is looking pretty good with regards to ImStr as all overhead is gone an the compiler (in this case) just figures out to just load two registers and call the FFI function.

[ShironekoBen]

Cool - that rust codegen looks really nice!
I've added initial support for ImStr to the code in 49ea6de - as suspected, it was mostly a case of flagging ImStr as a value type, and then for themoment I've added a manual helper function to convert simple char*s to ImStr.
I'll look at adding "implict" helpers too soon but I figured that getting basic support in would let everyone looking to use this with non-C languages move forward and was worth getting in quickly.

[emoon]

Great! Thanks :)

[emoon]

Tested this out yesterday and it seems to work fine 👍

[emoon]

Just wanted to give a small update that I have my wrapper up and running in it's early stages :)

        imgui::new_frame();
        imgui::begin("test");

        if imgui::button("test") {
            println!("test");
        }

        imgui::end();

[ShironekoBen]

So a further small update on this - e1ec95a adds auto-generation of helper functions that take const char* instead of ImStr, so C users can use the ImStr-enabled branch without needing to do their own conversion.

Conversion functions aren't inline at the moment - I started looking into that but then realised that having inline functions in the API would torpedo anyone who wanted to use the const char* variants of functions from a non-C language. I'm not sure how we deal with that, aside from possibly having the convertor output two sets of headers, one for native C code users and one for binding to other languages...?

For the moment the older helper function ImStr_FromCharStr() still exists but I'm not sure if it's worth keeping around or not now there are implicit conversions. I can't think of a use-case that isn't either simple enough that the auto-generated variants will do the job, or sufficiently complex that the user would probably just supply their own conversion code, but they may well be one that hasn't sprung to mind yet.

For the Rust bindings you probably want to check for is_imstr_helper on the function metadata and just ignore any functions with that set.

[emoon]

Thanks! I will take a look.

[ShironekoBen]

I'm going to close this for now - please feel free to reopen if you run into further problems.

[ocornut]

Following #11 (comment)

And:

So a further small update on this - e1ec95a adds auto-generation of helper functions that take const char* instead of ImStr, so C users can use the ImStr-enabled branch without needing to do their own conversion.

Confused about current state of is_imstr_helper has_imstr_helper fields and output, afaik they are always false.
Note- there's no urgency since C users can generate from a source that doesn't use ImStrV.

Conversion functions aren't inline at the moment - I started looking into that but then realised that having inline functions in the API would torpedo anyone who wanted to use the const char* variants of functions from a non-C language. I'm not sure how we deal with that, aside from possibly having the convertor output two sets of headers, one for native C code users and one for binding to other languages...?

Probably something like that (and CI can output both variant are artifacts)
It would simply be that both .h and .cpp implementation are generated accordingly to that flag, so from user point of view there's only 1 api at a time (and technically both can be linked together).

In theory could have up to three:

• (Optional) 1 for C end-user with const char* where the strlen() are done in macros -> cimgui_xxx
• 1 for tools/binding end-user with const char* -> cimgui.h/cimgui.cpp
• 1 for tools/binding end-user with ImStrv -> cimgui_strv.h/cimgui_strv.cpp

First one is merely an optimization tbh and could be skipped for simplicity, although well, in theory it is a nice thing to have! Can be implemented later. Struggling to find the best name for it.

[ShironekoBen]

On the subject of is_imstr_helper and has_imstr_helper - as far as I can see they are working? If I process an imgui.h that uses ImStr (i.e. the string view branch) then I get helpers generated for ImStr functions, so for example:

CIMGUI_API void ImGui_TreePushImStr(ImStr str_id); <-- Original ImStr function, has has_imstr_helper set to true
CIMGUI_API void ImGui_TreePush(const char* str_id); <-- char* helper, has is_imstr_helper set to true

As for the conversion functions - I think the helpers (as above) probably cover 90%+ of the use-cases here, so the explicit conversion functions (ImStr_FromCharStr() and friends) are probably only relevant if the user is trying to do something "clever" like convert a string once and then pass it to multiple ImGui functions. And if you're making bindings for something that isn't C and want to use the ImStr variants then is_imstr_helper and original_fully_qualified_name in the JSON metadata mean that it's trivial to ignore the helpers and expose the original functions as accepting whatever your language's native string_view type is.

As such, I'm currently thinking that whilst from a performance perspective inlining/macros would be nice, they probably aren't worth the pain of having separate headers/ifdefs/something until someone has a concrete use-case where inline conversion makes a big difference. It doesn't really affect the API either way (aside from the addition of an extra header/ifdef) so it's a relatively easy change to drop in later.