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

Keyboard layout changes not handled #895

Closed
kimci86 opened this Issue May 23, 2015 · 9 comments

Comments

Projects
None yet
3 participants
@kimci86
Contributor

kimci86 commented May 23, 2015

The issue #879 was closed but the issue of keyboard layout changes remains.

Using Ubuntu 14.04 and SFML 2.3, changing the keyboard layout while an application is running is not handled properly. Both KeyEvent and TextEntered events still follow the layout in effect when the application was launched.
Moreover, a warning message appears. (See this comment)

@binary1248 binary1248 self-assigned this May 23, 2015

binary1248 added a commit that referenced this issue May 23, 2015

Fixed keyboard mapping not being correct after the user changes their…
… keyboard layout while an SFML application is running. Fixes #895
@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 May 23, 2015

Member

Try out d25cb9c.

Member

binary1248 commented May 23, 2015

Try out d25cb9c.

@kimci86

This comment has been minimized.

Show comment
Hide comment
@kimci86

kimci86 May 24, 2015

Contributor

I tried it. The warning message disappear but both KeyEvent and TextEntered events still follow the first layout.
By digging a little in the source, I found out that the buildMap functions from WindowImplX11.cpp and InputImpl.cpp are based on the keysymMap from Display.cpp. By making Display's buildMap function available to WindowImplX11 (by moving it to the sf::priv namespace) and calling it just before WindowImplX11's buildMap so that keysymMap gets updated, the WindowImplX11's sfKeyMap is updated correctly as well as InputImpl's keycodeMap. Then, the KeyEvent events contain the right Keyboard::Key value and isKeyPressed behaves correctly.

To summarize, Display.cpp buildMap function must be called first.
However, TextEntered events still follow the first layout.

Besides, reading the source, I felt that the way InputImpl's keycodeMap is built is somewhat brutal. Moreover, I noticed that several map rebuild are triggered for a single keyboard layout change (12 times when I change the layout and 22 times when I plug my keyboard in). If I understand correctly, building keycodeMap could be done in a single loop by "inverting" WindowImplX11's sfKeyMap (even if it is not exactly bijective), but maybe this is a little off-topic.

Contributor

kimci86 commented May 24, 2015

I tried it. The warning message disappear but both KeyEvent and TextEntered events still follow the first layout.
By digging a little in the source, I found out that the buildMap functions from WindowImplX11.cpp and InputImpl.cpp are based on the keysymMap from Display.cpp. By making Display's buildMap function available to WindowImplX11 (by moving it to the sf::priv namespace) and calling it just before WindowImplX11's buildMap so that keysymMap gets updated, the WindowImplX11's sfKeyMap is updated correctly as well as InputImpl's keycodeMap. Then, the KeyEvent events contain the right Keyboard::Key value and isKeyPressed behaves correctly.

To summarize, Display.cpp buildMap function must be called first.
However, TextEntered events still follow the first layout.

Besides, reading the source, I felt that the way InputImpl's keycodeMap is built is somewhat brutal. Moreover, I noticed that several map rebuild are triggered for a single keyboard layout change (12 times when I change the layout and 22 times when I plug my keyboard in). If I understand correctly, building keycodeMap could be done in a single loop by "inverting" WindowImplX11's sfKeyMap (even if it is not exactly bijective), but maybe this is a little off-topic.

binary1248 added a commit that referenced this issue May 24, 2015

Fixed keyboard mapping not being correct after the user changes their…
… keyboard layout while an SFML application is running. Fixes #895
@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 May 24, 2015

Member

Try out c94536a.

Member

binary1248 commented May 24, 2015

Try out c94536a.

@kimci86

This comment has been minimized.

Show comment
Hide comment
@kimci86

kimci86 May 24, 2015

Contributor

This is not working, but I noticed that xkbType is always 0.
I replaced if (xkbType == XCB_XKB_MAP_NOTIFY)
by if (xkbType == XCB_XKB_NEW_KEYBOARD_NOTIFY || xkbType == XCB_XKB_MAP_NOTIFY)
and everything gets updated as intended. I don't know if this would be valid for every system though.
As before, TextEntered events follow the first layout.

Contributor

kimci86 commented May 24, 2015

This is not working, but I noticed that xkbType is always 0.
I replaced if (xkbType == XCB_XKB_MAP_NOTIFY)
by if (xkbType == XCB_XKB_NEW_KEYBOARD_NOTIFY || xkbType == XCB_XKB_MAP_NOTIFY)
and everything gets updated as intended. I don't know if this would be valid for every system though.
As before, TextEntered events follow the first layout.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 May 24, 2015

Member

Define "not working". What are you doing? Changing the keyboard layout or re-plugging the device?

Member

binary1248 commented May 24, 2015

Define "not working". What are you doing? Changing the keyboard layout or re-plugging the device?

@kimci86

This comment has been minimized.

Show comment
Hide comment
@kimci86

kimci86 May 25, 2015

Contributor

Define "not working".

I mean that the commit c94536a does not handle keyboard layout changes. Nothing is updated when I switch the keyboard layout because this if condition never passes: if (xkbType == XCB_XKB_MAP_NOTIFY)

What are you doing? Changing the keyboard layout or re-plugging the device?

Yesterday I was only changing the keyboard layout. As surprising as it may seem, xkbType is always equal to 0 (that is to say XCB_XKB_NEW_KEYBOARD_NOTIFY). Today I also tested re-plugging the keyboard and xkbType is also 0.

Contributor

kimci86 commented May 25, 2015

Define "not working".

I mean that the commit c94536a does not handle keyboard layout changes. Nothing is updated when I switch the keyboard layout because this if condition never passes: if (xkbType == XCB_XKB_MAP_NOTIFY)

What are you doing? Changing the keyboard layout or re-plugging the device?

Yesterday I was only changing the keyboard layout. As surprising as it may seem, xkbType is always equal to 0 (that is to say XCB_XKB_NEW_KEYBOARD_NOTIFY). Today I also tested re-plugging the keyboard and xkbType is also 0.

binary1248 added a commit that referenced this issue May 25, 2015

Fixed keyboard mapping not being correct after the user changes their…
… keyboard layout while an SFML application is running. Fixes #895
@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 May 25, 2015

Member

a5de801 should do the trick. Was a bit more involved than I originally thought.

Member

binary1248 commented May 25, 2015

a5de801 should do the trick. Was a bit more involved than I originally thought.

@kimci86

This comment has been minimized.

Show comment
Hide comment
@kimci86

kimci86 May 25, 2015

Contributor

Now, KeyEvent and TextEvent data follow the new layout. 👏
For me, this commit solves the issue.

Contributor

kimci86 commented May 25, 2015

Now, KeyEvent and TextEvent data follow the new layout. 👏
For me, this commit solves the issue.

@eXpl0it3r eXpl0it3r added this to the 2.3.1 milestone May 27, 2015

@eXpl0it3r eXpl0it3r added s:accepted and removed s:undecided labels Jun 4, 2015

binary1248 added a commit that referenced this issue Jun 4, 2015

Fixed keyboard mapping not being correct after the user changes their…
… keyboard layout while an SFML application is running. Fixes #895
@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Jun 4, 2015

Member

Fixed in 0df1c97 on branch 2.3.x

Member

eXpl0it3r commented Jun 4, 2015

Fixed in 0df1c97 on branch 2.3.x

@eXpl0it3r eXpl0it3r closed this Jun 4, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment