SDL: Change keyboard repeat to apply on unmapped events

The keyboard repeat event generator is used when building against SDL1.
Previously the repeat events would generate based on the event stream
produced by the keymapper which is not guaranteed to have matching up
and down events in the case the keymaps are changed while a key is
pressed.

Fixes #11417.

backends/events/default/default-events.cpp

@@ -42,8 +42,7 @@ DefaultEventManager::DefaultEventManager(Common::EventSource *boss) :
 	_modifierState(0),
 	_shouldQuit(false),
 	_shouldRTL(false),
-	_confirmExitDialogActive(false),
-	_shouldGenerateKeyRepeatEvents(false) {
+	_confirmExitDialogActive(false) {
 
 	assert(boss);
 
@@ -52,9 +51,6 @@ DefaultEventManager::DefaultEventManager(Common::EventSource *boss) :
 
 	_dispatcher.registerObserver(this, kEventManPriority, false);
 
-	// Reset key repeat
-	_keyRepeatTime = 0;
-
 #ifdef ENABLE_VKEYBD
 	_vk = nullptr;
 #endif
@@ -88,10 +84,6 @@ void DefaultEventManager::init() {
 bool DefaultEventManager::pollEvent(Common::Event &event) {
 	_dispatcher.dispatch();
 
-	if (_shouldGenerateKeyRepeatEvents) {
-		handleKeyRepeat();
-	}
-
 	if (g_engine)
 		// Handle autosaves if enabled
 		g_engine->handleAutoSave();
@@ -116,7 +108,6 @@ bool DefaultEventManager::pollEvent(Common::Event &event) {
 			// 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;
 
@@ -235,45 +226,6 @@ bool DefaultEventManager::pollEvent(Common::Event &event) {
 	return forwardEvent;
 }
 
-void DefaultEventManager::handleKeyRepeat() {
-	uint32 time = g_system->getMillis(true);
-
-	if (!_eventQueue.empty()) {
-		// Peek in the event queue
-		const Common::Event &nextEvent = _eventQueue.front();
-
-		switch (nextEvent.type) {
-		case Common::EVENT_KEYDOWN:
-			// init continuous event stream
-			_currentKeyDown = nextEvent.kbd;
-			_keyRepeatTime = time + kKeyRepeatInitialDelay;
-			break;
-
-		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;
-
-		default:
-			break;
-		}
-	} else {
-		// Check if event should be sent again (keydown)
-		if (_currentKeyDown.keycode != Common::KEYCODE_INVALID && _keyRepeatTime <= time) {
-			// fire event
-			Common::Event repeatEvent;
-			repeatEvent.type = Common::EVENT_KEYDOWN;
-			repeatEvent.kbdRepeat = true;
-			repeatEvent.kbd = _currentKeyDown;
-			_keyRepeatTime = time + kKeyRepeatSustainDelay;
-
-			_eventQueue.push(repeatEvent);
-		}
-	}
-}
-
 void DefaultEventManager::pushEvent(const Common::Event &event) {
 	// If already received an EVENT_QUIT, don't add another one
 	if (event.type == Common::EVENT_QUIT) {

backends/events/default/default-events.h

@@ -59,17 +59,6 @@ class DefaultEventManager : public Common::EventManager, Common::EventObserver {
 	bool _shouldRTL;
 	bool _confirmExitDialogActive;
 
-	// for continuous events (keyDown)
-	enum {
-		kKeyRepeatInitialDelay = 400,
-		kKeyRepeatSustainDelay = 100
-	};
-
-	bool _shouldGenerateKeyRepeatEvents;
-	Common::KeyState _currentKeyDown;
-	uint32 _keyRepeatTime;
-
-	void handleKeyRepeat();
 public:
 	DefaultEventManager(Common::EventSource *boss);
 	~DefaultEventManager();
@@ -91,17 +80,6 @@ public:
 
 	Common::Keymapper *getKeymapper() override { return _keymapper; }
 	Common::Keymap *getGlobalKeymap() override;
-
-	/**
-	 * 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

backends/platform/sdl/sdl.cpp

@@ -85,6 +85,7 @@ OSystem_SDL::OSystem_SDL()
 	_logger(0),
 	_mixerManager(0),
 	_eventSource(0),
+	_eventSourceWrapper(nullptr),
 	_window(0) {
 }
 
@@ -106,6 +107,8 @@ OSystem_SDL::~OSystem_SDL() {
 	_window = 0;
 	delete _eventManager;
 	_eventManager = 0;
+	delete _eventSourceWrapper;
+	_eventSourceWrapper = nullptr;
 	delete _eventSource;
 	_eventSource = 0;
 	delete _audiocdManager;
@@ -198,16 +201,17 @@ void OSystem_SDL::initBackend() {
 
 	// Create the default event source, in case a custom backend
 	// manager didn't provide one yet.
-	if (_eventSource == 0)
+	if (!_eventSource)
 		_eventSource = new SdlEventSource();
 
-	if (_eventManager == nullptr) {
-		DefaultEventManager *eventManager = new DefaultEventManager(_eventSource);
 #if !SDL_VERSION_ATLEAST(2, 0, 0)
-		// SDL 1 does not generate its own keyboard repeat events.
-		eventManager->setGenerateKeyRepeatEvents(true);
+	// SDL 1 does not generate its own keyboard repeat events.
+	assert(!_eventSourceWrapper);
+	_eventSourceWrapper = makeKeyboardRepeatingEventSource(_eventSource);
 #endif
-		_eventManager = eventManager;
+
+	if (!_eventManager) {
+		_eventManager = new DefaultEventManager(_eventSourceWrapper ? _eventSourceWrapper : _eventSource);
 	}
 
 

backends/platform/sdl/sdl.h

@@ -117,6 +117,7 @@ protected:
 	 * The event source we use for obtaining SDL events.
 	 */
 	SdlEventSource *_eventSource;
+	Common::EventSource *_eventSourceWrapper;
 
 	/**
 	 * The SDL output window.

common/events.cpp

@@ -22,6 +22,8 @@
 
 #include "common/events.h"
 
+#include "common/system.h"
+
 namespace Common {
 
 bool isMouseEvent(const Event &event) {
@@ -175,4 +177,78 @@ void EventDispatcher::dispatchPoll() {
 	}
 }
 
+class KeyboardRepeatEventSourceWrapper : public Common::EventSource {
+public:
+	KeyboardRepeatEventSourceWrapper(Common::EventSource *delegate) :
+			_delegate(delegate),
+			_keyRepeatTime(0) {
+		assert(delegate);
+	}
+
+	// EventSource API
+	bool pollEvent(Common::Event &event) override {
+		uint32 time = g_system->getMillis(true);
+		bool gotEvent = _delegate->pollEvent(event);
+
+		if (gotEvent) {
+			switch (event.type) {
+			case Common::EVENT_KEYDOWN:
+				// init continuous event stream
+				_currentKeyDown = event.kbd;
+				_keyRepeatTime = time + kKeyRepeatInitialDelay;
+				break;
+
+			case Common::EVENT_KEYUP:
+				if (event.kbd.keycode == _currentKeyDown.keycode) {
+					// Only stop firing events if it's the current key
+					_currentKeyDown.keycode = Common::KEYCODE_INVALID;
+				}
+				break;
+
+			default:
+				break;
+			}
+
+			return true;
+		} else {
+			// Check if event should be sent again (keydown)
+			if (_currentKeyDown.keycode != Common::KEYCODE_INVALID && _keyRepeatTime <= time) {
+				// fire event
+				event.type = Common::EVENT_KEYDOWN;
+				event.kbdRepeat = true;
+				event.kbd = _currentKeyDown;
+				_keyRepeatTime = time + kKeyRepeatSustainDelay;
+
+				return true;
+			}
+
+			return false;
+		}
+	}
+
+	bool allowMapping() const override {
+		return _delegate->allowMapping();
+	}
+
+private:
+	// for continuous events (keyDown)
+	enum {
+		kKeyRepeatInitialDelay = 400,
+		kKeyRepeatSustainDelay = 100
+	};
+
+	Common::EventSource *_delegate;
+
+	Common::KeyState _currentKeyDown;
+	uint32 _keyRepeatTime;
+};
+
+EventSource *makeKeyboardRepeatingEventSource(EventSource *eventSource) {
+	if (!eventSource) {
+		return nullptr;
+	}
+
+	return new KeyboardRepeatEventSourceWrapper(eventSource);
+}
+
 } // End of namespace Common

common/events.h

@@ -536,6 +536,14 @@ protected:
 	EventDispatcher _dispatcher;
 };
 
+/**
+ * Wrap an event source so the key down events are repeated while
+ * keys are held down.
+ *
+ * Does not take ownership of the wrapped EventSource.
+ */
+EventSource *makeKeyboardRepeatingEventSource(EventSource *eventSource);
+
 } // End of namespace Common
 
 #endif