Merge branch 'master' of github.com:scummvm/scummvm

5 files changed, 311 insertions, 296 deletions

diff --git a/engines/sci/engine/kevent.cpp b/engines/sci/engine/kevent.cpp
index e5a9931605..2540861a93 100644
--- a/engines/sci/engine/kevent.cpp
+++ b/engines/sci/engine/kevent.cpp
@@ -46,17 +46,18 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 	SegManager *segMan = s->_segMan;
 	Common::Point mousePos;
 
-	// Limit the mouse cursor position, if necessary
-	g_sci->_gfxCursor->refreshPosition();
-	mousePos = g_sci->_gfxCursor->getPosition();
-#ifdef ENABLE_SCI32
-	if (getSciVersion() >= SCI_VERSION_2_1)
-		g_sci->_gfxCoordAdjuster->fromDisplayToScript(mousePos.y, mousePos.x);
-#endif
-
 	// If there's a simkey pending, and the game wants a keyboard event, use the
 	// simkey instead of a normal event
 	if (g_debug_simulated_key && (mask & SCI_EVENT_KEYBOARD)) {
+		// In case we use a simulated event we query the current mouse position
+		mousePos = g_sci->_gfxCursor->getPosition();
+#ifdef ENABLE_SCI32
+		if (getSciVersion() >= SCI_VERSION_2_1)
+			g_sci->_gfxCoordAdjuster->fromDisplayToScript(mousePos.y, mousePos.x);
+#endif
+		// Limit the mouse cursor position, if necessary
+		g_sci->_gfxCursor->refreshPosition();
+
 		writeSelectorValue(segMan, obj, SELECTOR(type), SCI_EVENT_KEYBOARD); // Keyboard event
 		writeSelectorValue(segMan, obj, SELECTOR(message), g_debug_simulated_key);
 		writeSelectorValue(segMan, obj, SELECTOR(modifiers), SCI_KEYMOD_NUMLOCK); // Numlock on
@@ -68,6 +69,15 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 
 	curEvent = g_sci->getEventManager()->getSciEvent(mask);
 
+	// For a real event we use its associated mouse position
+	mousePos = curEvent.mousePos;
+#ifdef ENABLE_SCI32
+	if (getSciVersion() >= SCI_VERSION_2_1)
+		g_sci->_gfxCoordAdjuster->fromDisplayToScript(mousePos.y, mousePos.x);
+#endif
+	// Limit the mouse cursor position, if necessary
+	g_sci->_gfxCursor->refreshPosition();
+
 	if (g_sci->getVocabulary())
 		g_sci->getVocabulary()->parser_event = NULL_REG; // Invalidate parser event
 
diff --git a/engines/sci/event.cpp b/engines/sci/event.cpp
index d607a5314f..cb0e6b3c03 100644
--- a/engines/sci/event.cpp
+++ b/engines/sci/event.cpp
@@ -35,12 +35,6 @@
 
 namespace Sci {
 
-EventManager::EventManager(bool fontIsExtended) : _fontIsExtended(fontIsExtended), _modifierStates(0) {
-}
-
-EventManager::~EventManager() {
-}
-
 struct ScancodeRow {
 	int offset;
 	const char *keys;
@@ -52,27 +46,6 @@ const ScancodeRow s_scancodeRows[] = {
 	{ 0x2c, "ZXCVBNM,./"    }
 };
 
-static int altify(int ch) {
-	// Calculates a PC keyboard scancode from a character */
-	int row;
-	int c = toupper((char)ch);
-
-	for (row = 0; row < ARRAYSIZE(s_scancodeRows); row++) {
-		const char *keys = s_scancodeRows[row].keys;
-		int offset = s_scancodeRows[row].offset;
-
-		while (*keys) {
-			if (*keys == c)
-				return offset << 8;
-
-			offset++;
-			keys++;
-		}
-	}
-
-	return ch;
-}
-
 const byte codepagemap_88591toDOS[0x80] = {
 	 '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?', // 0x8x
 	 '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?',  '?', // 0x9x
@@ -120,147 +93,178 @@ const SciKeyConversion keyMappings[] = {
 	{ Common::KEYCODE_KP_DIVIDE   , '/'            , '/'            },
 };
 
+struct MouseEventConversion {
+	Common::EventType commonType;
+	short sciType;
+	short data;
+};
+
+const MouseEventConversion mouseEventMappings[] = {
+	{ Common::EVENT_LBUTTONDOWN,   SCI_EVENT_MOUSE_PRESS, 1 },
+	{ Common::EVENT_RBUTTONDOWN,   SCI_EVENT_MOUSE_PRESS, 2 },
+	{ Common::EVENT_MBUTTONDOWN,   SCI_EVENT_MOUSE_PRESS, 3 },
+	{   Common::EVENT_LBUTTONUP, SCI_EVENT_MOUSE_RELEASE, 1 },
+	{   Common::EVENT_LBUTTONUP, SCI_EVENT_MOUSE_RELEASE, 2 },
+	{   Common::EVENT_LBUTTONUP, SCI_EVENT_MOUSE_RELEASE, 3 }
+};
+
+EventManager::EventManager(bool fontIsExtended) : _fontIsExtended(fontIsExtended) {
+}
+
+EventManager::~EventManager() {
+}
+
+static int altify(int ch) {
+	// Calculates a PC keyboard scancode from a character */
+	int row;
+	int c = toupper((char)ch);
+
+	for (row = 0; row < ARRAYSIZE(s_scancodeRows); row++) {
+		const char *keys = s_scancodeRows[row].keys;
+		int offset = s_scancodeRows[row].offset;
+
+		while (*keys) {
+			if (*keys == c)
+				return offset << 8;
+
+			offset++;
+			keys++;
+		}
+	}
+
+	return ch;
+}
+
 SciEvent EventManager::getScummVMEvent() {
-	SciEvent input = { SCI_EVENT_NONE, 0, 0, 0 };
+	SciEvent input = { SCI_EVENT_NONE, 0, 0, 0, Common::Point(0, 0) };
+	SciEvent noEvent = { SCI_EVENT_NONE, 0, 0, 0, Common::Point(0, 0) };
 
 	Common::EventManager *em = g_system->getEventManager();
 	Common::Event ev;
 
 	bool found = em->pollEvent(ev);
-	Common::Point p = ev.mouse;
 
 	// Don't generate events for mouse movement
-	while (found && ev.type == Common::EVENT_MOUSEMOVE) {
+	while (found && ev.type == Common::EVENT_MOUSEMOVE)
 		found = em->pollEvent(ev);
+
+	// Save the mouse position
+	//
+	// We call getMousePos of the event manager here, since we also want to
+	// store the mouse position in case of keyboard events, which do not feature
+	// any mouse position information itself.
+	// This should be safe, since the mouse position in the event manager should
+	// only be updated when a mouse related event has been taken from the queue
+	// via pollEvent.
+	noEvent.mousePos = input.mousePos = em->getMousePos();
+
+	if (!found || ev.type == Common::EVENT_MOUSEMOVE)
+		return noEvent;
+
+	if (ev.type == Common::EVENT_QUIT) {
+		input.type = SCI_EVENT_QUIT;
+		return input;
 	}
 
-	if (found && ev.type != Common::EVENT_MOUSEMOVE) {
-		int modifiers = em->getModifierState();
-		bool numlockOn = (ev.kbd.flags & Common::KBD_NUM);
-
-		// We add the modifier key status to buckybits
-		//TODO: SCI_EVM_INSERT
-
-		input.modifiers =
-		    ((modifiers & Common::KBD_ALT) ? SCI_KEYMOD_ALT : 0) |
-		    ((modifiers & Common::KBD_CTRL) ? SCI_KEYMOD_CTRL : 0) |
-		    ((modifiers & Common::KBD_SHIFT) ? SCI_KEYMOD_LSHIFT | SCI_KEYMOD_RSHIFT : 0) |
-		    ((ev.kbd.flags & Common::KBD_CAPS) ? SCI_KEYMOD_CAPSLOCK : 0) |
-			((ev.kbd.flags & Common::KBD_SCRL) ? SCI_KEYMOD_SCRLOCK : 0) |
-			_modifierStates;
-
-		switch (ev.type) {
-			// Keyboard events
-		case Common::EVENT_KEYDOWN:
-			input.data = ev.kbd.keycode;
-			input.character = ev.kbd.ascii;
-
-			// Debug console
-			if (ev.kbd.hasFlags(Common::KBD_CTRL) && ev.kbd.keycode == Common::KEYCODE_d) {
-				// Open debug console
-				Console *con = g_sci->getSciDebugger();
-				con->attach();
-
-				// Clear keyboard event
-				input.type = SCI_EVENT_NONE;
-				input.character = 0;
-				input.data = 0;
-				input.modifiers = 0;
-
-				return input;
-			}
+	// Handle mouse events
+	for (int i = 0; i < ARRAYSIZE(mouseEventMappings); i++) {
+		if (mouseEventMappings[i].commonType == ev.type) {
+			input.type = mouseEventMappings[i].sciType;
+			input.data = mouseEventMappings[i].data;
+			return input;
+		}
+	}
 
-			if (!(input.data & 0xFF00)) {
-				// Directly accept most common keys without conversion
-				input.type = SCI_EVENT_KEYBOARD;
-				if ((input.character >= 0x80) && (input.character <= 0xFF)) {
-					// If there is no extended font, we will just clear the current event
-					//  Sierra SCI actually accepted those characters, but didn't display them inside textedit-controls
-					//  because the characters were missing inside the font(s)
-					//  We filter them out for non-multilingual games because of that
-					if (!_fontIsExtended) {
-						input.type = SCI_EVENT_NONE;
-						input.character = 0;
-						input.data = 0;
-						input.modifiers = 0;
-						return input;
-					}
-					// We get a 8859-1 character, we need dos (cp850/437) character for multilingual sci01 games
-					input.character = codepagemap_88591toDOS[input.character & 0x7f];
-				}
-				if (input.data == Common::KEYCODE_TAB) {
-					// Tab
-					input.type = SCI_EVENT_KEYBOARD;
-					input.data = SCI_KEY_TAB;
-					if (input.modifiers & (SCI_KEYMOD_LSHIFT | SCI_KEYMOD_RSHIFT))
-						input.character = SCI_KEY_SHIFT_TAB;
-					else
-						input.character = SCI_KEY_TAB;
-				}
-				if (input.data == Common::KEYCODE_DELETE) {
-					// Delete key
-					input.type = SCI_EVENT_KEYBOARD;
-					input.data = input.character = SCI_KEY_DELETE;
-				}
-			} else if ((input.data >= Common::KEYCODE_F1) && input.data <= Common::KEYCODE_F10) {
-				// F1-F10
-				input.type = SCI_EVENT_KEYBOARD;
-				// SCI_K_F1 == 59 << 8
-				// SCI_K_SHIFT_F1 == 84 << 8
-				input.data = SCI_KEY_F1 + ((input.data - Common::KEYCODE_F1)<<8);
-				if (input.modifiers & (SCI_KEYMOD_LSHIFT | SCI_KEYMOD_RSHIFT))
-					input.character = input.data + 0x1900;
-				else
-					input.character = input.data;
-			} else {
-				// Special keys that need conversion
-				input.type = SCI_EVENT_KEYBOARD;
-				for (int i = 0; i < ARRAYSIZE(keyMappings); i++) {
-					if (keyMappings[i].scummVMKey == ev.kbd.keycode) {
-						input.data = numlockOn ? keyMappings[i].sciKeyNumlockOn : keyMappings[i].sciKeyNumlockOff;
-						break;
-					}
-				}
-				input.character = input.data;
-			}
-			break;
+	// If we reached here, make sure that it's a keydown event
+	if (ev.type != Common::EVENT_KEYDOWN)
+		return noEvent;
 
-			// Mouse events
-		case Common::EVENT_LBUTTONDOWN:
-			input.type = SCI_EVENT_MOUSE_PRESS;
-			input.data = 1;
-			break;
-		case Common::EVENT_RBUTTONDOWN:
-			input.type = SCI_EVENT_MOUSE_PRESS;
-			input.data = 2;
-			break;
-		case Common::EVENT_MBUTTONDOWN:
-			input.type = SCI_EVENT_MOUSE_PRESS;
-			input.data = 3;
-			break;
-		case Common::EVENT_LBUTTONUP:
-			input.type = SCI_EVENT_MOUSE_RELEASE;
-			input.data = 1;
-			break;
-		case Common::EVENT_RBUTTONUP:
-			input.type = SCI_EVENT_MOUSE_RELEASE;
-			input.data = 2;
-			break;
-		case Common::EVENT_MBUTTONUP:
-			input.type = SCI_EVENT_MOUSE_RELEASE;
-			input.data = 3;
-			break;
-
-			// Misc events
-		case Common::EVENT_QUIT:
-			input.type = SCI_EVENT_QUIT;
-			break;
+	// Check for Control-D (debug console)
+	if (ev.kbd.hasFlags(Common::KBD_CTRL) && ev.kbd.keycode == Common::KEYCODE_d) {
+		// Open debug console
+		Console *con = g_sci->getSciDebugger();
+		con->attach();
+		return noEvent;
+	}
 
-		default:
-			break;
+	// Process keyboard events
+
+	int modifiers = em->getModifierState();
+	bool numlockOn = (ev.kbd.flags & Common::KBD_NUM);
+
+	input.data = ev.kbd.keycode;
+	input.character = ev.kbd.ascii;
+	input.type = SCI_EVENT_KEYBOARD;
+
+	input.modifiers =
+		((modifiers & Common::KBD_ALT) ? SCI_KEYMOD_ALT : 0) |
+		((modifiers & Common::KBD_CTRL) ? SCI_KEYMOD_CTRL : 0) |
+		((modifiers & Common::KBD_SHIFT) ? SCI_KEYMOD_LSHIFT | SCI_KEYMOD_RSHIFT : 0);
+
+	// Caps lock and Scroll lock have been removed, cause we already handle upper
+	// case keys ad Scroll lock doesn't seem to be used anywhere
+		//((ev.kbd.flags & Common::KBD_CAPS) ? SCI_KEYMOD_CAPSLOCK : 0) |
+		//((ev.kbd.flags & Common::KBD_SCRL) ? SCI_KEYMOD_SCRLOCK : 0) |
+
+	if (!(input.data & 0xFF00)) {
+		// Directly accept most common keys without conversion
+		if ((input.character >= 0x80) && (input.character <= 0xFF)) {
+			// If there is no extended font, we will just clear the
+			// current event.
+			// Sierra SCI actually accepted those characters, but
+			// didn't display them inside text edit controls because
+			// the characters were missing inside the font(s).
+			// We filter them out for non-multilingual games because
+			// of that.
+			if (!_fontIsExtended)
+				return noEvent;
+			// Convert 8859-1 characters to DOS (cp850/437) for
+			// multilingual SCI01 games
+			input.character = codepagemap_88591toDOS[input.character & 0x7f];
+		}
+		if (input.data == Common::KEYCODE_TAB) {
+			input.character = input.data = SCI_KEY_TAB;
+			if (modifiers & Common::KBD_SHIFT)
+				input.character = SCI_KEY_SHIFT_TAB;
+		}
+		if (input.data == Common::KEYCODE_DELETE)
+			input.data = input.character = SCI_KEY_DELETE;
+	} else if ((input.data >= Common::KEYCODE_F1) && input.data <= Common::KEYCODE_F10) {
+		// SCI_K_F1 == 59 << 8
+		// SCI_K_SHIFT_F1 == 84 << 8
+		input.character = input.data = SCI_KEY_F1 + ((input.data - Common::KEYCODE_F1)<<8);
+		if (modifiers & Common::KBD_SHIFT)
+			input.character = input.data + 0x1900;
+	} else {
+		// Special keys that need conversion
+		for (int i = 0; i < ARRAYSIZE(keyMappings); i++) {
+			if (keyMappings[i].scummVMKey == ev.kbd.keycode) {
+				input.character = input.data = numlockOn ? keyMappings[i].sciKeyNumlockOn : keyMappings[i].sciKeyNumlockOff;
+				break;
+			}
 		}
 	}
+
+	// When Ctrl AND Alt are pressed together with a regular key, Linux will give us control-key, Windows will give
+	//  us the actual key. My opinion is that windows is right, because under DOS the keys worked the same, anyway
+	//  we support the other case as well
+	if ((modifiers & Common::KBD_SHIFT) && input.character < 27)
+		input.character += 96; // 0x01 -> 'a'
+
+	if (getSciVersion() <= SCI_VERSION_1_MIDDLE) {
+		// TODO: find out if altify is also not needed for sci1late+, couldnt find any game that uses those keys
+		// Scancodify if appropriate
+		if (modifiers & Common::KBD_ALT)
+			input.character = altify(input.character);
+		else if ((modifiers & Common::KBD_CTRL) && input.character > 0 && input.character < 27)
+			input.character += 96; // 0x01 -> 'a'
+	}
 	
+	// If no actual key was pressed (e.g. if only a modifier key was pressed),
+	// ignore the event
+	if (!input.character)
+		return noEvent;
+
 	return input;
 }
 
@@ -282,8 +286,7 @@ void EventManager::updateScreen() {
 }
 
 SciEvent EventManager::getSciEvent(unsigned int mask) {
-	//sci_event_t error_event = { SCI_EVT_ERROR, 0, 0, 0 };
-	SciEvent event = { 0, 0, 0, 0 };
+	SciEvent event = { 0, 0, 0, 0, Common::Point(0, 0) };
 
 	EventManager::updateScreen();
 
@@ -304,9 +307,8 @@ SciEvent EventManager::getSciEvent(unsigned int mask) {
 		event = *iter;
 
 		// If not peeking at the queue, remove the event
-		if (!(mask & SCI_EVENT_PEEK)) {
+		if (!(mask & SCI_EVENT_PEEK))
 			_events.erase(iter);
-		}
 	} else {
 		// No event found: we must return a SCI_EVT_NONE event.
 
@@ -314,29 +316,6 @@ SciEvent EventManager::getSciEvent(unsigned int mask) {
 		// there is no need to change it.
 	}
 
-	if (event.type == SCI_EVENT_KEYBOARD) {
-		// Do we still have to translate the key?
-
-		// When Ctrl AND Alt are pressed together with a regular key, Linux will give us control-key, Windows will give
-		//  us the actual key. My opinion is that windows is right, because under DOS the keys worked the same, anyway
-		//  we support the other case as well
-		if (event.modifiers & SCI_KEYMOD_ALT) {
-			if (event.character < 27)
-				event.character += 96; // 0x01 -> 'a'
-		}
-
-		if (getSciVersion() <= SCI_VERSION_1_MIDDLE) {
-			// TODO: find out if altify is also not needed for sci1late+, couldnt find any game that uses those keys
-			// Scancodify if appropriate
-			if (event.modifiers & SCI_KEYMOD_ALT) {
-				event.character = altify(event.character);
-			} else if (event.modifiers & SCI_KEYMOD_CTRL) {
-				if (event.character < 27)
-					event.character += 96; // 0x01 -> 'a'
-			}
-		}
-	}
-
 	return event;
 }
 
diff --git a/engines/sci/event.h b/engines/sci/event.h
index 7c83476294..be0322f2a4 100644
--- a/engines/sci/event.h
+++ b/engines/sci/event.h
@@ -27,6 +27,7 @@
 #define SCI_EVENT_H
 
 #include "common/list.h"
+#include "common/rect.h"
 
 namespace Sci {
 
@@ -46,6 +47,13 @@ struct SciEvent {
 	 * PC keyboard scancodes.
 	 */
 	short character;
+
+	/**
+	 * The mouse position at the time the event was created.
+	 *
+	 * These are display coordinates!
+	 */
+	Common::Point mousePos;
 };
 
 /*Values for type*/
@@ -56,11 +64,8 @@ struct SciEvent {
 #define SCI_EVENT_DIRECTION       (1<<6)
 #define SCI_EVENT_SAID            (1<<7)
 /*Fake values for other events*/
-#define SCI_EVENT_ERROR           (1<<10)
 #define SCI_EVENT_QUIT            (1<<11)
 #define SCI_EVENT_PEEK            (1<<15)
-/* The QUIT event may be used to signal an external 'quit' command being
-** issued to the gfx driver.  */
 #define SCI_EVENT_ANY             0x7fff
 
 /* Keycodes of special keys: */
@@ -121,7 +126,6 @@ private:
 	SciEvent getScummVMEvent();
 
 	const bool _fontIsExtended;
-	int _modifierStates;
 	Common::List<SciEvent> _events;
 };
 
diff --git a/engines/sci/graphics/menu.cpp b/engines/sci/graphics/menu.cpp
index b2e564c288..b0d12038e8 100644
--- a/engines/sci/graphics/menu.cpp
+++ b/engines/sci/graphics/menu.cpp
@@ -399,7 +399,6 @@ void GfxMenu::calculateMenuAndItemWidth() {
 reg_t GfxMenu::kernelSelect(reg_t eventObject, bool pauseSound) {
 	int16 eventType = readSelectorValue(_segMan, eventObject, SELECTOR(type));
 	int16 keyPress, keyModifier;
-	Common::Point mousePosition;
 	GuiMenuItemList::iterator itemIterator = _itemList.begin();
 	GuiMenuItemList::iterator itemEnd = _itemList.end();
 	GuiMenuItemEntry *itemEntry = NULL;
@@ -457,15 +456,17 @@ reg_t GfxMenu::kernelSelect(reg_t eventObject, bool pauseSound) {
 			itemEntry = NULL;
 		break;
 
-	case SCI_EVENT_MOUSE_PRESS:
-		mousePosition = _cursor->getPosition();
+	case SCI_EVENT_MOUSE_PRESS: {
+		Common::Point mousePosition;
+		mousePosition.x = readSelectorValue(_segMan, eventObject, SELECTOR(x));
+		mousePosition.y = readSelectorValue(_segMan, eventObject, SELECTOR(y));
 		if (mousePosition.y < 10) {
 			interactiveStart(pauseSound);
 			itemEntry = interactiveWithMouse();
 			interactiveEnd(pauseSound);
 			forceClaimed = true;
 		}
-		break;
+		} break;
 	}
 
 	if (!_menuSaveHandle.isNull()) {
@@ -715,7 +716,6 @@ GuiMenuItemEntry *GfxMenu::interactiveWithKeyboard() {
 	uint16 newItemId = _curItemId;
 	GuiMenuItemEntry *curItemEntry = findItem(_curMenuId, _curItemId);
 	GuiMenuItemEntry *newItemEntry = curItemEntry;
-	Common::Point mousePosition;
 
 	// We don't 100% follow Sierra here: we select last item instead of
 	// selecting first item of first menu every time. Also sierra sci didn't
@@ -793,9 +793,9 @@ GuiMenuItemEntry *GfxMenu::interactiveWithKeyboard() {
 			}
 			break;
 
-		case SCI_EVENT_MOUSE_PRESS:
-			mousePosition = _cursor->getPosition();
-			if (_cursor->getPosition().y < 10) {
+		case SCI_EVENT_MOUSE_PRESS: {
+			Common::Point mousePosition = curEvent.mousePos;
+			if (mousePosition.y < 10) {
 				// Somewhere on the menubar
 				newMenuId = mouseFindMenuSelection(mousePosition);
 				if (newMenuId) {
@@ -824,7 +824,8 @@ GuiMenuItemEntry *GfxMenu::interactiveWithKeyboard() {
 				}
 				newItemId = curItemEntry->id;
 			}
-			break;
+			} break;
+
 		case SCI_EVENT_NONE:
 			g_sci->sleep(2500 / 1000);
 			break;
@@ -840,7 +841,6 @@ GuiMenuItemEntry *GfxMenu::interactiveWithMouse() {
 	SciEvent curEvent;
 	uint16 newMenuId = 0, newItemId = 0;
 	uint16 curMenuId = 0, curItemId = 0;
-	Common::Point mousePosition = _cursor->getPosition();
 	bool firstMenuChange = true;
 	GuiMenuItemEntry *curItemEntry = NULL;
 
@@ -871,7 +871,7 @@ GuiMenuItemEntry *GfxMenu::interactiveWithMouse() {
 		}
 
 		// Find out where mouse is currently pointing to
-		mousePosition = _cursor->getPosition();
+		Common::Point mousePosition = curEvent.mousePos;
 		if (mousePosition.y < 10) {
 			// Somewhere on the menubar
 			newMenuId = mouseFindMenuSelection(mousePosition);
diff --git a/engines/sci/graphics/view.cpp b/engines/sci/graphics/view.cpp
index fd74714495..b99861319a 100644
--- a/engines/sci/graphics/view.cpp
+++ b/engines/sci/graphics/view.cpp
@@ -375,116 +375,138 @@ void unpackCelData(byte *inBuffer, byte *celBitmap, byte clearColor, int pixelCo
 	byte *outPtr = celBitmap;
 	byte curByte, runLength;
 	byte *rlePtr = inBuffer + rlePos;
+	// The existence of a literal position pointer signifies data with two
+	// separate streams, most likely a SCI1.1 view
 	byte *literalPtr = inBuffer + literalPos;
 	int pixelNr = 0;
 
 	memset(celBitmap, clearColor, pixelCount);
 
-	if (!literalPos) {
-		// decompression for data that has only one combined stream
-		switch (viewType) {
-		case kViewEga:
-			while (pixelNr < pixelCount) {
-				curByte = *rlePtr++;
-				runLength = curByte >> 4;
-				memset(outPtr + pixelNr, curByte & 0x0F, MIN<uint16>(runLength, pixelCount - pixelNr));
-				pixelNr += runLength;
+	// View unpacking:
+	//
+	// EGA:
+	// Each byte is like XXXXYYYY (XXXX: 0 - 15, YYYY: 0 - 15)
+	// Set the next XXXX pixels to YYYY
+	//
+	// Amiga:
+	// Each byte is like XXXXXYYY (XXXXX: 0 - 31, YYY: 0 - 7)
+	// - Case A: YYY != 0
+	//   Set the next YYY pixels to XXXXX
+	// - Case B: YYY == 0
+	//   Skip the next XXXXX pixels (i.e. transparency)
+	//
+	// Amiga 64:
+	// Each byte is like XXYYYYYY (XX: 0 - 3, YYYYYY: 0 - 63)
+	// - Case A: XX != 0
+	//   Set the next XX pixels to YYYYYY
+	// - Case B: XX == 0
+	//   Skip the next YYYYYY pixels (i.e. transparency)
+	//
+	// VGA:
+	// Each byte is like XXYYYYYY (YYYYY: 0 - 63)
+	// - Case A: XX == 00 (binary)
+	//   Copy next YYYYYY bytes as-is
+	// - Case B: XX == 01 (binary)
+	//   Same as above, copy YYYYYY + 64 bytes as-is
+	// - Case C: XX == 10 (binary)
+	//   Set the next YYYYY pixels to the next byte value
+	// - Case D: XX == 11 (binary)
+	//   Skip the next YYYYY pixels (i.e. transparency)
+
+	if (literalPos && isMacSci11ViewData) {
+		// KQ6/Freddy Pharkas use byte lengths, all others use uint16
+		// The SCI devs must have realized that a max of 255 pixels wide
+		// was not very good for 320 or 640 width games.
+		bool hasByteLengths = (g_sci->getGameId() == GID_KQ6 || g_sci->getGameId() == GID_FREDDYPHARKAS);
+
+		// compression for SCI1.1+ Mac
+		while (pixelNr < pixelCount) {
+			uint32 pixelLine = pixelNr;
+		
+			if (hasByteLengths) {
+				pixelNr += *rlePtr++;
+				runLength = *rlePtr++;
+			} else {
+				pixelNr += READ_BE_UINT16(rlePtr);
+				runLength = READ_BE_UINT16(rlePtr + 2);
+				rlePtr += 4;
 			}
-			break;
-		case kViewAmiga:
-			while (pixelNr < pixelCount) {
-				curByte = *rlePtr++;
-				if (curByte & 0x07) { // fill with color
-					runLength = curByte & 0x07;
-					curByte = curByte >> 3;
-					while (runLength-- && pixelNr < pixelCount)
-						outPtr[pixelNr++] = curByte;
-				} else { // fill with transparent
-					runLength = curByte >> 3;
-					pixelNr += runLength;
-				}
+
+			while (runLength-- && pixelNr < pixelCount)
+				outPtr[pixelNr++] = *literalPtr++;
+
+			pixelNr = pixelLine + width;
+		}
+		return;
+	}
+
+	switch (viewType) {
+	case kViewEga:
+		while (pixelNr < pixelCount) {
+			curByte = *rlePtr++;
+			runLength = curByte >> 4;
+			memset(outPtr + pixelNr,        curByte & 0x0F, MIN<uint16>(runLength, pixelCount - pixelNr));
+			pixelNr += runLength;
+		}
+		break;
+	case kViewAmiga:
+		while (pixelNr < pixelCount) {
+			curByte = *rlePtr++;
+			if (curByte & 0x07) { // fill with color
+				runLength = curByte & 0x07;
+				curByte = curByte >> 3;
+				memset(outPtr + pixelNr,           curByte, MIN<uint16>(runLength, pixelCount - pixelNr));
+			} else { // skip the next pixels (transparency)
+				runLength = curByte >> 3;
 			}
-			break;
-		case kViewAmiga64:
-			// TODO: This isn't 100% right. Implement it fully.
-			while (pixelNr < pixelCount) {
-				curByte = *rlePtr++;
+			pixelNr += runLength;
+		}
+		break;
+	case kViewAmiga64:
+		while (pixelNr < pixelCount) {
+			curByte = *rlePtr++;
+			if (curByte & 0xC0) { // fill with color
 				runLength = curByte >> 6;
-				memset(outPtr + pixelNr, curByte & 0x3F, MIN<uint16>(runLength, pixelCount - pixelNr));
-				pixelNr += runLength;
-			}
-			break;
-		case kViewVga:
-		case kViewVga11:
-			while (pixelNr < pixelCount) {
-				curByte = *rlePtr++;
+				memset(outPtr + pixelNr,    curByte & 0x3F, MIN<uint16>(runLength, pixelCount - pixelNr));
+			} else { // skip the next pixels (transparency)
 				runLength = curByte & 0x3F;
-				switch (curByte & 0xC0) {
-				case 0: // copy bytes as-is
-					while (runLength-- && pixelNr < pixelCount)
-						outPtr[pixelNr++] = *rlePtr++;
-					break;
-				case 0x40: // copy bytes as is (In copy case, runLength can go upto 127 i.e. pixel & 0x40). Fixes bug #3135872.
-					runLength += 64;
-					break;
-				case 0x80: // fill with color
-					memset(outPtr + pixelNr, *rlePtr++, MIN<uint16>(runLength, pixelCount - pixelNr));
-					pixelNr += runLength;
-					break;
-				case 0xC0: // fill with transparent
-					pixelNr += runLength;
-					break;
-				}
 			}
-			break;
-		default:
-			error("Unsupported picture viewtype");
+			pixelNr += runLength;
 		}
-	} else {
-		// decompression for data that has two separate streams (probably a SCI 1.1 view)
-		if (isMacSci11ViewData) {
-			// KQ6/Freddy Pharkas use byte lengths, all others use uint16
-			// The SCI devs must have realized that a max of 255 pixels wide
-			// was not very good for 320 or 640 width games.
-			bool hasByteLengths = (g_sci->getGameId() == GID_KQ6 || g_sci->getGameId() == GID_FREDDYPHARKAS);
-
-			// compression for SCI1.1+ Mac
-			while (pixelNr < pixelCount) {
-				uint32 pixelLine = pixelNr;
-		
-				if (hasByteLengths) {
-					pixelNr += *rlePtr++;
-					runLength = *rlePtr++;
+		break;
+	case kViewVga:
+	case kViewVga11:
+		while (pixelNr < pixelCount) {
+			curByte = *rlePtr++;
+			runLength = curByte & 0x3F;
+
+			switch (curByte & 0xC0) {
+			case 0x40: // copy bytes as is (In copy case, runLength can go up to 127 i.e. pixel & 0x40). Fixes bug #3135872.
+				runLength += 64;
+			case 0x00: // copy bytes as-is
+				if (!literalPos) {
+					memcpy(outPtr + pixelNr,        rlePtr, MIN<uint16>(runLength, pixelCount - pixelNr));
+					rlePtr += runLength;
 				} else {
-					pixelNr += READ_BE_UINT16(rlePtr);
-					runLength = READ_BE_UINT16(rlePtr + 2);
-					rlePtr += 4;
-				}
-
-				while (runLength-- && pixelNr < pixelCount)
-					outPtr[pixelNr++] = *literalPtr++;
-
-				pixelNr = pixelLine + width;
-			}
-		} else {
-			while (pixelNr < pixelCount) {
-				curByte = *rlePtr++;
-				runLength = curByte & 0x3F;
-				switch (curByte & 0xC0) {
-				case 0: // copy bytes as-is
-					while (runLength-- && pixelNr < pixelCount)
-						outPtr[pixelNr++] = *literalPtr++;
-					break;
-				case 0x80: // fill with color
+					memcpy(outPtr + pixelNr,    literalPtr, MIN<uint16>(runLength, pixelCount - pixelNr));
+					literalPtr += runLength;
+				}			
+				break;
+			case 0x80: // fill with color
+				if (!literalPos)
+					memset(outPtr + pixelNr,     *rlePtr++, MIN<uint16>(runLength, pixelCount - pixelNr));
+				else
 					memset(outPtr + pixelNr, *literalPtr++, MIN<uint16>(runLength, pixelCount - pixelNr));
-					pixelNr += runLength;
-					break;
-				case 0xC0: // fill with transparent
-					pixelNr += runLength;
-					break;
-				}
+				break;
+			case 0xC0: // skip the next pixels (transparency)
+				break;
 			}
+
+			pixelNr += runLength;
 		}
+		break;
+	default:
+		error("Unsupported picture viewtype");
 	}
 }