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

MouseReleased double trigger #359

Closed
Pagghiu opened this issue Oct 2, 2015 · 2 comments
Closed

MouseReleased double trigger #359

Pagghiu opened this issue Oct 2, 2015 · 2 comments

Comments

@Pagghiu
Copy link
Contributor

Pagghiu commented Oct 2, 2015

This change should be reverted in my opinion, because it has broken current MouseRelease handling.
It makes MouseReleased == true for two subsequent frames after mouseDown[i] changes from true to false.

     g.IO.MousePosPrev = g.IO.MousePos;
     for (int i = 0; i < IM_ARRAYSIZE(g.IO.MouseDown); i++)
     {
+        g.IO.MouseClicked[i] = g.IO.MouseDown[i] && g.IO.MouseDownDuration[i] < 0.0f;
+        g.IO.MouseReleased[i] = !g.IO.MouseDown[i] && g.IO.MouseDownDurationPrev[i] >= 0.0f;
         g.IO.MouseDownDurationPrev[i] = g.IO.MouseDownDuration[i];
         g.IO.MouseDownDuration[i] = g.IO.MouseDown[i] ? (g.IO.MouseDownDuration[i] < 0.0f ? 0.0f : g.IO.MouseDownDuration[i] + g.IO.DeltaTime) : -1.0f;
-        g.IO.MouseClicked[i] = g.IO.MouseDownDuration[i] == 0.0f;
-        g.IO.MouseReleased[i] = g.IO.MouseDownDurationPrev[i] >= 0.0f && !g.IO.MouseDown[i];
         g.IO.MouseDoubleClicked[i] = false;

It's trivial to check wrong behaviour by modifying the demo as follows.
You will see that you get two mouse releases for each click.

        if (ImGui::TreeNode("Keyboard & Mouse State"))
        {
            ImGuiIO& io = ImGui::GetIO();

            ImGui::Text("MousePos: (%g, %g)", io.MousePos.x, io.MousePos.y);
            static int numReleases = 0;
            ImGui::Text("Mouse down:");     for (int i = 0; i < IM_ARRAYSIZE(io.MouseDown); i++) if (io.MouseDownDuration[i] >= 0.0f)   { ImGui::SameLine(); ImGui::Text("b%d (%.02f secs)", i, io.MouseDownDuration[i]); }
            ImGui::Text("Mouse clicked:");  for (int i = 0; i < IM_ARRAYSIZE(io.MouseDown); i++) if (ImGui::IsMouseClicked(i))          { ImGui::SameLine(); ImGui::Text("b%d", i); }
            ImGui::Text("Mouse dbl-clicked:"); for (int i = 0; i < IM_ARRAYSIZE(io.MouseDown); i++) if (ImGui::IsMouseDoubleClicked(i)) { ImGui::SameLine(); ImGui::Text("b%d", i); }
            ImGui::Text("Mouse released (%d):", numReleases); for (int i = 0; i < IM_ARRAYSIZE(io.MouseDown); i++) if (ImGui::IsMouseReleased(i))         { ImGui::SameLine(); ImGui::Text("b%d", i); numReleases++; }
            ImGui::Text("MouseWheel: %.1f", io.MouseWheel);
@ocornut
Copy link
Owner

ocornut commented Oct 2, 2015

The change was meant to make mouse functions with 0.0 deltatime so ideally we'd want that without the new bug. I'll look into it maybe tonight, if you can find the correct fix before feel free to PR :)

Pagghiu added a commit to Pagghiu/imgui that referenced this issue Oct 5, 2015
- The idea is that you can't have two MouseReleased (for the same button) on two consecutive frames
@ocornut
Copy link
Owner

ocornut commented Oct 8, 2015

Thanks and sorry I couldn't look earlier.

I haven't merged your change from #362

But changed
g.IO.MouseReleased[i] = !g.IO.MouseDown[i] && g.IO.MouseDownDurationPrev[i] >= 0.0f;
to
g.IO.MouseReleased[i] = !g.IO.MouseDown[i] && g.IO.MouseDownDuration[i] >= 0.0f;

Which I think it's more clear. Testing Prev[] was a mistake here. It would miss a release event if mouse was held for a single frame.
Those lines are rather confusing but such a simple operation. Can probably clarify when we add support for timestamped events.

@ocornut ocornut closed this as completed Oct 8, 2015
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