SCI32: Detect and handle tight loops around kGetEvent

In SSCI, mouse events are received through a hardware interrupt
and the cursor is drawn directly to the graphics card by the
interrupt handler. This allows game scripts to omit calls to
kFrameOut without stopping the mouse cursor from being redrawn
in response to mouse movement.

ScummVM, in contrast, needs to poll for events and submit screen
updates explicitly from the main thread. Submitting screen updates
may block on vsync, which means that this call should really only
be made once per frame, just after the game has finished updating
its back buffer. The closest signal in SCI32 for having completed
drawing a frame is the kFrameOut call, so this is where the
update is submitted (by calling OSystem::updateScreen).

The problem with the approach in ScummVM is that, even though the
mouse position is being updated (by calls to kGetEvent) and drawn
to the backend's back buffer (by GfxCursor32::drawToHardware),
OSystem::updateScreen is never called during game loops that omit
calls to kFrameOut.

This commit introduces a workaround that looks at the number of
times kGetEvent is called between calls to kFrameOut. If the
number of kGetEvent calls is higher than usual (where "usual"
seems to be 0 or 1), we assume that the game is running one of
these tight event loops, and kGetEvent starts calling
OSystem::updateScreen until the next kFrameOut call. We also
then start throttling the calls to kGetEvent to keep CPU usage
down. This fixes at least two such known loops:

1. When interacting with the menu bar at the top of the screen
   in LSL6hires;
2. When restoring a game in Phant2 and sitting on the "click mouse"
   screen.

A similar workaround may also be needed for kGetTime, though loops
around kGetTime should preferably be replaced using a script patch
to call kWait instead.

4 files changed, 27 insertions, 4 deletions

diff --git a/engines/sci/engine/kevent.cpp b/engines/sci/engine/kevent.cpp
index 18205a4219..389277b9bc 100644
--- a/engines/sci/engine/kevent.cpp
+++ b/engines/sci/engine/kevent.cpp
@@ -83,9 +83,29 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 
 	// For a real event we use its associated mouse position
 #ifdef ENABLE_SCI32
-	if (getSciVersion() >= SCI_VERSION_2)
+	if (getSciVersion() >= SCI_VERSION_2) {
 		mousePos = curEvent.mousePosSci;
-	else {
+
+		// Some games, like LSL6hires (when interacting with the menu bar) and
+		// Phant2 (when on the "click mouse" screen after restoring a game),
+		// have unthrottled loops that call kGetEvent but do not call kFrameOut.
+		// In these cases we still need to call OSystem::updateScreen to update
+		// the mouse cursor (in SSCI this was not necessary because mouse
+		// updates were made directly to hardware from an interrupt handler),
+		// and we need to throttle these calls so the game does not use 100%
+		// CPU.
+		// This situation seems to be detectable by looking at how many times
+		// kGetEvent has been called between calls to kFrameOut. During normal
+		// game operation, there are usually just 0 or 1 kGetEvent calls between
+		// kFrameOut calls; any more than that indicates that we are probably in
+		// one of these ugly loops and should be updating the screen &
+		// throttling the VM.
+		if (++s->_eventCounter > 2) {
+			g_system->updateScreen();
+			s->speedThrottler(10); // 10ms is an arbitrary value
+			s->_throttleTrigger = true;
+		}
+	} else {
 #endif
 		mousePos = curEvent.mousePos;
 		// Limit the mouse cursor position, if necessary
diff --git a/engines/sci/engine/kgraphics32.cpp b/engines/sci/engine/kgraphics32.cpp
index b0a1c70ed8..38c760fabc 100644
--- a/engines/sci/engine/kgraphics32.cpp
+++ b/engines/sci/engine/kgraphics32.cpp
@@ -234,6 +234,7 @@ reg_t kGetHighPlanePri(EngineState *s, int argc, reg_t *argv) {
 reg_t kFrameOut(EngineState *s, int argc, reg_t *argv) {
 	bool showBits = argc > 0 ? argv[0].toUint16() : true;
 	g_sci->_gfxFrameout->kernelFrameOut(showBits);
+	s->_eventCounter = 0;
 	return s->r_acc;
 }
 
diff --git a/engines/sci/engine/state.cpp b/engines/sci/engine/state.cpp
index 44aecdf0ff..4abf5ce303 100644
--- a/engines/sci/engine/state.cpp
+++ b/engines/sci/engine/state.cpp
@@ -101,7 +101,7 @@ void EngineState::reset(bool isRestoring) {
 
 	gcCountDown = 0;
 
-	_throttleCounter = 0;
+	_eventCounter = 0;
 	_throttleLastTime = 0;
 	_throttleTrigger = false;
 	_gameIsBenchmarking = false;
diff --git a/engines/sci/engine/state.h b/engines/sci/engine/state.h
index c41f86c781..57839ab04e 100644
--- a/engines/sci/engine/state.h
+++ b/engines/sci/engine/state.h
@@ -128,7 +128,9 @@ public:
 	void speedThrottler(uint32 neededSleep);
 	void wait(int16 ticks);
 
-	uint32 _throttleCounter; /**< total times kAnimate was invoked */
+#ifdef ENABLE_SCI32
+	uint32 _eventCounter; /**< total times kGetEvent was invoked since the last call to kFrameOut */
+#endif
 	uint32 _throttleLastTime; /**< last time kAnimate was invoked */
 	bool _throttleTrigger;
 	bool _gameIsBenchmarking;