Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buttons with identical labels do not work #74

Closed
UUSim opened this issue Nov 7, 2014 · 5 comments
Closed

Buttons with identical labels do not work #74

UUSim opened this issue Nov 7, 2014 · 5 comments
Labels
label/id and id stack implicit identifiers, pushid(), id stack

Comments

@UUSim
Copy link

UUSim commented Nov 7, 2014

Before rattling any discussion: This could be considered improper usage of ImGui, however in my ignorance I did use ImGui like this. I noticed the issue, and I think you should now about it.

Onto the issue:
Having Buttons with the same label, can cause unpredictable results.
In my case, with 2 buttons controlling two separate settings:

ImGui::Text("Settting 1:");
ImGui::SameLine();
if (ImGui::Button((setting1enabled) ? "Yes" : "No")){
    setting1enabled = !setting1enabled;
}

ImGui::Text("Settting 2:");
ImGui::SameLine();
if (ImGui::Button((setting2enabled) ? "Yes" : "No")){
    setting2enabled = !setting2enabled;
}

In both settings are true, the label is identical ("Yes") for both buttons.
When the 2nd button is then clicked, ImGui::Button will return false because
window->GetID(label); (imgui.cpp: line 3076) will return the ID of the 1st item with that label.
Since it will check the mouse coordinates against the coordinates of the wrong button, it will report as not being pressed.

When setting1enabled = false (or setting1enabled != setting2enabled), the label on each button will be different, and everything works as expected.

In case more than two buttons have an identical label, button Bn will only respond if the labels of B0,B1,...,Bn-1 != Bn (I have not tried this, but this is what will happen most likely).

Idea for a solution:

  • Allow supplying an extra ID to the ImGui::Button function call (ID=-1 by default).
  • If the optional ID is given, use that instead of the text label to index the Button in ImGui's storage (hash table?).

This problem may also apply to other widgets.
Note: You may close this issue I you regard this issue as being unintended library usage. That being said, I think it would be nice if users could choose to use it like this anyway

@xythobuz
Copy link
Contributor

xythobuz commented Nov 7, 2014

Looking at imgui.cpp line 115ff (if you are confused about the meaning or use of ID in ImGui), I think there is already a solution to your problem: Either use PushID() or PopID(), or add extra ID information to the label using "Yes##Foo".

@UUSim
Copy link
Author

UUSim commented Nov 7, 2014

Ouch! A very big sorry indeed! I did not see that.

For other users making the same mistake (not reading the troubleshooting tips in the cpp that is..), here is the help section covering this issue:

  • some widgets requires state to be carried over multiple frames (most typically ImGui often wants remember what is the "active" widget).
    to do so they need an unique ID. unique ID are typically derived from a string label, an indice or a pointer.
    when you call Button("OK") the button shows "OK" and also use "OK" as an ID.
  • ID are uniquely scoped within Windows so no conflict can happen if you have two buttons called "OK" in two different Windows.
    within a same Window, use PushID() / PopID() to easily create scopes and avoid ID conflicts.
    so if you have a loop creating "multiple" items, you can use PushID() / PopID() with the index of each item, or their pointer, etc.
    some functions like TreeNode() implicitly creates a scope for you by calling PushID()
  • when dealing with trees, ID are important because you want to preserve the opened/closed state of tree nodes.
    depending on your use cases you may want to use strings, indices or pointers as ID. experiment and see what makes more sense!
    • e.g. When displaying a single object, using a static string as ID will preserve your node open/closed state when the targeted object change
    • e.g. When displaying a list of objects, using indices or pointers as ID will preserve the node open/closed state per object
  • when passing a label you can optionally specify extra unique ID information within the same string using "##". This helps solving the simpler collision cases.
    • e.g. "Label" display "Label" and uses "Label" as ID
    • e.g. "Label##Foobar" display "Label" and uses "Label##Foobar" as ID
    • e.g. "##Foobar" display an empty label and uses "##Foobar" as ID

In my case, I used the final option.

@UUSim UUSim closed this as completed Nov 7, 2014
@ocornut
Copy link
Owner

ocornut commented Nov 10, 2014

This is indeed a common question especially for first-time users of immediate mode GUI library.
I have now added an "index" at the very top of the file, listing all sections in the file so hopefully people will be more aware of the existence of the FAQ.

@xaxxon
Copy link

xaxxon commented Jun 6, 2016

and use label###ID if you don't want the label to be part of the ID

@JG-Adams
Copy link

JG-Adams commented Jan 2, 2023

This ought to be made automatic by IamGui. Because, it can be a security issue, performance, memory, and a waste of codespace. calling push and pop, and keeping track of id. (Deep nesting issue)

If I understand this right, it is queued into the Graphic library for every draw instruction made?
If that is the case, it should go from 0-on by itself allowing every button to be accessed regardless of name.
Why use namespace to begin with? It is not cache friendly. (Array would work)
Just my thought. Cause, I expected it to always work until recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
label/id and id stack implicit identifiers, pushid(), id stack
Projects
None yet
Development

No branches or pull requests

5 participants