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

Fixed same-frame click/release bug #2525

Closed
wants to merge 1 commit into from

Conversation

BlaiseRideout
Copy link

Fixes Issue #1992 but also fixes the same issue in a general way for all platform impls.

Currently only tested for Win32.

There may be a better approach that involves a larger change to the way ImGui and its components handle mouse button input, but I'll leave that to those who are more familiar with the codebase as a whole and the consequences of such a change.

@ocornut
Copy link
Owner

ocornut commented May 25, 2020

Thank you Blaise, I realize I never got around to reply to this specific PR and apologize for that.
The reason I didn't reply was that I am hoping to incorporate an IO queue handled by core imgui which would create a centralized solution for this issue. I hope we can take care of that soon but for now this PR is a good reference and it's tempting to merge some of it sooner. Will get back to it later, thanks for your patience.

case WM_MBUTTONUP: { button = 2; } break;
case WM_XBUTTONUP: { button = (GET_XBUTTON_WPARAM(wParam) == XBUTTON1) ? 3 : 4; } break;
}
io.InputMouseReleased[button] = true;
if (!ImGui::IsAnyMouseDown() && ::GetCapture() == hwnd)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not get called in case of a mouseup happening before NewFrame() since this is where io.MouseDown gets updated.
As a result clicking on an imgui window does not release mouse capture and trying to click on the main window bar at the top to move, minimise or close it does not work.
An easy fix is to make ::ReleaseCapture not based on io.MouseDown but on the actual hardware inputs WM_LBUTTONUP etc...

@ocornut ocornut force-pushed the master branch 2 times, most recently from 0c1e5bd to bb6a60b Compare August 27, 2021 19:10
@ocornut ocornut force-pushed the master branch 2 times, most recently from 8b83e0a to d735066 Compare December 13, 2021 11:31
@ocornut ocornut force-pushed the master branch 2 times, most recently from b3b85d8 to 0755767 Compare January 17, 2022 14:21
ocornut pushed a commit that referenced this pull request Jan 17, 2022
…nt() api + updated all Backends. (#4858) (input queue code will be next commit)

Details: note that SDL, OSX and GLFW backends removed recording of MouseJustPressed[] which will be unnecessary with input queue (which is the NEXT commit). (#2787, #1992, #3383, #2525, #1320)
@ocornut
Copy link
Owner

ocornut commented Jan 17, 2022

Better late than never, apologies @BlaiseRitchie for not reacting sooner. Your PR was the closest intermediary solution but we wanted to overhaul and fix this more generally, this is now done.

Dear imgui 1.87 WIP now uses a new IO api (e.g. io.AddKeyEvent() io.AddMouseButtonEvent() etc.) which implement an input trickling queue, normally solving this problem correctly and everywhere for all types of inputs. We'll monitor feedback and expect bugs to have creeped as we reoverhauled that system.

See e.g.

  • new IO api b8e56dc
  • input queue 7374b96
    There are many other commits but those should provide a good general sense of what changed.

Also see #4858 for a general overview (we'll improve this topic soon).

@ocornut ocornut closed this Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants