ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
59.73k stars 10.17k forks source link

IsItemHovered when overlapping doesn't return true when it once did #6610

Open sarchar opened 1 year ago

sarchar commented 1 year ago

Version/Branch of Dear ImGui:

Version: latest Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_glfw.cpp + imgui_impl_opengl.cpp Compiler: MSVC2022 Operating System: Win11

My Issue/Question:

Updated to latest on docking and noticed some features in my application weren't working anymore and tracked it down to IsMouseDoubleClicked(0) never returning true. I did a bisect and found that a88e5be was bad but 6b01114 still worked. It seems to be only on elements where I'm using Selectable and SpanAllColumns|AllowItemOverlap. Between those two commits is indeed work related to those flags. I'm not familiar enough with ImGui internals to dig deeper.

Standalone, minimal, complete and verifiable example:

I was unable to find the error in the demo but the code that triggers the problem is basically:

ImGui::Selectable(..., SpanAllColumns | AllowItemOverlap)
..
..
ImGui::Text("foobar")
if(ImGui::IsItemHovered() && ImGui::IsMouseDoubleClicked(0)) { do stuff; }

I can try and provide more detailed code if this isn't enough to go on.

Cheers

ocornut commented 1 year ago

I can confirm an issue here, Full repro:

ImGui::Selectable("...", false, ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap);
ImGui::SameLine();
ImGui::Text("foobar");
bool hovered = ImGui::IsItemHovered();
bool double_clicked = ImGui::IsMouseDoubleClicked(0);
ImGui::Text("%s && %s == %s", hovered ? "true" : "false", double_clicked ? "true" : "false", (hovered && double_clicked) ? "true" : "false");
ocornut commented 1 year ago

Followup:

The general issue this fixed was that when using AllowOverlap mode, you could click-hold on a Selectable with AllowOverlap and move the mouse over to another button not necessarily over the Selectable and that would highlight. This was definitively a bug, even if not super problematic: when an item claims active id by default other items shouldn't highlight on hover.

The problem is that this idiom you are using in the same situation: as the Text() doesn't claim HoveredId, hen clicking down the Selectable() claims both HoveredId and ActiveId, and then at that point IsItemHovered() on the Text() will start returning false. So your issue isn't due to IsMouseDoubleClicked() but due to the IsItemHovered() call.

The correct workaround would be to use IsItemHovered(ImGuiHoveredFlags_AllowWhenBlockedByActiveItem):

ImGui::Selectable("...", false, ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowItemOverlap);
ImGui::SameLine();
ImGui::Text("foobar");
bool hovered = ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenBlockedByActiveItem);
bool double_clicked = ImGui::IsMouseDoubleClicked(0);
ImGui::Text("%s\n%s\n== %s", hovered ? "true" : "false", double_clicked ? "true" : "false", (hovered && double_clicked) ? "true" : "false");

But I appreciate it may be unintuitive to the user that this would work without the flag when using Button() and not Text().

I am going to investigate if it would make sense for non-hoverable items e.g Text() to clear HoveredId if hovered and overlapping an item with AllowOverlap.

sarchar commented 1 year ago

OK, thanks. It didn't occur to me that Hovered was an actual widget state other than the literal is-mouse-over state. Would it be unreasonable to have both an IsItemHovered() and IsItemMouseOver() ?

At least, in this case, I can pass that flag to IsItemHovered to get my project to conform to the new code.

ocornut commented 1 year ago

Would it be unreasonable to have both an IsItemHovered() and IsItemMouseOver() ?

The various flags to IsItemHovered() are already solving this. A function like you imagine would be the source of many bugs as people would hastily use it instead of disabling specific filter (see ImGuoHoveredFlags_ values to see what is possible to enable/disable).

DctrNoob commented 1 year ago

Not sure if I have the same issue but it does seem related.

I have a custom canvas class on which I draw custom buttons (it's a timeline with elements on it). My canvas class acts as an invisible button because I want to manipulate the timeline. At the same time my custom buttons also act as invisible buttons on top (I want to click, double click, and drag them).

So the code is like this:

im::SetItemAllowOverlap();
im::InvisibleButton(<some-id>, <canvas-size>);
...
im::InvisibleButton(<some-id>, <button-size>);
if (im::IsItemHovered() && im::IsMouseDragging(ImGuiMouseButton_Left)) {
    // Still works
}
if (im::IsItemHovered() && im::IsMouseDoubleClicked(ImGuiMouseButton_Left)) {
    // Doesn't work anymore
}

In this use case, IsItemHovered() on the buttons does return true but IsMouseDoubleClicked() does not. The flags ImGuiHoveredFlags_AllowWhenBlockedByActiveItem and ImGuiHoveredFlags_AllowWhenOverlapped make no difference (IsItemHovered() already returns true without flags).

I am on the docking branch v1.89.6.

ocornut commented 1 year ago

im::SetItemAllowOverlap(); im::InvisibleButton(, );

Seems like you are using the wrong function here, should be using SetNextItemAllowOverlap(). As a result your canvas is taking inputs.

DctrNoob commented 1 year ago

Thanks for your input. I updated my branch to v1.89.7 (version where SetNextItemAllowOverlap() was introduced as a replacement for SetItemAllowOverlap() I think) and now the code works as expected. Thanks!