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

Call GtkImContextFilterKeypress after issuing the KeyDown event #2075

Merged
merged 2 commits into from
Nov 12, 2018

Conversation

tdaffin
Copy link
Contributor

@tdaffin tdaffin commented Nov 3, 2018

I'm writing an app in avalonia that depends on being able to accurately read all of the KeyDown and KeyUp events.
I'm doing my development on Linux but I'm also testing on windows.
I've found that in order to get the same sequence of events for Control.KeyDown, KeyUp and TextInput on Linux as I get on windows I need to have the KeyDown event issued before the call to GtkImContextFilterKeypress, as that call will then issue the TextInput event via the OnCommit callback in WindowBaseImpl.

The other effect of this change is that both the KeyDown and the TextInput event now get issued on Linux, whereas before the KeyDown event would never be issues if GtkImContextFilterKeypress returned true.

With this change I now get the same events issued to my app when it runs on Linux as I do when I run it on Windows.

Checklist:

@kekekeks
Copy link
Member

kekekeks commented Nov 5, 2018

https://developer.gnome.org/gtk3/stable/GtkIMContext.html#gtk-im-context-filter-keypress

Allow an input method to internally handle key press and release events. If this function returns TRUE, then no further processing should be done for this key event.

@kekekeks
Copy link
Member

kekekeks commented Nov 5, 2018

Basically this allows the input method to preprocess key events and signal application to ignore them. Your change breaks proper handling of eastern writing systems

@kekekeks
Copy link
Member

kekekeks commented Nov 5, 2018

#2076

@kekekeks kekekeks closed this Nov 5, 2018
@grokys
Copy link
Member

grokys commented Nov 10, 2018

I wonder, given that we don't actually handle eastern text properly (in rendering, or in input) at the moment, and given that proper handling of eastern text is probably a little way off, would it not be worth merging this PR in the interim?

@kekekeks
Copy link
Member

GtkImContextSimple is still handling Compose key sequences that don't require special rendering. RAlt + O, C prints "©", for example.

So we currently do support multi-key input in our GTK backend.

@grokys
Copy link
Member

grokys commented Nov 11, 2018

Ok, so I think the question is whether handling key sequences is more important than receiving the same events between operating systems for the moment.

@kekekeks
Copy link
Member

We could implement basic infrastructure for checking if focused control expects text input, but I'm not quite sure if my initial approach in #2076 is correct. "Virtual" text store and some kind of per-control IM-context seems to be a better solution.

@grokys
Copy link
Member

grokys commented Nov 11, 2018

Yeah, I'm really not sure either... I just don't have enough experience in this area to be able to give useful input. @tdaffin do you have any experience in this area?

@kekekeks
Copy link
Member

We could try somewhat mirroring GTK's API, but that will require to pass the native handle pointer through the whole event pipeline. In that case the control handling events could decide if it wants them to be processed via IM.

The problem with that approach (aside from the need to handle native objects) is that in Avalonia it's way too easy to intercept events for child controls, so some top-level control logic might break IM interaction, which will be unnoticed by application developer (who will often only do testing on european language localized Windows-machine) until app is deployed on some CJK-machine. That way even our standard TextBox could be broken, which is a bad experience for ends users.

@tdaffin
Copy link
Contributor Author

tdaffin commented Nov 11, 2018

I'm certainly not an expert, but it appears to me that there shouldn't be any reason not to treat the KeyDown/KeyUp events differently from the TextInput events.
In my use case I'm using the keys for an interactive game where I need to know every time any particular key on the keyboard is pressed or released, and I don't need to know if it is going to create a character sent to TextInput or not -- as far as I know the letters associated with keys being used [W, A, S, D, Ctrl, Shift, Alt, Spacebar] are not important at all.
Is it really necessary to prevent code from knowing when keys are pressed or released? As far as I could see the change I made would still allow the input information into the IME even though it still allows the KeyDown/KeyUp through.
It seems to be that that 'no further processing' is really only relevant to the output of the TextInput event...

@kekekeks
Copy link
Member

I'm using the keys for an interactive game

That's the problem. Some controls (most of them actually) don't care about text input and IME, some controls need key events to be handled and filtered by IME. The problem is that if key events should be handled by IME, we shouldn't allow parent controls to intercept them through the routed events system.

For now your change is breaking the following scenario:

  • application has a hotkey for Alt+C
  • user types Alt+(O,C) and expects to see © without triggering Alt+C hotkey

@kekekeks
Copy link
Member

kekekeks commented Nov 12, 2018

I'm going to merge this right now since it's how OSX backend currently works.

We'll introduce a proper event filtering for text input later.

@kekekeks kekekeks reopened this Nov 12, 2018
@kekekeks kekekeks merged commit bb6cdd5 into AvaloniaUI:master Nov 12, 2018
@tdaffin
Copy link
Contributor Author

tdaffin commented Nov 12, 2018

Ok, I see -- on windows you hold Alt and type '0169' on the numeric keypad to get ©
I note that this does work in Avalonia on windows -- I et all the relevant KeyDown and KeyUp events but no TextInput events until I let go of the Alt key, at which point I get a single TextInput with just the copyright symbol.
I'll see if I can find a way to get Linux to produce the copyright symbol in case I can find a simple way to support that as well as the KeyDown/KeyUp events.

@kekekeks
Copy link
Member

kekekeks commented Nov 13, 2018

That won't work for complex input methods that present their own UI and need more key presses which shouldn't be handled by application.

Please, install a proper input method of all three OS-es (Windows, Linux, OS X) and try entering Chinese or Japanese text before further discussion on input methods.

I understand, that input filtering behavior is not desirable by controls not dealing with text input, but it's crucial for text input fields. That's why we'll be introducing a special input handling mode for the case when TextBox is focused.

The PR is merged, you'll be getting all of your key input for non-textbox controls in the future.

@tdaffin tdaffin deleted the key_events branch November 14, 2018 11:10
@grokys grokys added this to the 0.8.0 milestone Apr 3, 2019
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.

3 participants