After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 697007 - status/keyboard: Synchronize input source switching with key events
status/keyboard: Synchronize input source switching with key events
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 691397 692726 695334 697018 698542 700467 (view as bug list)
Depends on: 696996 697001
Blocks:
 
 
Reported: 2013-03-31 22:41 UTC by Rui Matos
Modified: 2013-05-24 22:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
status/keyboard: Synchronize input source switching with key events (4.51 KB, patch)
2013-03-31 22:41 UTC, Rui Matos
none Details | Review
status/keyboard: Synchronize input source switching with key events (4.05 KB, patch)
2013-04-03 22:01 UTC, Rui Matos
none Details | Review
status/keyboard: Synchronize input source switching with key events (5.27 KB, patch)
2013-04-12 14:20 UTC, Rui Matos
accepted-commit_now Details | Review
status/keyboard: Synchronize input source switching with key events (5.03 KB, patch)
2013-05-13 21:01 UTC, Rui Matos
committed Details | Review

Description Rui Matos 2013-03-31 22:41:47 UTC
.
Comment 1 Rui Matos 2013-03-31 22:41:50 UTC
Created attachment 240266 [details] [review]
status/keyboard: Synchronize input source switching with key events

Currently we simply set the gsettings key when activating an input
source. This obviously introduces a time window, between the event that
activates the switch and when the switch is complete, under which key
events are being delivered to applications and interpreted according
to the previous input source.

The patches in bug 696996 introduce a DBus API in g-s-d that allows us
to know when an input source if effectively active. Using that and
freezing keyboard events in the X server until we hear back from g-s-d
we can ensure that events won't be misinterpreted after an input
source switch.
Comment 2 Rui Matos 2013-04-01 10:35:44 UTC
*** Bug 697018 has been marked as a duplicate of this bug. ***
Comment 3 Rui Matos 2013-04-02 22:50:41 UTC
*** Bug 695334 has been marked as a duplicate of this bug. ***
Comment 4 Rui Matos 2013-04-02 22:56:13 UTC
*** Bug 691397 has been marked as a duplicate of this bug. ***
Comment 5 Rui Matos 2013-04-02 23:02:43 UTC
*** Bug 692726 has been marked as a duplicate of this bug. ***
Comment 6 Rui Matos 2013-04-03 22:01:03 UTC
Created attachment 240550 [details] [review]
status/keyboard: Synchronize input source switching with key events

--

* Noticed that I could simplify the code a lot and still make it work
  reliably in all conditions.

* Added a timeout (2 secs) to the DBus call so that we don't keep the
  keyboard frozen for potentially long periods (whatever the default
  DBus timeout is).
Comment 7 Rui Matos 2013-04-12 14:20:20 UTC
Created attachment 241354 [details] [review]
status/keyboard: Synchronize input source switching with key events

--

* Updated patch according to changes in bug 696996.

* Increased the maximum time we wait with a frozen keyboard to 4
  seconds since I was seeing that it still wasn't enough in the case
  of ibus-anthy being activated from a cold start.
Comment 8 Rui Matos 2013-04-22 09:06:08 UTC
*** Bug 698542 has been marked as a duplicate of this bug. ***
Comment 9 Aleksei Lissitsin 2013-04-30 14:15:24 UTC
IMHO, this solution is not good enough.

In the pre-gnome 3.6 (or pre-g-s-d-switching) era the layout switching took just an instant. It was just a simple increment in XKBLockGroup. All the layouts were already loaded. And it did not bother the user at all. 

Now whether this bug is fixed (this way) or not, the user will have to wait. Not exactly 4 seconds all the time, but the usual amount is about 1 sec. Which is a lot, it really spoils the experience.

I did not really investigate in depth how this works now in gnome nor how it worked via xkblockgroup. But

What introduces this lag? 
It might be gsettings call. But probably not.

When using xkbsettings, one common way to change layout was to run setxkbmap to recreate xkbmaps each time. This approach had almost exactly all the same problems as the current layout switching had in gnome. It broke keyboard shortcuts in non-latin layouts and it was slow (up to a sec).

