Merge pull request #1063 from bgK/keyboard-repeat

SDL2: Improve handling of keyboard repeat events

12 files changed, 208 insertions, 162 deletions

diff --git a/backends/events/default/default-events.cpp b/backends/events/default/default-events.cpp
index 667914b023..8dd9599d20 100644
--- a/backends/events/default/default-events.cpp
+++ b/backends/events/default/default-events.cpp
@@ -40,7 +40,8 @@ DefaultEventManager::DefaultEventManager(Common::EventSource *boss) :
 	_modifierState(0),
 	_shouldQuit(false),
 	_shouldRTL(false),
-	_confirmExitDialogActive(false) {
+	_confirmExitDialogActive(false),
+	_shouldGenerateKeyRepeatEvents(true) {
 
 	assert(boss);
 
@@ -50,10 +51,6 @@ DefaultEventManager::DefaultEventManager(Common::EventSource *boss) :
 	_dispatcher.registerObserver(this, kEventManPriority, false);
 
 	// Reset key repeat
-	_currentKeyDown.keycode = 0;
-	_currentKeyDown.ascii = 0;
-	_currentKeyDown.flags = 0;
-
 	_keyRepeatTime = 0;
 
 #ifdef ENABLE_VKEYBD
@@ -86,163 +83,181 @@ void DefaultEventManager::init() {
 }
 
 bool DefaultEventManager::pollEvent(Common::Event &event) {
-	// Skip recording of these events
-	uint32 time = g_system->getMillis(true);
-	bool result = false;
-
 	_dispatcher.dispatch();
-	if (!_eventQueue.empty()) {
-		event = _eventQueue.pop();
-		result = true;
+
+	if (_shouldGenerateKeyRepeatEvents) {
+		handleKeyRepeat();
 	}
 
-	if (result) {
-		event.synthetic = false;
-		switch (event.type) {
-		case Common::EVENT_KEYDOWN:
-			_modifierState = event.kbd.flags;
-			// init continuous event stream
-			_currentKeyDown.ascii = event.kbd.ascii;
-			_currentKeyDown.keycode = event.kbd.keycode;
-			_currentKeyDown.flags = event.kbd.flags;
-			_keyRepeatTime = time + kKeyRepeatInitialDelay;
+	if (_eventQueue.empty()) {
+		return false;
+	}
 
-			if (event.kbd.keycode == Common::KEYCODE_BACKSPACE) {
-				// WORKAROUND: Some engines incorrectly attempt to use the
-				// ascii value instead of the keycode to detect the backspace
-				// key (a non-portable behavior). This fails at least on
-				// Mac OS X, possibly also on other systems.
-				// As a workaround, we force the ascii value for backspace
-				// key pressed. A better fix would be for engines to stop
-				// making invalid assumptions about ascii values.
-				event.kbd.ascii = Common::KEYCODE_BACKSPACE;
-				_currentKeyDown.ascii = Common::KEYCODE_BACKSPACE;
+	event = _eventQueue.pop();
+	bool forwardEvent = true;
+
+	switch (event.type) {
+	case Common::EVENT_KEYDOWN:
+		_modifierState = event.kbd.flags;
+
+		if (event.kbd.keycode == Common::KEYCODE_BACKSPACE) {
+			// WORKAROUND: Some engines incorrectly attempt to use the
+			// ascii value instead of the keycode to detect the backspace
+			// key (a non-portable behavior). This fails at least on
+			// Mac OS X, possibly also on other systems.
+			// As a workaround, we force the ascii value for backspace
+			// key pressed. A better fix would be for engines to stop
+			// making invalid assumptions about ascii values.
+			event.kbd.ascii = Common::KEYCODE_BACKSPACE;
+			_currentKeyDown.ascii = Common::KEYCODE_BACKSPACE;
+		}
+		break;
+
+	case Common::EVENT_KEYUP:
+		_modifierState = event.kbd.flags;
+		break;
+
+	case Common::EVENT_MOUSEMOVE:
+		_mousePos = event.mouse;
+		break;
+
+	case Common::EVENT_LBUTTONDOWN:
+		_mousePos = event.mouse;
+		_buttonState |= LBUTTON;
+		break;
+
+	case Common::EVENT_LBUTTONUP:
+		_mousePos = event.mouse;
+		_buttonState &= ~LBUTTON;
+		break;
+
+	case Common::EVENT_RBUTTONDOWN:
+		_mousePos = event.mouse;
+		_buttonState |= RBUTTON;
+		break;
+
+	case Common::EVENT_RBUTTONUP:
+		_mousePos = event.mouse;
+		_buttonState &= ~RBUTTON;
+		break;
+
+	case Common::EVENT_MAINMENU:
+		if (g_engine && !g_engine->isPaused())
+			g_engine->openMainMenuDialog();
+
+		if (_shouldQuit)
+			event.type = Common::EVENT_QUIT;
+		else if (_shouldRTL)
+			event.type = Common::EVENT_RTL;
+		break;
+#ifdef ENABLE_VKEYBD
+	case Common::EVENT_VIRTUAL_KEYBOARD:
+		if (_vk->isDisplaying()) {
+			_vk->close(true);
+		} else {
+			if (g_engine)
+				g_engine->pauseEngine(true);
+			_vk->show();
+			if (g_engine)
+				g_engine->pauseEngine(false);
+			forwardEvent = false;
+		}
+		break;
+#endif
+#ifdef ENABLE_KEYMAPPER
+	case Common::EVENT_KEYMAPPER_REMAP:
+		if (!_remap) {
+			_remap = true;
+			Common::RemapDialog _remapDialog;
+			if (g_engine)
+				g_engine->pauseEngine(true);
+			_remapDialog.runModal();
+			if (g_engine)
+				g_engine->pauseEngine(false);
+			_remap = false;
+		}
+		break;
+#endif
+	case Common::EVENT_RTL:
+		if (ConfMan.getBool("confirm_exit")) {
+			if (g_engine)
+				g_engine->pauseEngine(true);
+			GUI::MessageDialog alert(_("Do you really want to return to the Launcher?"), _("Launcher"), _("Cancel"));
+			forwardEvent = _shouldRTL = (alert.runModal() == GUI::kMessageOK);
+			if (g_engine)
+				g_engine->pauseEngine(false);
+		} else
+			_shouldRTL = true;
+		break;
+
+	case Common::EVENT_MUTE:
+		if (g_engine)
+			g_engine->flipMute();
+		break;
+
+	case Common::EVENT_QUIT:
+		if (ConfMan.getBool("confirm_exit")) {
+			if (_confirmExitDialogActive) {
+				forwardEvent = false;
+				break;
 			}
-			break;
+			_confirmExitDialogActive = true;
+			if (g_engine)
+				g_engine->pauseEngine(true);
+			GUI::MessageDialog alert(_("Do you really want to quit?"), _("Quit"), _("Cancel"));
+			forwardEvent = _shouldQuit = (alert.runModal() == GUI::kMessageOK);
+			if (g_engine)
+				g_engine->pauseEngine(false);
+			_confirmExitDialogActive = false;
+		} else
+			_shouldQuit = true;
 
-		case Common::EVENT_KEYUP:
-			_modifierState = event.kbd.flags;
-			if (event.kbd.keycode == _currentKeyDown.keycode) {
-				// Only stop firing events if it's the current key
-				_currentKeyDown.keycode = 0;
-			}
-			break;
+		break;
 
-		case Common::EVENT_MOUSEMOVE:
-			_mousePos = event.mouse;
-			break;
+	default:
+		break;
+	}
 
-		case Common::EVENT_LBUTTONDOWN:
-			_mousePos = event.mouse;
-			_buttonState |= LBUTTON;
-			break;
+	return forwardEvent;
+}
 
-		case Common::EVENT_LBUTTONUP:
-			_mousePos = event.mouse;
-			_buttonState &= ~LBUTTON;
-			break;
+void DefaultEventManager::handleKeyRepeat() {
+	uint32 time = g_system->getMillis(true);
 
-		case Common::EVENT_RBUTTONDOWN:
-			_mousePos = event.mouse;
-			_buttonState |= RBUTTON;
-			break;
+	if (!_eventQueue.empty()) {
+		// Peek in the event queue
+		const Common::Event &nextEvent = _eventQueue.front();
 
-		case Common::EVENT_RBUTTONUP:
-			_mousePos = event.mouse;
-			_buttonState &= ~RBUTTON;
+		switch (nextEvent.type) {
+		case Common::EVENT_KEYDOWN:
+			// init continuous event stream
+			_currentKeyDown = nextEvent.kbd;
+			_keyRepeatTime = time + kKeyRepeatInitialDelay;
 			break;
 
-		case Common::EVENT_MAINMENU:
-			if (g_engine && !g_engine->isPaused())
-				g_engine->openMainMenuDialog();
-
-			if (_shouldQuit)
-				event.type = Common::EVENT_QUIT;
-			else if (_shouldRTL)
-				event.type = Common::EVENT_RTL;
-			break;
-#ifdef ENABLE_VKEYBD
-		case Common::EVENT_VIRTUAL_KEYBOARD:
-			if (_vk->isDisplaying()) {
-				_vk->close(true);
-			} else {
-				if (g_engine)
-					g_engine->pauseEngine(true);
-				_vk->show();
-				if (g_engine)
-					g_engine->pauseEngine(false);
-				result = false;
-			}
-			break;
-#endif
-#ifdef ENABLE_KEYMAPPER
-		case Common::EVENT_KEYMAPPER_REMAP:
-			if (!_remap) {
-				_remap = true;
-				Common::RemapDialog _remapDialog;
-				if (g_engine)
-					g_engine->pauseEngine(true);
-				_remapDialog.runModal();
-				if (g_engine)
-					g_engine->pauseEngine(false);
-				_remap = false;
+		case Common::EVENT_KEYUP:
+			if (nextEvent.kbd.keycode == _currentKeyDown.keycode) {
+				// Only stop firing events if it's the current key
+				_currentKeyDown.keycode = Common::KEYCODE_INVALID;
 			}
 			break;
-#endif
-		case Common::EVENT_RTL:
-			if (ConfMan.getBool("confirm_exit")) {
-				if (g_engine)
-					g_engine->pauseEngine(true);
-				GUI::MessageDialog alert(_("Do you really want to return to the Launcher?"), _("Launcher"), _("Cancel"));
-				result = _shouldRTL = (alert.runModal() == GUI::kMessageOK);
-				if (g_engine)
-					g_engine->pauseEngine(false);
-			} else
-				_shouldRTL = true;
-			break;
-
-		case Common::EVENT_MUTE:
-			if (g_engine)
-				g_engine->flipMute();
-			break;
-
-		case Common::EVENT_QUIT:
-			if (ConfMan.getBool("confirm_exit")) {
-				if (_confirmExitDialogActive) {
-					result = false;
-					break;
-				}
-				_confirmExitDialogActive = true;
-				if (g_engine)
-					g_engine->pauseEngine(true);
-				GUI::MessageDialog alert(_("Do you really want to quit?"), _("Quit"), _("Cancel"));
-				result = _shouldQuit = (alert.runModal() == GUI::kMessageOK);
-				if (g_engine)
-					g_engine->pauseEngine(false);
-				_confirmExitDialogActive = false;
-			} else
-				_shouldQuit = true;
-
-			break;
 
 		default:
 			break;
 		}
 	} else {
 		// Check if event should be sent again (keydown)
-		if (_currentKeyDown.keycode != 0 && _keyRepeatTime < time) {
+		if (_currentKeyDown.keycode != Common::KEYCODE_INVALID && _keyRepeatTime <= time) {
 			// fire event
-			event.type = Common::EVENT_KEYDOWN;
-			event.synthetic = true;
-			event.kbd.ascii = _currentKeyDown.ascii;
-			event.kbd.keycode = (Common::KeyCode)_currentKeyDown.keycode;
-			event.kbd.flags = _currentKeyDown.flags;
+			Common::Event repeatEvent;
+			repeatEvent.type = Common::EVENT_KEYDOWN;
+			repeatEvent.kbdRepeat = true;
+			repeatEvent.kbd = _currentKeyDown;
 			_keyRepeatTime = time + kKeyRepeatSustainDelay;
-			result = true;
+
+			_eventQueue.push(repeatEvent);
 		}
 	}
-
-	return result;
 }
 
 void DefaultEventManager::pushEvent(const Common::Event &event) {
diff --git a/backends/events/default/default-events.h b/backends/events/default/default-events.h
index 38406c25aa..df6ebd2f14 100644
--- a/backends/events/default/default-events.h
+++ b/backends/events/default/default-events.h
@@ -67,12 +67,11 @@ class DefaultEventManager : public Common::EventManager, Common::EventObserver {
 		kKeyRepeatSustainDelay = 100
 	};
 
-	struct {
-		uint16 ascii;
-		byte flags;
-		int keycode;
-	} _currentKeyDown;
+	bool _shouldGenerateKeyRepeatEvents;
+	Common::KeyState _currentKeyDown;
 	uint32 _keyRepeatTime;
+
+	void handleKeyRepeat();
 public:
 	DefaultEventManager(Common::EventSource *boss);
 	~DefaultEventManager();
@@ -97,6 +96,17 @@ public:
 	 // this, please talk to tsoliman and/or LordHoto.
 	virtual Common::Keymapper *getKeymapper() { return _keymapper; }
 #endif
+
+	/**
+	 * Controls whether repeated key down events are generated while a key is pressed
+	 *
+	 * Backends that generate their own keyboard repeat events should disable this.
+	 *
+	 * @param generateKeyRepeatEvents
+	 */
+	void setGenerateKeyRepeatEvents(bool generateKeyRepeatEvents) {
+		_shouldGenerateKeyRepeatEvents = generateKeyRepeatEvents;
+	}
 };
 
 #endif
diff --git a/backends/events/sdl/sdl-events.cpp b/backends/events/sdl/sdl-events.cpp
index 91ca0f5df6..d01d9c6c4a 100644
--- a/backends/events/sdl/sdl-events.cpp
+++ b/backends/events/sdl/sdl-events.cpp
@@ -696,6 +696,10 @@ bool SdlEventSource::handleKeyDown(SDL_Event &ev, Common::Event &event) {
 	event.kbd.keycode = SDLToOSystemKeycode(sdlKeycode);
 	event.kbd.ascii = mapKey(sdlKeycode, (SDLMod)ev.key.keysym.mod, obtainUnicode(ev.key.keysym));
 
+#if SDL_VERSION_ATLEAST(2, 0, 0)
+	event.kbdRepeat = ev.key.repeat;
+#endif
+
 	return true;
 }
 
diff --git a/backends/platform/android/events.cpp b/backends/platform/android/events.cpp
index b146945a01..d13d381f95 100644
--- a/backends/platform/android/events.cpp
+++ b/backends/platform/android/events.cpp
@@ -443,7 +443,7 @@ void OSystem_Android::pushEvent(int type, int arg1, int arg2, int arg3,
 		}
 
 		if (arg5 > 0)
-			e.synthetic = true;
+			e.kbdRepeat = true;
 
 		// map special keys to 'our' ascii codes
 		switch (e.kbd.keycode) {
diff --git a/backends/platform/sdl/sdl.cpp b/backends/platform/sdl/sdl.cpp
index f2bf9590c5..68a987bc61 100644
--- a/backends/platform/sdl/sdl.cpp
+++ b/backends/platform/sdl/sdl.cpp
@@ -43,6 +43,7 @@
 #include "backends/audiocd/sdl/sdl-audiocd.h"
 #endif
 
+#include "backends/events/default/default-events.h"
 #include "backends/events/sdl/sdl-events.h"
 #include "backends/mutex/sdl/sdl-mutex.h"
 #include "backends/timer/sdl/sdl-timer.h"
@@ -207,6 +208,16 @@ void OSystem_SDL::initBackend() {
 	if (_eventSource == 0)
 		_eventSource = new SdlEventSource();
 
+	if (_eventManager == nullptr) {
+		DefaultEventManager *eventManager = new DefaultEventManager(_eventSource);
+#if SDL_VERSION_ATLEAST(2, 0, 0)
+		// SDL 2 generates its own keyboard repeat events.
+		eventManager->setGenerateKeyRepeatEvents(false);
+#endif
+		_eventManager = eventManager;
+	}
+
+
 #ifdef USE_OPENGL
 #if SDL_VERSION_ATLEAST(2, 0, 0)
 	SDL_DisplayMode displayMode;
diff --git a/backends/platform/tizen/form.cpp b/backends/platform/tizen/form.cpp
index 2a9a3967cc..22e15f61c4 100644
--- a/backends/platform/tizen/form.cpp
+++ b/backends/platform/tizen/form.cpp
@@ -243,7 +243,6 @@ void TizenAppForm::pushEvent(Common::EventType type, const Point &currentPositio
 void TizenAppForm::pushKey(Common::KeyCode keycode) {
 	if (_eventQueueLock) {
 		Common::Event e;
-		e.synthetic = false;
 		e.kbd.keycode = keycode;
 		e.kbd.ascii = keycode;
 		e.kbd.flags = 0;
diff --git a/backends/vkeybd/virtual-keyboard.cpp b/backends/vkeybd/virtual-keyboard.cpp
index ce19e9d462..80d7313a8c 100644
--- a/backends/vkeybd/virtual-keyboard.cpp
+++ b/backends/vkeybd/virtual-keyboard.cpp
@@ -242,7 +242,6 @@ void VirtualKeyboard::show() {
 
 		// push keydown & keyup events into the event manager
 		Event evt;
-		evt.synthetic = false;
 		while (!_keyQueue.empty()) {
 			evt.kbd = _keyQueue.pop();
 			evt.type = EVENT_KEYDOWN;
diff --git a/common/events.h b/common/events.h
index e5bb8cab50..a514ea291e 100644
--- a/common/events.h
+++ b/common/events.h
@@ -90,22 +90,29 @@ enum EventType {
 };
 
 typedef uint32 CustomEventType;
+
 /**
  * Data structure for an event. A pointer to an instance of Event
  * can be passed to pollEvent.
  */
 struct Event {
+
 	/** The type of the event. */
 	EventType type;
-	/** Flag to indicate if the event is real or synthetic. E.g. keyboard
-	  * repeat events are synthetic.
-	  */
-	bool synthetic;
+
+	/**
+	 * True if this is a key down repeat event.
+	 *
+	 * Only valid for EVENT_KEYDOWN events.
+	 */
+	bool kbdRepeat;
+
 	/**
 	  * Keyboard data; only valid for keyboard events (EVENT_KEYDOWN and
 	  * EVENT_KEYUP). For all other event types, content is undefined.
 	  */
 	KeyState kbd;
+
 	/**
 	 * The mouse coordinates, in virtual screen coordinates. Only valid
 	 * for mouse events.
@@ -120,7 +127,7 @@ struct Event {
 	CustomEventType customType;
 #endif
 
-	Event() : type(EVENT_INVALID), synthetic(false) {
+	Event() : type(EVENT_INVALID), kbdRepeat(false) {
 #ifdef ENABLE_KEYMAPPER
 		customType = 0;
 #endif
@@ -383,6 +390,7 @@ public:
 	 * @note	called after graphics system has been set up
 	 */
 	virtual void init() {}
+
 	/**
 	 * Get the next event in the event queue.
 	 * @param event	point to an Event struct, which will be filled with the event data.
diff --git a/common/recorderfile.cpp b/common/recorderfile.cpp
index 71f8272b44..1f283715d0 100644
--- a/common/recorderfile.cpp
+++ b/common/recorderfile.cpp
@@ -390,7 +390,7 @@ void PlaybackFile::readEvent(RecorderEvent& event) {
 		}
 		break;
 	}
-	event.synthetic = true;
+	event.kbdRepeat = true;
 }
 
 void PlaybackFile::readEventsToBuffer(uint32 size) {
diff --git a/engines/agi/keyboard.cpp b/engines/agi/keyboard.cpp
index 8fb49fdf02..8fd30eda54 100644
--- a/engines/agi/keyboard.cpp
+++ b/engines/agi/keyboard.cpp
@@ -164,42 +164,42 @@ void AgiEngine::processScummVMEvents() {
 				switch (event.kbd.keycode) {
 				case Common::KEYCODE_LEFT:
 				case Common::KEYCODE_KP4:
-					if (_allowSynthetic || !event.synthetic)
+					if (_allowSynthetic || !event.kbdRepeat)
 						key = AGI_KEY_LEFT;
 					break;
 				case Common::KEYCODE_RIGHT:
 				case Common::KEYCODE_KP6:
-					if (_allowSynthetic || !event.synthetic)
+					if (_allowSynthetic || !event.kbdRepeat)
 						key = AGI_KEY_RIGHT;
 					break;
 				case Common::KEYCODE_UP:
 				case Common::KEYCODE_KP8:
-					if (_allowSynthetic || !event.synthetic)
+					if (_allowSynthetic || !event.kbdRepeat)
 						key = AGI_KEY_UP;
 					break;
 				case Common::KEYCODE_DOWN:
 				case Common::KEYCODE_KP2:
-					if (_allowSynthetic || !event.synthetic)
+					if (_allowSynthetic || !event.kbdRepeat)
 						key = AGI_KEY_DOWN;
 					break;
 				case Common::KEYCODE_PAGEUP:
 				case Common::KEYCODE_KP9:
-					if (_allowSynthetic || !event.synthetic)
+					if (_allowSynthetic || !event.kbdRepeat)
 						key = AGI_KEY_UP_RIGHT;
 					break;
 				case Common::KEYCODE_PAGEDOWN:
 				case Common::KEYCODE_KP3:
-					if (_allowSynthetic || !event.synthetic)
+					if (_allowSynthetic || !event.kbdRepeat)
 						key = AGI_KEY_DOWN_RIGHT;
 					break;
 				case Common::KEYCODE_HOME:
 				case Common::KEYCODE_KP7:
-					if (_allowSynthetic || !event.synthetic)
+					if (_allowSynthetic || !event.kbdRepeat)
 						key = AGI_KEY_UP_LEFT;
 					break;
 				case Common::KEYCODE_END:
 				case Common::KEYCODE_KP1:
-					if (_allowSynthetic || !event.synthetic)
+					if (_allowSynthetic || !event.kbdRepeat)
 						key = AGI_KEY_DOWN_LEFT;
 					break;
 				case Common::KEYCODE_KP5:
diff --git a/engines/dm/eventman.cpp b/engines/dm/eventman.cpp
index f424672b67..1d914bb183 100644
--- a/engines/dm/eventman.cpp
+++ b/engines/dm/eventman.cpp
@@ -609,7 +609,7 @@ Common::EventType EventManager::processInput(Common::Event *grabKey, Common::Eve
 	while (g_system->getEventManager()->pollEvent(event)) {
 		switch (event.type) {
 		case Common::EVENT_KEYDOWN: {
-			if (event.synthetic)
+			if (event.kbdRepeat)
 				break;
 
 			if (event.kbd.keycode == Common::KEYCODE_d && event.kbd.hasFlags(Common::KBD_CTRL)) {
diff --git a/gui/EventRecorder.cpp b/gui/EventRecorder.cpp
index 3f91cfa259..f1b486d394 100644
--- a/gui/EventRecorder.cpp
+++ b/gui/EventRecorder.cpp
@@ -177,7 +177,7 @@ bool EventRecorder::processDelayMillis() {
 }
 
 void EventRecorder::checkForKeyCode(const Common::Event &event) {
-	if ((event.type == Common::EVENT_KEYDOWN) && (event.kbd.flags & Common::KBD_CTRL) && (event.kbd.keycode == Common::KEYCODE_p) && (!event.synthetic)) {
+	if ((event.type == Common::EVENT_KEYDOWN) && (event.kbd.flags & Common::KBD_CTRL) && (event.kbd.keycode == Common::KEYCODE_p) && (!event.kbdRepeat)) {
 		togglePause();
 	}
 }
@@ -445,7 +445,7 @@ Common::List<Common::Event> EventRecorder::mapEvent(const Common::Event &ev, Com
 	evt.mouse.y = evt.mouse.y * (g_system->getOverlayHeight() / g_system->getHeight());
 	switch (_recordMode) {
 	case kRecorderPlayback:
-		if (ev.synthetic != true) {
+		if (ev.kbdRepeat != true) {
 			return Common::List<Common::Event>();
 		}
 		return Common::DefaultEventMapper::mapEvent(ev, source);