GetUniqueName crashes because stack is empty

[thomasgeissl]

And another one, GetUniqueName was crashing, as usedNames is empty initially. I don't really know what's going on, need to double check with a working brain. Here is a quick fix, but there are probably things missing. Probably this is also the case in Jason's original code. Or maybe related to how I create my UI.

Any Ideas?

const char * ofxImGui::GetUniqueName(const std::string& candidate)
{
    std::string result = candidate;
    if(windowOpen.usedNames.empty()){
//        TODO: should be probably added to the sack of string vectors, need to understand whats happening there
        return result.c_str();
    }
    while (std::find(windowOpen.usedNames.top().begin(), windowOpen.usedNames.top().end(), result) != windowOpen.usedNames.top().end())
    {
        result += " ";
    }
    windowOpen.usedNames.top().push_back(result);
    return windowOpen.usedNames.top().back().c_str();
}

[Daandelange]

Hello Thomas, I've never really used the ImHelpers part of this addon, and I've limited knowledge on this part. (They seem to heavily rely on GetUniqueName). Moebius is doing some modifications with them; @moebiussurfing, I remember you saying something similar by mail ? (something like switching to ImGui::PushID(i++); instead of GetUniqueName())

[moebiussurfing]

hey @thomasgeissl ,

can you try this branch? https://github.com/Daandelange/ofxImGui/tree/ofParameters-Helpers-Test today I made some changes trying to figure out how to solve these crashes...

I am using the group helper it like the original repo:

.h

ofxImGui::Settings mainSettings = ofxImGui::Settings();

.cpp

//setup
gui.setup(nullptr, true, ImGuiConfigFlags_DockingEnable, true, false);//this do not matters

//draw
ofxImGui::AddGroup(params_1, mainSettings);
ofxImGui::AddGroup(params_2, mainSettings);
ofxImGui::AddGroup(params_3, mainSettings);

I made modifications to collapse the group/tree by default.

Should be better to allow passing the window/tree flags with the group, then we can customize better the window props...

but is crashing sometimes... You can see on the commented zones my frustrated attempts...

I think we don't need this mess of settings maybe but I am not sure. Something like this could be nice:

void ofxImGui::AddGroup(ofParameterGroup& group, ImGuiTreeNodeFlags flags = ImGuiTreeNodeFlags_None)

[moebiussurfing]

I encourage you to use this fork that @Daandelange is working on, I am using two "instances" of ImGui, I mean some panels coming from an addon, another from the ofApp, and all working fine.

[moebiussurfing]

there's a fork with a simpler params helpers approach that could be inspiration: https://github.com/yumataesu/ofxImGui_v3/blob/master/src/Helper.h

[Daandelange]

Just re-checked the helpers code a bit, and your changes to it. The Helpers feel quite obsolete indeed, and could be stripped / refactored, I think (saying this without having any experience with them). It could be great to have a more exhaustive example of the Helpers to address these questions and test them.

What feels obsolete to me :

• The mechanism with getUniqueName : could be replaced with pushID() and popID() or something similar. This is bound to windowOpen, which doesn't look to be used for other stuff. Except that it's a public variable, so people could be using it (= slightly breaking).
• jvcleave's original Helpers implementation seems to handle some custom layout tricks, which interfere with advanced use cases and newer imgui features such as docking and viewports. @moebiussurfing : Yes, where possible, passing optional flags to helper functions sounds like a good solution. Everything feels a bit hard-baked in the original helpers. (for this reason, I've never used them)
• ofxImGui::Settings::Settings seem to be a bit more complicated, it seems to serve as a state machine for improving the default imgui behaviour, combined with custom layout helpers. Removing this will probably break the layout of existing projects, probably best not changing it too much. (?)
  On the other side, it feels like most of its features are handled (or handleable) by native ImGui functions, but I don't know how exactly how much. What's the Helpers' "layout engine" doing compared to native Imgui behaviour ?

Note : Remember, as useful as they are, usage of the Handler functions remain optional. If you need too specific layouts, you better write your own alternatives.

Yamataesu's helper functions seem way more minimal and thus quite different from the original code. He stripped the whole ofxImGui::Settings part, so that could give an example of how ofxImGui would look like without the original layout tricks.

