Skip to content

Commit 6d87064

Browse files
committed
Fix Linux keyboard event handling (issue #2226)
Fixes a bug where key release events reported incorrect key codes when multiple keys were pressed simultaneously. The original implementation used a shared static KeyEvent variable that was overwritten by each key press, causing releases to report the code of the last pressed key rather than the key being released. Removes the onCharInput callback and shared static variable, instead calculating the KeyEvent directly from GLFW's parameters on both press and release. This ensures each key event uses the correct code for that specific key. Note: This is a pragmatic fix for the immediate bug, but the underlying architecture remains flawed. KeyEvent conflates two distinct concepts: physical key events (for shortcuts, game input) and character input (for text entry, Unicode). Issue #2337 highlights this confusion - users expect KeyEvent codes to match shifted characters like '?', but codes should represent physical keys. The proper solution requires adding a separate keyChar() event type, similar to Windows' WM_KEYDOWN/WM_KEYUP vs WM_CHAR separation. This architectural change would affect all platforms and is beyond the scope of this fix. Fixes #2226 Related to #2337
1 parent 6cadaca commit 6d87064

File tree

1 file changed

+12
-30
lines changed

1 file changed

+12
-30
lines changed

src/cinder/app/linux/AppImplLinuxGlfw.cpp

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ class GlfwCallbacks {
3636
public:
3737

3838
static std::map<GLFWwindow*, std::pair<AppImplLinux*,WindowRef>> sWindowMapping;
39-
static KeyEvent sKeyEvent;
4039
static bool sCapsLockDown, sNumLockDown, sScrollLockDown;
4140

4241
static void registerWindowEvents( GLFWwindow *glfwWindow, AppImplLinux* cinderAppImpl, const WindowRef& cinderWindow ) {
@@ -45,7 +44,7 @@ class GlfwCallbacks {
4544
::glfwSetWindowSizeCallback( glfwWindow, GlfwCallbacks::onWindowSize );
4645
::glfwSetWindowPosCallback( glfwWindow, GlfwCallbacks::onWindowMove );
4746
::glfwSetKeyCallback( glfwWindow, GlfwCallbacks::onKeyboard );
48-
::glfwSetCharCallback( glfwWindow, GlfwCallbacks::onCharInput );
47+
// Note: NOT using glfwSetCharCallback - we compute characters directly in onKeyboard
4948
::glfwSetCursorPosCallback( glfwWindow, GlfwCallbacks::onMousePos );
5049
::glfwSetMouseButtonCallback( glfwWindow, GlfwCallbacks::onMouseButton );
5150
::glfwSetScrollCallback( glfwWindow, GlfwCallbacks::onMouseWheel );
@@ -201,7 +200,7 @@ class GlfwCallbacks {
201200
cinderAppImpl->setWindow( cinderWindow );
202201

203202
int32_t nativeKeyCode = KeyEvent::translateNativeKeyCode( key );
204-
203+
205204
if( glfwGetKey( glfwWindow, GLFW_KEY_CAPS_LOCK ) )
206205
sCapsLockDown = ! sCapsLockDown;
207206
if( glfwGetKey( glfwWindow, GLFW_KEY_NUM_LOCK ) )
@@ -210,38 +209,22 @@ class GlfwCallbacks {
210209
sScrollLockDown = !sScrollLockDown;
211210

212211
auto modifiers = extractKeyModifiers( mods );
213-
auto convertedChar = modifyChar( key, modifiers, sCapsLockDown );
214-
215-
if( GLFW_PRESS == action ) {
216-
sKeyEvent = KeyEvent( cinderWindow, nativeKeyCode, 0, 0, modifiers, scancode );
217-
// if convertedChar != 0, we'll get the unicode char value and wait until onCharInput is called.
218-
// if convertedChar == 0, that means it is a non-unicode key (ex ctrl), so we'll emit the keydown here.
219-
if( convertedChar == 0 ) {
220-
cinderWindow->emitKeyDown( &sKeyEvent );
221-
}
212+
char convertedChar = modifyChar( key, modifiers, sCapsLockDown );
213+
uint32_t charUtf32 = convertedChar ? (uint32_t)(unsigned char)convertedChar : 0;
214+
215+
// Calculate KeyEvent from GLFW's parameters on both press and release
216+
KeyEvent keyEvent( cinderWindow, nativeKeyCode, charUtf32, convertedChar, modifiers, scancode );
217+
218+
if( GLFW_PRESS == action || GLFW_REPEAT == action ) {
219+
cinderWindow->emitKeyDown( &keyEvent );
222220
}
223221
else if( GLFW_RELEASE == action ) {
224-
KeyEvent event( cinderWindow, sKeyEvent.getCode(), sKeyEvent.getCharUtf32(), sKeyEvent.getChar(), modifiers, scancode );
225-
sKeyEvent = {};
226-
cinderWindow->emitKeyUp( &event );
222+
cinderWindow->emitKeyUp( &keyEvent );
227223
}
228224
}
229225
}
230226

231-
static void onCharInput( GLFWwindow *glfwWindow, unsigned int codepoint )
232-
{
233-
auto iter = sWindowMapping.find( glfwWindow );
234-
if( sWindowMapping.end() != iter ) {
235-
auto& cinderAppImpl = iter->second.first;
236-
auto& cinderWindow = iter->second.second;
237-
cinderAppImpl->setWindow( cinderWindow );
238-
239-
// create new sKeyEvent that includes same info as previous but also the utf32 char. emit key down signal with that
240-
char asciiChar = codepoint < 256 ? (char)codepoint : 0;
241-
sKeyEvent = KeyEvent( cinderWindow, sKeyEvent.getCode(), codepoint, asciiChar, sKeyEvent.getModifiers(), sKeyEvent.getNativeKeyCode() );
242-
cinderWindow->emitKeyDown( &sKeyEvent );
243-
}
244-
}
227+
// onCharInput removed - we now compute characters directly in onKeyboard
245228

246229
static void onMousePos( GLFWwindow* glfwWindow, double mouseX, double mouseY ) {
247230
auto iter = sWindowMapping.find( glfwWindow );
@@ -327,7 +310,6 @@ class GlfwCallbacks {
327310
};
328311

329312
std::map<GLFWwindow*, std::pair<AppImplLinux*,WindowRef>> GlfwCallbacks::sWindowMapping;
330-
KeyEvent GlfwCallbacks::sKeyEvent;
331313
bool GlfwCallbacks::sCapsLockDown = false;
332314
bool GlfwCallbacks::sNumLockDown = false;
333315
bool GlfwCallbacks::sScrollLockDown = false;

0 commit comments

Comments
 (0)