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

Reset the keyboard status after ToUnicodeEx #1583

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pedronavf
Copy link

After getting a dead key from ToUnicodeEx, add an additional VK_SPACE to the keyboard state so we reset the dead key flag and subsequent calls with modifiers, like shift, return the right result (-1) instead of 1.

This happened because without reseting the dead key status the new one was attempted to be composed with the old one, which failed and the end result was a single unicode codepoint not marked as a dead key.

This opens the door to potentially use the returned unicode from the second call to ToUnicodeEx as the key character instead of maintaining the getDeadKey function.

This should, hopefully, address issues like #1267 or #727 #1143 or #795

I have submitted the same patch to Synergy.

Contributor Checklist:

  • I have created a file in the doc/newsfragments directory IF it is a
    user-visible change (and make sure to read the README.md in that directory)


After getting a dead key from `ToUnicodeEx`, add an additional `VK_SPACE` to the keyboard state so we reset the dead key flag and subsequent calls with modifiers, like shift, return the right result (-1) instead of 1.

This happened because without reseting the dead key status the new one was attempted to be composed with the old one, which failed and the end result was a single unicode codepoint not marked as a dead key.

This opens the door to potentially use the returned unicode from the second call to `ToUnicodeEx` as the key character instead of maintaining the getDeadKey function.

This should, hopefully, address issues like debauchee#1267 or debauchee#727 debauchee#1143 or debauchee#795

I have submitted the same patch to Synergy.
@pedronavf
Copy link
Author

Hey, what's the process for getting my PR reviewed?

@elegos
Copy link

elegos commented May 12, 2022

Hello @pedronavf,

I'm very interested in this fix, though it handles only Windows hosts (I guess?). Would it be possible to extend this to XWindowsKeyState.cpp at least (Linux), and possibly tell me if that could directly fix composite keys? I'm a developer, but it's too much time I don't write C/C++ code. If you wish to explain the key detection flow, I might able to help you experimenting via logging.

@pedronavf
Copy link
Author

Hi @elegos, what problem are you having? My synergy server runs on Linux and I had problem with the composite keys when I switch to a Windows client. I have a Spanish layout keyboard and I have no problems with composite keys at all on Linux. Is your case the opposite, in which your server is Windows and your client Linux?

@elegos
Copy link

elegos commented May 12, 2022

Hello @pedronavf so this is a client fix? With the current version of Barrier (not modified), when I try, for example, to compose SHIFT+', SPACE it writes only a space instead of the quote symbol " (using US international layout), or any vocal instead of space for accented vocals (i.e. ë). Other combinations work fine, for example `+e = è (any vocal).

Other example might be the fractions, for example ⅚ (made via AltGR+5+6) doesn't work on the Windows client... AltGr+5 doesn't write the Euro symbol (though CTRL+ALT+5 does).

@entomoio
Copy link

entomoio commented Jun 1, 2022

can we generate the executable directly from this PR?

@elegos
Copy link

elegos commented Jun 2, 2022

@entomoio I've compiled and succesfully installed the modified version of barrier, applying the patch on the master branch, in a virtual machine. I haven't tested the functionaly yet, but you can download the installer from here (obviously at your own risk):
BarrierSetup-2.4.0-release.zip

To build it I had to:

  • install the vs_buildtools.exe (2017) (C++ build tools only)
  • install Inno Setup 5
  • install git (required, as it will init the submodules) (remember to check the add to the PATH variable option)
  • install 7z (and add its bin folder to the Path environment variable)
  • git clone the master branch (do not download the zip, it just won't work as per the submodules)
  • from elevated PowerShell, execute Set-ExecutionPolicy -ExecutionPolicy Unrestricted
  • run the two ps1 download scripts in azure-pipelines (with Powershell)
    • .\azure-pipelines\download_install_bonjour_sdk_like.ps1
    • .\azure-pipelines\download_install_qt.ps1
  • copy azure-pipelines/build_env_tmpl.bat in build_env.bat
    • set B_BUILD_TYPE to "Release"
  • apply the patch
  • open the Developer Command Prompt for VS 2017 and enter the repo directory
  • .\clean_build.bat
  • .\build_installer.bat
  • get / use build/installer-inno/bin/BarrierSetup-2.4.0-release.exe

@entomoio
Copy link

entomoio commented Jun 2, 2022

Lovely, @elegos, thank you very much!

It was possible to avoid the dead key quote not being sent by setting the keyboard on client machine to use EN-US keyboard, and since I am using an US keyboard on server machine, setting it to PT-US.international.

*one more thing: I had to Reload the server every time the keyboard changed (on client or server), so it would use the new config.

@pedronavf
Copy link
Author

Sorry @elegos, I missed the notifications for this thread. To answer your question: yes, it's a client only patch. The correct keys were being sent to Windows but they were being mishandled there. What's your setup? Linux server and Windows client both with the layout set to US?

@elegos
Copy link

elegos commented Jun 2, 2022

Hello @pedronavf,

Yes exactly, they're both set to US international.

Will try my own build with your patch as soon as I'll power my job's laptop up :)

