From 92119ef98936767a29b740bf54dc8706f5cfc254 Mon Sep 17 00:00:00 2001 From: "David T. Lewis" Date: Mon, 18 Jan 2021 17:37:41 -0500 Subject: [PATCH 1/2] Fix missing KeyRelease events when multiple keys are depressed. Reference Mantis 0007597 http://bugs.squeak.org/view.php?id=7597. Rather than keep a single lastKey to remember the last previously pressed key value, maintain an array size 256 of last key pressed values indexed by X11 KeyCode. Works for any number of simultaneous keys. --- platforms/unix/vm-display-X11/sqUnixX11.c | 73 +++++++++++++---------- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/platforms/unix/vm-display-X11/sqUnixX11.c b/platforms/unix/vm-display-X11/sqUnixX11.c index 43cbbde8db..481b9e9472 100644 --- a/platforms/unix/vm-display-X11/sqUnixX11.c +++ b/platforms/unix/vm-display-X11/sqUnixX11.c @@ -349,7 +349,6 @@ int fullScreenDirect= 0; int iconified= 0; int withSpy= 0; - /*xxx REMOVE REFS TO THESE IN sqUnixSound*.* */ void feedback(int offset, int pixel) {} @@ -373,6 +372,9 @@ int inModalLoop= 0, dpyPitch= 0, dpyPixels= 0; static char *defaultWindowLabel = shortImageName; static long launchDropTimeoutMsecs = 1000; /* 1 second default launch drop timeout */ +/* Value of last key pressed indexed by KeyCode in the range 8 - 255 */ +static int lastKeyValue[256]; + /*** Functions ***/ @@ -1715,6 +1717,25 @@ static int recordPendingKeys(void) } } +storeLastKeyValue(XKeyEvent *xevt, int value) +{ + static int initialized= 0; + if (!initialized) + { + for (int i= 0; i<256; i++) lastKeyValue[i]= -1; + initialized= -1; + } + lastKeyValue[xevt->keycode]= value; + return value; +} + +int retrieveLastKeyValue(XKeyEvent *xevt) +{ + int value= lastKeyValue[xevt->keycode]; + lastKeyValue[xevt->keycode]= -1; + return value; +} + static int xkeysym2ucs4(KeySym keysym); static int x2sqKeyInput(XKeyEvent *xevt, KeySym *symbolic) @@ -1722,7 +1743,6 @@ static int x2sqKeyInput(XKeyEvent *xevt, KeySym *symbolic) static int initialised= 0; static XIM im= 0; static XIC ic= 0; - static int lastKey= -1; if (!initialised) { @@ -1757,11 +1777,7 @@ static int x2sqKeyInput(XKeyEvent *xevt, KeySym *symbolic) } if (KeyPress != xevt->type) - { - int key= lastKey; - lastKey= -1; - return key; - } + return retrieveLastKeyValue(xevt); DCONV_PRINTF("keycode %u\n", xevt->keycode); @@ -1779,9 +1795,8 @@ static int x2sqKeyInput(XKeyEvent *xevt, KeySym *symbolic) DCONV_PRINTERR("x2sqKey XLookupChars count %d\n", count); case XLookupBoth: DCONV_PRINTERR("x2sqKey XLookupBoth count %d\n", count); - lastKey= (count ? string[0] : -1); - DCONV_PRINTERR("x2sqKey == %d\n", lastKey); - return lastKey; + DCONV_PRINTERR("x2sqKey == %d\n", count ? string[0] : -1); + return storeLastKeyValue(xevt, count ? string[0] : -1); case XLookupKeySym: DCONV_PRINTERR("x2sqKey XLookupKeySym\n"); @@ -1792,14 +1807,14 @@ static int x2sqKeyInput(XKeyEvent *xevt, KeySym *symbolic) return -1; /* unknown key */ if ((charCode == 127) && mapDelBs) charCode= 8; - return lastKey= charCode; + return storeLastKeyValue(xevt, charCode); } default: fprintf(stderr, "this cannot happen\n"); - return lastKey= -1; + return storeLastKeyValue(xevt, -1); } - return lastKey= -1; + return storeLastKeyValue(xevt, -1); } revertInput: @@ -1809,8 +1824,6 @@ static int x2sqKeyInput(XKeyEvent *xevt, KeySym *symbolic) static int x2sqKeyCompositionInput(XKeyEvent *xevt, KeySym *symbolic) { - static int lastKey= -1; - # if defined(INIT_INPUT_WHEN_KEY_PRESSED) if (!inputContext) { @@ -1821,11 +1834,7 @@ static int x2sqKeyCompositionInput(XKeyEvent *xevt, KeySym *symbolic) # endif if (KeyPress != xevt->type) - { - int key= lastKey; - lastKey= -1; - return key; - } + return retrieveLastKeyValue(xevt); DCONV_PRINTERR("keycode %u\n", xevt->keycode); @@ -1835,13 +1844,13 @@ static int x2sqKeyCompositionInput(XKeyEvent *xevt, KeySym *symbolic) if (localeEncoding == sqTextEncoding) { if (!(inputBuf= lookupKeys(XmbLookupString, xevt, inputString, sizeof(inputString), &inputCount, symbolic, &status))) - return lastKey= -1; + return storeLastKeyValue(xevt, -1); } # if defined(X_HAVE_UTF8_STRING) else if (uxUTF8Encoding == sqTextEncoding) { if (!(inputBuf= lookupKeys(Xutf8LookupString, xevt, inputString, sizeof(inputString), &inputCount, symbolic, &status))) - return lastKey= -1; + return storeLastKeyValue(xevt, -1); } # endif else @@ -1850,7 +1859,7 @@ static int x2sqKeyCompositionInput(XKeyEvent *xevt, KeySym *symbolic) if (!(aBuf= lookupKeys(XmbLookupString, xevt, aStr, sizeof(aStr), &inputCount, symbolic, &status))) { fprintf(stderr, "status xmb2: %d\n", status); - return lastKey= -1; + return storeLastKeyValue(xevt, -1); } if (inputCount > sizeof(inputString)) { @@ -1860,7 +1869,7 @@ static int x2sqKeyCompositionInput(XKeyEvent *xevt, KeySym *symbolic) fprintf(stderr, "x2sqKeyInput: out of memory\n"); if (aStr != aBuf) free(aBuf); - return lastKey= -1; + return storeLastKeyValue(xevt, -1); } } else @@ -1880,11 +1889,11 @@ static int x2sqKeyCompositionInput(XKeyEvent *xevt, KeySym *symbolic) case XLookupBoth: DCONV_PRINTERR("x2sqKey XLookupBoth count %d\n", inputCount); if (inputCount == 0) - return lastKey= -1; + return storeLastKeyValue(xevt, -1); else if (inputCount == 1) { inputCount= 0; - return lastKey= inputBuf[0]; + return storeLastKeyValue(xevt, inputBuf[0]); } else { @@ -1899,10 +1908,10 @@ static int x2sqKeyCompositionInput(XKeyEvent *xevt, KeySym *symbolic) /* unclear which is best value for lastKey... */ # if 1 - lastKey= (inputCount == 1 ? inputBuf[0] : -1); + storeLastKeyValue(xevt, inputCount == 1 ? inputBuf[0] : -1); # else - lastKey= (inputCount ? inputBuf[0] : -1); - lastKey= (inputCount > 0 ? inputBuf[inputCount - 1] : -1); + storeLastKeyValue(xevt, inputCount ? inputBuf[0] : -1); + storeLastKeyValue(xevt, inputCount > 0 ? inputBuf[inputCount - 1] : -1); # endif return -1; /* we've already recorded the key events */ @@ -1926,14 +1935,14 @@ static int x2sqKeyCompositionInput(XKeyEvent *xevt, KeySym *symbolic) return -1; /* unknown key */ if ((charCode == 127) && mapDelBs) charCode= 8; - return lastKey= charCode; + return storeLastKeyValue(xevt, charCode); } default: fprintf(stderr, "this cannot happen\n"); - return lastKey= -1; + return storeLastKeyValue(xevt, -1); } - return lastKey= -1; + return storeLastKeyValue(xevt, -1); } } From 018c35e194afb7ce21ee2a7340bcd39317f7b5b4 Mon Sep 17 00:00:00 2001 From: "David T. Lewis" Date: Tue, 19 Jan 2021 11:28:34 -0500 Subject: [PATCH 2/2] Initialize lastKeyValue array at compile time. For safety, mask index into lastKeyValue to ensure array bounds are honored. --- platforms/unix/vm-display-X11/sqUnixX11.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/platforms/unix/vm-display-X11/sqUnixX11.c b/platforms/unix/vm-display-X11/sqUnixX11.c index 481b9e9472..ceeab13b35 100644 --- a/platforms/unix/vm-display-X11/sqUnixX11.c +++ b/platforms/unix/vm-display-X11/sqUnixX11.c @@ -373,8 +373,17 @@ static char *defaultWindowLabel = shortImageName; static long launchDropTimeoutMsecs = 1000; /* 1 second default launch drop timeout */ /* Value of last key pressed indexed by KeyCode in the range 8 - 255 */ -static int lastKeyValue[256]; - +static int lastKeyValue[]= + { + -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1 + }; /*** Functions ***/ @@ -1719,20 +1728,14 @@ static int recordPendingKeys(void) storeLastKeyValue(XKeyEvent *xevt, int value) { - static int initialized= 0; - if (!initialized) - { - for (int i= 0; i<256; i++) lastKeyValue[i]= -1; - initialized= -1; - } - lastKeyValue[xevt->keycode]= value; + lastKeyValue[xevt->keycode & 0xff]= value; return value; } int retrieveLastKeyValue(XKeyEvent *xevt) { int value= lastKeyValue[xevt->keycode]; - lastKeyValue[xevt->keycode]= -1; + lastKeyValue[xevt->keycode & 0xff]= -1; return value; }