Maybe this is what happens now. Each call just recreates the layout from scratch?
Then maybe we can eliminate the lag using some kind of layout caching as xkb did? At least for simple (non-Asian) layouts?
Comment 10 Matthias Clasen 2013-05-01 00:29:59 UTC
what, wait 1 second ?!
where would that kind of lag come from ? that would be totally unacceptable
Comment 11 Rui Matos 2013-05-01 13:23:52 UTC
(In reply to comment #9)
> IMHO, this solution is not good enough.

Did you try it?

FWIW here's a few figures, switching between 'us' and 'ru':

      JS LOG: >>> 136.987 ms
      JS LOG: >>> 107.416 ms
      JS LOG: >>> 100.686 ms
      JS LOG: >>> 87.01 ms

Note that key events are frozen inside the server during this interval but they *are* delivered after it. That means that no key event is lost, you can switch and continue to type and after, yes, some delay, all the typed characters will be there. Most of the times, in practice, the delay isn't noticeable IME.
Comment 12 Aleksei Lissitsin 2013-05-01 14:26:45 UTC
> > IMHO, this solution is not good enough.

> Did you try it?

No, I didn't. And I did not mean that this patch is not good. It will certainly make things better than they are now. Maybe the lag would not be felt at all, then.

Ok, it is not a second on a fast computer, and, yes, slow ones do not usually run gnome. But 100ms is still not perfect. Almost every time I switch layout I manage to type 2-3 characters in the old layout. Maybe this patch will fix it and it would not even feel unresponsive. That would be good. I'll try to test it.

But why is this lag there in the first place?
Comment 13 Rui Matos 2013-05-01 14:36:56 UTC
(In reply to comment #12)
> > > IMHO, this solution is not good enough.
> 
> > Did you try it?
> 
> No, I didn't. And I did not mean that this patch is not good. It will certainly
> make things better than they are now. Maybe the lag would not be felt at all,
> then.

Note that this patch is not enough. It depends on mutter and g-s-d patches too. They are all linked from here though.

> Ok, it is not a second on a fast computer, and, yes, slow ones do not usually
> run gnome. But 100ms is still not perfect. Almost every time I switch layout I
> manage to type 2-3 characters in the old layout. Maybe this patch will fix it
> and it would not even feel unresponsive. That would be good. I'll try to test
> it.

Yes this does fix the 2-3 characters in the old layout problem.

> But why is this lag there in the first place?

Because we're are indeed using the same method that running 'setxkbmap <layouts>' which causes the X server to compile a new keymap each time since it indeed doesn't cache them.

Why are using that now? See this commit message https://git.gnome.org/browse/gnome-settings-daemon/commit/?id=b6e011ad16311e2985aa523cf9bceef74168f95e . Also see this comment in the code: https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/keyboard/gsd-keyboard-manager.c?id=b6e011ad16311e2985aa523cf9bceef74168f95e#n311 .
Comment 14 Florian Müllner 2013-05-13 13:17:04 UTC
Review of attachment 241354 [details] [review]:

OK

::: js/ui/status/keyboard.js
@@ +259,2 @@
     activate: function() {
+        holdKeyboard();

I'd prefer this closer to the releaseKeyboard() call, e.g. in the signal handler. Or am I overlooking something?
Comment 15 Rui Matos 2013-05-13 21:01:30 UTC
Created attachment 244109 [details] [review]
status/keyboard: Synchronize input source switching with key events

Currently we simply set the gsettings key when activating an input
source. This obviously introduces a time window, between the event that
activates the switch and when the switch is complete, under which key
events are being delivered to applications and interpreted according
to the previous input source.

The patches in bug 696996 introduce a DBus API in g-s-d that allows us
to know when an input source if effectively active. Using that and
freezing keyboard events in the X server until we hear back from g-s-d
we can ensure that events won't be misinterpreted after an input
source switch.
Comment 16 Rui Matos 2013-05-13 21:03:18 UTC
(In reply to comment #14)
> ::: js/ui/status/keyboard.js
> @@ +259,2 @@
>      activate: function() {
> +        holdKeyboard();
> 
> I'd prefer this closer to the releaseKeyboard() call, e.g. in the signal
> handler. Or am I overlooking something?

Yes, it looks better this way. I also changed the remote DBus call to follow the traditional syntax and instead set the max timeout on the proxy instance itself.
Comment 17 Florian Müllner 2013-05-13 21:24:45 UTC
Review of attachment 244109 [details] [review]:

(In reply to comment #16)
> I also changed the remote DBus call to follow the traditional syntax and 
> instead set the max timeout on the proxy instance itself.

Nice!
Comment 18 Rui Matos 2013-05-16 16:20:23 UTC
*** Bug 700467 has been marked as a duplicate of this bug. ***
Comment 19 Rui Matos 2013-05-24 22:19:20 UTC
Attachment 244109 [details] pushed as cd7197e - status/keyboard: Synchronize input source switching with key events