[Scummvm-git-logs] scummvm master -> 8e933b79ec0be71a2bc5d1fe1c17f06d4ad862a7

[csnover]

This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
604ac1a375 SCI32: Update cursor even when position appears unchanged
57dd79d4dc SCI32: Detect and handle tight loops around kGetEvent
8e933b79ec SCI32: Add Spanish Windows KQ7 to detection table


Commit: 604ac1a375ca76765133e2b56329763ac5c9874c
    https://github.com/scummvm/scummvm/commit/604ac1a375ca76765133e2b56329763ac5c9874c
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-06T19:00:04-05:00

Commit Message:
SCI32: Update cursor even when position appears unchanged

This may be masking another bug, but at least what happens in
Phant1 is that the mouse does not get redrawn after being moved
programmatically to the fast-forward button when deviceMoved
returns early.

Changed paths:
    engines/sci/graphics/cursor32.cpp


diff --git a/engines/sci/graphics/cursor32.cpp b/engines/sci/graphics/cursor32.cpp
index 6247146..fc853bb 100644
--- a/engines/sci/graphics/cursor32.cpp
+++ b/engines/sci/graphics/cursor32.cpp
@@ -380,10 +380,6 @@ void GfxCursor32::deviceMoved(Common::Point &position) {
 		position.y = _restrictedArea.bottom - 1;
 	}
 
-	if (_position == position) {
-		return;
-	}
-
 	_position = position;
 
 	g_system->warpMouse(position.x, position.y);


Commit: 57dd79d4dcb977f9803c8c4d8fe6c19eaac35d90
    https://github.com/scummvm/scummvm/commit/57dd79d4dcb977f9803c8c4d8fe6c19eaac35d90
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-06T19:03:54-05:00

Commit Message:
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.

Changed paths:
    engines/sci/engine/kevent.cpp
    engines/sci/engine/kgraphics32.cpp
    engines/sci/engine/state.cpp
    engines/sci/engine/state.h


diff --git a/engines/sci/engine/kevent.cpp b/engines/sci/engine/kevent.cpp
index 18205a4..389277b 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 b0a1c70..38c760f 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 44aecdf..4abf5ce 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 c41f86c..57839ab 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;


Commit: 8e933b79ec0be71a2bc5d1fe1c17f06d4ad862a7
    https://github.com/scummvm/scummvm/commit/8e933b79ec0be71a2bc5d1fe1c17f06d4ad862a7
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-06T20:58:06-05:00

Commit Message:
SCI32: Add Spanish Windows KQ7 to detection table

(The DOS version was already listed.)

Changed paths:
    engines/sci/detection_tables.h


diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index 536a41d..de03a0e 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -1931,6 +1931,14 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		AD_LISTEND},
 		Common::ES_ESP, Common::kPlatformDOS, ADGF_TESTING | ADGF_CD, GUIO_KQ7 },
 
+	// King's Quest 7 - Spanish Windows (from jvprat)
+	// Executable scanning reports "2.100.002", VERSION file reports "2.00"
+	{"kq7", "", {
+		{"resource.map", 0, "0b62693cbe87e3aaca3e8655a437f27f", 18709},
+		{"resource.000", 0, "51c1ead1163e19a2de8f121c39df7a76", 200764100},
+		AD_LISTEND},
+		Common::ES_ESP, Common::kPlatformWindows, ADGF_TESTING | ADGF_CD, GUIO_KQ7 },
+
 	// King's Quest 7 - English DOS Non-Interactive Demo
 	// SCI interpreter version 2.100.002
 	{"kq7", "Demo", {