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

Inputs are missed when FPS is low #2787

Closed
niexuchina opened this issue Sep 17, 2019 · 18 comments
Closed

Inputs are missed when FPS is low #2787

niexuchina opened this issue Sep 17, 2019 · 18 comments
Labels

Comments

@niexuchina
Copy link

We are using imgui in a real time application like game. When FPS is very low, blow 1 frame per second, it becomes very difficult to click a button. For some buttons with extended menu triggered by long pressing, it's almost impossible to click. I checked codes. And I found that imgui translates mouse key states to key click events. That is key down to event. When a key is pressed and released in one frame, the last state is the same as begin. So the key is not clicked.

Have you considered using event based approach? We can pass key pressed and released events to imgui. And imgui can use them to generate event flags.

This annoys us. We want to try to fix it. We can create pull request. :)

@heroboy
Copy link
Contributor

heroboy commented Sep 17, 2019

I think you should use this one:
https://gist.github.com/ocornut/8417344f3506790304742b07887adf9f

@ocornut ocornut added the inputs label Sep 17, 2019
@ocornut
Copy link
Owner

ocornut commented Sep 17, 2019

Correct, using a trickling input queue as linked above is the recommended solution.
We will revamp the ImGuiIO api to include this trickling input feature, but in the meanwhile it is perfect possible to use the same code on application side.

@niexuchina
Copy link
Author

Thanks. I'll take a look. :)

@niexuchina
Copy link
Author

I tried the new codes a little bit. I think double click can't be generated when FPS is low. What do you think?

@niexuchina niexuchina reopened this Sep 18, 2019
@stgatilov
Copy link

I'm using ImGui for internal CAD model viewer.
With COVID forcing us to use RDP with its slow software rendering, this issue has become an unexpected nuisance =)
I guess I'll try to hack the referenced code with GLFW bindings...

@stgatilov
Copy link

It turned out to be harder than I expected.

The main problem that I'm trying to solve is when I click a checkbox and move the mouse away in one frame. In this case the default handling of input does not register a click, and checkbox state does not change. The sequence of events is:

  1. Mouse down at A.
  2. Mouse up at A.
  3. Mouse move to B.

The obvious problem with the referenced gist is that it does not store mouse coordinates in MouseButton events. I use stock GLFW bindings, and I guess they overwrite mouse coordinates every frame. So if event 1 is processed during the first frame, on the second frame mouse position has already changed to B instead of staying at A.

I changed the code to store the actual mouse coordinates in MouseButton event, and I assign them like io.MousePos = evt.Pos; when event is handled. But it does not work.

I'm afraid the gist is not enough for mere mortals who don't understand the internals well =(

@RoryO
Copy link

RoryO commented Aug 3, 2020

I ran into something similar while adding a pure X11 impl. It's not just limited to low fps and mouse usage, it's any non-character down and up within the same frame. I found this happens frequently with backspace where it's common to input multiple backspace down and up events within one frame at a frame locked 60fps. Other solutions collect all the input events into a queue when processing the event queue at the start of the frame, while also setting the keydown/up booleans while processing the queue. Then when it comes time to scan for inputs replay the whole queue and replay all the inputs. Fast typing isn't an issue because we have a character input queue with text fields and AddInputCharacterXXX. Other keys, like ctrl, alt, backspace are booleans and not in a queue, leading to potential missed inputs between frames.

It feels like there's probably a lot of work switching the all internals to use an input stream rather than setting and checking booleans each frame. Though, perhaps adding an additional event queue api, like AddEventInput(Event e) along side the current boolean system, and then switch widgets over to using event stream system as wanted may be a better path?

Actually, after thinking about it a bit more, it may possibly be a smaller impact changing KeysDown from bool to an integer, with the value being the number of times the key was pressed in between frames. Current existing logic would stay the same, and widgets who care about multiple inputs could process the key N times based upon the value.

@ocornut
Copy link
Owner

ocornut commented Aug 3, 2020

It feels like there's probably a lot of work switching the all internals to use an input stream rather than setting and checking booleans each frame.

We can use a hybrid approach, this is what the gist linked above does and suggests to do:
https://gist.github.com/ocornut/8417344f3506790304742b07887adf9f
Merging inputs and trickling others over frames when required. This should be incorporated in a newer IO api.

We can certainly work toward some more support in widget code, as you mention.

You are however the first person to even mention that specifically a key like Backspace can be toggled at a rate faste than FPS, is that due to the automatic repeat rate in your OS/window manager?

@RoryO
Copy link

RoryO commented Aug 3, 2020

Nothing to do with key repeat, only missed inputs per frame. This also happens with any special key input where there is a key down and up event within the same frame.

Here's a small patch for the win32 impl which visualizes the issue

diff --git a/examples/imgui_impl_win32.cpp b/examples/imgui_impl_win32.cpp
index f6dd5b82..c5383c19 100644
--- a/examples/imgui_impl_win32.cpp
+++ b/examples/imgui_impl_win32.cpp
@@ -7,11 +7,12 @@
 //  [X] Platform: Keyboard arrays indexed using VK_* Virtual Key Codes, e.g. ImGui::IsKeyPressed(VK_SPACE).
 //  [X] Platform: Gamepad support. Enabled with 'io.ConfigFlags |= ImGuiConfigFlags_NavEnableGamepad'.

-#include "imgui.h"
+#include "../imgui.h"
 #include "imgui_impl_win32.h"
 #ifndef WIN32_LEAN_AND_MEAN
 #define WIN32_LEAN_AND_MEAN
 #endif
+#include <stdio.h>
 #include <windows.h>
 #include <tchar.h>

@@ -59,6 +60,7 @@ static INT64                g_TicksPerSecond = 0;
 static ImGuiMouseCursor     g_LastMouseCursor = ImGuiMouseCursor_COUNT;
 static bool                 g_HasGamepad = false;
 static bool                 g_WantUpdateHasGamepad = true;
+static INT64 g_n_bs_keypress;

 // Functions
 bool    ImGui_ImplWin32_Init(void* hwnd)
@@ -215,7 +217,16 @@ void    ImGui_ImplWin32_NewFrame()
 {
     ImGuiIO& io = ImGui::GetIO();
     IM_ASSERT(io.Fonts->IsBuilt() && "Font atlas not built! It is generally built by the renderer back-end. Missing call to renderer _NewFrame() function? e.g. ImGui_ImplOpenGL3_NewFrame().");
+    if(g_n_bs_keypress) {
+        printf("%I64u keypresses last frame\n", g_n_bs_keypress);
+        if(io.KeysDown[VK_BACK])
+            printf("backspace confirmed\n");
+        else
+            printf("backspace ~~~unconfirmed~~~\n");

+    }
+
+    g_n_bs_keypress = 0;
     // Setup display size (every frame to accommodate for window resizing)
     RECT rect;
     ::GetClientRect(g_hWnd, &rect);
@@ -315,6 +326,8 @@ IMGUI_IMPL_API LRESULT ImGui_ImplWin32_WndProcHandler(HWND hwnd, UINT msg, WPARA
         return 0;
     case WM_KEYDOWN:
     case WM_SYSKEYDOWN:
+        if(wParam == VK_BACK)
+            g_n_bs_keypress++;
         if (wParam < 256)
             io.KeysDown[wParam] = 1;
         return 0;

And then building the directx 12 example

f2

This can happen with any modifier key, ctrl-tab is another thing I noticed gets missed.

I quickly experimented with changing KeysDown/MouseDown from bool[] to int[], it looks promising. It compiles and everything works fine so far. Then we could use KeysDown & MouseDown as 'how many times input pressed this frame', and it would immediately solve the problem without changing any other code.. I think I'll have a PR for that later today.

@ocornut
Copy link
Owner

ocornut commented Aug 4, 2020

Interesting. I tried here and managed to get a total of 2 unconfirmed cases while agressively trying to mash the backspace in various fast or subtle ways for several minutes. If it was happening nearly as much as in your gif I would have reacted way earlier. I wonder if the keyboard hardware or drivers would make a difference here.

@sergeyn
Copy link
Contributor

sergeyn commented Aug 4, 2020

Hi,
I'm sorry if it was said before. Current input processing in samples is a bit too simplistic, thus this problem. Currently all current inputs are fed into a struct before that struct gets released to ImGui for processing. A fix would be to release a struct earlier (i.e.- stop processing inputs and draw frame) when a condition resulting in an event loss is met (when pressing/releasing special keys too quickly for example).

@RoryO
Copy link

RoryO commented Aug 4, 2020

I wonder if the keyboard hardware or drivers would make a difference here.

You're probably on to something. I use ergodox keyboards which have higher quality switches and firmware than a normal off the shelf keyboard. It probably has much faster switch response times and firmware scan rate, exasperating the issue.

@RoryO
Copy link

RoryO commented Aug 4, 2020

#3383

@RoryO
Copy link

RoryO commented Aug 5, 2020

After thinking about it more I cannot think of a low impact way of making mouse inputs with lagged frames more responsive. If we try the same trick as keyboard input, checking for up/down in alternate frames instead of the same frame, that would lead to a probably worse experience with lagged frames. Currently nothing encodes the coordinate of the click. The state of the mouse button and the position are independent. Fixing this requires encoding a click with the exact vector of the position. If we try and carry over all the mouse information into the next frame all we have is 'mouse 1 is clicked, cursor currently at x,y' and 'x,y' might be distant from where the user actually clicked in the last lagged frame, causing a click in a distant area.

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2020 via email

@RoryO
Copy link

RoryO commented Aug 5, 2020

I deserved that, sorry. That looks like it solves the issue for a single input, not in the really rare case of multiple hits per frame. Is there a reason why it's not added into the main code?

@ocornut
Copy link
Owner

ocornut commented Aug 5, 2020 via email

ocornut pushed a commit that referenced this issue Jan 10, 2022
…vent(), GetKeyName(), IMGUI_DISABLE_OBSOLETE_KEYIO. Obsoleted GetKeyIndex(), io.KeyMap[], io.KeysDown[]. (#2625, #4858, #2787)
ocornut added a commit that referenced this issue Jan 10, 2022
…simplify backends, asserts on misuses, tested backward compat. (#2625, #4858, #2787)

(edit: simplified backends merged into previous commits to make history clearer)
ocornut pushed a commit that referenced this issue 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

FYI 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 everywhere for all types of inputs. We'll monitor feedback and expect bugs to have creeped as we reoverhauled that system.

See e.g.

Also see #4858 for a general overview.

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

No branches or pull requests

6 participants