@thomasgeissl To get back to your initial question, I've commited a slightly different version of your code, so the new name gets added to the list of taken names. I don't know if it's due to your use case that the stack is empty, but better add a crash protection anyways !

[thomasgeissl]

thanks.

if I find time, I will have a deeper look. I think I did not pass settings to the function. I think your fix will still crash, if usedNames is emtpy then top() will be null.

https://github.com/Daandelange/ofxImGui/commit/5d5144968713b35a17622ebf93c5ae77cd4ac0d8#diff-420a3b5cbd7833c478b7f723c9bd0edeef83a6938cc07fbd1588034b54a06626R34

[Daandelange]

You're right. Usednames is pushed()/poped() in begin()/end() in tree and window. So outside of a window, it's supposedly empty. So maybe creating a "top-level"/"default" stack for windowless zones as a quick fix ? (will probably never get popped() unless cleanly implemented)
Or as discussed above, switch away from getUniqueName().

If you have some time, yes please. :) For now, the quick fix is pushed, still untested. On long-term, IMO, the 2nd option is better, but needs a deeper insight for confirmation.

[thomasgeissl]

I agree with what you are proposing. I think I should have some time next week. Thanks for your work on updating the addon.

[moebiussurfing]

• ofxImGui::Settings::Settings seem to be a bit more complicated, it seems to serve as a state machine for improving the default imgui behaviour, combined with custom layout helpers. Removing this will probably break the layout of existing projects, probably best not changing it too much. (?) On the other side, it feels like most of its features are handled (or handleable) by native ImGui functions, but I don't know how exactly how much. What's the Helpers' "layout engine" doing compared to native Imgui behaviour ?

Yamataesu's helper functions seem way more minimal and thus quite different from the original code. He stripped the whole ofxImGui::Settings part, so that could give an example of how ofxImGui would look like without the original layout tricks.

Hey @Daandelange ,

The more observable layout behavior when using helpers is that: when you add a container ofParameterGroup, with other nested groups with more params etc etc, the official repo makes something like:

• The root group is created with a collapsable/bigger/darker bar with the arrow to collapse.
• Deeper/child groups are created as TreeNode or TreeNodeEx. tinier/without dark background, small size text.

But the layout states (collapsing, closed windows..) are not being handled with .ini ImGui workflow. (Only handles the position and windows, docking sizes).

So in my case I ended using additional extra params bools in OF that I need to handle by myself, declaring, serializing on exit, deserializing on starting, etc. But unfortunately I am not being able to restore the collapsing states. I must change this workflow to let do this easy directly by ImGui.

Personally, what I would like to have is to make my custom extra layout helpers as you said above; something like flag some widgets to be drawn with the desired style. (Besides the argument to add the flags when adding single ungrouped widgets/parameters)

For example:

• you add a group using a handy one-single-line (AddGroup(g, flags)) with many ofParameters bools and floats inside.
• you want that some floats params are sliders but other +/- steppers, draggable numbers, or text input'ables...
• or bool used add checkBoxes or bigger toggle buttons etc...
• that could be done queueing a vector the names and the style; something like a getUniqueNames queue ;).

(maybe a pair/struct/map with the parameter name and an enum with _STYLE_CHECKBOX, STYLE_SLIDER, STYLESTEPPER...etc. And then when populating all the group params, to check if the name is on the queue and then apply the queued style. )

EDITED

[Daandelange]

Ok, I see, thanks for these details; so it's about the layout and not directly related to GetUniqueName.

Your other points sound like new ideas/features and could become a new issue. (?)

• Improve how the gui state restores when loading from imgui.ini (header collapse, open tabs, etc.)
  I see 2 solutions for this, in ofxImGui namespace (with extra params as you explained), or in imgui namespace.
  They say it's "in the air": https://github.com/ocornut/imgui/issues/437#issuecomment-380403870.
• Find a way to customize the imgui widget of an ofParameter. Now ofxImGui has one per type, and AddGroup() has to guess from an AbstractParameter. Maybe an AddGroup(g, appearance_flags_list[] ) will do instead of making pairs.