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

PushID and ImageButton #2464

Closed
darkgnostic opened this issue Apr 2, 2019 · 6 comments
Closed

PushID and ImageButton #2464

darkgnostic opened this issue Apr 2, 2019 · 6 comments

Comments

@darkgnostic
Copy link

darkgnostic commented Apr 2, 2019

Hi Omar,

I have an issue with ImageButtons. I have following method in my app:

void Foo(int index, vec4  uv) {
        static int hov_index = -1;
	
	ImGui::PushID(index);
	if (ImGui::ImageButton((void*)(intptr_t)(hov_index == index ? icon_texture_flat_ : icon_texture_), ImVec2(24, 24), ImVec2(uv.x, uv.w), ImVec2(uv.z, uv.y), 0 )) {
	       Foonction();
	}
	ImGui::PopID();

	if (ImGui::IsItemHovered()) {
		hov_index = (ImGui::IsMouseDown(0) ? index : -1);
	}
}

I wanted to achieve than if user press and hold the button, image is changed (flat and normal icon), which worked perfectly. The problem was that from that point ImageButton never returned true. After debugging a bit, I presumed that since ImGuiID is generated from texture id (and I have two different textures), it actually never recognizes that the item is pressed.

My solution was to overload the ImGui::ImageButton as ImageButton(ImTextureID user_texture_id, ImTextureID user_texture_id_down ....

copy/paste complete ImageButton definition, then change last line in definition to: window->DrawList->AddImage(!held ? user_texture_id : user_texture_id_down, image_bb.Min, image_bb.Max, uv0, uv1, GetColorU32(tint_col));

From that point it worked as expected.

I have two questions:

  1. Why there is PushID((void*)(intptr_t)user_texture_id); if for example I PushID myself? Shouldn't ImGui check if there is a pushed ID already by the user, and in case there is none, use your existent solution of adding texture_id as ID?
  2. Is there any chance/plan to add overloaded ImageButton with 3 states normal, pushed, hovered?

UI: saw there is something similar at #1390

Thanks

@ocornut
Copy link
Owner

ocornut commented Apr 2, 2019

Shouldn't ImGui check if there is a pushed ID already by the user,

That's not possible, there's always a pushed id in the stack.

This is a flaw of ImageButton(), the code is very small I suggest just created your own custom function (in your own source file, never in imgui.cpp).

@darkgnostic
Copy link
Author

Thank you for your fast response.

I suggest just created your own custom function (in your own source file, never in imgui.cpp).

That was exactly what I did :)

@ocornut
Copy link
Owner

ocornut commented Apr 2, 2019

I'm adding a note that this is an issue with ImageButton.

I would like to later redesign the API of Image() and ImageButton() they are old and have other problems.

@ocornut
Copy link
Owner

ocornut commented Sep 21, 2019

Someone came to me yesterday pointing out that this flaw is even more problematic as with some back-end it is possible that the ImTextureId be unstable from frame to frame, therefore making it impossible to use ImageButton() as-is.

@ocornut
Copy link
Owner

ocornut commented Aug 22, 2024

FYI the old ImageButton() entry point which was made obsolete on Aug 3, 2022, has now been commented out.

For reference:

#ifndef IMGUI_DISABLE_OBSOLETE_FUNCTIONS
// Legacy API obsoleted in 1.89. Two differences with new ImageButton()
// - old ImageButton() used ImTextureId as item id (created issue with multiple buttons with same image, transient texture id values, opaque computation of ID)
// - new ImageButton() requires an explicit 'const char* str_id'
// - old ImageButton() had frame_padding' override argument.
// - new ImageButton() always use style.FramePadding.
bool ImGui::ImageButton(ImTextureID user_texture_id, const ImVec2& size, const ImVec2& uv0, const ImVec2& uv1, int frame_padding, const ImVec4& bg_col, const ImVec4& tint_col)
{
    // Default to using texture ID as ID. User can still push string/integer prefixes.
    PushID((void*)(intptr_t)user_texture_id);
    if (frame_padding >= 0)
        PushStyleVar(ImGuiStyleVar_FramePadding, ImVec2((float)frame_padding, (float)frame_padding));
    bool ret = ImageButton("", user_texture_id, size, uv0, uv1, bg_col, tint_col);
    if (frame_padding >= 0)
        PopStyleVar();
    PopID();
    return ret;
}
#endif // #ifndef IMGUI_DISABLE_OBSOLETE_FUNCTIONS

@ocornut
Copy link
Owner

ocornut commented Aug 22, 2024

Also I had forgotten to close this issue back but it's been solved since that date :)

@ocornut ocornut closed this as completed Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants