SimulaVR / Simula

Linux VR Desktop
MIT License
2.91k stars 87 forks source link

Keyboard focus should only be activated on button presses, not pointing #87

Closed georgewsinger closed 4 years ago

georgewsinger commented 4 years ago

Otherwise it is impossible to type when windows get configured like the following:

Here we cannot type in the inner window, since setting down the controller will necessarily activate the outer window.

georgewsinger commented 4 years ago

The following simple adjustment doesn't seem to work:

-- Old
processClickEvent :: GodotSimulaViewSprite
                  -> InputEventType
                  -> GodotVector3
                  -> IO ()
processClickEvent gsvs evt clickPos = do
  -- ..
  case evt of
    Motion                -> do pointerNotifyEnter wlrSeat godotWlrSurface subSurfaceLocalCoords
                                pointerNotifyMotion wlrSeat subSurfaceLocalCoords
                                keyboardNotifyEnter wlrSeat godotWlrSurface -- HACK: shouldn't have to call this every frame we're pointing at a surface
    Button pressed button -> do pointerNotifyButton wlrSeat evt
  -- ..

-- New: for some reason doesn't work
processClickEvent :: GodotSimulaViewSprite
                  -> InputEventType
                  -> GodotVector3
                  -> IO ()
processClickEvent gsvs evt clickPos = do
  -- ..
  case evt of
    Motion                -> do pointerNotifyEnter wlrSeat godotWlrSurface subSurfaceLocalCoords
                                pointerNotifyMotion wlrSeat subSurfaceLocalCoords
    Button pressed button -> do keyboardNotifyEnter wlrSeat godotWlrSurface
                                pointerNotifyButton wlrSeat evt
  -- ..
georgewsinger commented 4 years ago

I worked on this all day yesterday until realizing something that wasn't obvious to me:

In [gd]wlroots, both keyboard_notify_enter and pointer_notify_enter cause the keyboard to be focused on a surface. I've been assuming this whole time that it was just keyboard_notify_enter that was doing this, but I was wrong. And this is actually the normal behavior in other Linux window managers as well, such as i3:

https://www.youtube.com/watch?v=jE0S66yHY_k&feature=youtu.be

so Simula isn't "broken" in the sense that it's not acting like a normal window manager, but it is "broken" in the sense that normal window manager behavior isn't appropriate for our VR compositor.

Possible solution: keep track of the active keyboard window at the Simula/Haskell level. That is we can create a field gssKeyboardFocusedSprite:

data GodotSimulaServer = GodotSimulaServer
  { _gssObj                  :: GodotObject
  -- ..
  , _gssKeyboardFocusedSprite :: TVar (Maybe GodotSimulaViewSprite) -- <- Here
  }

which is swapped whenever someone clicks a window (not just points at one):

focus :: GodotSimulaViewSprite -> IO ()
focus gsvs = do
  -- ..
  gss <- readTVarIO $ (gsvs ^. gsvsServer)
  atomically $ writeTVar (gss ^. gssKeyboardFocusedSprite) (Just gsvs)
  -- ..
processClickEvent :: GodotSimulaViewSprite
                  -> InputEventType
                  -> GodotVector3
                  -> IO ()
processClickEvent gsvs evt clickPos = do
   -- ...
  case evt of
    Motion                -> do pointerNotifyEnter wlrSeat godotWlrSurface subSurfaceLocalCoords
                                pointerNotifyMotion wlrSeat subSurfaceLocalCoords

    Button pressed button -> do focus gsvs -- keyboardNotifyEnter wlrSeat godotWlrSurface
                                pointerNotifyButton wlrSeat evt

such that when a Simula user presses a key, we force the proper surface to be focused before passing the event off to gdwlroots:

_on_wlr_key :: GodotSimulaServer -> [GodotVariant] -> IO ()
_on_wlr_key gss [keyboardGVar, eventGVar] = do
  gsvsFocused <- readTVarIO (gss ^. gssKeyboardFocusedSprite)
  focus gsvsFocused
  wlrSeat <- readTVarIO (gss ^. gssWlrSeat)
  G.keyboard_notify_key wlrSeat eventGVar
  return ()

_on_wlr_modifiers :: GodotSimulaServer -> [GodotVariant] -> IO ()
_on_wlr_modifiers gss [keyboardGVar] = do
  gsvsFocused <- readTVarIO (gss ^. gssKeyboardFocusedSprite)
  focus gsvsFocused
  wlrSeat <- readTVarIO (gss ^. gssWlrSeat)
  G.keyboard_notify_modifiers wlrSeat
  return ()
georgewsinger commented 4 years ago

Original problem. Here is the problem we're trying to solve:

trying to select a window in the middle of others and type is impossible because when I set the controller down to type, the controller passes [other windows] – @NerveCoordinator

The above solution half works. The solution layed out in my previous post doesn't quite work, but is still an improvement. Here is how it behaves:

  1. :heavy_check_mark: If you point & click on surface A, and then point at surface B, and then point at nothing: your typing only affects surface A.
  2. :x: If you point & click on surface A, and then point and hold focus on surface B, then typing will still occur on surface B (instead of surface A).

Behavior (1) improves the behavior of Simula, but behavior (2) is still a bug. I'm assuming it's being caused by some sort of race condition issue whereby surface B is getting re-focused in between (i) our call to force the focus on A and (ii) our call to send the key event. To get around this I tried adding the following functions in gdwlroots:

void WlrSeat::keyboard_notify_key_with_surface(Variant _key_event, Variant _surface) {
  keyboard_notify_enter(_surface);
  struct timespec now;
  clock_gettime(CLOCK_MONOTONIC, &now);
  auto key_event = dynamic_cast<WlrEventKeyboardKey *>((Object *)_key_event);
  auto event = key_event->get_wlr_event();
  std::cout << "keyboard_notify_key_with_surface surface: " << wlr_seat->keyboard_state.focused_surface << std::endl; 
  wlr_seat_keyboard_notify_key(wlr_seat, timespec_to_msec(&now),
                                event->keycode, event->state);
}

void WlrSeat::keyboard_notify_modifiers_with_surface(Variant _surface) {
  //cout << "WlrSeat::keyboard_notify_modifiers" << endl;
  keyboard_notify_enter(_surface);
  struct timespec now;
  clock_gettime(CLOCK_MONOTONIC, &now);
  struct wlr_keyboard *keyboard = wlr_seat_get_keyboard(wlr_seat);
  wlr_seat_keyboard_notify_modifiers(wlr_seat, &keyboard->modifiers);
}

but they don't fix the bug.

In any event, I committed the above solution to Simula's gdwlroots-xwayland branch.