@pedronavf
Copy link
Author

I can try that combination also @elegos. I'll take a look. Note that I'm using Synergy and not Barrier.

@elegos
Copy link

elegos commented Jun 6, 2022

@pedronavf Ok, here I am, I'm writing from the Windows client machine, using US international on both devices (with dead keys on Linux). Unfortunately pressing [SHIFT} + ', [SPACE] won't write me the quote symbol :(

Is there anything I can do to debug the keys being received by barrier client, to investigate whether it's a server or a client issue? (maybe continue on another thread?)

@pedronavf
Copy link
Author

@pedronavf Ok, here I am, I'm writing from the Windows client machine, using US international on both devices (with dead keys on Linux). Unfortunately pressing [SHIFT} + ', [SPACE] won't write me the quote symbol :(

I changed my Linux machine to use the "English (US) English (intl, with AltGr dead keys)". On Linux I can use all keys with shift and in dead key mode. For the ' key I get (I'm typing this on my Windows client machine)

  • '
  • " (shift ')
  • ´ (altgr ')
  • ¨ (altgr + shift ')

My keyboard does have an AltGr key. I don't know if that helps. Also, in my configuration I have for my windows machine:

meta = altgr
altgr = shift

I don't know if that helps. Keep in mind that I'm not using barrier but synergy, which has new code for handling keyboard layouts. You might want to give it a try.

@elegos
Copy link

elegos commented Jun 7, 2022

Yes, with the layout English (USA) international with AltGr dead keys it seems to work fine on my guest, too. I might switch to this layout (from English (USA) international with dead keys), though I'd like to know if there was a way to let the 2nd layout work, too.

@pedronavf
Copy link
Author

Oh, I didn't see that layout. I'll try that one and will report back.

@pedronavf
Copy link
Author

@elegos, does the "English internation with AltGr dead keys" work without my patch? I'm curious.

@pedronavf
Copy link
Author

I see what you mean. This is what Linux shows for the US Intl with dead keys keyboard layout:

kbd-layout

In this layout, the dead keys are used without modifiers, and you can access ' and " by using AltGr and Shift+AltGr. You are using the alternative method, which pressing the dead key and space so you get just the diacritic which gets converted to ' or ", much like you would do ^ followed by space to just get the ^ not composed on a letter.

The issue here is that shift + ¨ followed by space to get " is a Linux keyboard layout particularity. Synergy/Barrier send the raw keys over the wire to Windows, where the composing happens. You need to have in Windows the exact same layout you have on Linux.

I installed on my Windows machine the United States-International layout and I could type shift + ¨ followed by space to get ", but it looked like some other keys didn't exactly map (but I have a Spanish layout keyboard, so your mileage may vary). I think that's the way to go. Use that keyboard layout and, if it doesn't exactly map to your keyboard you could use Microsoft's Keyboard Layout Creator (https://www.microsoft.com/en-us/download/details.aspx?id=102134) to create your own or modify the United States-International one, like this guy did: https://github.com/thomasfaingnaert/win-us-intl-altgr

@elegos
Copy link

elegos commented Jun 7, 2022

Ah, awesome! Will try that. Strange thing though is that when I switch to direct Windows input, the SHIFT+", space press works just fine.

I didn't know the layout I'm using is the alt. one, as it fits the (roughly same) layout of the US int. layout on Windows, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants