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

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

Open
sarchar opened this issue Jul 17, 2023 · 7 comments
Open

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

sarchar opened this issue Jul 17, 2023 · 7 comments

Comments

@sarchar
Copy link

sarchar commented Jul 17, 2023

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
Copy link
Owner

ocornut commented Jul 18, 2023

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 ocornut changed the title IsMouseDoubleClicked doesn't return true when it once did IsItemHovered when overlapping doesn't return true when it once did Jul 18, 2023
@ocornut
Copy link
Owner

ocornut commented Jul 18, 2023

Followup:

  • This only happen when the overlapping items idoesn't claim active id, like a Text() button.
  • This was caused by 3ec128c which I should have labelled as "breaking" (will do, retroactively).

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.

ocornut added a commit that referenced this issue Jul 18, 2023
…on-reactive item after a reactive overlappable one. (#6610)
@sarchar
Copy link
Author

sarchar commented Jul 19, 2023

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
Copy link
Owner

ocornut commented Jul 19, 2023

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
Copy link
Contributor

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
Copy link
Owner

ocornut commented Aug 1, 2023

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
Copy link
Contributor

DctrNoob commented Aug 1, 2023

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants