opensmalltalk-vm
opensmalltalk-vm copied to clipboard
Linux Squeak VM X11 keyboard changes breaks 32b VM
Tim Johnson reported a problem with 32b Linux builds: http://forum.world.st/Linux-Squeak-VM-X11-keyboard-changes-td5099584.html
Pressing ctrl-d now acts like debugIt instead of doIt like in the 64b. Ctrl-p also stopping working.
I did a bisection of all versions in https://bintray.com/opensmalltalk/vm/cog/ and narrowed down the problem to
good squeak.cog.spur_linux32x86_201811272342.tar.gz bad squeak.cog.spur_linux32x86_201812301551.tar.gz
The error is consistent on both 18231 and the oldest 18221 released image for 32b VM but does not appear with 64b VMs.
Thanks for all the work you've done on this.
I (Tim J who reported the problem) have built a debug build and uploaded it to a personal web server. If you trust a strange binary built on a DigitalOcean server, it is here (tarball is named after the commit hash).
Thanks Tim,
The following lines in sqUnixX11.c doesn't seem right. When Ctrl and p are pressed at the same time, sq2KeyPlain should return ctrl-p (16r10). This code returns p instead (16r70). I don't understand why this change was made.
2065 if (charCode >= 1 && charCode <= 26) {
2066 /* check for Ctrl-letter that gets translated into charCode 1-26 instead of letters a-z */
2067 KeySym keysym = *symbolic;
2068 if (keysym >= XK_a && keysym <= XK_z)
2069 return (int)'a' + (keysym - XK_a);
2070 if (keysym >= XK_A && keysym <= XK_Z)
2071 return (int)'A' + (keysym - XK_A);
Could you try a build with this section removed and see if it fixes your problem?
Relevant commit: 1b837f94e96b93cb4d117ccdd010825d76dcde57
Tim,
I used gdb to nop out above code in the text segment as this was quicker and easier than trying to rebuild a 32b binary. Then the image worked like charm.
The comment in recode() is confusing and I need time to understand its rationale. Shift is keyboard specific and decides charCode and should be handled in the VM while Ctrl is a modifier and should be passed to the image. So I don't understand why a key event should be generated when Shift alone is pressed or released.
I also found a thread from 2012 where Guillermo Polito fixed a similar problem in Windows code http://forum.world.st/Windows-cog-vm-Keyboard-events-related-about-keycode-mapping-td4330359.html
HTH .. Subbu
Hi Subbu, I have rebuilt the 32-bit Linux x86/i386 Squeak-Cog-Spur VM with the above section #ifdef'ed out, and it works. I tested Ctrl-P, Ctrl-D, and Ctrl-Shift-D. Each of them do what is expected/desired. Thank you! Tim
So, if I understand the issue, the problem is that we always get keysym == XK_A
rather than keysym == XK_a
when CTRL is pressed, even if we did not press SHIFT.
The keysym
is produced by XLookupString
.
https://linux.die.net/man/3/xlookupstring sounds clear to me: it should honour the shift for the Keysym
output, and map to ASCII for the ASCII buffer output...
The XLookupString function translates a key event to a KeySym and a string. The KeySym is obtained by using the standard interpretation of the Shift, Lock, group, and numlock modifiers as defined in the X Protocol specification. If the KeySym has been rebound (see XRebindKeysym), the bound string will be stored in the buffer. Otherwise, the KeySym is mapped, if possible, to an ISO Latin-1 character or (if the Control modifier is on) to an ASCII control character, and that character is stored in the buffer. XLookupString returns the number of characters that are stored in the buffer.
Does it work like advertised? We'd better activate DEBUG_KEYBOARD_EVENTS
, but currently we print address of the keysym rather than the keysym!
Actually, the problem is that charCode in sq2KeyPlain has already translated ctrl and XK_a as ctrl-a (16r1), but the errant code returns 'a' instead of ctrl-a.
Hi Subbu, you made very good job at tracking the changes causing the problem, but I'm not sure that reverting is the right solution, this change was made for some reason. If we want to disentangle the code and remove some cruft we need better understanding. So let's focus on symptoms: CTRL+d is performing debugIt:
rather than doIt:
action.
So clearly, the CTRL/CMD or whatever modifier is taken into account, and transformation of keyValue to Ctrl-d code (0x4) is not necessary. It is not only not necessary, it also create various problems of interpretation of keyboard event, because 0x4 is also the encoding of END key, see implementors/senders of specialShiftCmdKeys
. I played with various combinations of CTRL,ALT,OPTION,CMD,SHIFT + d on MacOS and Windows VM, and HandMorph showEvents: true.
, and I can tell that it's messy all the way down!
The problem we observe in Squeak images is that the TextEditor
performs the shiftCmdActions
instead of the cmdActions
. See initializeCmdKeyShortcuts
and initializeShiftCmdKeyShortcuts
in SmalltalkEditor
. Why?
A good candidate for suspiscion is in TextEditor>>dispatchOnKeyboardEvent:
"the control key can be used to invoke shift-cmd shortcuts"
(honorCommandKeys and: [ aKeyboardEvent controlKeyPressed ])
ifTrue: [^ self
perform: (self class shiftCmdActions at: aKeyboardEvent keyValue + 1)
with: aKeyboardEvent].
Ahah: I don't know if this is a MacOS-centric piece of code or what, but if we generate CTRL modifier instead of CMD modifier, maybe the image code ain't gonna perform the cmdActions, but rather the shiftCmdActions as we observe. Since you seem to identify a change in modifier state between old and new VM, this is maybe where we start inquiring instead.
The keyboard event is composed of several parts:
- keyValue code (
eventBuffer third
at image side) that tries to be an ASCII code + special codes for special keys (cursor, etc...) - as produced bysq2KeyPlain
and friends. - ucs4 (utf32) code for the character (
eventBuffer sixth
) - keyboard modifiers (
eventBuffer fifth
) with a very complex dance in the VM.
But only one code is retained in the KeyboardEvent
distributed for Morph
consumption: either the keyValue, or the ucs4 code might be used, depending on Localized World currentHand keyboardInterpreter class
. Most probably the ucs4 one nowadays if non null. By the way, what is your keyboardInterpreter class?
Note that Ctrl-d has some mapping in shiftCmdActions, "thanks" to this super hack in initializeShiftCmdKeyShortcuts
1 to: cmds size by: 2 do: [ :i |
cmdMap at: ((cmds at: i) asciiValue + 1) put: (cmds at: i + 1). "plain keys"
cmdMap at: ((cmds at: i) asciiValue - 32 + 1) put: (cmds at: i + 1). "shifted keys"
cmdMap at: ((cmds at: i) asciiValue - 96 + 1) put: (cmds at: i + 1). "ctrl keys"
].
which implies that shift-command #debugIt:
associated to $d (100 0x64) will also be associated to $D (68 0x44) and to Ctrl-d (4 0x04).
But I do not see any thing like that in cmdActions, SmalltalkEditor cmdActions occurrencesOf: #doIt:
answers 1, it is associated to $d only.
You see that this would/could conflict with END key special actions (same for other special keys encoding), which is a bad thing. The Ctlr-d (0x4) value & co are a bad thing coming from the past (like console mode compatibility) that we do not need, neither at low level keyboard handling (KeyUp KeyDown), nor at higher level synthetic event handling (KeyStroke/KeyChar). I think we'd better keep the change you legitimately incriminated, and I would rather try to understand all the implications of translateCode
and when/how it handles the modifier states instead...
Hi Nicolas, Thank you for the insightful research. This does seem like an area that benefits from greater documentation, attention, and cleanup, because of many generations of backwards-compatibility, multilingual support, and cross-platform code.
I need to ask, though: why then is this only a problem on 32-bit Linux VMs but not 64-bit Linux VMs?
Thanks, Tim
I don't understand why this change was made (code link for more context)
2065 if (charCode >= 1 && charCode <= 26) {
2066 /* check for Ctrl-letter that gets translated into charCode 1-26 instead of letters a-z */
2067 KeySym keysym = *symbolic;
2068 if (keysym >= XK_a && keysym <= XK_z)
2069 return (int)'a' + (keysym - XK_a);
2070 if (keysym >= XK_A && keysym <= XK_Z)
2071 return (int)'A' + (keysym - XK_A);
Paraphrasing feenkcom/gtoolkit#9 found by @akgrant43 ...
doru: A piece of information about this bug. There is an inconsistency in Mac and Linux VMs when it comes to keyboard events: -- OSX generates keyDown: events for keys like shift , ctrl or meta when I press the corresponding keys, while -- Linux VM generates keyDown: events for ctrl when we release the key Our issue is at a lower level then Morphic. @syrel used logging statements in HandMorph>>#generateKeyboardEvent:. It appears that it is not called when we press a modifier key on Linux.
syrel: Indeed, the keyboard events for shift are sent but the question is when they are sent. We state that they are not sent when we press the shift button, but instead when we release it. This is a very important and crucial difference that does not let us to have simple and clear shortcut handling in Bloc on linux. In Bloc by design we assume that whenever a key is pressed - we get a keydown event (hence it has down in it), right after that for printable characters we get keystroke (or input) event that gets resent every X seconds depending on OS settings and finally when user releases her finger we get keyup event. Unfortunately this does not happen for keys like shift, alt and ctrl while perfectly works for regular keys like A, M, Z. @akgrant43 I have no idea why it works for you but doesn’t work for multiple other people. We are debugging this for the latest Pharo7 64bit with latest VM on Linux Mint 64bit.
alistair: I'm not seeing this behaviour (Ubuntu 16.04). Adding logging to: Morph>>handleKeyDown: Morph>>handleKeyUp: Shows them being called at the expected times, i.e. as the ctrl key is being pressed and released. Linux Mint, if I remember correctly, is typically based on a fairly old version of Ubuntu (not that mine is recent :-)). Maybe there's a change in the kernel or graphics stack?
john: I have gotten the keyboard shortcuts to work under my Linux Mint installation. There were a few issues. First, the KeyboardKey class was using the wrong key table for Unix. Second, the right Unix key table was missing the right control & alt keys. These were addressed in https://pharo.manuscript.com/f/cases/22769 and have been integrated into the latest Pharo build. . The other issues were VM issues. When you pressed and then released either the control or shift keys, the correct key would be recorded as pressed, but character code $? would be reported when the key was released. This causes the editor to believe the control key was still pressed even after it was released since it never got the key up for the control. The other VM issue was that A-Z keys being pressed when the control was pressed was being reported as character code 1-26 instead of the letters a-z. On the other platforms, they are reported as letters with the control key pressed. On Linux, they were reported with the control key down, but with character codes 1-26. To fix the VM issues, I added some code to the x2sqKeyPlain method in sqUnixX11.c file before the return (j-brant/opensmalltalk-vm@37e3abf):
Paraphasing http://forum.world.st/Linux-Squeak-VM-X11-keyboard-changes-td5099584.html
kksubbu: EventSensor classPool associationAt: #KeyDecodeTable is different between the good and bad VM. The good one has 184 entries while the bad one has only 16 entries :-( See attached pics taken from the bad and good VM. . The bad 32b VM is returning a different modifier bit (64 instead of 16) in the keyboard event compared to the good VMs. . With HandMorph showEvents: true Pressing Ctrl shows up as "keyDown Ctrl-Opt-û" (251). Now if I press p it shows "keyStroke Ctrl-p (112)". If I release p key, it continues to show "keyUp Ctrl-p (112)". If I release Ctrl now, it shows "keyUp û (251)". In the good image, pressing p after Ctrl shows up as "Cmd-p (112)"
Paraphasing http://forum.world.st/Windows-cog-vm-Keyboard-events-related-about-keycode-mapping-td4330359.html
guillep: I was playing to add Function Key support in the windows vm (yep, always the same :P), and looking at the code, I saw this in the recordKeyboardEvent:
evt = (sqKeyboardEvent*) sqNextEventPut();
...
evt->charCode = keymap[keyCode & 0xff];
the problem with that line is that KeyDown and KeyUp events send VirtualKeycodes as keycodes and the Char event sends a unicode char value. And, it makes collisions, since for example: -- $p char value is 112 ; and -- F1 virtual code value is 112 too
And other keys are the same, image-side having the same keycode with different keys. To distinguish these keys, the following provides to the image a Char event the keycode without mapping (code link for more context):
case WM_CHAR:
case WM_SYSCHAR:
/* Note: VK_RETURN is recorded as virtual key ONLY */
if(keyCode == 13) return 1;
+ charCode = keyCode;
pressCode = EventKeyChar;
break
...
- evt->charCode = keymap[keyCode & 0xff];
+ evt->charCode = charCode? charCode : keymap[keyCode & 0xff];
andreas: The change makes no sense (it will break most non-ascii input like accents, umlauts, etc). You really shouldn't be using character events for handling function keys. There is no 'F11 Character' in any character encoding world-wide so trying to represent F11 as a character is completely futile. You need to use keyDown and keyUp events, since F11 & friends are KEYs not CHARACTERs.
guille: Sure! I'm not generating Char events for F11 ;). Since F11 does not generate Char events. The problem is that the p key generates the same KeyDown event than F11 for the vm. I'm only fixing that p-key and F1-key have different keyCodes for KeyDown and KeyUp events.
Now, utf32Code should be only used on KeyChar events, and I don't care about them. . How does it work now? (KeyDown, KeyUp) and KeyChar events send different keyCodes to the image. Right now works because in no place in the image keyDown: is handled. . More on, in the unix vm, the behavior is the following: when pressing F1: -- key: 112 char: 112 type: keyDown -- key: 112 char: 112 type: keyUp when pressing p: -- key: 112 char: 112 type: keyDown -- key: 112 char: 112 type: keystroke -- key: 112 char: 112 type: keyUp when pressing shift + p (P): -- key: 254 char: 254 type: keyDown -- key: 80 char: 80 type: keyDown -- key: 80 char: 80 type: keystroke -- key: 80 char: 80 type: keyUp After adding support for Function keys, when pressing F1: key: 16 char: 16 type: keyDown key: 16 char: 16 type: keyUp . The question is: Should KeyDown, KeyUp and KeyChar events for the same key produce the same keyCode? I think yes. Because the keyCode indicates the key pressed in the keyboard. Doesn't it?
andreas: It does. Which is precisely the reason why it can't produce different values when you press the shift key with it. The KEY you are pressing does not change depending on the modifier; your OS decides that the combination of the Shift key and the P key produces an uppercase P character.
guille: The main problem is that I want keycodes to be the same through the three platforms (unix, mac and windows). And for that, I have to do a mapping somewhere (this piece of code I wrote today was to play and see how it behaves, because the codes are to be defined :) ). So, I thought those conversions could be done through the keymap array. Maybe that's not the way to do the mapping, but the mapping should be done somewhere.
andreas: There was general agreement that probably the best way to do this would be based on the X11/keysym.h because it seems to be the most complete source for key codes. It's just that that nobody sat down and wrote the mapping from VK_XXX to XK_YYY.
guillep: Hmm, that would be good, I'll have a look tomorrow :). Thanks!
What was the conclusion of looking at that @guillep?
Side comment 1 from http://forum.world.st/Windows-cog-vm-Keyboard-events-related-about-keycode-mapping-td4330359.html
@jvuletich: On the mac, when control or command are pressed, for any keystroke the unshifted code is returned, even if shift is pressed. For instance, let's assume an US keyboard. So, [shift]+[,] generates $< and [shift]+[.] generates $>. If you run 'Sensor kbdTest' and press [shift]+[,] you correctly get $<, but if you press [control]+[shift]+[,] or [command]+[shift]+[,] you get $,. This is bad, because to detect [command]+[<] or [control]+[<] you need to write code that not only needs to know about the platform, but also about the keyboard layout, as in many layouts $< is not generated by doing [shift]+[,], but by some other combination. The same happens with most non-alphabetic keys, that usually differ in different keyboard layouts.
Side comment 2 from http://forum.world.st/Windows-cog-vm-Keyboard-events-related-about-keycode-mapping-td4330359.html
@jvuletich: There is a completely separated issue, and it happens both in Windows and Mac. Here, [ctrl] + [an alphabetic key] substracts 64 from the code. So, [ctrl]+[c] generates code 3. This is consistent with the traditional meaning of the ctrl key (in dumb terminals and DOS), but it makes impossible for the image to tell (for example) between [ctrl]+[Enter] and [ctrl]+[m]. The image might want to use these keystrokes for different things, so it would be much better not to substract 64 in the VM and let the image handle it. I know it could be done by handling key down and key up events, but this would also require code that is not only platform dependent but also needs to know the mapping between key codes and characters in each platform. . In general, I think that [control], [command] and [alt/option] should not affect the character code of a keystroke, they should only set the appropriate flag so the image can decide what to do with them. This is completely different for [shift], as [shift] does indeed modify the character generated.
Thank you, Nicolas, for your detailed analysis of the code path for handling keystrokes in the editor. Initially, I too traced this path and noticed that EventSensor>>processEvent: maps keystrokes using KeyDecodeTable before calling processKeyboardEvent. Key alt-d correctly invoked cmdActions in both 32b and 64b images but ctrl-d invoked shiftCmdActions in 32b image instead of cmdActions. The KeyDecodeTable is different in these two images.
I agree that the sqUnixX11.c file is very messy and confusing. It mixes X11 terms (raw keycode and mapped keysym) with Squeak specific terms (keycode, charcode). JB's patch broke 32b image but not 64b. I proposed a partial rollback of this patch to get TimJ going while we sort out the mess.
The change was made for this reason:
The other VM issue was that A-Z keys being pressed when the control was pressed was being reported as character code 1-26 instead of the letters a-z. On the other platforms, they are reported as letters with the control key pressed. On Linux, they were reported with the control key down, but with character codes 1-26.
I only tested the change on a 64-bit Pharo image. Playing around a little with a 64-bit Squeak 5.2 image, it appears that Squeak is likely translating the character codes 1-26 specially. I added some debugging code to EventSensor>>fetchMoreEvents
(in the whileFalse: loop):
type = EventTypeKeyboard
ifTrue: [Transcript show: (eventBuffer at: 3) printString , ' - ', (eventBuffer at: 5) printString; cr; flush].
When running with the old vm and pressing/releasing Ctrl-a, I get this printed to the transcript:
251 - 6
1 - 2
1 - 2
1 - 2
63 - 0
When running with the new vm I get:
251 - 6
97 - 2
97 - 2
97 - 2
251 - 0
The new vm generates the correct codes, but Ctrl-a isn't selecting the whole text. Watching the events using HandMorph showEvents: true
, you can see that the image is converting the old vm's Ctrl-a into Cmd-a, but it isn't converting the new vm's Ctrl-a into Cmd-a.
I agree that the sqUnixX11.c file is very messy and confusing. It mixes X11 terms (raw keycode and mapped keysym) with Squeak specific terms (keycode, charcode). JB's patch broke 32b image but not 64b. I proposed a partial rollback of this patch to get TimJ going while we sort out the mess.
Are you sure that it is working in a 64-bit image? I wasn't able to use Ctrl-a in my 64-bit vm using a Squeak 5.2 image with the new vm.
Are you sure that it is working in a 64-bit image? I wasn't able to use Ctrl-a in my 64-bit vm using a Squeak 5.2 image with the new vm.
I haven't tested it. I am just taking others' word that they tested a 64-bit VM and could not reproduce the problem I was reporting on my 32-bit VM.
I wonder if some of what we're uncovering here is also related to why, on a Mac OS VM, the Alt/Option key will often be "stuck" after I Cmd-Tab to switch to a different task and then return. I often find that I have to hit Alt/Option once to "unstick" it after Cmd-Tab switching back to Squeak.
Thanks Subbu! KeyDecodeTable
was the piece that I missed. I searched where the hell in the VM this CTRL->CMD modifier dance was performed, but failed to find it. It was in the image of course! It certainly IS the key of the problem.
On 2019-06-12, at 1:10 PM, Nicolas Cellier [email protected] wrote:
Thanks Subbu! KeyDecodeTable was the piece that I missed. I searched where the hell in the VM this CTRL->CMD modifier dance was performed, but failed to find it. It was in the image of course! It certainly IS the key of the problem.
This is part of a problem I've seen too many times over the years; something needs a 'tweak' for platform A - change is made in the platform code. That leads to problems for platforms B & C, and changes are made in the image. We end up with a curious and impenetrable mix of image and platform code doing weird things... and years later no one can remember anything about it.
tim
tim Rowledge; [email protected]; http://www.rowledge.org/tim Strange OpCodes: LD: Lose Device
So why does CTRL+d work?
If you have Preferences duplicateAllControlAndAltKeys = true
, then
KeyDecodeTable at: { key bitAnd: 16r9F . 2 } put: { key . 8 }.
will replace Ctrl-d code (0x4) + CTRL modifier (2) by $d + CMD modifier (8).
Indeed, 0x4 = $d asInteger bitAnd: 16r9F
- since bitAnd: 16r9F is like subtracting 64 from uppercase letters, and subtracting 96 from lowercase letters. Note that such 16r9F may have all sort of unpalatable effects on internationalized characters + CTRL... It's not internationalization friendly.
If you have a VM outputting $d + CTRL, then this translation does NOT happen, and we encounter the strange code in TextEditor>>dispatchOnKeyboardEvent:
that interprets CTRL modifier as SHIFT+CMD... That's complete madness at image side, so IMO we'd better not fix the VM (well we should continue cleaning, but not for this bug)!
Why 32 and 64 bits image differ?
They don't. Only the preferences may differ or not... So some squeakers will reproduce the problem some not, it's just random.
If other preferences is selected, like Preferences duplicateControlAndAltKeys = true
, the problem will only ever happen with self defaultCrossPlatformKeys
, that is CTRL+a c x v f g s z.
It may also depends on the World currentHand keyboardInterpreter class
, because if VM delivers event at: 3 as Ctrl-d and event at: 6 as $d asInteger, then depending on which KeyDecodeTable translations happens, and which of the two codes is taken into account in the morphic KeybordEvent, that's quite many different possible behaviors, not all of which are inline with our expectations.
We see that fixing the VM to make Brick/Bloc/GT/Whatever work, may break Morphic/MVC because they already have crooked workarounds that now fail. We arrived at a point where we cannot change a single feature without breaking another. Unpalatable!
It does not mean we do not have to change things. We want to be able to evolve. We want a VM doing what it says and saying what it does (specs and docs please!), if possible not too complex things: that is transform OS specific events into platform agnostic events (preferably union of possible events rather than intersection IMO, or we'll never be on tablets and phones). While increasing VM consistency, we can start to disentangle image code and remove some hacks.
But for this to happen, we have to put all expectations on the table and make some choice. CTRL = SHIFT+CMD is highly questionable for example, I'd like to know where it comes from to begin with? This is not the place to debate that, it's Squeak specific.
I would rather close the issue and fix Squeak. Thoughts?
On 12.06.2019, at 23:20, Nicolas Cellier [email protected] wrote:
(preferably union of possible events rather than intersection IMO, or we'll never be on tablets and phones)
The SDL Events are pretty good, I recon…
I like the SDL events, and I already have them working on the Minheadless VM since 1-2 years ago: https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/minheadless/sdl2-window/sqWindow-SDL2.c
We can use this for fixing the Keyboard and MOUSE inconsistency in Morphic for both Squeak, and Pharo. Mouse button numbers are also not assigned the same way in all the platforms.
The VM events are pushed into queue (implemented with a circular buffer) that are fetched by the InputEventSensor >> #nextEvent method in Pharo.
The SDL2 based display support in the Minheadless VM also allows the VM to handle SDL2 events that are not directed to the main VM window by exposing the following three primitives:
- primitiveIsVMDisplayUsingSDL2 -primitivePollVMSDL2Event -> Copy the content of a SDL_Event that is not directed to the VM window into a byteArray. These events are stored in separate circular buffer when they are received by the VM.
- primitiveSetVMSDL2Input -> Registers an ExternalSemaphore which is signaled by the VM when an event is stored in this circular array.
The support for using these primitives is already implemented and integrated in the standard Pharo 7 image. Using this VM fixes OSWindow event handling in Mac and Windows (in Linux it have been always working perfectly).
This issue occurs for me in our 5.3beta builds. I can only use alt+c/v/x/p... to trigger the usual actions, not ctrl+c/v/x/p... as it used to be.
I tried reverting the mentioned commit on the Cog branch, which I can confirm re-enables using the key combos as expected for me (1b837f94e96b93cb4d117ccdd010825d76dcde57)
I have fixed (patched?) ctrl+a to ctrl+z on linux VM. Please check and report if we can close