[Scummvm-git-logs] scummvm master -> 12d24d5b46cc4017b159040da6f42c9cfde5cbf7

csnover csnover at users.noreply.github.com
Fri Jul 7 02:15:24 CEST 2017


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

Summary:
332fabcb8a SDL: Only recreate SDL2 window when necessary
a689fee663 SCI32: Speed up & deduplicate palette submission code
f40ea8c2e7 SCI32: Stop setting unused palette timestamp property
9f910535c9 SCI32: Centralise OSystem screen updates
1fbee2f51e SCI32: Allow skipping SEQ animations
7ce2e4cf08 GRAPHICS: Allow nearest neighbor scaling of 1Bpp and 2Bpp TransparentSurfaces
798c6bf34d COMMON: Add yet another GUIO option flag
1466fb247e SCI32: Add workaround for SQ6
35346bc71b SCI32: Update mouse position for rendering in all frameOuts
f7fcce24e1 SCI32: Remove unused method declaration
c6f5840196 SCI32: Remove magic numbers in HunkPalette
2d6fe2b8cd SCI32: Work around bogus palette entries in select Windows games
e9bef89646 SCI32: Remove useless call
8cb35442c0 SCI32: Improve kShowMovieWin (AVI) rendering
7057f232d7 SCI32: Improve kPlayVMD rendering
71256a0d3c SCI32: Improve playback quality of SEQ videos
f15f9e3b7c SCI32: Refactor Video32 code to reduce code & feature duplication
3f0e061eaa SCI32: Refactor DuckPlayer to use common video playback code
12d24d5b46 SCI32: Fix bad palette entries when built without USE_RGB_COLOR


Commit: 332fabcb8a0ff3957f0198ad3b93b4f3861f7eba
    https://github.com/scummvm/scummvm/commit/332fabcb8a0ff3957f0198ad3b93b4f3861f7eba
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:11:54-05:00

Commit Message:
SDL: Only recreate SDL2 window when necessary

Destroying and recreating the SDL window whenever the video mode
changes in SDL2 is not necessary and causes several problems:

1. In windowed mode, the game window shifts position;
2. In fullscreen mode in macOS, every time the window is
   recreated, it causes the OS to play its switch-to-fullscreen
   animation again and emit system alert noises;
3. The window content flickers; and
4. The engine loses events from the old destroyed window.

This patch changes the SDL backend code to avoid destroying and
recreating the SDL window when using SDL2, except when switching
OpenGL modes, since there is no way to change the OpenGL feature
of a window.

There are still some outstanding issues with OpenGL where window
size ends up getting reset even though the user has resized it;
this will probably need to be addressed at some point in another
patch.

Thanks to @bgK and @criezy for their feedback which made this
patch much better.

Co-Authored-By: Bastien Bouclet <bastien.bouclet at gmail.com>

Changed paths:
    backends/graphics/openglsdl/openglsdl-graphics.cpp
    backends/graphics/surfacesdl/surfacesdl-graphics.cpp
    backends/graphics/surfacesdl/surfacesdl-graphics.h
    backends/platform/sdl/sdl-window.cpp
    backends/platform/sdl/sdl-window.h


diff --git a/backends/graphics/openglsdl/openglsdl-graphics.cpp b/backends/graphics/openglsdl/openglsdl-graphics.cpp
index e411480..f664314 100644
--- a/backends/graphics/openglsdl/openglsdl-graphics.cpp
+++ b/backends/graphics/openglsdl/openglsdl-graphics.cpp
@@ -463,10 +463,6 @@ bool OpenGLSdlGraphicsManager::setupMode(uint width, uint height) {
 		// Remember our choice.
 		ConfMan.setInt("last_fullscreen_mode_width", _desiredFullscreenWidth, Common::ConfigManager::kApplicationDomain);
 		ConfMan.setInt("last_fullscreen_mode_height", _desiredFullscreenHeight, Common::ConfigManager::kApplicationDomain);
-
-		// Use our choice.
-		width  = _desiredFullscreenWidth;
-		height = _desiredFullscreenHeight;
 	}
 
 	// This is pretty confusing since RGBA8888 talks about the memory
@@ -489,13 +485,23 @@ bool OpenGLSdlGraphicsManager::setupMode(uint width, uint height) {
 		_glContext = nullptr;
 	}
 
-	_window->destroyWindow();
-
-	uint32 flags = SDL_WINDOW_OPENGL;
+	uint32 flags = SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE;
 	if (_wantsFullScreen) {
+		// On Linux/X11, when toggling to fullscreen, the window manager saves
+		// the window size to be able to restore it when going back to windowed mode.
+		// If the user configured ScummVM to start in fullscreen mode, we first
+		// create a window and then toggle it to fullscreen to give the window manager
+		// a chance to save the window size. That way if the user switches back
+		// to windowed mode, the window manager has a window size to apply instead
+		// of leaving the window at the fullscreen resolution size.
+		if (!_window->getSDLWindow()) {
+			_window->createOrUpdateWindow(width, height, flags);
+		}
+
+		width  = _desiredFullscreenWidth;
+		height = _desiredFullscreenHeight;
+
 		flags |= SDL_WINDOW_FULLSCREEN;
-	} else {
-		flags |= SDL_WINDOW_RESIZABLE;
 	}
 
 	// Request a OpenGL (ES) context we can use.
@@ -503,11 +509,11 @@ bool OpenGLSdlGraphicsManager::setupMode(uint width, uint height) {
 	SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, _glContextMajor);
 	SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, _glContextMinor);
 
-	if (!_window->createWindow(width, height, flags)) {
+	if (!_window->createOrUpdateWindow(width, height, flags)) {
 		// We treat fullscreen requests as a "hint" for now. This means in
 		// case it is not available we simply ignore it.
 		if (_wantsFullScreen) {
-			_window->createWindow(width, height, SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE);
+			_window->createOrUpdateWindow(width, height, SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE);
 		}
 
 		if (!_window->getSDLWindow()) {
@@ -541,6 +547,8 @@ bool OpenGLSdlGraphicsManager::setupMode(uint width, uint height) {
 
 	uint32 flags = SDL_OPENGL;
 	if (_wantsFullScreen) {
+		width  = _desiredFullscreenWidth;
+		height = _desiredFullscreenHeight;
 		flags |= SDL_FULLSCREEN;
 	} else {
 		flags |= SDL_RESIZABLE;
diff --git a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
index 9594587..2e658bc 100644
--- a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
+++ b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
@@ -214,6 +214,10 @@ SurfaceSdlGraphicsManager::SurfaceSdlGraphicsManager(SdlEventSource *sdlEventSou
 
 SurfaceSdlGraphicsManager::~SurfaceSdlGraphicsManager() {
 	unloadGFXMode();
+#if SDL_VERSION_ATLEAST(2, 0, 0)
+	if (_window)
+		_window->destroyWindow();
+#endif
 	if (_mouseSurface)
 		SDL_FreeSurface(_mouseSurface);
 	_mouseSurface = 0;
@@ -2644,13 +2648,11 @@ void SurfaceSdlGraphicsManager::notifyVideoExpose() {
 	_forceFull = true;
 }
 
-#ifdef USE_SDL_RESIZABLE_WINDOW
 void SurfaceSdlGraphicsManager::notifyResize(const uint width, const uint height) {
 #if SDL_VERSION_ATLEAST(2, 0, 0)
 	setWindowResolution(width, height);
 #endif
 }
-#endif
 
 void SurfaceSdlGraphicsManager::transformMouseCoordinates(Common::Point &point) {
 #if SDL_VERSION_ATLEAST(2, 0, 0)
@@ -2683,9 +2685,6 @@ void SurfaceSdlGraphicsManager::deinitializeRenderer() {
 
 	SDL_DestroyRenderer(_renderer);
 	_renderer = nullptr;
-
-	if (_window)
-		_window->destroyWindow();
 }
 
 void SurfaceSdlGraphicsManager::setWindowResolution(int width, int height) {
@@ -2746,7 +2745,7 @@ SDL_Surface *SurfaceSdlGraphicsManager::SDL_SetVideoMode(int width, int height,
 		createWindowFlags |= SDL_WINDOW_FULLSCREEN_DESKTOP;
 	}
 
-	if (!_window->createWindow(width, height, createWindowFlags)) {
+	if (!_window->createOrUpdateWindow(width, height, createWindowFlags)) {
 		return nullptr;
 	}
 
diff --git a/backends/graphics/surfacesdl/surfacesdl-graphics.h b/backends/graphics/surfacesdl/surfacesdl-graphics.h
index fa130cd..532399e 100644
--- a/backends/graphics/surfacesdl/surfacesdl-graphics.h
+++ b/backends/graphics/surfacesdl/surfacesdl-graphics.h
@@ -156,9 +156,7 @@ public:
 
 	// SdlGraphicsManager interface
 	virtual void notifyVideoExpose();
-#ifdef USE_SDL_RESIZABLE_WINDOW
 	virtual void notifyResize(const uint width, const uint height);
-#endif
 	virtual void transformMouseCoordinates(Common::Point &point);
 	virtual void notifyMousePos(Common::Point mouse);
 
diff --git a/backends/platform/sdl/sdl-window.cpp b/backends/platform/sdl/sdl-window.cpp
index 609186a..543320f 100644
--- a/backends/platform/sdl/sdl-window.cpp
+++ b/backends/platform/sdl/sdl-window.cpp
@@ -28,9 +28,12 @@
 
 #include "icons/scummvm.xpm"
 
+static const uint32 fullscreenMask = SDL_WINDOW_FULLSCREEN_DESKTOP | SDL_WINDOW_FULLSCREEN;
+
 SdlWindow::SdlWindow()
 #if SDL_VERSION_ATLEAST(2, 0, 0)
-	: _window(nullptr), _inputGrabState(false), _windowCaption("ScummVM")
+	: _window(nullptr), _inputGrabState(false), _windowCaption("ScummVM"),
+	_lastFlags(0), _lastX(SDL_WINDOWPOS_UNDEFINED), _lastY(SDL_WINDOWPOS_UNDEFINED)
 #endif
 	{
 }
@@ -199,25 +202,67 @@ SDL_Surface *copySDLSurface(SDL_Surface *src) {
 	return res;
 }
 
-bool SdlWindow::createWindow(int width, int height, uint32 flags) {
-	destroyWindow();
-
+bool SdlWindow::createOrUpdateWindow(int width, int height, uint32 flags) {
 	if (_inputGrabState) {
 		flags |= SDL_WINDOW_INPUT_GRABBED;
 	}
 
-	_window = SDL_CreateWindow(_windowCaption.c_str(), SDL_WINDOWPOS_UNDEFINED,
-	                           SDL_WINDOWPOS_UNDEFINED, width, height, flags);
+	// SDL_WINDOW_RESIZABLE can also be updated without recreating the window
+	// starting with SDL 2.0.5, but it is not treated as updateable here
+	// because:
+	// 1. It is currently only changed in conjunction with the SDL_WINDOW_OPENGL
+	//    flag, so the window will always be recreated anyway when changing
+	//    resizability; and
+	// 2. Users (particularly on Windows) will sometimes swap older SDL DLLs
+	//    to avoid bugs, which would be impossible if the feature was enabled
+	//    at compile time using SDL_VERSION_ATLEAST.
+	const uint32 updateableFlagsMask = fullscreenMask | SDL_WINDOW_INPUT_GRABBED;
+
+	const uint32 oldNonUpdateableFlags = _lastFlags & ~updateableFlagsMask;
+	const uint32 newNonUpdateableFlags = flags & ~updateableFlagsMask;
+
+	if (!_window || oldNonUpdateableFlags != newNonUpdateableFlags) {
+		destroyWindow();
+		_window = SDL_CreateWindow(_windowCaption.c_str(), _lastX,
+								   _lastY, width, height, flags);
+		if (_window) {
+			setupIcon();
+		}
+	} else {
+		const uint32 fullscreenFlags = flags & fullscreenMask;
+
+		if (fullscreenFlags) {
+			SDL_DisplayMode fullscreenMode;
+			fullscreenMode.w = width;
+			fullscreenMode.h = height;
+			fullscreenMode.driverdata = nullptr;
+			fullscreenMode.format = 0;
+			fullscreenMode.refresh_rate = 0;
+			SDL_SetWindowDisplayMode(_window, &fullscreenMode);
+		} else {
+			SDL_SetWindowSize(_window, width, height);
+		}
+
+		SDL_SetWindowFullscreen(_window, fullscreenFlags);
+		SDL_SetWindowGrab(_window, (flags & SDL_WINDOW_INPUT_GRABBED) ? SDL_TRUE : SDL_FALSE);
+	}
+
 	if (!_window) {
 		return false;
 	}
-	setupIcon();
+
+	_lastFlags = flags;
 
 	return true;
 }
 
 void SdlWindow::destroyWindow() {
-	SDL_DestroyWindow(_window);
-	_window = nullptr;
+	if (_window) {
+		if (!(_lastFlags & fullscreenMask)) {
+			SDL_GetWindowPosition(_window, &_lastX, &_lastY);
+		}
+		SDL_DestroyWindow(_window);
+		_window = nullptr;
+	}
 }
 #endif
diff --git a/backends/platform/sdl/sdl-window.h b/backends/platform/sdl/sdl-window.h
index b628609..d75e811 100644
--- a/backends/platform/sdl/sdl-window.h
+++ b/backends/platform/sdl/sdl-window.h
@@ -81,14 +81,14 @@ public:
 	SDL_Window *getSDLWindow() const { return _window; }
 
 	/**
-	 * Creates a new SDL window (and destroys the old one).
+	 * Creates or updates the SDL window.
 	 *
 	 * @param width   Width of the window.
 	 * @param height  Height of the window.
 	 * @param flags   SDL flags passed to SDL_CreateWindow
 	 * @return true on success, false otherwise
 	 */
-	bool createWindow(int width, int height, uint32 flags);
+	bool createOrUpdateWindow(int width, int height, uint32 flags);
 
 	/**
 	 * Destroys the current SDL window.
@@ -99,6 +99,15 @@ protected:
 	SDL_Window *_window;
 
 private:
+	uint32 _lastFlags;
+
+	/**
+	 * Switching between software and OpenGL modes requires the window to be
+	 * destroyed and recreated. These properties store the position of the last
+	 * window so the new window will be created in the same place.
+	 */
+	int _lastX, _lastY;
+
 	bool _inputGrabState;
 	Common::String _windowCaption;
 #endif


Commit: a689fee66320048824424c9b9f557e72538b8fb8
    https://github.com/scummvm/scummvm/commit/a689fee66320048824424c9b9f557e72538b8fb8
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:35-05:00

Commit Message:
SCI32: Speed up & deduplicate palette submission code

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


diff --git a/engines/sci/graphics/palette32.cpp b/engines/sci/graphics/palette32.cpp
index de85b71..3667e1f 100644
--- a/engines/sci/graphics/palette32.cpp
+++ b/engines/sci/graphics/palette32.cpp
@@ -415,12 +415,19 @@ int16 GfxPalette32::matchColor(const uint8 r, const uint8 g, const uint8 b) {
 }
 
 void GfxPalette32::submit(const Palette &palette) {
-	const Palette oldSourcePalette(_sourcePalette);
-	mergePalette(_sourcePalette, palette);
+	// If `_needsUpdate` is already set, there is no need to test whether
+	// this palette submission causes a change to `_sourcePalette` since it is
+	// going to be updated already anyway
+	if (_needsUpdate) {
+		mergePalette(_sourcePalette, palette);
+	} else {
+		const Palette oldSourcePalette(_sourcePalette);
+		mergePalette(_sourcePalette, palette);
 
-	if (!_needsUpdate && _sourcePalette != oldSourcePalette) {
-		++_version;
-		_needsUpdate = true;
+		if (_sourcePalette != oldSourcePalette) {
+			++_version;
+			_needsUpdate = true;
+		}
 	}
 }
 
@@ -429,15 +436,7 @@ void GfxPalette32::submit(const HunkPalette &hunkPalette) {
 		return;
 	}
 
-	const Palette oldSourcePalette(_sourcePalette);
-	const Palette palette = hunkPalette.toPalette();
-	mergePalette(_sourcePalette, palette);
-
-	if (!_needsUpdate && oldSourcePalette != _sourcePalette) {
-		++_version;
-		_needsUpdate = true;
-	}
-
+	submit(hunkPalette.toPalette());
 	hunkPalette.setVersion(_version);
 }
 


Commit: f40ea8c2e7380b512042ab171be45eec19962b8f
    https://github.com/scummvm/scummvm/commit/f40ea8c2e7380b512042ab171be45eec19962b8f
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:35-05:00

Commit Message:
SCI32: Stop setting unused palette timestamp property

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


diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index da2a199..8b804cf 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -866,7 +866,6 @@ void VMDPlayer::renderFrame() const {
 	const bool dirtyPalette = _decoder->hasDirtyPalette();
 	if (dirtyPalette && !_ignorePalettes) {
 		Palette palette;
-		palette.timestamp = g_sci->getTickCount();
 		for (uint16 i = 0; i < _startColor; ++i) {
 			palette.colors[i].used = false;
 		}


Commit: 9f910535c9d358262b68ad9200e041a9a6d77ce5
    https://github.com/scummvm/scummvm/commit/9f910535c9d358262b68ad9200e041a9a6d77ce5
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:35-05:00

Commit Message:
SCI32: Centralise OSystem screen updates

Changed paths:
    engines/sci/engine/kevent.cpp
    engines/sci/graphics/controls32.cpp
    engines/sci/graphics/frameout.cpp
    engines/sci/graphics/frameout.h
    engines/sci/graphics/transitions32.cpp
    engines/sci/graphics/video32.cpp
    engines/sci/sci.cpp


diff --git a/engines/sci/engine/kevent.cpp b/engines/sci/engine/kevent.cpp
index 389277b..b240c94 100644
--- a/engines/sci/engine/kevent.cpp
+++ b/engines/sci/engine/kevent.cpp
@@ -101,7 +101,7 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 		// one of these ugly loops and should be updating the screen &
 		// throttling the VM.
 		if (++s->_eventCounter > 2) {
-			g_system->updateScreen();
+			g_sci->_gfxFrameout->updateScreen();
 			s->speedThrottler(10); // 10ms is an arbitrary value
 			s->_throttleTrigger = true;
 		}
diff --git a/engines/sci/graphics/controls32.cpp b/engines/sci/graphics/controls32.cpp
index 0cd924b..7080c10 100644
--- a/engines/sci/graphics/controls32.cpp
+++ b/engines/sci/graphics/controls32.cpp
@@ -299,7 +299,6 @@ reg_t GfxControls32::kernelEditText(const reg_t controlObject) {
 		}
 
 		g_sci->_gfxFrameout->frameOut(true);
-		g_sci->getSciDebugger()->onFrame();
 		g_sci->_gfxFrameout->throttle();
 	}
 
diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index c969f91..390cc81 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -68,7 +68,8 @@ GfxFrameout::GfxFrameout(SegManager *segMan, GfxPalette32 *palette, GfxTransitio
 	_throttleState(0),
 	_remapOccurred(false),
 	_overdrawThreshold(0),
-	_palMorphIsOn(false) {
+	_palMorphIsOn(false),
+	_lastScreenUpdateTick(0) {
 
 	if (g_sci->getGameId() == GID_PHANTASMAGORIA) {
 		_currentBuffer = Buffer(630, 450, nullptr);
@@ -1008,7 +1009,7 @@ void GfxFrameout::mergeToShowList(const Common::Rect &drawRect, RectList &showLi
 
 void GfxFrameout::showBits() {
 	if (!_showList.size()) {
-		g_system->updateScreen();
+		updateScreen();
 		return;
 	}
 
@@ -1050,7 +1051,7 @@ void GfxFrameout::showBits() {
 	_cursor->donePainting();
 
 	_showList.clear();
-	g_system->updateScreen();
+	updateScreen();
 }
 
 void GfxFrameout::alterVmap(const Palette &palette1, const Palette &palette2, const int8 style, const int8 *const styleRanges) {
@@ -1123,6 +1124,20 @@ void GfxFrameout::alterVmap(const Palette &palette1, const Palette &palette2, co
 	}
 }
 
+void GfxFrameout::updateScreen(const int delta) {
+	// using OSystem::getMillis instead of Sci::getTickCount because these
+	// values need to be monotonically increasing for the duration of the
+	// GfxFrameout object or else the screen will stop updating
+	const uint32 now = g_system->getMillis() * 60 / 1000;
+	if (now <= _lastScreenUpdateTick + delta) {
+		return;
+	}
+
+	_lastScreenUpdateTick = now;
+	g_system->updateScreen();
+	g_sci->getSciDebugger()->onFrame();
+}
+
 void GfxFrameout::kernelFrameOut(const bool shouldShowBits) {
 	if (_transitions->hasShowStyles()) {
 		_transitions->processShowStyles();
@@ -1166,14 +1181,14 @@ void GfxFrameout::shakeScreen(int16 numShakes, const ShakeDirection direction) {
 			g_system->setShakePos(_isHiRes ? 8 : 4);
 		}
 
-		g_system->updateScreen();
+		updateScreen();
 		g_sci->getEngineState()->wait(3);
 
 		if (direction & kShakeVertical) {
 			g_system->setShakePos(0);
 		}
 
-		g_system->updateScreen();
+		updateScreen();
 		g_sci->getEngineState()->wait(3);
 	}
 }
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index e706f14..ab58f41 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -155,6 +155,11 @@ public:
 #pragma mark -
 #pragma mark Rendering
 private:
+	/**
+	 * The last time the hardware screen was updated.
+	 */
+	uint32 _lastScreenUpdateTick;
+
 	GfxTransitions32 *_transitions;
 
 	/**
@@ -255,8 +260,8 @@ private:
 	void mergeToShowList(const Common::Rect &drawRect, RectList &showList, const int overdrawThreshold);
 
 	/**
-	 * Writes the internal frame buffer out to hardware and
-	 * clears the show list.
+	 * Sends all dirty rects from the internal frame buffer to the backend,
+	 * then updates the hardware screen.
 	 */
 	void showBits();
 
@@ -285,6 +290,18 @@ private:
 	}
 
 public:
+	/**
+	 * Updates the hardware screen, no more than once per tick.
+	 *
+	 * @param delta An additional number of ticks that should elapse
+	 * since the last time the screen was updated before it gets updated now.
+	 * This is used for updating the screen within run_vm, where we normally
+	 * expect that a call to kFrameOut will occur later during the current
+	 * frame, but if it does not, then update the screen on the second frame
+	 * anyway since the game is doing something bad.
+	 */
+	void updateScreen(const int delta = 0);
+
 	void setPixelFormat(const Graphics::PixelFormat &format) const {
 		initGraphics(_currentBuffer.screenWidth, _currentBuffer.screenHeight, _isHiRes, &format);
 	}
diff --git a/engines/sci/graphics/transitions32.cpp b/engines/sci/graphics/transitions32.cpp
index 6da1772..b5e4985 100644
--- a/engines/sci/graphics/transitions32.cpp
+++ b/engines/sci/graphics/transitions32.cpp
@@ -83,7 +83,6 @@ void GfxTransitions32::addShowRect(const Common::Rect &rect) {
 
 void GfxTransitions32::sendShowRects() {
 	g_sci->_gfxFrameout->showBits();
-	g_sci->getSciDebugger()->onFrame();
 	clearShowRects();
 	throttle();
 }
@@ -126,7 +125,6 @@ void GfxTransitions32::processShowStyles() {
 
 		if (doFrameOut) {
 			g_sci->_gfxFrameout->frameOut(true);
-			g_sci->getSciDebugger()->onFrame();
 			throttle();
 		}
 	} while(continueProcessing && doFrameOut);
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index 8b804cf..f4b1551 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -145,7 +145,6 @@ void SEQPlayer::renderFrame(SciBitmap &bitmap) const {
 
 	g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
 	g_sci->_gfxFrameout->frameOut(true);
-	g_sci->getSciDebugger()->onFrame();
 }
 
 #pragma mark -
@@ -419,7 +418,6 @@ void AVIPlayer::renderFrame() const {
 
 		g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
 		g_sci->_gfxFrameout->frameOut(true);
-		g_sci->getSciDebugger()->onFrame();
 	} else {
 		assert(surface->format.bytesPerPixel == 4);
 
@@ -455,8 +453,7 @@ void AVIPlayer::renderFrame() const {
 			g_system->copyRectToScreen(surface->getPixels(), surface->pitch, drawRect.left, drawRect.top, surface->w, surface->h);
 		}
 
-		g_system->updateScreen();
-		g_sci->getSciDebugger()->onFrame();
+		g_sci->_gfxFrameout->updateScreen();
 	}
 }
 
@@ -885,7 +882,6 @@ void VMDPlayer::renderFrame() const {
 		g_sci->_gfxPalette32->submit(palette);
 		g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
 		g_sci->_gfxFrameout->frameOut(true);
-		g_sci->getSciDebugger()->onFrame();
 
 #if SCI_VMD_BLACK_PALETTE
 		if (_blackPalette) {
@@ -898,7 +894,6 @@ void VMDPlayer::renderFrame() const {
 	} else {
 		g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
 		g_sci->_gfxFrameout->frameOut(true);
-		g_sci->getSciDebugger()->onFrame();
 	}
 }
 
@@ -1164,8 +1159,7 @@ void DuckPlayer::renderFrame() const {
 		g_system->copyRectToScreen(surface->getPixels(), surface->pitch, _drawRect.left, _drawRect.top, surface->w, surface->h);
 	}
 
-	g_system->updateScreen();
-	g_sci->getSciDebugger()->onFrame();
+	g_sci->_gfxFrameout->updateScreen();
 }
 
 } // End of namespace Sci
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index a84f50e..634652c 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -849,7 +849,7 @@ void SciEngine::sleep(uint32 msecs) {
 		// movement is still occurring and the screen needs to be updated to
 		// reflect it
 		if (getSciVersion() >= SCI_VERSION_2) {
-			g_system->updateScreen();
+			g_sci->_gfxFrameout->updateScreen();
 		}
 #endif
 		time = g_system->getMillis();


Commit: 1fbee2f51ef8f1027a638d1a9895fc859568fa4a
    https://github.com/scummvm/scummvm/commit/1fbee2f51ef8f1027a638d1a9895fc859568fa4a
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:36-05:00

Commit Message:
SCI32: Allow skipping SEQ animations

In SSCI, SEQ animations cannot be skipped.

Changed paths:
    engines/sci/graphics/video32.cpp
    engines/sci/graphics/video32.h


diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index f4b1551..fc35d40 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -71,8 +71,9 @@ static bool flushEvents(EventManager *eventMan) {
 
 #pragma mark SEQPlayer
 
-SEQPlayer::SEQPlayer(SegManager *segMan) :
+SEQPlayer::SEQPlayer(SegManager *segMan, EventManager *eventMan) :
 	_segMan(segMan),
+	_eventMan(eventMan),
 	_decoder(nullptr),
 	_plane(nullptr),
 	_screenItem(nullptr) {}
@@ -114,7 +115,32 @@ void SEQPlayer::play(const Common::String &fileName, const int16 numTicks, const
 
 	while (!g_engine->shouldQuit() && !_decoder->endOfVideo()) {
 		g_sci->sleep(_decoder->getTimeToNextFrame());
-		renderFrame(bitmap);
+
+		while (_decoder->needsUpdate()) {
+			renderFrame(bitmap);
+		}
+
+		// SSCI did not allow SEQ animations to be bypassed like this
+		SciEvent event = _eventMan->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
+		if (event.type == SCI_EVENT_MOUSE_PRESS) {
+			break;
+		}
+
+		event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_PEEK);
+		if (event.type == SCI_EVENT_KEYBOARD) {
+			bool stop = false;
+			while ((event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD)),
+				   event.type != SCI_EVENT_NONE) {
+				if (event.character == SCI_KEY_ESC) {
+					stop = true;
+					break;
+				}
+			}
+
+			if (stop) {
+				break;
+			}
+		}
 	}
 
 	_segMan->freeBitmap(bitmapId);
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index 7b71b37..41e3016 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -50,7 +50,7 @@ struct Palette;
  */
 class SEQPlayer {
 public:
-	SEQPlayer(SegManager *segMan);
+	SEQPlayer(SegManager *segMan, EventManager *eventMan);
 
 	/**
 	 * Plays a SEQ animation with the given
@@ -61,6 +61,7 @@ public:
 
 private:
 	SegManager *_segMan;
+	EventManager *_eventMan;
 	SEQDecoder *_decoder;
 
 	/**
@@ -655,7 +656,7 @@ private:
 class Video32 : public Common::Serializable {
 public:
 	Video32(SegManager *segMan, EventManager *eventMan) :
-	_SEQPlayer(segMan),
+	_SEQPlayer(segMan, eventMan),
 	_AVIPlayer(segMan, eventMan),
 	_VMDPlayer(segMan, eventMan),
 	_robotPlayer(segMan),


Commit: 7ce2e4cf080d9a328be6d1b2f8594cbb52159c28
    https://github.com/scummvm/scummvm/commit/7ce2e4cf080d9a328be6d1b2f8594cbb52159c28
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:36-05:00

Commit Message:
GRAPHICS: Allow nearest neighbor scaling of 1Bpp and 2Bpp TransparentSurfaces

This is one small step toward allowing more shared usage of
existing scaling code.

Changed paths:
    graphics/transparent_surface.cpp
    graphics/transparent_surface.h


diff --git a/graphics/transparent_surface.cpp b/graphics/transparent_surface.cpp
index 48e6da6..83869cef 100644
--- a/graphics/transparent_surface.cpp
+++ b/graphics/transparent_surface.cpp
@@ -793,21 +793,17 @@ TransparentSurface *TransparentSurface::rotoscaleT(const TransformStruct &transf
 template <TFilteringMode filteringMode>
 TransparentSurface *TransparentSurface::scaleT(uint16 newWidth, uint16 newHeight) const {
 
-	Common::Rect srcRect(0, 0, (int16)w, (int16)h);
-	Common::Rect dstRect(0, 0, (int16)newWidth, (int16)newHeight);
-
 	TransparentSurface *target = new TransparentSurface();
 
-	assert(format.bytesPerPixel == 4);
-
-	int srcW = srcRect.width();
-	int srcH = srcRect.height();
-	int dstW = dstRect.width();
-	int dstH = dstRect.height();
+	int srcW = w;
+	int srcH = h;
+	int dstW = newWidth;
+	int dstH = newHeight;
 
-	target->create((uint16)dstW, (uint16)dstH, this->format);
+	target->create((uint16)dstW, (uint16)dstH, format);
 
 	if (filteringMode == FILTER_BILINEAR) {
+		assert(format.bytesPerPixel == 4);
 
 		bool flipx = false, flipy = false; // TODO: See mirroring comment in RenderTicket ctor
 
@@ -954,25 +950,29 @@ TransparentSurface *TransparentSurface::scaleT(uint16 newWidth, uint16 newHeight
 		delete[] say;
 
 	} else {
-
 		int *scaleCacheX = new int[dstW];
 		for (int x = 0; x < dstW; x++) {
 			scaleCacheX[x] = (x * srcW) / dstW;
 		}
 
-		for (int y = 0; y < dstH; y++) {
-			uint32 *destP = (uint32 *)target->getBasePtr(0, y);
-			const uint32 *srcP = (const uint32 *)getBasePtr(0, (y * srcH) / dstH);
-			for (int x = 0; x < dstW; x++) {
-				*destP++ = srcP[scaleCacheX[x]];
-			}
+		switch (format.bytesPerPixel) {
+		case 1:
+			scaleNN<uint8>(scaleCacheX, target);
+			break;
+		case 2:
+			scaleNN<uint16>(scaleCacheX, target);
+			break;
+		case 4:
+			scaleNN<uint32>(scaleCacheX, target);
+			break;
+		default:
+			error("Can only scale 8bpp, 16bpp, and 32bpp");
 		}
-		delete[] scaleCacheX;
 
+		delete[] scaleCacheX;
 	}
 
 	return target;
-
 }
 
 TransparentSurface *TransparentSurface::convertTo(const PixelFormat &dstFormat, const byte *palette) const {
@@ -1053,12 +1053,26 @@ TransparentSurface *TransparentSurface::convertTo(const PixelFormat &dstFormat,
 	return surface;
 }
 
+template <typename Size>
+void TransparentSurface::scaleNN(int *scaleCacheX, TransparentSurface *target) const {
+	for (int y = 0; y < target->h; y++) {
+		Size *destP = (Size *)target->getBasePtr(0, y);
+		const Size *srcP = (const Size *)getBasePtr(0, (y * h) / target->h);
+		for (int x = 0; x < target->w; x++) {
+			*destP++ = srcP[scaleCacheX[x]];
+		}
+	}
+}
 
 template TransparentSurface *TransparentSurface::rotoscaleT<FILTER_NEAREST>(const TransformStruct &transform) const;
 template TransparentSurface *TransparentSurface::rotoscaleT<FILTER_BILINEAR>(const TransformStruct &transform) const;
 template TransparentSurface *TransparentSurface::scaleT<FILTER_NEAREST>(uint16 newWidth, uint16 newHeight) const;
 template TransparentSurface *TransparentSurface::scaleT<FILTER_BILINEAR>(uint16 newWidth, uint16 newHeight) const;
 
+template void TransparentSurface::scaleNN<uint8>(int *scaleCacheX, TransparentSurface *target) const;
+template void TransparentSurface::scaleNN<uint16>(int *scaleCacheX, TransparentSurface *target) const;
+template void TransparentSurface::scaleNN<uint32>(int *scaleCacheX, TransparentSurface *target) const;
+
 TransparentSurface *TransparentSurface::rotoscale(const TransformStruct &transform) const {
 	return rotoscaleT<FILTER_BILINEAR>(transform);
 }
diff --git a/graphics/transparent_surface.h b/graphics/transparent_surface.h
index 6fcdac1..22fe045 100644
--- a/graphics/transparent_surface.h
+++ b/graphics/transparent_surface.h
@@ -150,6 +150,7 @@ struct TransparentSurface : public Graphics::Surface {
 	TransparentSurface *scaleT(uint16 newWidth, uint16 newHeight) const;
 
 	TransparentSurface *scale(uint16 newWidth, uint16 newHeight) const;
+
 	/**
 	 * @brief Rotoscale function; this returns a transformed version of this surface after rotation and
 	 * scaling. Please do not use this if angle == 0, use plain old scaling function.
@@ -176,6 +177,8 @@ struct TransparentSurface : public Graphics::Surface {
 private:
 	AlphaType _alphaMode;
 
+	template <typename Size>
+	void scaleNN(int *scaleCacheX, TransparentSurface *target) const;
 };
 
 /**


Commit: 798c6bf34ddfc64f23bac3bf3f26539aaab72f2d
    https://github.com/scummvm/scummvm/commit/798c6bf34ddfc64f23bac3bf3f26539aaab72f2d
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:36-05:00

Commit Message:
COMMON: Add yet another GUIO option flag

SCI engine has very many game options.

Changed paths:
    common/gui_options.cpp
    common/gui_options.h


diff --git a/common/gui_options.cpp b/common/gui_options.cpp
index 120edd0..500830a 100644
--- a/common/gui_options.cpp
+++ b/common/gui_options.cpp
@@ -80,6 +80,9 @@ const struct GameOpt {
 	{ GUIO_GAMEOPTIONS7, "gameOption7" },
 	{ GUIO_GAMEOPTIONS8, "gameOption8" },
 	{ GUIO_GAMEOPTIONS9, "gameOption9" },
+	// Option strings must not contain substrings of any other options, so
+	// "gameOption10" would be invalid here because it contains "gameOption1"
+	{ GUIO_GAMEOPTIONS10, "gameOptionA" },
 
 	{ GUIO_NONE, 0 }
 };
diff --git a/common/gui_options.h b/common/gui_options.h
index d51a29a..6c1aa2f 100644
--- a/common/gui_options.h
+++ b/common/gui_options.h
@@ -74,6 +74,7 @@
 #define GUIO_GAMEOPTIONS7    "\056"
 #define GUIO_GAMEOPTIONS8    "\057"
 #define GUIO_GAMEOPTIONS9    "\060"
+#define GUIO_GAMEOPTIONS10   "\061"
 
 #define GUIO0() (GUIO_NONE)
 #define GUIO1(a) (a)


Commit: 1466fb247e9f24bfcc61e977e71602fadf26c148
    https://github.com/scummvm/scummvm/commit/1466fb247e9f24bfcc61e977e71602fadf26c148
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:36-05:00

Commit Message:
SCI32: Add workaround for SQ6

Changed paths:
    engines/sci/engine/workarounds.cpp


diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index ae6e2f7..66c40d6 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -418,6 +418,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_SQ6,            -1, 64964,  0,              "DPath", "init",                            NULL,     1, { WORKAROUND_FAKE,   0 } }, // during the game
 	{ GID_SQ6,           210,   210,  0,       "buttonSecret", "doVerb",                          NULL,     0, { WORKAROUND_FAKE,   0 } }, // after winning the first round of stooge fighter 3
 	{ GID_SQ6,            -1, 64994, -1,               "Game", "restore",                         NULL,     1, { WORKAROUND_FAKE,   0 } }, // When trying to load an invalid save game from the launcher
+	{ GID_SQ6,            -1, 64921, -1,              "Print", "addEdit",                         NULL,     1, { WORKAROUND_FAKE,   0 } }, // When trying to use the game debugger's flag setting command
 	{ GID_TORIN,          -1, 64017,  0,             "oFlags", "clear",                           NULL,     0, { WORKAROUND_FAKE,   0 } }, // entering Torin's home in the French version
 	{ GID_TORIN,       10000, 64029,  0,          "oMessager", "nextMsg",                         NULL,     3, { WORKAROUND_FAKE,   0 } }, // start of chapter one
 	{ GID_TORIN,          -1, 64892,  0,      "oEventHandler", "killAllEventHogs",                NULL,     1, { WORKAROUND_FAKE,   0 } }, // when pressing the hint button when the game is about to transition to a new room (race condition) - Trac#9810


Commit: 35346bc71bbe8dd122e8167d6850e1fdc3280038
    https://github.com/scummvm/scummvm/commit/35346bc71bbe8dd122e8167d6850e1fdc3280038
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:37-05:00

Commit Message:
SCI32: Update mouse position for rendering in all frameOuts

Changed paths:
    engines/sci/graphics/frameout.cpp
    engines/sci/graphics/frameout.h


diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 390cc81..80800da 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -390,13 +390,7 @@ void GfxFrameout::kernelAddPicAt(const reg_t planeObject, const GuiResourceId pi
 #pragma mark Rendering
 
 void GfxFrameout::frameOut(const bool shouldShowBits, const Common::Rect &eraseRect) {
-	// In SSCI, mouse events were received via hardware interrupt, so the mouse
-	// cursor would always get updated whenever the user moved the mouse. Since
-	// we must poll for mouse events instead, poll here so that the mouse gets
-	// updated with its current position at render time. If we do not do this,
-	// the mouse gets "stuck" during loops that do not make calls to kGetEvent,
-	// like transitions.
-	g_sci->getEventManager()->getSciEvent(SCI_EVENT_PEEK);
+	updateMousePositionForRendering();
 
 	RobotDecoder &robotPlayer = g_sci->_video32->getRobotPlayer();
 	const bool robotIsActive = robotPlayer.getStatus() != RobotDecoder::kRobotStatusUninitialized;
@@ -452,6 +446,8 @@ void GfxFrameout::frameOut(const bool shouldShowBits, const Common::Rect &eraseR
 }
 
 void GfxFrameout::palMorphFrameOut(const int8 *styleRanges, PlaneShowStyle *showStyle) {
+	updateMousePositionForRendering();
+
 	Palette sourcePalette(_palette->getNextPalette());
 	alterVmap(sourcePalette, sourcePalette, -1, styleRanges);
 
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index ab58f41..1585a34 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -24,6 +24,7 @@
 #define SCI_GRAPHICS_FRAMEOUT_H
 
 #include "engines/util.h"                // for initGraphics
+#include "sci/event.h"
 #include "sci/graphics/plane32.h"
 #include "sci/graphics/screen_item32.h"
 
@@ -370,6 +371,17 @@ public:
 #pragma mark -
 #pragma mark Mouse cursor
 private:
+	void updateMousePositionForRendering() const {
+		// In SSCI, mouse events were received via hardware interrupt, so the
+		// mouse cursor would always get updated immediately when the user moved
+		// the mouse. ScummVM must poll for mouse events from the backend
+		// instead, so we poll just before rendering so that the latest mouse
+		// position is rendered instead of whatever position it was at the last
+		// time kGetEvent was called. Without this, the mouse appears stuck
+		// during loops that do not make calls to kGetEvent, like transitions.
+		g_sci->getEventManager()->getSciEvent(SCI_EVENT_PEEK);
+	}
+
 	/**
 	 * Determines whether or not the point given by
 	 * `position` is inside of the given screen item.


Commit: f7fcce24e1da22815a589fbefcf34458d558acb4
    https://github.com/scummvm/scummvm/commit/f7fcce24e1da22815a589fbefcf34458d558acb4
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:37-05:00

Commit Message:
SCI32: Remove unused method declaration

Changed paths:
    engines/sci/graphics/frameout.h


diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index 1585a34..4bdb172 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -358,12 +358,6 @@ public:
 	};
 
 	/**
-	 * Draws a portion of the current screen buffer to
-	 * hardware. Used to display show styles in SCI2.1mid+.
-	 */
-	void showRect(const Common::Rect &rect);
-
-	/**
 	 * Shakes the screen.
 	 */
 	void shakeScreen(const int16 numShakes, const ShakeDirection direction);


Commit: c6f58401969fa5ab90c809c46f490e7c0e5dc02e
    https://github.com/scummvm/scummvm/commit/c6f58401969fa5ab90c809c46f490e7c0e5dc02e
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:37-05:00

Commit Message:
SCI32: Remove magic numbers in HunkPalette

Changed paths:
    engines/sci/graphics/palette32.cpp
    engines/sci/graphics/palette32.h


diff --git a/engines/sci/graphics/palette32.cpp b/engines/sci/graphics/palette32.cpp
index 3667e1f..fcef2a7 100644
--- a/engines/sci/graphics/palette32.cpp
+++ b/engines/sci/graphics/palette32.cpp
@@ -44,7 +44,7 @@ HunkPalette::HunkPalette(const SciSpan<const byte> &rawPalette) :
 	// set to 14, but the *actual* size of the header structure used in SSCI is
 	// 13, which is reflected by `kHunkPaletteHeaderSize`.
 	// _headerSize(rawPalette[0]),
-	_numPalettes(rawPalette.getUint8At(10)),
+	_numPalettes(rawPalette.getUint8At(kNumPaletteEntriesOffset)),
 	_data() {
 	assert(_numPalettes == 0 || _numPalettes == 1);
 	if (_numPalettes) {
@@ -54,7 +54,7 @@ HunkPalette::HunkPalette(const SciSpan<const byte> &rawPalette) :
 }
 
 void HunkPalette::setVersion(const uint32 version) const {
-	if (_numPalettes != _data.getUint8At(10)) {
+	if (_numPalettes != _data.getUint8At(kNumPaletteEntriesOffset)) {
 		error("Invalid HunkPalette");
 	}
 
@@ -74,10 +74,10 @@ const HunkPalette::EntryHeader HunkPalette::getEntryHeader() const {
 	const SciSpan<const byte> data(getPalPointer());
 
 	EntryHeader header;
-	header.startColor = data.getUint8At(10);
-	header.numColors = data.getUint16SEAt(14);
-	header.used = data.getUint8At(16);
-	header.sharedUsed = data.getUint8At(17);
+	header.startColor = data.getUint8At(kEntryStartColorOffset);
+	header.numColors = data.getUint16SEAt(kEntryNumColorsOffset);
+	header.used = data.getUint8At(kEntryUsedOffset);
+	header.sharedUsed = data.getUint8At(kEntrySharedUsedOffset);
 	header.version = data.getUint32SEAt(kEntryVersionOffset);
 
 	return header;
diff --git a/engines/sci/graphics/palette32.h b/engines/sci/graphics/palette32.h
index 0fcd7e0..fb0b226 100644
--- a/engines/sci/graphics/palette32.h
+++ b/engines/sci/graphics/palette32.h
@@ -56,6 +56,12 @@ public:
 private:
 	enum {
 		/**
+		 * The offset into the HunkPalette header of the number of palettes in
+		 * the HunkPalette.
+		 */
+		kNumPaletteEntriesOffset = 10,
+
+		/**
 		 * The size of the HunkPalette header.
 		 */
 		kHunkPaletteHeaderSize = 13,
@@ -63,7 +69,32 @@ private:
 		/**
 		 * The size of a palette entry header.
 		 */
-		kEntryHeaderSize = 22,
+		kEntryHeaderSize = 22
+	};
+
+	enum {
+		/**
+		 * The offset of the start color within the palette entry header.
+		 */
+		kEntryStartColorOffset = 10,
+
+		/**
+		 * The offset of the color count within the palette entry header.
+		 */
+		kEntryNumColorsOffset = 14,
+
+		/**
+		 * The offset of the shared used palette index flag within the palette
+		 * entry header.
+		 */
+		kEntryUsedOffset = 16,
+
+		/**
+		 * The offset of the flag within the palette entry header that says
+		 * whether or not the corresponding palette data includes used flags for
+		 * each palette index individually.
+		 */
+		kEntrySharedUsedOffset = 17,
 
 		/**
 		 * The offset of the hunk palette version within the palette entry


Commit: 2d6fe2b8cdf4a410362c87c2eacc7aaef90ef35c
    https://github.com/scummvm/scummvm/commit/2d6fe2b8cdf4a410362c87c2eacc7aaef90ef35c
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:37-05:00

Commit Message:
SCI32: Work around bogus palette entries in select Windows games

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


diff --git a/engines/sci/graphics/palette32.cpp b/engines/sci/graphics/palette32.cpp
index fcef2a7..acf63bc 100644
--- a/engines/sci/graphics/palette32.cpp
+++ b/engines/sci/graphics/palette32.cpp
@@ -461,7 +461,23 @@ void GfxPalette32::updateHardware() {
 
 	byte bpal[3 * 256];
 
-	for (int i = 0; i < ARRAYSIZE(_currentPalette.colors) - 1; ++i) {
+	// HACK: There are resources in a couple of Windows-only games that seem to
+	// include bogus palette entries above 236. SSCI does a lot of extra work
+	// when running in Windows to shift palettes and rewrite view & pic pixel
+	// data on-the-fly to account for the way Windows palettes work, which
+	// seems to end up masking the fact that there is some bad palette data.
+	// Since only one demo and one game seem to have this problem, we instead
+	// "fix" the problem here by ignoring attempts to send high palette entries
+	// to the backend. This makes those high pixels render black, which seems to
+	// match what would happen in the original interpreter, and saves us from
+	// having to clutter up the engine with a bunch of palette shifting garbage.
+	int maxIndex = ARRAYSIZE(_currentPalette.colors) - 2;
+	if (g_sci->getGameId() == GID_HOYLE5 ||
+		(g_sci->getGameId() == GID_GK2 && g_sci->isDemo())) {
+		maxIndex = 235;
+	}
+
+	for (int i = 0; i <= maxIndex; ++i) {
 		_currentPalette.colors[i] = _nextPalette.colors[i];
 
 		// All color entries MUST be copied, not just "used" entries, otherwise


Commit: e9bef896462d6f99c4c89672de90c99e879415a2
    https://github.com/scummvm/scummvm/commit/e9bef896462d6f99c4c89672de90c99e879415a2
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:38-05:00

Commit Message:
SCI32: Remove useless call

The show list is already cleared by showBits so it does not need to
be cleared a second time.

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


diff --git a/engines/sci/graphics/transitions32.cpp b/engines/sci/graphics/transitions32.cpp
index b5e4985..8f18a60 100644
--- a/engines/sci/graphics/transitions32.cpp
+++ b/engines/sci/graphics/transitions32.cpp
@@ -83,7 +83,6 @@ void GfxTransitions32::addShowRect(const Common::Rect &rect) {
 
 void GfxTransitions32::sendShowRects() {
 	g_sci->_gfxFrameout->showBits();
-	clearShowRects();
 	throttle();
 }
 


Commit: 8cb35442c073b5ed5a0f3fa7d5e627bdd85af229
    https://github.com/scummvm/scummvm/commit/8cb35442c073b5ed5a0f3fa7d5e627bdd85af229
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:38-05:00

Commit Message:
SCI32: Improve kShowMovieWin (AVI) rendering

1. Added a new game option for linear interpolation when scaling
   video in ScummVM builds with USE_RGB_COLOR;
2. 8bpp videos that put black in a palette index other than 0
   (KQ7) should now always render correctly without the earlier
   game-specific workarounds which did not work very well;
3. Data from game scripts regarding video size and position are
   now ignored, since games always just try to show videos in the
   middle of the screen, but frequently get this a little bit
   wrong, causing either bad aspect ratios or off-center videos;
4. Builds without USE_RGB_COLOR support will not crash when
   attempting to play >8bpp AVIs, like those from KQ7 2.00b.

Fixes Trac#9843, Trac#9762.

Changed paths:
    engines/sci/detection.cpp
    engines/sci/detection_tables.h
    engines/sci/engine/kvideo.cpp
    engines/sci/graphics/video32.cpp
    engines/sci/graphics/video32.h
    engines/sci/sci.h


diff --git a/engines/sci/detection.cpp b/engines/sci/detection.cpp
index 7f84247..fe95b0c 100644
--- a/engines/sci/detection.cpp
+++ b/engines/sci/detection.cpp
@@ -424,6 +424,18 @@ static const ADExtraGuiOptionsMap optionsList[] = {
 		}
 	},
 
+#ifdef USE_RGB_COLOR
+	{
+		GAMEOPTION_HQ_VIDEO,
+		{
+			_s("Use high-quality video scaling"),
+			_s("Use linear interpolation when upscaling videos, where possible"),
+			"enable_hq_video",
+			true
+		}
+	},
+#endif
+
 	{
 		GAMEOPTION_PREFER_DIGITAL_SFX,
 		{
diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index 1ba3af4..b5c426d 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -724,9 +724,13 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #ifdef ENABLE_SCI32
 #define GUIO_GK1_FLOPPY GUIO2(GUIO_NOSPEECH, \
                               GAMEOPTION_ORIGINAL_SAVELOAD)
-#define GUIO_GK1_CD     GUIO3(GUIO_LINKSPEECHTOSFX, \
+#define GUIO_GK1_CD_DOS GUIO3(GUIO_LINKSPEECHTOSFX, \
                               GAMEOPTION_ORIGINAL_SAVELOAD, \
                               GAMEOPTION_HIGH_RESOLUTION_GRAPHICS)
+#define GUIO_GK1_CD_WIN GUIO4(GUIO_LINKSPEECHTOSFX, \
+                              GAMEOPTION_ORIGINAL_SAVELOAD, \
+                              GAMEOPTION_HIGH_RESOLUTION_GRAPHICS, \
+                              GAMEOPTION_HQ_VIDEO)
 #define GUIO_GK1_MAC    GUIO_GK1_FLOPPY
 
 	// Gabriel Knight - English DOS Floppy
@@ -775,7 +779,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "372d059f75856afa6d73dd84cbb8913d", 10996},
 		{"resource.000", 0, "69b7516962510f780d38519cc15fcc7c", 12581736},
 		AD_LISTEND},
-		Common::EN_ANY, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::EN_ANY, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
 
 	// Gabriel Knight - English Windows CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "01.100.000"
@@ -783,7 +787,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "372d059f75856afa6d73dd84cbb8913d", 10996},
 		{"resource.000", 0, "69b7516962510f780d38519cc15fcc7c", 12581736},
 		AD_LISTEND},
-		Common::EN_ANY, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::EN_ANY, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_WIN },
 
 	// Gabriel Knight - German DOS CD (from Tobis87)
 	// SCI interpreter version 2.000.000
@@ -791,7 +795,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "a7d3e55114c65647310373cb390815ba", 11392},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13400497},
 		AD_LISTEND},
-		Common::DE_DEU, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::DE_DEU, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
 
 	// Gabriel Knight - Spanish DOS CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "1.000.000, April 13, 1995"
@@ -799,7 +803,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "7cb6e9bba15b544ec7a635c45bde9953", 11404},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13381599},
 		AD_LISTEND},
-		Common::ES_ESP, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::ES_ESP, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
 
 	// Gabriel Knight - French DOS CD (from Hkz)
 	// VERSION file reports "1.000.000, May 3, 1994"
@@ -807,7 +811,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "55f909ba93a2515042a08d8a2da8414e", 11392},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13325145},
 		AD_LISTEND},
-		Common::FR_FRA, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::FR_FRA, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
 
 	// Gabriel Knight - Spanish Windows CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "1.000.000, April 13, 1995"
@@ -815,7 +819,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "7cb6e9bba15b544ec7a635c45bde9953", 11404},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13381599},
 		AD_LISTEND},
-		Common::ES_ESP, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::ES_ESP, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_WIN },
 
 	// Gabriel Knight - English Macintosh (Floppy!)
 	// This version is hi-res ONLY, so it should NOT get GAMEOPTION_HIGH_RESOLUTION_GRAPHICS
@@ -830,16 +834,18 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		Common::EN_ANY, Common::kPlatformMacintosh, ADGF_MACRESFORK | ADGF_UNSTABLE, GUIO_GK1_MAC },
 
 #undef GUIO_GK1_FLOPPY
-#undef GUIO_GK1_CD
+#undef GUIO_GK1_CD_DOS
+#undef GUIO_GK1_CD_WIN
 #undef GUIO_GK1_MAC
 
-#define GUIO_GK2_DEMO GUIO7(GUIO_NOSUBTITLES, \
+#define GUIO_GK2_DEMO GUIO8(GUIO_NOSUBTITLES, \
                             GUIO_NOMUSIC, \
                             GUIO_NOSFX, \
                             GUIO_NOSPEECH, \
                             GUIO_NOMIDI, \
                             GUIO_NOLAUNCHLOAD, \
-                            GUIO_NOASPECT)
+                            GUIO_NOASPECT, \
+                            GAMEOPTION_HQ_VIDEO)
 #define GUIO_GK2      GUIO7(GUIO_NOSUBTITLES, \
                             GUIO_NOSFX, \
                             GUIO_NOSPEECHVOLUME, \
@@ -1824,9 +1830,10 @@ static const struct ADGameDescription SciGameDescriptions[] = {
                             GUIO_LINKMUSICTOSFX, \
                             GUIO_LINKSPEECHTOSFX, \
                             GUIO_NOASPECT)
-#define GUIO_KQ7      GUIO3(GUIO_NOASPECT, \
+#define GUIO_KQ7      GUIO4(GUIO_NOASPECT, \
                             GUIO_LINKMUSICTOSFX, \
-                            GUIO_LINKSPEECHTOSFX)
+                            GUIO_LINKSPEECHTOSFX, \
+                            GAMEOPTION_HQ_VIDEO)
 
 	// King's Quest 7 - English Windows (from the King's Quest Collection)
 	// Executable scanning reports "2.100.002", VERSION file reports "1.4"
diff --git a/engines/sci/engine/kvideo.cpp b/engines/sci/engine/kvideo.cpp
index 9398e6d..37e4eee 100644
--- a/engines/sci/engine/kvideo.cpp
+++ b/engines/sci/engine/kvideo.cpp
@@ -332,11 +332,12 @@ reg_t kShowMovieWinInit(EngineState *s, int argc, reg_t *argv) {
 		--argc;
 	}
 
-	const int16 x = argv[0].toSint16();
-	const int16 y = argv[1].toSint16();
-	const int16 width = argc > 3 ? argv[2].toSint16() : 0;
-	const int16 height = argc > 3 ? argv[3].toSint16() : 0;
-	return make_reg(0, g_sci->_video32->getAVIPlayer().init1x(x, y, width, height));
+	// argv[0] is a broken x-coordinate
+	// argv[1] is a broken y-coordinate
+	// argv[2] is an optional broken width
+	// argv[3] is an optional broken height
+	const bool pixelDouble = argc > 3 && argv[2].toSint16() && argv[3].toSint16();
+	return make_reg(0, g_sci->_video32->getAVIPlayer().init(pixelDouble));
 }
 
 reg_t kShowMovieWinPlay(EngineState *s, int argc, reg_t *argv) {
@@ -386,9 +387,9 @@ reg_t kShowMovieWinPlayUntilEvent(EngineState *s, int argc, reg_t *argv) {
 
 reg_t kShowMovieWinInitDouble(EngineState *s, int argc, reg_t *argv) {
 	// argv[0] is a broken movie ID
-	const int16 x = argv[1].toSint16();
-	const int16 y = argv[2].toSint16();
-	return make_reg(0, g_sci->_video32->getAVIPlayer().init2x(x, y));
+	// argv[1] is a broken x-coordinate
+	// argv[2] is a broken y-coordinate
+	return make_reg(0, g_sci->_video32->getAVIPlayer().init(true));
 }
 
 reg_t kPlayVMD(EngineState *s, int argc, reg_t *argv) {
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index fc35d40..ecde85b 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -23,9 +23,15 @@
 #include "audio/mixer.h"                 // for Audio::Mixer::kSFXSoundType
 #include "common/config-manager.h"       // for ConfMan
 #include "common/textconsole.h"          // for warning, error
+#ifndef USE_RGB_COLOR
+#include "common/translation.h"          // for _
+#endif
 #include "common/util.h"                 // for ARRAYSIZE
 #include "common/system.h"               // for g_system
 #include "engine.h"                      // for Engine, g_engine
+#include "graphics/colormasks.h"         // for createPixelFormat
+#include "graphics/palette.h"            // for PaletteManager
+#include "graphics/transparent_surface.h" // for TransparentSurface
 #include "sci/console.h"                 // for Console
 #include "sci/engine/features.h"         // for GameFeatures
 #include "sci/engine/state.h"            // for EngineState
@@ -176,14 +182,9 @@ void SEQPlayer::renderFrame(SciBitmap &bitmap) const {
 #pragma mark -
 #pragma mark AVIPlayer
 
-AVIPlayer::AVIPlayer(SegManager *segMan, EventManager *eventMan) :
-	_segMan(segMan),
+AVIPlayer::AVIPlayer(EventManager *eventMan) :
 	_eventMan(eventMan),
 	_decoder(new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
-	_scaleBuffer(nullptr),
-	_plane(nullptr),
-	_screenItem(nullptr),
-	_bitmap(NULL_REG),
 	_status(kAVINotOpen) {}
 
 AVIPlayer::~AVIPlayer() {
@@ -200,125 +201,99 @@ AVIPlayer::IOStatus AVIPlayer::open(const Common::String &fileName) {
 		return kIOFileNotFound;
 	}
 
-	_status = kAVIOpen;
-	return kIOSuccess;
-}
-
-AVIPlayer::IOStatus AVIPlayer::init1x(const int16 x, const int16 y, int16 width, int16 height) {
-	if (_status == kAVINotOpen) {
+#ifndef USE_RGB_COLOR
+	// KQ7 2.00b videos are compressed in 24bpp Cinepak, so cannot play on
+	// a system with no RGB support
+	if (_decoder->getPixelFormat().bytesPerPixel != 1) {
+		void showScummVMDialog(const Common::String &message);
+		showScummVMDialog(Common::String::format(_("Cannot play back %dbpp video on a system with maximum color depth of 8bpp"), _decoder->getPixelFormat().bpp()));
+		_decoder->close();
 		return kIOFileNotFound;
 	}
+#endif
 
-	_pixelDouble = false;
-
-	if (!width || !height) {
-		const int16 screenWidth = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
-		const int16 screenHeight = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
-		const int16 scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
-		const int16 scriptHeight = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
-		const Ratio screenToScriptX(scriptWidth, screenWidth);
-		const Ratio screenToScriptY(scriptHeight, screenHeight);
-		width = (_decoder->getWidth() * screenToScriptX).toInt();
-		height = (_decoder->getHeight() * screenToScriptY).toInt();
-	}
-
-	// QFG4CD gives non-multiple-of-2 values for width and height of the intro
-	// video, which would normally be OK except the source video is a pixel
-	// bigger in each dimension so it just causes part of the video to get cut
-	// off
-	width = (width + 1) & ~1;
-	height = (height + 1) & ~1;
-
-	// GK1 CREDITS.AVI is not rendered correctly in SSCI because it is a 640x480
-	// video and the game script gives the wrong dimensions.
-	// Since this is the only high-resolution AVI ever used by any SCI game,
-	// just set the draw rectangle to draw across the entire screen
-	if (g_sci->getGameId() == GID_GK1 && _decoder->getWidth() > 320) {
-		_drawRect.left = 0;
-		_drawRect.top = 0;
-		_drawRect.right = 320;
-		_drawRect.bottom = 200;
-	} else {
-		_drawRect.left = x;
-		_drawRect.top = y;
-		_drawRect.right = x + width;
-		_drawRect.bottom = y + height;
-	}
-
-	init();
-
+	_status = kAVIOpen;
 	return kIOSuccess;
 }
 
-AVIPlayer::IOStatus AVIPlayer::init2x(const int16 x, const int16 y) {
+AVIPlayer::IOStatus AVIPlayer::init(const bool pixelDouble) {
+	// Calls to initialize the AVI player in SCI can be made in a few ways:
+	//
+	// * kShowMovie(WinInit, x, y) to render the video at (x,y) using its
+	//   original resolution, or
+	// * kShowMovie(WinInit, x, y, w, h) to render the video at (x,y) with
+	//   rescaling to the given width and height, or
+	// * kShowMovie(WinInitDouble, x, y) to render the video at (x,y) with
+	//   rescaling to double the original resolution.
+	//
+	// Unfortunately, the values passed by game scripts are frequently wrong:
+	//
+	// * KQ7 passes origin coordinates that cause videos to be misaligned on the
+	//   Y-axis;
+	// * GK1 passes width and height that change the aspect ratio of the videos,
+	//   even though they were rendered with square pixels (and in the case of
+	//   CREDITS.AVI, cause the video to be badly downscaled);
+	// * The GK2 demo does all of these things at the same time.
+	//
+	// Fortunately, whenever all of these games play an AVI, they are just
+	// trying to play a video at the center of the screen. So, we ignore the
+	// values that the game sends, and instead calculate the correct dimensions
+	// and origin based on the video data, only allowing games to specify
+	// whether or not the videos should be scaled up 2x.
+
 	if (_status == kAVINotOpen) {
 		return kIOFileNotFound;
 	}
 
-	_drawRect.left = x;
-	_drawRect.top = y;
-	_drawRect.right = x + _decoder->getWidth() * 2;
-	_drawRect.bottom = y + _decoder->getHeight() * 2;
-	_pixelDouble = true;
-
-	init();
-
-	return kIOSuccess;
-}
-
-void AVIPlayer::init() {
-	int16 xRes;
-	int16 yRes;
-
-	// GK1 CREDITS.AVI or KQ7 1.51 half-size videos
-	if ((g_sci->_gfxFrameout->_isHiRes && _decoder->getWidth() > 320) ||
-		(g_sci->getGameId() == GID_KQ7 && getSciVersion() == SCI_VERSION_2_1_EARLY && _drawRect.width() <= 160)) {
-		xRes = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
-		yRes = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
-	} else {
-		xRes = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
-
-		const Ratio videoRatio(_decoder->getWidth(), _decoder->getHeight());
-		const Ratio screenRatio(4, 3);
+	g_sci->_gfxCursor32->hide();
 
-		// Videos that already have a 4:3 aspect ratio should not receive any
-		// aspect ratio correction
-		if (videoRatio == screenRatio) {
-			yRes = 240;
-		} else {
-			yRes = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
-		}
+	int16 width = _decoder->getWidth();
+	int16 height = _decoder->getHeight();
+	if (pixelDouble) {
+		width *= 2;
+		height *= 2;
 	}
 
-	_plane = new Plane(_drawRect);
-	g_sci->_gfxFrameout->addPlane(*_plane);
-
-	if (_decoder->getPixelFormat().bytesPerPixel == 1) {
-		SciBitmap &bitmap = *_segMan->allocateBitmap(&_bitmap, _decoder->getWidth(), _decoder->getHeight(), kDefaultSkipColor, 0, 0, xRes, yRes, 0, false, false);
-		bitmap.getBuffer().fillRect(Common::Rect(_decoder->getWidth(), _decoder->getHeight()), 0);
-
-		CelInfo32 celInfo;
-		celInfo.type = kCelTypeMem;
-		celInfo.bitmap = _bitmap;
-
-		_screenItem = new ScreenItem(_plane->_object, celInfo, Common::Point(), ScaleInfo());
-		g_sci->_gfxFrameout->addScreenItem(*_screenItem);
-		g_sci->_gfxFrameout->frameOut(true);
+	const int16 screenWidth = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
+	const int16 screenHeight = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
+
+	// When scaling videos, they must not grow larger than the hardware screen
+	// or else the engine will crash. This is particularly important for the GK1
+	// CREDITS.AVI since the game sends extra width/height arguments, causing it
+	// to be treated as needing upscaling even though it does not.
+	width = MIN<int16>(width, screenWidth);
+	height = MIN<int16>(height, screenHeight);
+
+	_drawRect.left = (screenWidth - width) / 2;
+	_drawRect.top = (screenHeight - height) / 2;
+	_drawRect.setWidth(width);
+	_drawRect.setHeight(height);
+
+#ifdef USE_RGB_COLOR
+	// Optimize rendering performance for unscaled videos, and allow
+	// better-than-NN interpolation for videos that are scaled
+	if (ConfMan.getBool("enable_hq_video") &&
+		(_decoder->getWidth() != width || _decoder->getHeight() != height)) {
+
+		// TODO: Search for and use the best supported format (which may be
+		// lower than 32bpp) once the scaling code in Graphics supports
+		// 16bpp/24bpp, and once the SDL backend can correctly communicate
+		// supported pixel formats above whatever format is currently used by
+		// _hwsurface. Right now, this will just crash ScummVM if the backend
+		// does not support a 32bpp pixel format, which sucks since this code
+		// really ought to be able to fall back to NN scaling for games with
+		// 256-color videos.
+		const Graphics::PixelFormat format = Graphics::createPixelFormat<8888>();
+		g_sci->_gfxFrameout->setPixelFormat(format);
 	} else {
-		// Attempting to draw a palettized cursor into a 24bpp surface will
-		// cause memory corruption, so hide the cursor in this mode (SCI did not
-		// have a 24bpp mode but just directed VFW to display videos instead)
-		g_sci->_gfxCursor32->hide();
-
+#else
+	{
+#endif
 		const Graphics::PixelFormat format = _decoder->getPixelFormat();
 		g_sci->_gfxFrameout->setPixelFormat(format);
-
-		if (_pixelDouble) {
-			const int16 width = _drawRect.width();
-			const int16 height = _drawRect.height();
-			_scaleBuffer = calloc(1, width * height * format.bytesPerPixel);
-		}
 	}
+
+	return kIOSuccess;
 }
 
 AVIPlayer::IOStatus AVIPlayer::play(const int16 from, const int16 to, const int16, const bool async) {
@@ -344,6 +319,7 @@ AVIPlayer::IOStatus AVIPlayer::play(const int16 from, const int16 to, const int1
 
 void AVIPlayer::renderVideo() const {
 	_decoder->start();
+
 	while (!g_engine->shouldQuit() && !_decoder->endOfVideo()) {
 		g_sci->sleep(_decoder->getTimeToNextFrame());
 		while (_decoder->needsUpdate()) {
@@ -357,24 +333,18 @@ AVIPlayer::IOStatus AVIPlayer::close() {
 		return kIOSuccess;
 	}
 
-	free(_scaleBuffer);
-	_scaleBuffer = nullptr;
-
-	if (_decoder->getPixelFormat().bytesPerPixel != 1) {
+#ifdef USE_RGB_COLOR
+	if (g_system->getScreenFormat().bytesPerPixel != 1) {
 		const Graphics::PixelFormat format = Graphics::PixelFormat::createFormatCLUT8();
 		g_sci->_gfxFrameout->setPixelFormat(format);
-		g_sci->_gfxCursor32->unhide();
 	}
+#endif
+
+	g_system->fillScreen(0);
+	g_sci->_gfxCursor32->unhide();
 
 	_decoder->close();
 	_status = kAVINotOpen;
-	if (_bitmap != NULL_REG) {
-		_segMan->freeBitmap(_bitmap);
-		_bitmap = NULL_REG;
-	}
-	g_sci->_gfxFrameout->deletePlane(*_plane);
-	_plane = nullptr;
-	_screenItem = nullptr;
 	return kIOSuccess;
 }
 
@@ -395,91 +365,77 @@ uint16 AVIPlayer::getDuration() const {
 	return _decoder->getFrameCount();
 }
 
-void AVIPlayer::renderFrame() const {
-	const Graphics::Surface *surface = _decoder->decodeNextFrame();
+template <Graphics::TFilteringMode MODE>
+static void writeFrameToSystem(const Graphics::Surface *nextFrame, Video::VideoDecoder *decoder, const Common::Rect &drawRect) {
+	assert(nextFrame);
 
-	if (surface->format.bytesPerPixel == 1) {
-		SciBitmap &bitmap = *_segMan->lookupBitmap(_bitmap);
-		if (surface->w > bitmap.getWidth() || surface->h > bitmap.getHeight()) {
-			warning("Attempted to draw a video frame larger than the destination bitmap");
-			return;
+	bool freeConvertedFrame;
+	Graphics::Surface *convertedFrame;
+	// Avoid creating a duplicate copy of the surface when it is not necessary
+	if (decoder->getPixelFormat() == g_system->getScreenFormat()) {
+		freeConvertedFrame = false;
+		convertedFrame = const_cast<Graphics::Surface *>(nextFrame);
+	} else {
+		freeConvertedFrame = true;
+		convertedFrame = nextFrame->convertTo(g_system->getScreenFormat(), decoder->getPalette());
+	}
+	assert(convertedFrame);
+
+	if (decoder->getWidth() != drawRect.width() || decoder->getHeight() != drawRect.height()) {
+		Graphics::Surface *const unscaledFrame(convertedFrame);
+		const Graphics::TransparentSurface tsUnscaledFrame(*unscaledFrame);
+		convertedFrame = tsUnscaledFrame.scaleT<MODE>(drawRect.width(), drawRect.height());
+		assert(convertedFrame);
+		if (freeConvertedFrame) {
+			unscaledFrame->free();
+			delete unscaledFrame;
 		}
+		freeConvertedFrame = true;
+	}
 
-		// KQ7 1.51 encodes videos with palette entry 0 as white, which makes
-		// the area around the video turn white too, since it is coded to use
-		// palette entry 0. This happens to work in the original game because
-		// the video is rendered by VfW, not in the engine itself. To fix this,
-		// we just modify the incoming pixel data from the video so if a pixel
-		// is using entry 0, we change it to use entry 255, which is guaranteed
-		// to always be white
-		if (getSciVersion() == SCI_VERSION_2_1_EARLY && g_sci->getGameId() == GID_KQ7) {
-			uint8 *target = bitmap.getPixels();
-			const uint8 *source = (const uint8 *)surface->getPixels();
-			const uint8 *end = (const uint8 *)surface->getPixels() + surface->w * surface->h;
-
-			while (source != end) {
-				const uint8 value = *source++;
-				*target++ = value == 0 ? 255 : value;
-			}
-		} else {
-			bitmap.getBuffer().copyRectToSurface(*surface, 0, 0, Common::Rect(surface->w, surface->h));
-		}
+	g_system->copyRectToScreen(convertedFrame->getPixels(), convertedFrame->pitch, drawRect.left, drawRect.top, convertedFrame->w, convertedFrame->h);
+	g_sci->_gfxFrameout->updateScreen();
+	if (freeConvertedFrame) {
+		convertedFrame->free();
+		delete convertedFrame;
+	}
+}
 
-		const bool dirtyPalette = _decoder->hasDirtyPalette();
-		if (dirtyPalette) {
-			Palette palette;
-			const byte *rawPalette = _decoder->getPalette();
-			for (int i = 0; i < ARRAYSIZE(palette.colors); ++i) {
-				palette.colors[i].r = *rawPalette++;
-				palette.colors[i].g = *rawPalette++;
-				palette.colors[i].b = *rawPalette++;
-				palette.colors[i].used = true;
+void AVIPlayer::renderFrame() const {
+	// TODO: Improve efficiency by making changes to common Graphics code that
+	// allow reuse of a single conversion surface for all frames
+
+	const Graphics::Surface *nextFrame = _decoder->decodeNextFrame();
+	assert(nextFrame);
+
+	if (g_system->getScreenFormat().bytesPerPixel == 1 && _decoder->hasDirtyPalette()) {
+		const uint8 *palette = _decoder->getPalette();
+		assert(palette);
+		g_system->getPaletteManager()->setPalette(palette, 0, 256);
+
+		// KQ7 1.x has videos encoded using Microsoft Video 1 where palette 0 is
+		// white and 255 is black, which is basically the opposite of DOS/Win
+		// SCI palettes. So, when drawing to an 8bpp hwscreen, whenever a new
+		// palette is seen, the screen must be re-filled with the new black
+		// entry to ensure areas outside the video are always black and not some
+		// other color
+		for (int color = 0; color < 256; ++color) {
+			if (palette[0] == 0 && palette[1] == 0 && palette[2] == 0) {
+				g_system->fillScreen(color);
+				break;
 			}
-
-			// Prevent KQ7 1.51 from setting entry 0 to white
-			palette.colors[0].used = false;
-
-			g_sci->_gfxPalette32->submit(palette);
+			palette += 3;
 		}
+	}
 
-		g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
-		g_sci->_gfxFrameout->frameOut(true);
+#ifdef USE_RGB_COLOR
+	if (g_system->getScreenFormat().bytesPerPixel != 1) {
+		writeFrameToSystem<Graphics::FILTER_BILINEAR>(nextFrame, _decoder, _drawRect);
 	} else {
-		assert(surface->format.bytesPerPixel == 4);
-
-		const int16 screenWidth = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
-		const int16 screenHeight = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
-		const int16 scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
-		const int16 scriptHeight = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
-
-		Common::Rect drawRect(_drawRect);
-		mulru(drawRect, Ratio(screenWidth, scriptWidth), Ratio(screenHeight, scriptHeight), 1);
-
-		if (_pixelDouble) {
-			const uint32 *source = (const uint32 *)surface->getPixels();
-			uint32 *target = (uint32 *)_scaleBuffer;
-			// target pitch here is in uint32s, not bytes, because the surface
-			// bpp is 4
-			const uint16 pitch = surface->pitch / 2;
-			for (int y = 0; y < surface->h; ++y) {
-				for (int x = 0; x < surface->w; ++x) {
-					const uint32 value = *source++;
-
-					target[0] = value;
-					target[1] = value;
-					target[pitch] = value;
-					target[pitch + 1] = value;
-					target += 2;
-				}
-				target += pitch;
-			}
-
-			g_system->copyRectToScreen(_scaleBuffer, surface->pitch * 2, drawRect.left, drawRect.top, _drawRect.width(), _drawRect.height());
-		} else {
-			g_system->copyRectToScreen(surface->getPixels(), surface->pitch, drawRect.left, drawRect.top, surface->w, surface->h);
-		}
-
-		g_sci->_gfxFrameout->updateScreen();
+#else
+	{
+#endif
+		writeFrameToSystem<Graphics::FILTER_NEAREST>(nextFrame, _decoder, _drawRect);
 	}
 }
 
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index 41e3016..a023fd4 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -23,6 +23,9 @@
 #ifndef SCI_GRAPHICS_VIDEO32_H
 #define SCI_GRAPHICS_VIDEO32_H
 
+#ifdef USE_RGB_COLOR
+#include "common/config-manager.h" // for ConfMan
+#endif
 #include "common/rect.h"          // for Rect
 #include "common/scummsys.h"      // for int16, uint8, uint16, int32
 #include "common/str.h"           // for String
@@ -110,7 +113,7 @@ public:
 		kEventFlagHotRectangle = 8
 	};
 
-	AVIPlayer(SegManager *segMan, EventManager *eventMan);
+	AVIPlayer(EventManager *eventMan);
 	~AVIPlayer();
 
 	/**
@@ -122,14 +125,7 @@ public:
 	 * Initializes the AVI rendering parameters for the
 	 * current AVI. This must be called after `open`.
 	 */
-	IOStatus init1x(const int16 x, const int16 y, const int16 width, const int16 height);
-
-	/**
-	 * Initializes the AVI rendering parameters for the
-	 * current AVI, in pixel-doubling mode. This must
-	 * be called after `open`.
-	 */
-	IOStatus init2x(const int16 x, const int16 y);
+	IOStatus init(const bool pixelDouble);
 
 	/**
 	 * Begins playback of the current AVI.
@@ -160,7 +156,6 @@ public:
 private:
 	typedef Common::HashMap<uint16, AVIStatus> StatusMap;
 
-	SegManager *_segMan;
 	EventManager *_eventMan;
 	Video::AVIDecoder *_decoder;
 
@@ -170,48 +165,12 @@ private:
 	AVIStatus _status;
 
 	/**
-	 * The plane where the AVI will be drawn.
-	 */
-	Plane *_plane;
-
-	/**
-	 * The screen item representing the AVI surface,
-	 * in 8bpp mode. In 24bpp mode, video is drawn
-	 * directly to the screen.
-	 */
-	ScreenItem *_screenItem;
-
-	/**
-	 * The bitmap used to render video output in
-	 * 8bpp mode.
-	 */
-	reg_t _bitmap;
-
-	/**
 	 * The rectangle where the video will be drawn,
-	 * in game script coordinates.
+	 * in screen coordinates.
 	 */
 	Common::Rect _drawRect;
 
 	/**
-	 * The scale buffer for pixel-doubled videos
-	 * drawn in 24bpp mode.
-	 */
-	void *_scaleBuffer;
-
-	/**
-	 * In SCI2.1, whether or not the video should
-	 * be pixel doubled for playback.
-	 */
-	bool _pixelDouble;
-
-	/**
-	 * Performs common initialisation for both
-	 * scaled and unscaled videos.
-	 */
-	void init();
-
-	/**
 	 * Renders video without event input until the
 	 * video is complete.
 	 */
@@ -657,7 +616,7 @@ class Video32 : public Common::Serializable {
 public:
 	Video32(SegManager *segMan, EventManager *eventMan) :
 	_SEQPlayer(segMan, eventMan),
-	_AVIPlayer(segMan, eventMan),
+	_AVIPlayer(eventMan),
 	_VMDPlayer(segMan, eventMan),
 	_robotPlayer(segMan),
 	_duckPlayer(segMan, eventMan) {}
diff --git a/engines/sci/sci.h b/engines/sci/sci.h
index 8109317..ff3b515 100644
--- a/engines/sci/sci.h
+++ b/engines/sci/sci.h
@@ -56,6 +56,7 @@ namespace Sci {
 // HIGH_RESOLUTION_GRAPHICS availability is checked for in SciEngine::run()
 #define GAMEOPTION_HIGH_RESOLUTION_GRAPHICS GUIO_GAMEOPTIONS8
 #define GAMEOPTION_ENABLE_BLACK_LINED_VIDEO GUIO_GAMEOPTIONS9
+#define GAMEOPTION_HQ_VIDEO                 GUIO_GAMEOPTIONS10
 
 struct EngineState;
 class Vocabulary;


Commit: 7057f232d75732c320fb470a8632a4c2f055a47f
    https://github.com/scummvm/scummvm/commit/7057f232d75732c320fb470a8632a4c2f055a47f
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:38-05:00

Commit Message:
SCI32: Improve kPlayVMD rendering

1. Added a new game option for linear interpolation when scaling
   overlay-mode video in ScummVM builds with USE_RGB_COLOR;
2. Implemented SCI2.1-variant of the VMD player renderer (fixes
   Trac#9857), which bypasses the engine's normal rendering
   pipeline;
3. Improved accuracy of the SCI3-variant of the VMD player by
   writing HunkPalettes into the VMD's CelObjMem instead of
   submitting palettes directly to GfxPalette32.

Changed paths:
    engines/sci/detection_tables.h
    engines/sci/engine/segment.h
    engines/sci/graphics/frameout.cpp
    engines/sci/graphics/frameout.h
    engines/sci/graphics/helpers.h
    engines/sci/graphics/palette32.cpp
    engines/sci/graphics/palette32.h
    engines/sci/graphics/plane32.cpp
    engines/sci/graphics/video32.cpp
    engines/sci/graphics/video32.h


diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index b5c426d..9378f2b 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -846,13 +846,14 @@ static const struct ADGameDescription SciGameDescriptions[] = {
                             GUIO_NOLAUNCHLOAD, \
                             GUIO_NOASPECT, \
                             GAMEOPTION_HQ_VIDEO)
-#define GUIO_GK2      GUIO7(GUIO_NOSUBTITLES, \
+#define GUIO_GK2      GUIO8(GUIO_NOSUBTITLES, \
                             GUIO_NOSFX, \
                             GUIO_NOSPEECHVOLUME, \
                             GUIO_NOMIDI, \
                             GUIO_NOASPECT, \
                             GAMEOPTION_ORIGINAL_SAVELOAD, \
-                            GAMEOPTION_ENABLE_BLACK_LINED_VIDEO)
+                            GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                            GAMEOPTION_HQ_VIDEO)
 #define GUIO_GK2_MAC  GUIO_GK2
 
 	// Gabriel Knight 2 - English Windows Non-Interactive Demo
@@ -2695,10 +2696,11 @@ static const struct ADGameDescription SciGameDescriptions[] = {
                              GUIO_NOMIDI, \
                              GUIO_NOLAUNCHLOAD, \
                              GAMEOPTION_ORIGINAL_SAVELOAD)
-#define GUIO_LSL7      GUIO4(GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+#define GUIO_LSL7      GUIO5(GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
                              GUIO_NOASPECT, \
                              GUIO_NOMIDI, \
-                             GAMEOPTION_ORIGINAL_SAVELOAD)
+                             GAMEOPTION_ORIGINAL_SAVELOAD, \
+                             GAMEOPTION_HQ_VIDEO)
 
 	// Larry 7 - English DOS Demo (provided by richiefs in bug report #2670691)
 	// SCI interpreter version 2.100.002
@@ -2758,9 +2760,10 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #define GUIO_LIGHTHOUSE_DEMO GUIO3(GUIO_NOSPEECH, \
                                    GUIO_NOASPECT, \
                                    GAMEOPTION_ORIGINAL_SAVELOAD)
-#define GUIO_LIGHTHOUSE      GUIO3(GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+#define GUIO_LIGHTHOUSE      GUIO4(GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
                                    GUIO_NOASPECT, \
-                                   GAMEOPTION_ORIGINAL_SAVELOAD)
+                                   GAMEOPTION_ORIGINAL_SAVELOAD, \
+                                   GAMEOPTION_HQ_VIDEO)
 
 	// Lighthouse - English Windows Demo (from jvprat)
 	// Executable scanning reports "2.100.002", VERSION file reports "1.00"
@@ -2979,11 +2982,12 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 
 #ifdef ENABLE_SCI32
 
-#define GUIO_PHANTASMAGORIA_DEMO GUIO5(GUIO_NOSUBTITLES, \
+#define GUIO_PHANTASMAGORIA_DEMO GUIO6(GUIO_NOSUBTITLES, \
                                        GUIO_NOASPECT, \
                                        GUIO_NOLAUNCHLOAD, \
                                        GUIO_LINKSPEECHTOSFX, \
-                                       GAMEOPTION_ENABLE_BLACK_LINED_VIDEO)
+                                       GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                                       GAMEOPTION_HQ_VIDEO)
 #define GUIO_PHANTASMAGORIA      GUIO_PHANTASMAGORIA_DEMO
 #define GUIO_PHANTASMAGORIA_MAC  GUIO_PHANTASMAGORIA_DEMO
 
@@ -3509,12 +3513,13 @@ static const struct ADGameDescription SciGameDescriptions[] = {
                                GUIO_LINKSPEECHTOSFX, \
                                GUIO_NOASPECT, \
                                GUIO_NOLAUNCHLOAD)
-#define GUIO_PQSWAT      GUIO6(GUIO_NOSUBTITLES, \
+#define GUIO_PQSWAT      GUIO7(GUIO_NOSUBTITLES, \
                                GUIO_NOMIDI, \
                                GUIO_LINKMUSICTOSFX, \
                                GUIO_LINKSPEECHTOSFX, \
                                GUIO_NOASPECT, \
-                               GAMEOPTION_ENABLE_BLACK_LINED_VIDEO)
+                               GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                               GAMEOPTION_HQ_VIDEO)
 
 	// Police Quest: SWAT - English DOS/Windows Demo (from jvprat)
 	// Executable scanning reports "2.100.002", VERSION file reports "0.001.200"
@@ -3957,14 +3962,16 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #undef GUIO_QFG4_CD
 
 // TODO: Correct GUIOs
-#define GUIO_RAMA_DEMO GUIO4(GUIO_NOMIDI, \
+#define GUIO_RAMA_DEMO GUIO5(GUIO_NOMIDI, \
                              GUIO_NOLAUNCHLOAD, \
                              GUIO_NOASPECT, \
-                             GAMEOPTION_ENABLE_BLACK_LINED_VIDEO)
-#define GUIO_RAMA      GUIO4(GUIO_NOMIDI, \
+                             GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                             GAMEOPTION_HQ_VIDEO)
+#define GUIO_RAMA      GUIO5(GUIO_NOMIDI, \
                              GUIO_NOASPECT, \
                              GAMEOPTION_ORIGINAL_SAVELOAD, \
-                             GAMEOPTION_ENABLE_BLACK_LINED_VIDEO)
+                             GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                             GAMEOPTION_HQ_VIDEO)
 
 	// RAMA - English DOS/Windows Demo
 	// Executable scanning reports "2.100.002", VERSION file reports "000.000.008"
@@ -4040,11 +4047,12 @@ static const struct ADGameDescription SciGameDescriptions[] = {
                                 GUIO_LINKSPEECHTOSFX, \
                                 GUIO_LINKMUSICTOSFX, \
                                 GUIO_NOASPECT)
-#define GUIO_SHIVERS      GUIO5(GUIO_NOMIDI, \
+#define GUIO_SHIVERS      GUIO6(GUIO_NOMIDI, \
                                 GUIO_LINKSPEECHTOSFX, \
                                 GUIO_LINKMUSICTOSFX, \
                                 GUIO_NOASPECT, \
-                                GAMEOPTION_ENABLE_BLACK_LINED_VIDEO)
+                                GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                                GAMEOPTION_HQ_VIDEO)
 
 	// Shivers - English Windows (from jvprat)
 	// Executable scanning reports "2.100.002", VERSION file reports "1.02"
@@ -4661,10 +4669,11 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #define GUIO_SQ6_DEMO GUIO3(GUIO_NOLAUNCHLOAD, \
                             GUIO_LINKSPEECHTOSFX, \
                             GUIO_NOASPECT)
-#define GUIO_SQ6      GUIO4(GUIO_LINKSPEECHTOSFX, \
+#define GUIO_SQ6      GUIO5(GUIO_LINKSPEECHTOSFX, \
                             GUIO_NOASPECT, \
                             GAMEOPTION_ORIGINAL_SAVELOAD, \
-                            GAMEOPTION_ENABLE_BLACK_LINED_VIDEO)
+                            GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                            GAMEOPTION_HQ_VIDEO)
 
 	// Space Quest 6 - English DOS/Win3.11 CD (from the Space Quest Collection)
 	// Executable scanning reports "2.100.002", VERSION file reports "1.0"
@@ -4740,10 +4749,11 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #define GUIO_TORIN_DEMO GUIO3(GUIO_NOMIDI, \
                               GUIO_NOLAUNCHLOAD, \
                               GUIO_NOASPECT)
-#define GUIO_TORIN      GUIO4(GUIO_NOMIDI, \
+#define GUIO_TORIN      GUIO5(GUIO_NOMIDI, \
                               GUIO_NOASPECT, \
                               GAMEOPTION_ORIGINAL_SAVELOAD, \
-                              GAMEOPTION_ENABLE_BLACK_LINED_VIDEO)
+                              GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                              GAMEOPTION_HQ_VIDEO)
 #define GUIO_TORIN_MAC  GUIO_TORIN
 
 	// Torin's Passage - English Windows Interactive Demo
diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index adc8f13..8eca671 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -29,6 +29,9 @@
 #include "sci/engine/vm.h"
 #include "sci/engine/vm_types.h"	// for reg_t
 #include "sci/util.h"
+#ifdef ENABLE_SCI32
+#include "sci/graphics/palette32.h"
+#endif
 
 namespace Sci {
 
@@ -1026,7 +1029,7 @@ public:
 		setRemap(remap);
 		setDataSize(width * height);
 		WRITE_SCI11ENDIAN_UINT32(_data + 16, 0);
-		setHunkPaletteOffset(paletteSize > 0 ? (width * height) : 0);
+		setHunkPaletteOffset(paletteSize > 0 ? (bitmapHeaderSize + width * height) : 0);
 		setDataOffset(bitmapHeaderSize);
 		setUncompressedDataOffset(bitmapHeaderSize);
 		setControlOffset(0);
@@ -1099,17 +1102,7 @@ public:
 
 	BITMAP_PROPERTY(32, DataSize, 12);
 
-	inline uint32 getHunkPaletteOffset() const {
-		return READ_SCI11ENDIAN_UINT32(_data + 20);
-	}
-
-	inline void setHunkPaletteOffset(uint32 hunkPaletteOffset) {
-		if (hunkPaletteOffset) {
-			hunkPaletteOffset += getBitmapHeaderSize();
-		}
-
-		WRITE_SCI11ENDIAN_UINT32(_data + 20, hunkPaletteOffset);
-	}
+	BITMAP_PROPERTY(32, HunkPaletteOffset, 20);
 
 	BITMAP_PROPERTY(32, DataOffset, 24);
 
@@ -1162,6 +1155,14 @@ public:
 		return _data + getHunkPaletteOffset();
 	}
 
+	inline void setPalette(const Palette &palette) {
+		byte *paletteData = getHunkPalette();
+		if (paletteData != nullptr) {
+			SciSpan<byte> paletteSpan(paletteData, getRawSize() - getHunkPaletteOffset());
+			HunkPalette::write(paletteSpan, palette);
+		}
+	}
+
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 
 	void applyRemap(SciArray &clut) {
diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 80800da..7415979 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -46,6 +46,7 @@
 #include "sci/graphics/cursor32.h"
 #include "sci/graphics/font.h"
 #include "sci/graphics/frameout.h"
+#include "sci/graphics/helpers.h"
 #include "sci/graphics/paint32.h"
 #include "sci/graphics/palette32.h"
 #include "sci/graphics/plane32.h"
@@ -548,47 +549,20 @@ void GfxFrameout::palMorphFrameOut(const int8 *styleRanges, PlaneShowStyle *show
 	showBits();
 }
 
-/**
- * Determines the parts of `r` that aren't overlapped by `other`.
- * Returns -1 if `r` and `other` have no intersection.
- * Returns number of returned parts (in `outRects`) otherwise.
- * (In particular, this returns 0 if `r` is contained in `other`.)
- */
-int splitRects(Common::Rect r, const Common::Rect &other, Common::Rect(&outRects)[4]) {
-	if (!r.intersects(other)) {
-		return -1;
-	}
-
-	int splitCount = 0;
-	if (r.top < other.top) {
-		Common::Rect &t = outRects[splitCount++];
-		t = r;
-		t.bottom = other.top;
-		r.top = other.top;
-	}
-
-	if (r.bottom > other.bottom) {
-		Common::Rect &t = outRects[splitCount++];
-		t = r;
-		t.top = other.bottom;
-		r.bottom = other.bottom;
-	}
-
-	if (r.left < other.left) {
-		Common::Rect &t = outRects[splitCount++];
-		t = r;
-		t.right = other.left;
-		r.left = other.left;
-	}
-
-	if (r.right > other.right) {
-		Common::Rect &t = outRects[splitCount++];
-		t = r;
-		t.left = other.right;
-	}
+void GfxFrameout::directFrameOut(const Common::Rect &showRect) {
+	updateMousePositionForRendering();
+	_showList.add(showRect);
+	showBits();
+}
 
-	return splitCount;
+#ifdef USE_RGB_COLOR
+void GfxFrameout::resetHardware() {
+	updateMousePositionForRendering();
+	_showList.add(Common::Rect(getCurrentBuffer().screenWidth, getCurrentBuffer().screenHeight));
+	g_system->getPaletteManager()->setPalette(_palette->getHardwarePalette(), 0, 256);
+	showBits();
 }
+#endif
 
 /**
  * Determines the parts of `middleRect` that aren't overlapped
@@ -1041,7 +1015,21 @@ void GfxFrameout::showBits() {
 			continue;
 		}
 
-		g_system->copyRectToScreen(sourceBuffer, _currentBuffer.screenWidth, rounded.left, rounded.top, rounded.width(), rounded.height());
+#ifdef USE_RGB_COLOR
+		if (g_system->getScreenFormat() != _currentBuffer.format) {
+			// This happens (at least) when playing a video in Shivers with
+			// HQ video on & subtitles on
+			Graphics::Surface *screenSurface = _currentBuffer.getSubArea(rounded).convertTo(g_system->getScreenFormat(), _palette->getHardwarePalette());
+			assert(screenSurface);
+			g_system->copyRectToScreen(screenSurface->getPixels(), screenSurface->pitch, rounded.left, rounded.top, screenSurface->w, screenSurface->h);
+			screenSurface->free();
+			delete screenSurface;
+		} else {
+#else
+		{
+#endif
+			g_system->copyRectToScreen(sourceBuffer, _currentBuffer.screenWidth, rounded.left, rounded.top, rounded.width(), rounded.height());
+		}
 	}
 
 	_cursor->donePainting();
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index 4bdb172..26732d5 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -340,6 +340,19 @@ public:
 	void palMorphFrameOut(const int8 *styleRanges, PlaneShowStyle *showStyle);
 
 	/**
+	 * Draws the given rect from the internal screen buffer to hardware without
+	 * processing any other graphics updates except for cursor changes.
+	 */
+	void directFrameOut(const Common::Rect &showRect);
+
+#ifdef USE_RGB_COLOR
+	/**
+	 * Sends the entire internal screen buffer and palette to hardware.
+	 */
+	void resetHardware();
+#endif
+
+	/**
 	 * Modifies the raw pixel data for the next frame with
 	 * new palette indexes based on matched style ranges.
 	 */
diff --git a/engines/sci/graphics/helpers.h b/engines/sci/graphics/helpers.h
index 1da3749..52699c6 100644
--- a/engines/sci/graphics/helpers.h
+++ b/engines/sci/graphics/helpers.h
@@ -187,6 +187,48 @@ inline void mulru(Common::Rect &rect, const Common::Rational &ratioX, const Comm
 	rect.bottom = mulru(rect.bottom - 1, ratioY, extra) + 1;
 }
 
+/**
+ * Determines the parts of `r` that aren't overlapped by `other`.
+ * Returns -1 if `r` and `other` have no intersection.
+ * Returns number of returned parts (in `outRects`) otherwise.
+ * (In particular, this returns 0 if `r` is contained in `other`.)
+ */
+inline int splitRects(Common::Rect r, const Common::Rect &other, Common::Rect(&outRects)[4]) {
+	if (!r.intersects(other)) {
+		return -1;
+	}
+
+	int splitCount = 0;
+	if (r.top < other.top) {
+		Common::Rect &t = outRects[splitCount++];
+		t = r;
+		t.bottom = other.top;
+		r.top = other.top;
+	}
+
+	if (r.bottom > other.bottom) {
+		Common::Rect &t = outRects[splitCount++];
+		t = r;
+		t.top = other.bottom;
+		r.bottom = other.bottom;
+	}
+
+	if (r.left < other.left) {
+		Common::Rect &t = outRects[splitCount++];
+		t = r;
+		t.right = other.left;
+		r.left = other.left;
+	}
+
+	if (r.right > other.right) {
+		Common::Rect &t = outRects[splitCount++];
+		t = r;
+		t.left = other.right;
+	}
+
+	return splitCount;
+}
+
 struct Buffer : public Graphics::Surface {
 	uint16 screenWidth;
 	uint16 screenHeight;
diff --git a/engines/sci/graphics/palette32.cpp b/engines/sci/graphics/palette32.cpp
index acf63bc..339461d 100644
--- a/engines/sci/graphics/palette32.cpp
+++ b/engines/sci/graphics/palette32.cpp
@@ -53,6 +53,29 @@ HunkPalette::HunkPalette(const SciSpan<const byte> &rawPalette) :
 	}
 }
 
+void HunkPalette::write(SciSpan<byte> &out, const Palette &palette) {
+	const uint8 numPalettes = 1;
+	const uint16 paletteOffset = kHunkPaletteHeaderSize + 2 * numPalettes;
+
+	out[kNumPaletteEntriesOffset] = numPalettes;
+	out[kHunkPaletteHeaderSize + 2] = paletteOffset;
+
+	SciSpan<byte> entry = out.subspan(paletteOffset);
+	entry[kEntryStartColorOffset] = 0;
+	entry.setUint16SEAt(kEntryNumColorsOffset, ARRAYSIZE(palette.colors));
+	entry[kEntryUsedOffset] = 1;
+	entry[kEntrySharedUsedOffset] = 0;
+	entry.setUint32SEAt(kEntryVersionOffset, 1);
+
+	SciSpan<byte> paletteData = entry.subspan(kEntryHeaderSize);
+	for (uint i = 0; i < ARRAYSIZE(palette.colors); ++i) {
+		*paletteData++ = palette.colors[i].used;
+		*paletteData++ = palette.colors[i].r;
+		*paletteData++ = palette.colors[i].g;
+		*paletteData++ = palette.colors[i].b;
+	}
+}
+
 void HunkPalette::setVersion(const uint32 version) const {
 	if (_numPalettes != _data.getUint8At(kNumPaletteEntriesOffset)) {
 		error("Invalid HunkPalette");
@@ -333,6 +356,9 @@ static const uint8 gammaTables[GfxPalette32::numGammaTables][256] = {
 	// Palette versioning
 	_version(1),
 	_needsUpdate(false),
+#ifdef USE_RGB_COLOR
+	_hardwarePalette(),
+#endif
 	_currentPalette(),
 	_sourcePalette(),
 	_nextPalette(),
@@ -459,7 +485,11 @@ void GfxPalette32::updateHardware() {
 		return;
 	}
 
-	byte bpal[3 * 256];
+#ifdef USE_RGB_COLOR
+	uint8 *bpal = _hardwarePalette;
+#else
+	uint8 bpal[256 * 3];
+#endif
 
 	// HACK: There are resources in a couple of Windows-only games that seem to
 	// include bogus palette entries above 236. SSCI does a lot of extra work
@@ -508,7 +538,13 @@ void GfxPalette32::updateHardware() {
 		bpal[255 * 3 + 2] = 0;
 	}
 
-	g_system->getPaletteManager()->setPalette(bpal, 0, 256);
+	// If the system is in a high color mode, which can happen during video
+	// playback, attempting to send the palette to OSystem is illegal and will
+	// result in a crash
+	if (g_system->getScreenFormat().bytesPerPixel == 1) {
+		g_system->getPaletteManager()->setPalette(bpal, 0, 256);
+	}
+
 	_gammaChanged = false;
 }
 
diff --git a/engines/sci/graphics/palette32.h b/engines/sci/graphics/palette32.h
index fb0b226..b8ba85e 100644
--- a/engines/sci/graphics/palette32.h
+++ b/engines/sci/graphics/palette32.h
@@ -37,6 +37,16 @@ class HunkPalette {
 public:
 	HunkPalette(const SciSpan<const byte> &rawPalette);
 
+	static void write(SciSpan<byte> &out, const Palette &palette);
+
+	static uint32 calculateHunkPaletteSize(const uint16 numIndexes = 256, const bool sharedUsed = true) {
+		const int numPalettes = 1;
+		return kHunkPaletteHeaderSize +
+			/* slack bytes between hunk header & palette offset table */ 2 +
+			/* palette offset table */ 2 * numPalettes +
+			/* palette data */ (kEntryHeaderSize + numIndexes * (/* RGB */ 3 + !sharedUsed)) * numPalettes;
+	}
+
 	/**
 	 * Gets the version of the palette. Used to avoid resubmitting a HunkPalette
 	 * which has already been submitted for the next frame.
@@ -237,6 +247,14 @@ public:
 	 */
 	inline const Palette &getCurrentPalette() const { return _currentPalette; };
 
+#ifdef USE_RGB_COLOR
+	/**
+	 * Gets the raw hardware palette in RGB format. This should be used instead
+	 * of `::PaletteManager::grabPalette` when the OSystem screen is >8bpp.
+	 */
+	inline const uint8 *getHardwarePalette() const { return _hardwarePalette; };
+#endif
+
 	/**
 	 * Loads a palette into GfxPalette32 with the given resource ID.
 	 */
@@ -288,6 +306,15 @@ private:
 	 */
 	bool _needsUpdate;
 
+#ifdef USE_RGB_COLOR
+	/**
+	 * A local copy of the hardware palette. Used when the backend is in a true
+	 * color mode and a change to the game's internal framebuffer occurs that
+	 * needs to be reconverted from 8bpp to the backend's bit depth.
+	 */
+	uint8 _hardwarePalette[256 * 3];
+#endif
+
 	/**
 	 * The currently displayed palette.
 	 */
diff --git a/engines/sci/graphics/plane32.cpp b/engines/sci/graphics/plane32.cpp
index 0a69ab2..47d41bc 100644
--- a/engines/sci/graphics/plane32.cpp
+++ b/engines/sci/graphics/plane32.cpp
@@ -26,6 +26,7 @@
 #include "sci/engine/selector.h"
 #include "sci/engine/state.h"
 #include "sci/graphics/frameout.h"
+#include "sci/graphics/helpers.h"
 #include "sci/graphics/lists32.h"
 #include "sci/graphics/plane32.h"
 #include "sci/graphics/remap32.h"
@@ -262,8 +263,6 @@ void Plane::deleteAllPics() {
 #pragma mark -
 #pragma mark Plane - Rendering
 
-extern int splitRects(Common::Rect r, const Common::Rect &other, Common::Rect(&outRects)[4]);
-
 void Plane::breakDrawListByPlanes(DrawList &drawList, const PlaneList &planeList) const {
 	const int nextPlaneIndex = planeList.findIndexByObject(_object) + 1;
 	const PlaneList::size_type planeCount = planeList.size();
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index ecde85b..b05b9e6 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -562,9 +562,10 @@ VMDPlayer::IOStatus VMDPlayer::open(const Common::String &fileName, const OpenFl
 	return kIOError;
 }
 
-void VMDPlayer::init(const int16 x, const int16 y, const PlayFlags flags, const int16 boostPercent, const int16 boostStartColor, const int16 boostEndColor) {
-	_x = getSciVersion() >= SCI_VERSION_3 ? x : (x & ~1);
-	_y = y;
+void VMDPlayer::init(int16 x, const int16 y, const PlayFlags flags, const int16 boostPercent, const int16 boostStartColor, const int16 boostEndColor) {
+	if (getSciVersion() < SCI_VERSION_3) {
+		x &= ~1;
+	}
 	_doublePixels = flags & kPlayFlagDoublePixels;
 	_blackLines = ConfMan.getBool("enable_black_lined_video") && (flags & kPlayFlagBlackLines);
 	// If ScummVM has been configured to disable black lines on video playback,
@@ -579,6 +580,11 @@ void VMDPlayer::init(const int16 x, const int16 y, const PlayFlags flags, const
 	_blackPalette = flags & kPlayFlagBlackPalette;
 #endif
 	_stretchVertical = flags & kPlayFlagStretchVertical;
+
+	_drawRect = Common::Rect(x,
+							 y,
+							 x + (_decoder->getWidth() << _doublePixels),
+							 y + (_decoder->getHeight() << (_doublePixels || _stretchVertical)));
 }
 
 VMDPlayer::IOStatus VMDPlayer::close() {
@@ -586,32 +592,10 @@ VMDPlayer::IOStatus VMDPlayer::close() {
 		return kIOSuccess;
 	}
 
-	_decoder->close();
-	_isOpen = false;
-	_isInitialized = false;
-	_ignorePalettes = false;
-
-	if (_bundledVmd) {
-		g_sci->getResMan()->unlockResource(_bundledVmd);
-		_bundledVmd = nullptr;
-	}
-
-	if (_bitmapId != NULL_REG) {
-		_segMan->freeBitmap(_bitmapId);
-		_bitmapId = NULL_REG;
-	}
-
-	if (!_planeIsOwned && _screenItem != nullptr) {
-		g_sci->_gfxFrameout->deleteScreenItem(*_screenItem);
-		_screenItem = nullptr;
-	} else if (_plane != nullptr) {
-		g_sci->_gfxFrameout->deletePlane(*_plane);
-		_plane = nullptr;
-	}
-
-	if (!_leaveLastFrame && _leaveScreenBlack) {
-		// This call *actually* deletes the plane/screen item
-		g_sci->_gfxFrameout->frameOut(true);
+	if (_isComposited) {
+		closeComposited();
+	} else {
+		closeOverlay();
 	}
 
 	if (_blackoutPlane != nullptr) {
@@ -624,13 +608,24 @@ VMDPlayer::IOStatus VMDPlayer::close() {
 		g_sci->_gfxFrameout->frameOut(true);
 	}
 
+	_decoder->close();
+
+	if (_bundledVmd) {
+		g_sci->getResMan()->unlockResource(_bundledVmd);
+		_bundledVmd = nullptr;
+	}
+
 	if (!_showCursor) {
 		g_sci->_gfxCursor32->unhide();
 	}
 
+	_isOpen = false;
+	_isInitialized = false;
+	_ignorePalettes = false;
 	_lastYieldedFrameNo = 0;
 	_planeIsOwned = true;
 	_priority = 0;
+	_drawRect = Common::Rect();
 	return kIOSuccess;
 }
 
@@ -695,70 +690,21 @@ VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags) {
 			g_sci->_gfxCursor32->hide();
 		}
 
-		Common::Rect vmdRect(_x,
-							 _y,
-							 _x + _decoder->getWidth(),
-							 _y + _decoder->getHeight());
-		ScaleInfo vmdScaleInfo;
-
 		if (!_blackoutRect.isEmpty() && _planeIsOwned) {
 			_blackoutPlane = new Plane(_blackoutRect);
 			g_sci->_gfxFrameout->addPlane(*_blackoutPlane);
 		}
 
-		if (_doublePixels) {
-			vmdScaleInfo.x = 256;
-			vmdScaleInfo.y = 256;
-			vmdScaleInfo.signal = kScaleSignalManual;
-			vmdRect.right += vmdRect.width();
-			vmdRect.bottom += vmdRect.height();
-		} else if (_stretchVertical) {
-			vmdScaleInfo.y = 256;
-			vmdScaleInfo.signal = kScaleSignalManual;
-			vmdRect.bottom += vmdRect.height();
-		}
-
-		const int16 screenWidth = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
-		const int16 screenHeight = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
-		const int16 scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
-		const int16 scriptHeight = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
-
-		SciBitmap &vmdBitmap = *_segMan->allocateBitmap(&_bitmapId, vmdRect.width(), vmdRect.height(), 255, 0, 0, screenWidth, screenHeight, 0, false, false);
-		vmdBitmap.getBuffer().fillRect(Common::Rect(vmdRect.width(), vmdRect.height()), 0);
-
-		if (screenWidth != scriptWidth || screenHeight != scriptHeight) {
-			mulru(vmdRect, Ratio(scriptWidth, screenWidth), Ratio(scriptHeight, screenHeight), 1);
-		}
-
-		CelInfo32 vmdCelInfo;
-		vmdCelInfo.bitmap = _bitmapId;
-		_decoder->setSurfaceMemory(vmdBitmap.getPixels(), vmdBitmap.getWidth(), vmdBitmap.getHeight(), 1);
-
-		if (_planeIsOwned) {
-			_x = 0;
-			_y = 0;
-			_plane = new Plane(vmdRect, kPlanePicColored);
-			if (_priority) {
-				_plane->_priority = _priority;
-			}
-			g_sci->_gfxFrameout->addPlane(*_plane);
-			_screenItem = new ScreenItem(_plane->_object, vmdCelInfo, Common::Point(), vmdScaleInfo);
+		if (shouldUseCompositing()) {
+			_isComposited = true;
+			_usingHighColor = false;
+			initComposited();
 		} else {
-			_screenItem = new ScreenItem(_plane->_object, vmdCelInfo, Common::Point(_x, _y), vmdScaleInfo);
-			if (_priority) {
-				_screenItem->_priority = _priority;
-			}
-		}
-
-		if (_blackLines) {
-			_screenItem->_drawBlackLines = true;
+			_isComposited = false;
+			_usingHighColor = shouldUseHighColor();
+			initOverlay();
 		}
 
-		// NOTE: There was code for positioning the screen item using insetRect
-		// here, but none of the game scripts seem to use this functionality.
-
-		g_sci->_gfxFrameout->addScreenItem(*_screenItem);
-
 		_decoder->start();
 	}
 
@@ -830,66 +776,245 @@ VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags) {
 	return stopFlag;
 }
 
-#pragma mark -
-#pragma mark VMDPlayer - Rendering
+void VMDPlayer::initOverlay() {
+	g_sci->_gfxFrameout->frameOut(true);
 
-void VMDPlayer::renderFrame() const {
-	// This writes directly to the CelObjMem we already created,
-	// so no need to take its return value
-	_decoder->decodeNextFrame();
+#ifdef USE_RGB_COLOR
+	// TODO: Allow interpolation for videos where the cursor is drawn, either by
+	// writing to an intermediate 4bpp surface and using that surface during
+	// cursor drawing, or by promoting the cursor code to use CursorMan, if
+	// possible
+	if (_usingHighColor) {
+		// TODO: 8888 is used here because 4bpp is the only format currently
+		// supported by the common scaling code
+		const Graphics::PixelFormat format = Graphics::createPixelFormat<8888>();
+		g_sci->_gfxFrameout->setPixelFormat(format);
+		redrawGameScreen();
+	}
+#endif
+}
 
-	// NOTE: Normally this would write a hunk palette at the end of the
-	// video bitmap that CelObjMem would read out and submit, but instead
-	// we are just submitting it directly here because the decoder exposes
-	// this information a little bit differently than the one in SSCI
-	const bool dirtyPalette = _decoder->hasDirtyPalette();
-	if (dirtyPalette && !_ignorePalettes) {
-		Palette palette;
-		for (uint16 i = 0; i < _startColor; ++i) {
-			palette.colors[i].used = false;
+#ifdef USE_RGB_COLOR
+void VMDPlayer::redrawGameScreen() const {
+	Graphics::Surface *game = g_sci->_gfxFrameout->getCurrentBuffer().convertTo(g_system->getScreenFormat(), g_sci->_gfxPalette32->getHardwarePalette());
+
+	Common::Rect rects[4];
+	int splitCount = splitRects(Common::Rect(game->w, game->h), _drawRect, rects);
+	if (splitCount != -1) {
+		while (splitCount--) {
+			const Common::Rect &drawRect = rects[splitCount];
+			g_system->copyRectToScreen(game->getBasePtr(drawRect.left, drawRect.top), game->pitch, drawRect.left, drawRect.top, drawRect.width(), drawRect.height());
 		}
-		for (uint16 i = _endColor; i < 256; ++i) {
-			palette.colors[i].used = false;
+	}
+
+	game->free();
+	delete game;
+}
+#endif
+
+void VMDPlayer::renderOverlay() const {
+	const Graphics::Surface *nextFrame = _decoder->decodeNextFrame();
+
+#ifdef USE_RGB_COLOR
+	if (_usingHighColor) {
+		if (updatePalette()) {
+			redrawGameScreen();
 		}
-#if SCI_VMD_BLACK_PALETTE
-		if (_blackPalette) {
-			for (uint16 i = _startColor; i <= _endColor; ++i) {
-				palette.colors[i].r = palette.colors[i].g = palette.colors[i].b = 0;
-				palette.colors[i].used = true;
+
+		writeFrameToSystem<Graphics::FILTER_BILINEAR>(nextFrame, _decoder, _drawRect);
+	} else {
+#else
+	{
+#endif
+		updatePalette();
+
+		Graphics::Surface out = g_sci->_gfxFrameout->getCurrentBuffer().getSubArea(_drawRect);
+
+		const int lineCount = _blackLines ? 2 : 1;
+		if (_doublePixels) {
+			for (int16 y = 0; y < _drawRect.height(); y += lineCount) {
+				const uint8 *source = (uint8 *)nextFrame->getBasePtr(0, y >> 1);
+				uint8 *target = (uint8 *)out.getBasePtr(0, y);
+				for (int16 x = 0; x < _decoder->getWidth(); ++x) {
+					*target++ = *source;
+					*target++ = *source++;
+				}
+			}
+		} else if (_blackLines) {
+			for (int16 y = 0; y < _drawRect.height(); y += lineCount) {
+				const uint8 *source = (uint8 *)nextFrame->getBasePtr(0, y);
+				uint8 *target = (uint8 *)out.getBasePtr(0, y);
+				memcpy(target, source, _drawRect.width());
 			}
-		} else
+		} else {
+			out.copyRectToSurface(nextFrame->getPixels(), nextFrame->pitch, 0, 0, nextFrame->w, nextFrame->h);
+		}
+
+		g_sci->_gfxFrameout->directFrameOut(_drawRect);
+	}
+}
+
+bool VMDPlayer::updatePalette() const {
+	if (_ignorePalettes || !_decoder->hasDirtyPalette()) {
+		return false;
+	}
+
+	Palette palette;
+	for (uint16 i = 0; i < _startColor; ++i) {
+		palette.colors[i].used = false;
+	}
+	for (uint16 i = _endColor + 1; i < ARRAYSIZE(palette.colors); ++i) {
+		palette.colors[i].used = false;
+	}
+#if SCI_VMD_BLACK_PALETTE
+	if (_blackPalette) {
+		for (uint16 i = _startColor; i <= _endColor; ++i) {
+			palette.colors[i].r = palette.colors[i].g = palette.colors[i].b = 0;
+			palette.colors[i].used = true;
+		}
+	} else
 #endif
-			fillPalette(palette);
+		fillPalette(palette);
 
-		g_sci->_gfxPalette32->submit(palette);
+	if (_isComposited) {
+		SciBitmap *bitmap = _segMan->lookupBitmap(_bitmapId);
+		bitmap->setPalette(palette);
 		g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
 		g_sci->_gfxFrameout->frameOut(true);
+	} else {
+		g_sci->_gfxPalette32->submit(palette);
+		g_sci->_gfxPalette32->updateForFrame();
+		g_sci->_gfxPalette32->updateHardware();
+	}
 
 #if SCI_VMD_BLACK_PALETTE
-		if (_blackPalette) {
-			fillPalette(palette);
-			g_sci->_gfxPalette32->submit(palette);
-			g_sci->_gfxPalette32->updateForFrame();
-			g_sci->_gfxPalette32->updateHardware();
+	if (_blackPalette) {
+		fillPalette(palette);
+		if (_isComposited) {
+			SciBitmap *bitmap = _segMan->lookupBitmap(_bitmapId);
+			bitmap->setPalette(palette);
 		}
+		g_sci->_gfxPalette32->submit(palette);
+		g_sci->_gfxPalette32->updateForFrame();
+		g_sci->_gfxPalette32->updateHardware();
+	}
 #endif
+
+	return true;
+}
+
+void VMDPlayer::closeOverlay() {
+#ifdef USE_RGB_COLOR
+	if (_usingHighColor) {
+		g_sci->_gfxFrameout->setPixelFormat(Graphics::PixelFormat::createFormatCLUT8());
+		g_sci->_gfxFrameout->resetHardware();
 	} else {
+#else
+	{
+#endif
+		g_sci->_gfxFrameout->frameOut(true, _drawRect);
+	}
+}
+
+void VMDPlayer::initComposited() {
+	ScaleInfo vmdScaleInfo;
+
+	if (_doublePixels) {
+		vmdScaleInfo.x *= 2;
+		vmdScaleInfo.y *= 2;
+		vmdScaleInfo.signal = kScaleSignalManual;
+	} else if (_stretchVertical) {
+		vmdScaleInfo.y *= 2;
+		vmdScaleInfo.signal = kScaleSignalManual;
+	}
+
+	const uint32 hunkPaletteSize = HunkPalette::calculateHunkPaletteSize(256, false);
+	const int16 screenWidth = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
+	const int16 screenHeight = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
+
+	SciBitmap &vmdBitmap = *_segMan->allocateBitmap(&_bitmapId, _drawRect.width(), _drawRect.height(), 255, 0, 0, screenWidth, screenHeight, hunkPaletteSize, false, false);
+	vmdBitmap.getBuffer().fillRect(Common::Rect(_drawRect.width(), _drawRect.height()), 0);
+
+	CelInfo32 vmdCelInfo;
+	vmdCelInfo.bitmap = _bitmapId;
+	_decoder->setSurfaceMemory(vmdBitmap.getPixels(), vmdBitmap.getWidth(), vmdBitmap.getHeight(), 1);
+
+	if (_planeIsOwned) {
+		_plane = new Plane(_drawRect, kPlanePicColored);
+		if (_priority) {
+			_plane->_priority = _priority;
+		}
+		g_sci->_gfxFrameout->addPlane(*_plane);
+		_screenItem = new ScreenItem(_plane->_object, vmdCelInfo, Common::Point(), vmdScaleInfo);
+	} else {
+		_screenItem = new ScreenItem(_plane->_object, vmdCelInfo, Common::Point(_drawRect.left, _drawRect.top), vmdScaleInfo);
+		if (_priority) {
+			_screenItem->_priority = _priority;
+		}
+	}
+
+	if (_blackLines) {
+		_screenItem->_drawBlackLines = true;
+	}
+
+	// NOTE: There was code for positioning the screen item using insetRect
+	// here, but none of the game scripts seem to use this functionality.
+
+	g_sci->_gfxFrameout->addScreenItem(*_screenItem);
+}
+
+void VMDPlayer::renderComposited() const {
+	// This writes directly to the CelObjMem we already created,
+	// so no need to take its return value
+	_decoder->decodeNextFrame();
+	if (!updatePalette()) {
 		g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
 		g_sci->_gfxFrameout->frameOut(true);
 	}
 }
 
+void VMDPlayer::closeComposited() {
+	if (_bitmapId != NULL_REG) {
+		_segMan->freeBitmap(_bitmapId);
+		_bitmapId = NULL_REG;
+	}
+
+	if (!_planeIsOwned && _screenItem != nullptr) {
+		g_sci->_gfxFrameout->deleteScreenItem(*_screenItem);
+		_screenItem = nullptr;
+	} else if (_plane != nullptr) {
+		g_sci->_gfxFrameout->deletePlane(*_plane);
+		_plane = nullptr;
+	}
+
+	if (!_leaveLastFrame && _leaveScreenBlack) {
+		// This call *actually* deletes the plane/screen item
+		g_sci->_gfxFrameout->frameOut(true);
+	}
+}
+
+#pragma mark -
+#pragma mark VMDPlayer - Rendering
+
+void VMDPlayer::renderFrame() const {
+	if (_isComposited) {
+		renderComposited();
+	} else {
+		renderOverlay();
+	}
+}
+
 void VMDPlayer::fillPalette(Palette &palette) const {
 	const byte *vmdPalette = _decoder->getPalette() + _startColor * 3;
 	for (uint16 i = _startColor; i <= _endColor; ++i) {
-		int16 r = *vmdPalette++;
-		int16 g = *vmdPalette++;
-		int16 b = *vmdPalette++;
+		uint8 r = *vmdPalette++;
+		uint8 g = *vmdPalette++;
+		uint8 b = *vmdPalette++;
 
 		if (_boostPercent != 100 && i >= _boostStartColor && i <= _boostEndColor) {
-			r = CLIP<int16>(r * _boostPercent / 100, 0, 255);
-			g = CLIP<int16>(g * _boostPercent / 100, 0, 255);
-			b = CLIP<int16>(b * _boostPercent / 100, 0, 255);
+			r = CLIP(r * _boostPercent / 100, 0, 255);
+			g = CLIP(g * _boostPercent / 100, 0, 255);
+			b = CLIP(b * _boostPercent / 100, 0, 255);
 		}
 
 		palette.colors[i].r = r;
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index a023fd4..0f63996 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -308,6 +308,14 @@ private:
 	 */
 	int _lastYieldedFrameNo;
 
+	void initOverlay();
+	void renderOverlay() const;
+	void closeOverlay();
+
+	void initComposited();
+	void renderComposited() const;
+	void closeComposited();
+
 	/**
 	 * Plays the VMD until an event occurs (e.g. user
 	 * presses escape, clicks, etc.).
@@ -330,10 +338,9 @@ public:
 
 private:
 	/**
-	 * The location of the VMD plane, in game script
-	 * coordinates.
+	 * The rectangle where the video will be drawn, in screen coordinates.
 	 */
-	int16 _x, _y;
+	Common::Rect _drawRect;
 
 	/**
 	 * The plane where the VMD will be drawn.
@@ -399,17 +406,74 @@ private:
 	bool _ignorePalettes;
 
 	/**
+	 * Whether or not rendering mode is composited.
+	 */
+	bool _isComposited;
+
+	/**
+	 * Whether or not rendering of the video is being performed in high color or
+	 * better.
+	 */
+	bool _usingHighColor;
+
+	/**
 	 * Renders a frame of video to the output bitmap.
 	 */
 	void renderFrame() const;
 
 	/**
+	 * Updates the system with palette data from the video.
+	 */
+	bool updatePalette() const;
+
+	/**
 	 * Fills the given palette with RGB values from
 	 * the VMD palette, applying brightness boost if
 	 * it is enabled.
 	 */
 	void fillPalette(Palette &palette) const;
 
+#ifdef USE_RGB_COLOR
+	/**
+	 * Redraws areas of the screen outside of the video to the system buffer.
+	 * This is used when
+	 */
+	void redrawGameScreen() const;
+#endif
+
+	/**
+	 * Determines whether or not the VMD player should upgrade the renderer to
+	 * high color depth when rendering the video.
+	 *
+	 * @TODO It should be possible in the future to allow high color composited
+	 * video, but this will require additional work in GfxFrameout and
+	 * GfxCursor32 since the internal buffer and cursor code are 8bpp only.
+	 */
+	bool shouldUseHighColor() const {
+#ifdef USE_RGB_COLOR
+		return ConfMan.getBool("enable_hq_video") &&
+			_priority == 0 &&
+			(_doublePixels || _stretchVertical) &&
+			!_leaveLastFrame &&
+			!_showCursor &&
+			!_blackLines;
+#else
+		return false;
+#endif
+	}
+
+	/**
+	 * Determines whether or not the video should use the compositing renderer
+	 * instead of the overlay renderer.
+	 */
+	bool shouldUseCompositing() const {
+#ifdef USE_RGB_COLOR
+		return getSciVersion() == SCI_VERSION_3 && !shouldUseHighColor();
+#else
+		return getSciVersion() == SCI_VERSION_3;
+#endif
+	}
+
 #pragma mark -
 #pragma mark VMDPlayer - Blackout
 public:


Commit: 71256a0d3c2136d21d943513766ec2acd623f6c1
    https://github.com/scummvm/scummvm/commit/71256a0d3c2136d21d943513766ec2acd623f6c1
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:39-05:00

Commit Message:
SCI32: Improve playback quality of SEQ videos

Changed paths:
    engines/sci/detection_tables.h
    engines/sci/graphics/video32.cpp
    engines/sci/graphics/video32.h


diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index 9378f2b..4230a3b 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -724,10 +724,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #ifdef ENABLE_SCI32
 #define GUIO_GK1_FLOPPY GUIO2(GUIO_NOSPEECH, \
                               GAMEOPTION_ORIGINAL_SAVELOAD)
-#define GUIO_GK1_CD_DOS GUIO3(GUIO_LINKSPEECHTOSFX, \
-                              GAMEOPTION_ORIGINAL_SAVELOAD, \
-                              GAMEOPTION_HIGH_RESOLUTION_GRAPHICS)
-#define GUIO_GK1_CD_WIN GUIO4(GUIO_LINKSPEECHTOSFX, \
+#define GUIO_GK1_CD     GUIO4(GUIO_LINKSPEECHTOSFX, \
                               GAMEOPTION_ORIGINAL_SAVELOAD, \
                               GAMEOPTION_HIGH_RESOLUTION_GRAPHICS, \
                               GAMEOPTION_HQ_VIDEO)
@@ -779,7 +776,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "372d059f75856afa6d73dd84cbb8913d", 10996},
 		{"resource.000", 0, "69b7516962510f780d38519cc15fcc7c", 12581736},
 		AD_LISTEND},
-		Common::EN_ANY, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
+		Common::EN_ANY, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
 
 	// Gabriel Knight - English Windows CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "01.100.000"
@@ -787,7 +784,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "372d059f75856afa6d73dd84cbb8913d", 10996},
 		{"resource.000", 0, "69b7516962510f780d38519cc15fcc7c", 12581736},
 		AD_LISTEND},
-		Common::EN_ANY, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_WIN },
+		Common::EN_ANY, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
 
 	// Gabriel Knight - German DOS CD (from Tobis87)
 	// SCI interpreter version 2.000.000
@@ -795,7 +792,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "a7d3e55114c65647310373cb390815ba", 11392},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13400497},
 		AD_LISTEND},
-		Common::DE_DEU, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
+		Common::DE_DEU, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
 
 	// Gabriel Knight - Spanish DOS CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "1.000.000, April 13, 1995"
@@ -803,7 +800,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "7cb6e9bba15b544ec7a635c45bde9953", 11404},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13381599},
 		AD_LISTEND},
-		Common::ES_ESP, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
+		Common::ES_ESP, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
 
 	// Gabriel Knight - French DOS CD (from Hkz)
 	// VERSION file reports "1.000.000, May 3, 1994"
@@ -811,7 +808,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "55f909ba93a2515042a08d8a2da8414e", 11392},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13325145},
 		AD_LISTEND},
-		Common::FR_FRA, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
+		Common::FR_FRA, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
 
 	// Gabriel Knight - Spanish Windows CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "1.000.000, April 13, 1995"
@@ -819,7 +816,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "7cb6e9bba15b544ec7a635c45bde9953", 11404},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13381599},
 		AD_LISTEND},
-		Common::ES_ESP, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_WIN },
+		Common::ES_ESP, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
 
 	// Gabriel Knight - English Macintosh (Floppy!)
 	// This version is hi-res ONLY, so it should NOT get GAMEOPTION_HIGH_RESOLUTION_GRAPHICS
@@ -834,8 +831,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		Common::EN_ANY, Common::kPlatformMacintosh, ADGF_MACRESFORK | ADGF_UNSTABLE, GUIO_GK1_MAC },
 
 #undef GUIO_GK1_FLOPPY
-#undef GUIO_GK1_CD_DOS
-#undef GUIO_GK1_CD_WIN
+#undef GUIO_GK1_CD
 #undef GUIO_GK1_MAC
 
 #define GUIO_GK2_DEMO GUIO8(GUIO_NOSUBTITLES, \
@@ -3915,8 +3911,9 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 
 #define GUIO_QFG4_FLOPPY GUIO2(GUIO_NOSPEECH, \
                                GAMEOPTION_ORIGINAL_SAVELOAD)
-#define GUIO_QFG4_CD     GUIO2(GUIO_LINKSPEECHTOSFX, \
-                               GAMEOPTION_ORIGINAL_SAVELOAD)
+#define GUIO_QFG4_CD     GUIO3(GUIO_LINKSPEECHTOSFX, \
+                               GAMEOPTION_ORIGINAL_SAVELOAD, \
+                               GAMEOPTION_HQ_VIDEO)
 
 	// Quest for Glory 4 1.1 Floppy - English DOS (supplied by markcool in bug report #2723852)
 	// SCI interpreter version 2.000.000 (a guess?)
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index b05b9e6..7d2daf7 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -75,55 +75,138 @@ static bool flushEvents(EventManager *eventMan) {
 	return false;
 }
 
+static void directWriteToSystem(Video::VideoDecoder *decoder, const Common::Rect &drawRect, const bool setSystemPalette, const Graphics::Surface *nextFrame = nullptr) {
+
+	// VMDPlayer needs to decode the frame early so it can submit palette
+	// updates; calling decodeNextFrame again loses frames
+	if (!nextFrame) {
+		nextFrame = decoder->decodeNextFrame();
+	}
+	assert(nextFrame);
+
+	if (setSystemPalette &&
+		g_system->getScreenFormat().bytesPerPixel == 1 &&
+		decoder->hasDirtyPalette()) {
+
+		const uint8 *palette = decoder->getPalette();
+		assert(palette);
+		g_system->getPaletteManager()->setPalette(palette, 0, 256);
+
+		// KQ7 1.x has videos encoded using Microsoft Video 1 where palette 0 is
+		// white and 255 is black, which is basically the opposite of DOS/Win
+		// SCI palettes. So, when drawing to an 8bpp hwscreen, whenever a new
+		// palette is seen, the screen must be re-filled with the new black
+		// entry to ensure areas outside the video are always black and not some
+		// other color
+		for (int color = 0; color < 256; ++color) {
+			if (palette[0] == 0 && palette[1] == 0 && palette[2] == 0) {
+				g_system->fillScreen(color);
+				break;
+			}
+			palette += 3;
+		}
+	}
+
+	bool freeConvertedFrame;
+	Graphics::Surface *convertedFrame;
+	// Avoid creating a duplicate copy of the surface when it is not necessary
+	if (decoder->getPixelFormat() == g_system->getScreenFormat()) {
+		freeConvertedFrame = false;
+		convertedFrame = const_cast<Graphics::Surface *>(nextFrame);
+	} else {
+		freeConvertedFrame = true;
+		convertedFrame = nextFrame->convertTo(g_system->getScreenFormat(), decoder->getPalette());
+	}
+	assert(convertedFrame);
+
+	if (decoder->getWidth() != drawRect.width() || decoder->getHeight() != drawRect.height()) {
+		Graphics::Surface *const unscaledFrame(convertedFrame);
+		const Graphics::TransparentSurface tsUnscaledFrame(*unscaledFrame);
+#ifdef USE_RGB_COLOR
+		if (g_system->getScreenFormat().bytesPerPixel != 1) {
+			convertedFrame = tsUnscaledFrame.scaleT<Graphics::FILTER_BILINEAR>(drawRect.width(), drawRect.height());
+		} else {
+#else
+		{
+#endif
+			convertedFrame = tsUnscaledFrame.scaleT<Graphics::FILTER_NEAREST>(drawRect.width(), drawRect.height());
+		}
+		assert(convertedFrame);
+		if (freeConvertedFrame) {
+			unscaledFrame->free();
+			delete unscaledFrame;
+		}
+		freeConvertedFrame = true;
+	}
+
+	g_system->copyRectToScreen(convertedFrame->getPixels(), convertedFrame->pitch, drawRect.left, drawRect.top, convertedFrame->w, convertedFrame->h);
+	g_sci->_gfxFrameout->updateScreen();
+	if (freeConvertedFrame) {
+		convertedFrame->free();
+		delete convertedFrame;
+	}
+}
+
 #pragma mark SEQPlayer
 
 SEQPlayer::SEQPlayer(SegManager *segMan, EventManager *eventMan) :
 	_segMan(segMan),
 	_eventMan(eventMan),
-	_decoder(nullptr),
-	_plane(nullptr),
-	_screenItem(nullptr) {}
+	_decoder(nullptr) {}
 
 void SEQPlayer::play(const Common::String &fileName, const int16 numTicks, const int16 x, const int16 y) {
-	delete _decoder;
+
+	close();
+
 	_decoder = new SEQDecoder(numTicks);
 	if (!_decoder->loadFile(fileName)) {
 		warning("[SEQPlayer::play]: Failed to load %s", fileName.c_str());
+		delete _decoder;
 		return;
 	}
 
-	// NOTE: In the original engine, video was output directly to the hardware,
-	// bypassing the game's rendering engine. Instead of doing this, we use a
-	// mechanism that is very similar to that used by the VMD player, which
-	// allows the SEQ to be drawn into a bitmap ScreenItem and displayed using
-	// the normal graphics system.
-	reg_t bitmapId;
-	SciBitmap &bitmap = *_segMan->allocateBitmap(&bitmapId, _decoder->getWidth(), _decoder->getHeight(), kDefaultSkipColor, 0, 0, kLowResX, kLowResY, 0, false, false);
-	bitmap.getBuffer().fillRect(Common::Rect(_decoder->getWidth(), _decoder->getHeight()), 0);
-
-	CelInfo32 celInfo;
-	celInfo.type = kCelTypeMem;
-	celInfo.bitmap = bitmapId;
-
-	_plane = new Plane(Common::Rect(kLowResX, kLowResY), kPlanePicColored);
-	g_sci->_gfxFrameout->addPlane(*_plane);
-
-	// Normally we would use the x, y coordinates passed into the play function
-	// to position the screen item, but because the video frame bitmap is
-	// drawn in low-resolution coordinates, it gets automatically scaled up by
-	// the engine (pixel doubling with aspect ratio correction). As a result,
-	// the animation does not need the extra offsets from the game in order to
-	// be correctly positioned in the middle of the window, so we ignore them.
-	_screenItem = new ScreenItem(_plane->_object, celInfo, Common::Point(0, 0), ScaleInfo());
-	g_sci->_gfxFrameout->addScreenItem(*_screenItem);
-	g_sci->_gfxFrameout->frameOut(true);
+	const int16 scriptWidth = g_sci->_gfxFrameout->getCurrentBuffer().scriptWidth;
+	const int16 scriptHeight = g_sci->_gfxFrameout->getCurrentBuffer().scriptHeight;
+	const int16 screenWidth = g_sci->_gfxFrameout->getCurrentBuffer().screenWidth;
+	const int16 screenHeight = g_sci->_gfxFrameout->getCurrentBuffer().screenHeight;
+
+	const int16 scaledWidth = (_decoder->getWidth() * Ratio(screenWidth, scriptWidth)).toInt();
+	const int16 scaledHeight = (_decoder->getHeight() * Ratio(screenHeight, scriptHeight)).toInt();
+
+	// Normally we would use the coordinates passed into the play function
+	// to position the video, but since we are scaling the video (which SSCI
+	// did not do), the coordinates are not correct. Since videos are always
+	// intended to play in the center of the screen, we just recalculate the
+	// origin here.
+	_drawRect.left = (screenWidth - scaledWidth) / 2;
+	_drawRect.top = (screenHeight - scaledHeight) / 2;
+	_drawRect.setWidth(scaledWidth);
+	_drawRect.setHeight(scaledHeight);
+
+#ifdef USE_RGB_COLOR
+	// Optimize rendering performance for unscaled videos, and allow
+	// better-than-NN interpolation for videos that are scaled
+	if (ConfMan.getBool("enable_hq_video") &&
+		(_decoder->getWidth() != scaledWidth || _decoder->getHeight() != scaledHeight)) {
+		// TODO: Search for and use the best supported format (which may be
+		// lower than 32bpp) once the scaling code in Graphics supports
+		// 16bpp/24bpp, and once the SDL backend can correctly communicate
+		// supported pixel formats above whatever format is currently used by
+		// _hwsurface. Right now, this will just crash ScummVM if the backend
+		// does not support a 32bpp pixel format, which sucks since this code
+		// really ought to be able to fall back to NN scaling for games with
+		// 256-color videos.
+		const Graphics::PixelFormat format = Graphics::createPixelFormat<8888>();
+		g_sci->_gfxFrameout->setPixelFormat(format);
+	}
+#endif
+
 	_decoder->start();
 
 	while (!g_engine->shouldQuit() && !_decoder->endOfVideo()) {
 		g_sci->sleep(_decoder->getTimeToNextFrame());
-
 		while (_decoder->needsUpdate()) {
-			renderFrame(bitmap);
+			renderFrame();
 		}
 
 		// SSCI did not allow SEQ animations to be bypassed like this
@@ -149,34 +232,24 @@ void SEQPlayer::play(const Common::String &fileName, const int16 numTicks, const
 		}
 	}
 
-	_segMan->freeBitmap(bitmapId);
-	g_sci->_gfxFrameout->deletePlane(*_plane);
-	g_sci->_gfxFrameout->frameOut(true);
-	_screenItem = nullptr;
-	_plane = nullptr;
+	close();
 }
 
-void SEQPlayer::renderFrame(SciBitmap &bitmap) const {
-	const Graphics::Surface *surface = _decoder->decodeNextFrame();
-
-	bitmap.getBuffer().copyRectToSurface(*surface, 0, 0, Common::Rect(surface->w, surface->h));
-
-	const bool dirtyPalette = _decoder->hasDirtyPalette();
-	if (dirtyPalette) {
-		Palette palette;
-		const byte *rawPalette = _decoder->getPalette();
-		for (int i = 0; i < ARRAYSIZE(palette.colors); ++i) {
-			palette.colors[i].r = *rawPalette++;
-			palette.colors[i].g = *rawPalette++;
-			palette.colors[i].b = *rawPalette++;
-			palette.colors[i].used = true;
-		}
+void SEQPlayer::renderFrame() const {
+	directWriteToSystem(_decoder, _drawRect, true);
+}
 
-		g_sci->_gfxPalette32->submit(palette);
+void SEQPlayer::close() {
+#ifdef USE_RGB_COLOR
+	if (g_system->getScreenFormat().bytesPerPixel != 1) {
+		const Graphics::PixelFormat format = Graphics::PixelFormat::createFormatCLUT8();
+		g_sci->_gfxFrameout->setPixelFormat(format);
 	}
+#endif
 
-	g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
-	g_sci->_gfxFrameout->frameOut(true);
+	g_system->fillScreen(0);
+	delete _decoder;
+	_decoder = nullptr;
 }
 
 #pragma mark -
@@ -365,78 +438,8 @@ uint16 AVIPlayer::getDuration() const {
 	return _decoder->getFrameCount();
 }
 
-template <Graphics::TFilteringMode MODE>
-static void writeFrameToSystem(const Graphics::Surface *nextFrame, Video::VideoDecoder *decoder, const Common::Rect &drawRect) {
-	assert(nextFrame);
-
-	bool freeConvertedFrame;
-	Graphics::Surface *convertedFrame;
-	// Avoid creating a duplicate copy of the surface when it is not necessary
-	if (decoder->getPixelFormat() == g_system->getScreenFormat()) {
-		freeConvertedFrame = false;
-		convertedFrame = const_cast<Graphics::Surface *>(nextFrame);
-	} else {
-		freeConvertedFrame = true;
-		convertedFrame = nextFrame->convertTo(g_system->getScreenFormat(), decoder->getPalette());
-	}
-	assert(convertedFrame);
-
-	if (decoder->getWidth() != drawRect.width() || decoder->getHeight() != drawRect.height()) {
-		Graphics::Surface *const unscaledFrame(convertedFrame);
-		const Graphics::TransparentSurface tsUnscaledFrame(*unscaledFrame);
-		convertedFrame = tsUnscaledFrame.scaleT<MODE>(drawRect.width(), drawRect.height());
-		assert(convertedFrame);
-		if (freeConvertedFrame) {
-			unscaledFrame->free();
-			delete unscaledFrame;
-		}
-		freeConvertedFrame = true;
-	}
-
-	g_system->copyRectToScreen(convertedFrame->getPixels(), convertedFrame->pitch, drawRect.left, drawRect.top, convertedFrame->w, convertedFrame->h);
-	g_sci->_gfxFrameout->updateScreen();
-	if (freeConvertedFrame) {
-		convertedFrame->free();
-		delete convertedFrame;
-	}
-}
-
 void AVIPlayer::renderFrame() const {
-	// TODO: Improve efficiency by making changes to common Graphics code that
-	// allow reuse of a single conversion surface for all frames
-
-	const Graphics::Surface *nextFrame = _decoder->decodeNextFrame();
-	assert(nextFrame);
-
-	if (g_system->getScreenFormat().bytesPerPixel == 1 && _decoder->hasDirtyPalette()) {
-		const uint8 *palette = _decoder->getPalette();
-		assert(palette);
-		g_system->getPaletteManager()->setPalette(palette, 0, 256);
-
-		// KQ7 1.x has videos encoded using Microsoft Video 1 where palette 0 is
-		// white and 255 is black, which is basically the opposite of DOS/Win
-		// SCI palettes. So, when drawing to an 8bpp hwscreen, whenever a new
-		// palette is seen, the screen must be re-filled with the new black
-		// entry to ensure areas outside the video are always black and not some
-		// other color
-		for (int color = 0; color < 256; ++color) {
-			if (palette[0] == 0 && palette[1] == 0 && palette[2] == 0) {
-				g_system->fillScreen(color);
-				break;
-			}
-			palette += 3;
-		}
-	}
-
-#ifdef USE_RGB_COLOR
-	if (g_system->getScreenFormat().bytesPerPixel != 1) {
-		writeFrameToSystem<Graphics::FILTER_BILINEAR>(nextFrame, _decoder, _drawRect);
-	} else {
-#else
-	{
-#endif
-		writeFrameToSystem<Graphics::FILTER_NEAREST>(nextFrame, _decoder, _drawRect);
-	}
+	directWriteToSystem(_decoder, _drawRect, true);
 }
 
 AVIPlayer::EventFlags AVIPlayer::playUntilEvent(EventFlags flags) {
@@ -821,7 +824,7 @@ void VMDPlayer::renderOverlay() const {
 			redrawGameScreen();
 		}
 
-		writeFrameToSystem<Graphics::FILTER_BILINEAR>(nextFrame, _decoder, _drawRect);
+		directWriteToSystem(_decoder, _drawRect, false, nextFrame);
 	} else {
 #else
 	{
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index 0f63996..474851c 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -68,19 +68,20 @@ private:
 	SEQDecoder *_decoder;
 
 	/**
-	 * The plane where the SEQ will be drawn.
+	 * Renders a single frame of video.
 	 */
-	Plane *_plane;
+	void renderFrame() const;
 
 	/**
-	 * The screen item representing the SEQ surface.
+	 * Stops playback and closes the currently open SEQ stream.
 	 */
-	ScreenItem *_screenItem;
+	void close();
 
 	/**
-	 * Renders a single frame of video.
+	 * The rectangle where the video will be drawn,
+	 * in screen coordinates.
 	 */
-	void renderFrame(SciBitmap &bitmap) const;
+	Common::Rect _drawRect;
 };
 
 #pragma mark -


Commit: f15f9e3b7c9dd7594a60aa230fed05b965a7a587
    https://github.com/scummvm/scummvm/commit/f15f9e3b7c9dd7594a60aa230fed05b965a7a587
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:39-05:00

Commit Message:
SCI32: Refactor Video32 code to reduce code & feature duplication

Changed paths:
    engines/sci/detection_tables.h
    engines/sci/graphics/video32.cpp
    engines/sci/graphics/video32.h
    engines/sci/sci.cpp


diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index 4230a3b..5061876 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -724,10 +724,13 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #ifdef ENABLE_SCI32
 #define GUIO_GK1_FLOPPY GUIO2(GUIO_NOSPEECH, \
                               GAMEOPTION_ORIGINAL_SAVELOAD)
-#define GUIO_GK1_CD     GUIO4(GUIO_LINKSPEECHTOSFX, \
+#define GUIO_GK1_CD_DOS GUIO4(GUIO_LINKSPEECHTOSFX, \
                               GAMEOPTION_ORIGINAL_SAVELOAD, \
                               GAMEOPTION_HIGH_RESOLUTION_GRAPHICS, \
                               GAMEOPTION_HQ_VIDEO)
+#define GUIO_GK1_CD_WIN GUIO3(GUIO_LINKSPEECHTOSFX, \
+                              GAMEOPTION_ORIGINAL_SAVELOAD, \
+                              GAMEOPTION_HQ_VIDEO)
 #define GUIO_GK1_MAC    GUIO_GK1_FLOPPY
 
 	// Gabriel Knight - English DOS Floppy
@@ -776,7 +779,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "372d059f75856afa6d73dd84cbb8913d", 10996},
 		{"resource.000", 0, "69b7516962510f780d38519cc15fcc7c", 12581736},
 		AD_LISTEND},
-		Common::EN_ANY, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::EN_ANY, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
 
 	// Gabriel Knight - English Windows CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "01.100.000"
@@ -784,7 +787,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "372d059f75856afa6d73dd84cbb8913d", 10996},
 		{"resource.000", 0, "69b7516962510f780d38519cc15fcc7c", 12581736},
 		AD_LISTEND},
-		Common::EN_ANY, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::EN_ANY, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_WIN },
 
 	// Gabriel Knight - German DOS CD (from Tobis87)
 	// SCI interpreter version 2.000.000
@@ -792,7 +795,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "a7d3e55114c65647310373cb390815ba", 11392},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13400497},
 		AD_LISTEND},
-		Common::DE_DEU, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::DE_DEU, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
 
 	// Gabriel Knight - Spanish DOS CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "1.000.000, April 13, 1995"
@@ -800,7 +803,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "7cb6e9bba15b544ec7a635c45bde9953", 11404},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13381599},
 		AD_LISTEND},
-		Common::ES_ESP, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::ES_ESP, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
 
 	// Gabriel Knight - French DOS CD (from Hkz)
 	// VERSION file reports "1.000.000, May 3, 1994"
@@ -808,7 +811,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "55f909ba93a2515042a08d8a2da8414e", 11392},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13325145},
 		AD_LISTEND},
-		Common::FR_FRA, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::FR_FRA, Common::kPlatformDOS, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_DOS },
 
 	// Gabriel Knight - Spanish Windows CD (from jvprat)
 	// Executable scanning reports "2.000.000", VERSION file reports "1.000.000, April 13, 1995"
@@ -816,7 +819,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		{"resource.map", 0, "7cb6e9bba15b544ec7a635c45bde9953", 11404},
 		{"resource.000", 0, "091cf08910780feabc56f8551b09cb36", 13381599},
 		AD_LISTEND},
-		Common::ES_ESP, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD },
+		Common::ES_ESP, Common::kPlatformWindows, ADGF_CD | ADGF_TESTING, GUIO_GK1_CD_WIN },
 
 	// Gabriel Knight - English Macintosh (Floppy!)
 	// This version is hi-res ONLY, so it should NOT get GAMEOPTION_HIGH_RESOLUTION_GRAPHICS
@@ -831,7 +834,8 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		Common::EN_ANY, Common::kPlatformMacintosh, ADGF_MACRESFORK | ADGF_UNSTABLE, GUIO_GK1_MAC },
 
 #undef GUIO_GK1_FLOPPY
-#undef GUIO_GK1_CD
+#undef GUIO_GK1_CD_DOS
+#undef GUIO_GK1_CD_WIN
 #undef GUIO_GK1_MAC
 
 #define GUIO_GK2_DEMO GUIO8(GUIO_NOSUBTITLES, \
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index 7d2daf7..ec37d1c 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -56,80 +56,191 @@ namespace Graphics { struct Surface; }
 
 namespace Sci {
 
-/**
- * @returns true if the player should quit
- */
-static bool flushEvents(EventManager *eventMan) {
+static void flushEvents(EventManager *eventMan) {
 	// Flushing all the keyboard and mouse events out of the event manager
 	// keeps events queued from before the start of playback from accidentally
 	// activating a video stop flag
 	for (;;) {
-		const SciEvent event = eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_MOUSE_PRESS | SCI_EVENT_MOUSE_RELEASE | SCI_EVENT_HOT_RECTANGLE | SCI_EVENT_QUIT);
+		const SciEvent event = eventMan->getSciEvent(SCI_EVENT_ANY & ~SCI_EVENT_QUIT);
 		if (event.type == SCI_EVENT_NONE) {
 			break;
-		} else if (event.type == SCI_EVENT_QUIT) {
-			return true;
 		}
 	}
+}
+
+bool VideoPlayer::open(const Common::String &fileName) {
+	if (!_decoder->loadFile(fileName)) {
+		warning("Failed to load %s", fileName.c_str());
+		return false;
+	}
+
+#ifndef USE_RGB_COLOR
+	// KQ7 2.00b videos are compressed in 24bpp Cinepak, so cannot play on
+	// a system with no RGB support
+	if (_decoder->getPixelFormat().bytesPerPixel != 1) {
+		void showScummVMDialog(const Common::String &message);
+		showScummVMDialog(Common::String::format(_("Cannot play back %dbpp video on a system with maximum color depth of 8bpp"), _decoder->getPixelFormat().bpp()));
+		_decoder->close();
+		return false;
+	}
+#endif
+
+	return true;
+}
+
+bool VideoPlayer::startHQVideo() {
+#ifdef USE_RGB_COLOR
+	// Optimize rendering performance for unscaled videos, and allow
+	// better-than-NN interpolation for videos that are scaled
+	if (shouldStartHQVideo()) {
+		// TODO: Search for and use the best supported format (which may be
+		// lower than 32bpp) once the scaling code in Graphics supports
+		// 16bpp/24bpp, and once the SDL backend can correctly communicate
+		// supported pixel formats above whatever format is currently used by
+		// _hwsurface. Right now, this will either show an error dialog (OpenGL)
+		// or just crash entirely (SDL) if the backend does not support this
+		// 32bpp pixel format, which sucks since this code really ought to be
+		// able to fall back to NN scaling for games with 256-color videos
+		// without any error.
+		const Graphics::PixelFormat format(4, 8, 8, 8, 8, 24, 16, 8, 0);
+		g_sci->_gfxFrameout->setPixelFormat(format);
+		_hqVideoMode = (g_system->getScreenFormat() == format);
+		return _hqVideoMode;
+	} else {
+		_hqVideoMode = false;
+	}
+#endif
 
 	return false;
 }
 
-static void directWriteToSystem(Video::VideoDecoder *decoder, const Common::Rect &drawRect, const bool setSystemPalette, const Graphics::Surface *nextFrame = nullptr) {
+bool VideoPlayer::endHQVideo() {
+#ifdef USE_RGB_COLOR
+	if (g_system->getScreenFormat().bytesPerPixel != 1) {
+		const Graphics::PixelFormat format = Graphics::PixelFormat::createFormatCLUT8();
+		g_sci->_gfxFrameout->setPixelFormat(format);
+		assert(g_system->getScreenFormat() == format);
+		_hqVideoMode = false;
+		return true;
+	}
+#endif
 
-	// VMDPlayer needs to decode the frame early so it can submit palette
-	// updates; calling decodeNextFrame again loses frames
-	if (!nextFrame) {
-		nextFrame = decoder->decodeNextFrame();
+	return false;
+}
+
+VideoPlayer::EventFlags VideoPlayer::playUntilEvent(const EventFlags flags, const uint32 maxSleepMs) {
+	flushEvents(_eventMan);
+	_decoder->start();
+
+	EventFlags stopFlag = kEventFlagNone;
+	for (;;) {
+		g_sci->sleep(MIN(_decoder->getTimeToNextFrame(), maxSleepMs));
+		const Graphics::Surface *nextFrame = nullptr;
+		// If a decoder needs more than one update per loop, this means we are
+		// running behind and should skip rendering these frames (but must still
+		// submit any palettes from skipped frames)
+		while (_decoder->needsUpdate()) {
+			nextFrame = _decoder->decodeNextFrame();
+			if (_decoder->hasDirtyPalette()) {
+				submitPalette(_decoder->getPalette());
+			}
+		}
+
+		// Some frames may contain only audio and/or palette data; this occurs
+		// with Duck videos and is not an error
+		if (nextFrame) {
+			renderFrame(*nextFrame);
+		}
+
+		stopFlag = checkForEvent(flags);
+		if (stopFlag != kEventFlagNone) {
+			break;
+		}
 	}
-	assert(nextFrame);
 
-	if (setSystemPalette &&
-		g_system->getScreenFormat().bytesPerPixel == 1 &&
-		decoder->hasDirtyPalette()) {
+	return stopFlag;
+}
 
-		const uint8 *palette = decoder->getPalette();
-		assert(palette);
-		g_system->getPaletteManager()->setPalette(palette, 0, 256);
+VideoPlayer::EventFlags VideoPlayer::checkForEvent(const EventFlags flags) {
+	if (g_engine->shouldQuit() || _decoder->endOfVideo()) {
+		return kEventFlagEnd;
+	}
 
-		// KQ7 1.x has videos encoded using Microsoft Video 1 where palette 0 is
-		// white and 255 is black, which is basically the opposite of DOS/Win
-		// SCI palettes. So, when drawing to an 8bpp hwscreen, whenever a new
-		// palette is seen, the screen must be re-filled with the new black
-		// entry to ensure areas outside the video are always black and not some
-		// other color
-		for (int color = 0; color < 256; ++color) {
-			if (palette[0] == 0 && palette[1] == 0 && palette[2] == 0) {
-				g_system->fillScreen(color);
-				break;
+	SciEvent event = _eventMan->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
+	if ((flags & kEventFlagMouseDown) && event.type == SCI_EVENT_MOUSE_PRESS) {
+		return kEventFlagMouseDown;
+	}
+
+	event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_PEEK);
+	if ((flags & kEventFlagEscapeKey) && event.type == SCI_EVENT_KEYBOARD) {
+		if (getSciVersion() < SCI_VERSION_3) {
+			while ((event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD)),
+				   event.type != SCI_EVENT_NONE) {
+				if (event.character == SCI_KEY_ESC) {
+					return kEventFlagEscapeKey;
+				}
 			}
-			palette += 3;
+		} else if (event.character == SCI_KEY_ESC) {
+			return kEventFlagEscapeKey;
 		}
 	}
 
+	return kEventFlagNone;
+}
+
+void VideoPlayer::submitPalette(const uint8 palette[256 * 3]) const {
+#ifdef USE_RGB_COLOR
+	if (g_system->getScreenFormat().bytesPerPixel != 1) {
+		return;
+	}
+#endif
+
+	assert(palette);
+	g_system->getPaletteManager()->setPalette(palette, 0, 256);
+
+	// KQ7 1.x has videos encoded using Microsoft Video 1 where palette 0 is
+	// white and 255 is black, which is basically the opposite of DOS/Win
+	// SCI palettes. So, when drawing to an 8bpp hwscreen, whenever a new
+	// palette is seen, the screen must be re-filled with the new black
+	// entry to ensure areas outside the video are always black and not some
+	// other color
+	for (int color = 0; color < 256; ++color) {
+		if (palette[0] == 0 && palette[1] == 0 && palette[2] == 0) {
+			g_system->fillScreen(color);
+			break;
+		}
+		palette += 3;
+	}
+}
+
+void VideoPlayer::renderFrame(const Graphics::Surface &nextFrame) const {
 	bool freeConvertedFrame;
 	Graphics::Surface *convertedFrame;
 	// Avoid creating a duplicate copy of the surface when it is not necessary
-	if (decoder->getPixelFormat() == g_system->getScreenFormat()) {
+	if (_decoder->getPixelFormat() == g_system->getScreenFormat()) {
 		freeConvertedFrame = false;
-		convertedFrame = const_cast<Graphics::Surface *>(nextFrame);
+		convertedFrame = const_cast<Graphics::Surface *>(&nextFrame);
 	} else {
 		freeConvertedFrame = true;
-		convertedFrame = nextFrame->convertTo(g_system->getScreenFormat(), decoder->getPalette());
+		convertedFrame = nextFrame.convertTo(g_system->getScreenFormat(), _decoder->getPalette());
 	}
 	assert(convertedFrame);
 
-	if (decoder->getWidth() != drawRect.width() || decoder->getHeight() != drawRect.height()) {
+	if (_decoder->getWidth() != _drawRect.width() || _decoder->getHeight() != _drawRect.height()) {
 		Graphics::Surface *const unscaledFrame(convertedFrame);
+		// TODO: The only reason TransparentSurface is used here because it is
+		// where common scaler code is right now.
 		const Graphics::TransparentSurface tsUnscaledFrame(*unscaledFrame);
 #ifdef USE_RGB_COLOR
-		if (g_system->getScreenFormat().bytesPerPixel != 1) {
-			convertedFrame = tsUnscaledFrame.scaleT<Graphics::FILTER_BILINEAR>(drawRect.width(), drawRect.height());
+		if (_hqVideoMode) {
+			convertedFrame = tsUnscaledFrame.scaleT<Graphics::FILTER_BILINEAR>(_drawRect.width(), _drawRect.height());
 		} else {
-#else
+#elif 1
 		{
+#else
+		}
 #endif
-			convertedFrame = tsUnscaledFrame.scaleT<Graphics::FILTER_NEAREST>(drawRect.width(), drawRect.height());
+			convertedFrame = tsUnscaledFrame.scaleT<Graphics::FILTER_NEAREST>(_drawRect.width(), _drawRect.height());
 		}
 		assert(convertedFrame);
 		if (freeConvertedFrame) {
@@ -139,29 +250,50 @@ static void directWriteToSystem(Video::VideoDecoder *decoder, const Common::Rect
 		freeConvertedFrame = true;
 	}
 
-	g_system->copyRectToScreen(convertedFrame->getPixels(), convertedFrame->pitch, drawRect.left, drawRect.top, convertedFrame->w, convertedFrame->h);
+	g_system->copyRectToScreen(convertedFrame->getPixels(), convertedFrame->pitch, _drawRect.left, _drawRect.top, convertedFrame->w, convertedFrame->h);
 	g_sci->_gfxFrameout->updateScreen();
+
 	if (freeConvertedFrame) {
 		convertedFrame->free();
 		delete convertedFrame;
 	}
 }
 
+template <typename PixelType>
+void VideoPlayer::renderLQToSurface(Graphics::Surface &out, const Graphics::Surface &nextFrame, const bool doublePixels, const bool blackLines) const {
+
+	const int lineCount = blackLines ? 2 : 1;
+	if (doublePixels) {
+		for (int16 y = 0; y < nextFrame.h * 2; y += lineCount) {
+			const PixelType *source = (const PixelType *)nextFrame.getBasePtr(0, y >> 1);
+			PixelType *target = (PixelType *)out.getBasePtr(0, y);
+			for (int16 x = 0; x < nextFrame.w; ++x) {
+				*target++ = *source;
+				*target++ = *source++;
+			}
+		}
+	} else if (blackLines) {
+		for (int16 y = 0; y < nextFrame.h; y += lineCount) {
+			const PixelType *source = (const PixelType *)nextFrame.getBasePtr(0, y);
+			PixelType *target = (PixelType *)out.getBasePtr(0, y);
+			memcpy(target, source, out.w * sizeof(PixelType));
+		}
+	} else {
+		out.copyRectToSurface(nextFrame.getPixels(), nextFrame.pitch, 0, 0, nextFrame.w, nextFrame.h);
+	}
+}
+
 #pragma mark SEQPlayer
 
-SEQPlayer::SEQPlayer(SegManager *segMan, EventManager *eventMan) :
-	_segMan(segMan),
-	_eventMan(eventMan),
-	_decoder(nullptr) {}
+SEQPlayer::SEQPlayer(EventManager *eventMan) :
+	VideoPlayer(eventMan) {}
 
-void SEQPlayer::play(const Common::String &fileName, const int16 numTicks, const int16 x, const int16 y) {
+void SEQPlayer::play(const Common::String &fileName, const int16 numTicks, const int16, const int16) {
 
-	close();
+	_decoder.reset(new SEQDecoder(numTicks));
 
-	_decoder = new SEQDecoder(numTicks);
-	if (!_decoder->loadFile(fileName)) {
-		warning("[SEQPlayer::play]: Failed to load %s", fileName.c_str());
-		delete _decoder;
+	if (!VideoPlayer::open(fileName)) {
+		_decoder.reset();
 		return;
 	}
 
@@ -183,113 +315,34 @@ void SEQPlayer::play(const Common::String &fileName, const int16 numTicks, const
 	_drawRect.setWidth(scaledWidth);
 	_drawRect.setHeight(scaledHeight);
 
-#ifdef USE_RGB_COLOR
-	// Optimize rendering performance for unscaled videos, and allow
-	// better-than-NN interpolation for videos that are scaled
-	if (ConfMan.getBool("enable_hq_video") &&
-		(_decoder->getWidth() != scaledWidth || _decoder->getHeight() != scaledHeight)) {
-		// TODO: Search for and use the best supported format (which may be
-		// lower than 32bpp) once the scaling code in Graphics supports
-		// 16bpp/24bpp, and once the SDL backend can correctly communicate
-		// supported pixel formats above whatever format is currently used by
-		// _hwsurface. Right now, this will just crash ScummVM if the backend
-		// does not support a 32bpp pixel format, which sucks since this code
-		// really ought to be able to fall back to NN scaling for games with
-		// 256-color videos.
-		const Graphics::PixelFormat format = Graphics::createPixelFormat<8888>();
-		g_sci->_gfxFrameout->setPixelFormat(format);
-	}
-#endif
-
-	_decoder->start();
-
-	while (!g_engine->shouldQuit() && !_decoder->endOfVideo()) {
-		g_sci->sleep(_decoder->getTimeToNextFrame());
-		while (_decoder->needsUpdate()) {
-			renderFrame();
-		}
-
-		// SSCI did not allow SEQ animations to be bypassed like this
-		SciEvent event = _eventMan->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
-		if (event.type == SCI_EVENT_MOUSE_PRESS) {
-			break;
-		}
-
-		event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_PEEK);
-		if (event.type == SCI_EVENT_KEYBOARD) {
-			bool stop = false;
-			while ((event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD)),
-				   event.type != SCI_EVENT_NONE) {
-				if (event.character == SCI_KEY_ESC) {
-					stop = true;
-					break;
-				}
-			}
-
-			if (stop) {
-				break;
-			}
-		}
-	}
-
-	close();
-}
-
-void SEQPlayer::renderFrame() const {
-	directWriteToSystem(_decoder, _drawRect, true);
-}
-
-void SEQPlayer::close() {
-#ifdef USE_RGB_COLOR
-	if (g_system->getScreenFormat().bytesPerPixel != 1) {
-		const Graphics::PixelFormat format = Graphics::PixelFormat::createFormatCLUT8();
-		g_sci->_gfxFrameout->setPixelFormat(format);
-	}
-#endif
-
+	startHQVideo();
+	playUntilEvent(kEventFlagMouseDown | kEventFlagEscapeKey);
+	endHQVideo();
 	g_system->fillScreen(0);
-	delete _decoder;
-	_decoder = nullptr;
+	_decoder.reset();
 }
 
 #pragma mark -
 #pragma mark AVIPlayer
 
 AVIPlayer::AVIPlayer(EventManager *eventMan) :
-	_eventMan(eventMan),
-	_decoder(new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
+	VideoPlayer(eventMan, new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
 	_status(kAVINotOpen) {}
 
-AVIPlayer::~AVIPlayer() {
-	close();
-	delete _decoder;
-}
-
 AVIPlayer::IOStatus AVIPlayer::open(const Common::String &fileName) {
 	if (_status != kAVINotOpen) {
 		close();
 	}
 
-	if (!_decoder->loadFile(fileName)) {
+	if (!VideoPlayer::open(fileName)) {
 		return kIOFileNotFound;
 	}
 
-#ifndef USE_RGB_COLOR
-	// KQ7 2.00b videos are compressed in 24bpp Cinepak, so cannot play on
-	// a system with no RGB support
-	if (_decoder->getPixelFormat().bytesPerPixel != 1) {
-		void showScummVMDialog(const Common::String &message);
-		showScummVMDialog(Common::String::format(_("Cannot play back %dbpp video on a system with maximum color depth of 8bpp"), _decoder->getPixelFormat().bpp()));
-		_decoder->close();
-		return kIOFileNotFound;
-	}
-#endif
-
 	_status = kAVIOpen;
 	return kIOSuccess;
 }
 
-AVIPlayer::IOStatus AVIPlayer::init(const bool pixelDouble) {
+AVIPlayer::IOStatus AVIPlayer::init(const bool doublePixels) {
 	// Calls to initialize the AVI player in SCI can be made in a few ways:
 	//
 	// * kShowMovie(WinInit, x, y) to render the video at (x,y) using its
@@ -322,7 +375,7 @@ AVIPlayer::IOStatus AVIPlayer::init(const bool pixelDouble) {
 
 	int16 width = _decoder->getWidth();
 	int16 height = _decoder->getHeight();
-	if (pixelDouble) {
+	if (doublePixels) {
 		width *= 2;
 		height *= 2;
 	}
@@ -342,28 +395,8 @@ AVIPlayer::IOStatus AVIPlayer::init(const bool pixelDouble) {
 	_drawRect.setWidth(width);
 	_drawRect.setHeight(height);
 
-#ifdef USE_RGB_COLOR
-	// Optimize rendering performance for unscaled videos, and allow
-	// better-than-NN interpolation for videos that are scaled
-	if (ConfMan.getBool("enable_hq_video") &&
-		(_decoder->getWidth() != width || _decoder->getHeight() != height)) {
-
-		// TODO: Search for and use the best supported format (which may be
-		// lower than 32bpp) once the scaling code in Graphics supports
-		// 16bpp/24bpp, and once the SDL backend can correctly communicate
-		// supported pixel formats above whatever format is currently used by
-		// _hwsurface. Right now, this will just crash ScummVM if the backend
-		// does not support a 32bpp pixel format, which sucks since this code
-		// really ought to be able to fall back to NN scaling for games with
-		// 256-color videos.
-		const Graphics::PixelFormat format = Graphics::createPixelFormat<8888>();
-		g_sci->_gfxFrameout->setPixelFormat(format);
-	} else {
-#else
-	{
-#endif
-		const Graphics::PixelFormat format = _decoder->getPixelFormat();
-		g_sci->_gfxFrameout->setPixelFormat(format);
+	if (!startHQVideo() && _decoder->getPixelFormat().bytesPerPixel != 1) {
+		g_sci->_gfxFrameout->setPixelFormat(_decoder->getPixelFormat());
 	}
 
 	return kIOSuccess;
@@ -379,10 +412,8 @@ AVIPlayer::IOStatus AVIPlayer::play(const int16 from, const int16 to, const int1
 		_decoder->setEndFrame(to);
 	}
 
-	if (!async) {
-		renderVideo();
-	} else if (getSciVersion() == SCI_VERSION_2_1_EARLY) {
-		playUntilEvent((EventFlags)(kEventFlagEnd | kEventFlagEscapeKey));
+	if (!async || getSciVersion() == SCI_VERSION_2_1_EARLY) {
+		playUntilEvent(kEventFlagNone);
 	} else {
 		_status = kAVIPlaying;
 	}
@@ -390,15 +421,11 @@ AVIPlayer::IOStatus AVIPlayer::play(const int16 from, const int16 to, const int1
 	return kIOSuccess;
 }
 
-void AVIPlayer::renderVideo() const {
-	_decoder->start();
-
-	while (!g_engine->shouldQuit() && !_decoder->endOfVideo()) {
-		g_sci->sleep(_decoder->getTimeToNextFrame());
-		while (_decoder->needsUpdate()) {
-			renderFrame();
-		}
-	}
+AVIPlayer::EventFlags AVIPlayer::playUntilEvent(const EventFlags flags, const uint32 maxSleepMs) {
+	// NOTE: In SSCI, whether or not a video could be skipped was controlled by
+	// game scripts; here, we always allow skipping video with the mouse or
+	// escape key, to improve the user experience
+	return VideoPlayer::playUntilEvent(flags | kEventFlagMouseDown | kEventFlagEscapeKey, maxSleepMs);
 }
 
 AVIPlayer::IOStatus AVIPlayer::close() {
@@ -406,16 +433,15 @@ AVIPlayer::IOStatus AVIPlayer::close() {
 		return kIOSuccess;
 	}
 
-#ifdef USE_RGB_COLOR
-	if (g_system->getScreenFormat().bytesPerPixel != 1) {
-		const Graphics::PixelFormat format = Graphics::PixelFormat::createFormatCLUT8();
-		g_sci->_gfxFrameout->setPixelFormat(format);
+	if (!endHQVideo()) {
+		// This fixes a single-frame white flash after playback of the KQ7 1.x
+		// videos, which replace palette entry 0 with white
+		const uint8 black[3] = { 0, 0, 0 };
+		g_system->getPaletteManager()->setPalette(black, 0, 1);
 	}
-#endif
 
 	g_system->fillScreen(0);
 	g_sci->_gfxCursor32->unhide();
-
 	_decoder->close();
 	_status = kAVINotOpen;
 	return kIOSuccess;
@@ -438,59 +464,12 @@ uint16 AVIPlayer::getDuration() const {
 	return _decoder->getFrameCount();
 }
 
-void AVIPlayer::renderFrame() const {
-	directWriteToSystem(_decoder, _drawRect, true);
-}
-
-AVIPlayer::EventFlags AVIPlayer::playUntilEvent(EventFlags flags) {
-	_decoder->start();
-
-	EventFlags stopFlag = kEventFlagNone;
-	while (!g_engine->shouldQuit()) {
-		if (_decoder->endOfVideo()) {
-			stopFlag = kEventFlagEnd;
-			break;
-		}
-
-		g_sci->sleep(_decoder->getTimeToNextFrame());
-		while (_decoder->needsUpdate()) {
-			renderFrame();
-		}
-
-		SciEvent event = _eventMan->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
-		if ((flags & kEventFlagMouseDown) && event.type == SCI_EVENT_MOUSE_PRESS) {
-			stopFlag = kEventFlagMouseDown;
-			break;
-		}
-
-		event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_PEEK);
-		if ((flags & kEventFlagEscapeKey) && event.type == SCI_EVENT_KEYBOARD) {
-			bool stop = false;
-			while ((event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD)),
-				   event.type != SCI_EVENT_NONE) {
-				if (event.character == SCI_KEY_ESC) {
-					stop = true;
-					break;
-				}
-			}
-
-			if (stop) {
-				stopFlag = kEventFlagEscapeKey;
-				break;
-			}
-		}
-	}
-
-	return stopFlag;
-}
-
 #pragma mark -
 #pragma mark VMDPlayer
 
-VMDPlayer::VMDPlayer(SegManager *segMan, EventManager *eventMan) :
+VMDPlayer::VMDPlayer(EventManager *eventMan, SegManager *segMan) :
+	VideoPlayer(eventMan, new Video::AdvancedVMDDecoder(Audio::Mixer::kSFXSoundType)),
 	_segMan(segMan),
-	_eventMan(eventMan),
-	_decoder(new Video::AdvancedVMDDecoder(Audio::Mixer::kSFXSoundType)),
 
 	_isOpen(false),
 	_isInitialized(false),
@@ -525,7 +504,6 @@ VMDPlayer::VMDPlayer(SegManager *segMan, EventManager *eventMan) :
 
 VMDPlayer::~VMDPlayer() {
 	close();
-	delete _decoder;
 }
 
 #pragma mark -
@@ -673,14 +651,11 @@ VMDPlayer::EventFlags VMDPlayer::kernelPlayUntilEvent(const EventFlags flags, co
 	return playUntilEvent(flags);
 }
 
-VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags) {
-	if (flushEvents(_eventMan)) {
-		return kEventFlagEnd;
-	}
-
+VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags, const uint32) {
 	if (flags & kEventFlagReverse) {
 		// NOTE: This flag may not work properly since SSCI does not care
 		// if a video has audio, but the VMD decoder does.
+		warning("VMD reverse playback flag was set. Please report this event to the bug tracker");
 		const bool success = _decoder->setReverse(true);
 		assert(success);
 		_decoder->setVolume(0);
@@ -700,86 +675,53 @@ VMDPlayer::EventFlags VMDPlayer::playUntilEvent(const EventFlags flags) {
 
 		if (shouldUseCompositing()) {
 			_isComposited = true;
-			_usingHighColor = false;
 			initComposited();
 		} else {
 			_isComposited = false;
-			_usingHighColor = shouldUseHighColor();
 			initOverlay();
 		}
-
-		_decoder->start();
 	}
 
-	EventFlags stopFlag = kEventFlagNone;
-	while (!g_engine->shouldQuit()) {
-		if (_decoder->endOfVideo()) {
-			stopFlag = kEventFlagEnd;
-			break;
-		}
-
-		// Sleeping any more than 1/60th of a second will make the mouse feel
-		// very sluggish during VMD action sequences because the frame rate of
-		// VMDs is usually only 15fps
-		g_sci->sleep(MIN<uint32>(10, _decoder->getTimeToNextFrame()));
-		while (_decoder->needsUpdate()) {
-			renderFrame();
-		}
+	// Sleeping any more than 1/60th of a second will make the mouse feel
+	// very sluggish during VMD action sequences because the frame rate of
+	// VMDs is usually only 15fps
+	return VideoPlayer::playUntilEvent(flags, 10);
+}
 
-		const int currentFrameNo = _decoder->getCurFrame();
+VMDPlayer::EventFlags VMDPlayer::checkForEvent(const EventFlags flags) {
+	const int currentFrameNo = _decoder->getCurFrame();
 
-		if (currentFrameNo == _yieldFrame) {
-			stopFlag = kEventFlagEnd;
-			break;
-		}
+	if (currentFrameNo == _yieldFrame) {
+		return kEventFlagEnd;
+	}
 
-		if (_yieldInterval > 0 &&
-			currentFrameNo != _lastYieldedFrameNo &&
-			(currentFrameNo % _yieldInterval) == 0
-		) {
-			_lastYieldedFrameNo = currentFrameNo;
-			stopFlag = kEventFlagYieldToVM;
-			break;
-		}
+	if (_yieldInterval > 0 &&
+		currentFrameNo != _lastYieldedFrameNo &&
+		(currentFrameNo % _yieldInterval) == 0) {
 
-		SciEvent event = _eventMan->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
-		if ((flags & kEventFlagMouseDown) && event.type == SCI_EVENT_MOUSE_PRESS) {
-			stopFlag = kEventFlagMouseDown;
-			break;
-		}
-
-		event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_PEEK);
-		if ((flags & kEventFlagEscapeKey) && event.type == SCI_EVENT_KEYBOARD) {
-			bool stop = false;
-			if (getSciVersion() < SCI_VERSION_3) {
-				while ((event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD)),
-					   event.type != SCI_EVENT_NONE) {
-					if (event.character == SCI_KEY_ESC) {
-						stop = true;
-						break;
-					}
-				}
-			} else {
-				stop = (event.character == SCI_KEY_ESC);
-			}
+		_lastYieldedFrameNo = currentFrameNo;
+		return kEventFlagYieldToVM;
+	}
 
-			if (stop) {
-				stopFlag = kEventFlagEscapeKey;
-				break;
-			}
-		}
+	EventFlags stopFlag = VideoPlayer::checkForEvent(flags);
+	if (stopFlag) {
+		return stopFlag;
+	}
 
-		event = _eventMan->getSciEvent(SCI_EVENT_HOT_RECTANGLE | SCI_EVENT_PEEK);
-		if ((flags & kEventFlagHotRectangle) && event.type == SCI_EVENT_HOT_RECTANGLE) {
-			stopFlag = kEventFlagHotRectangle;
-			break;
-		}
+	const SciEvent event = _eventMan->getSciEvent(SCI_EVENT_HOT_RECTANGLE | SCI_EVENT_PEEK);
+	if ((flags & kEventFlagHotRectangle) && event.type == SCI_EVENT_HOT_RECTANGLE) {
+		return kEventFlagHotRectangle;
 	}
 
-	return stopFlag;
+	return kEventFlagNone;
 }
 
 void VMDPlayer::initOverlay() {
+	// Make sure that any pending graphics changes from the game are submitted
+	// before starting playback, since if they aren't, and the video player
+	// yields back to the VM in the middle of playback, there may be a flash of
+	// content that draws over the video. (This happens when subtitles are
+	// enabled in Shivers.)
 	g_sci->_gfxFrameout->frameOut(true);
 
 #ifdef USE_RGB_COLOR
@@ -787,11 +729,7 @@ void VMDPlayer::initOverlay() {
 	// writing to an intermediate 4bpp surface and using that surface during
 	// cursor drawing, or by promoting the cursor code to use CursorMan, if
 	// possible
-	if (_usingHighColor) {
-		// TODO: 8888 is used here because 4bpp is the only format currently
-		// supported by the common scaling code
-		const Graphics::PixelFormat format = Graphics::createPixelFormat<8888>();
-		g_sci->_gfxFrameout->setPixelFormat(format);
+	if (startHQVideo()) {
 		redrawGameScreen();
 	}
 #endif
@@ -799,7 +737,12 @@ void VMDPlayer::initOverlay() {
 
 #ifdef USE_RGB_COLOR
 void VMDPlayer::redrawGameScreen() const {
+	if (!_hqVideoMode) {
+		return;
+	}
+
 	Graphics::Surface *game = g_sci->_gfxFrameout->getCurrentBuffer().convertTo(g_system->getScreenFormat(), g_sci->_gfxPalette32->getHardwarePalette());
+	assert(game);
 
 	Common::Rect rects[4];
 	int splitCount = splitRects(Common::Rect(game->w, game->h), _drawRect, rects);
@@ -815,51 +758,22 @@ void VMDPlayer::redrawGameScreen() const {
 }
 #endif
 
-void VMDPlayer::renderOverlay() const {
-	const Graphics::Surface *nextFrame = _decoder->decodeNextFrame();
-
+void VMDPlayer::renderOverlay(const Graphics::Surface &nextFrame) const {
 #ifdef USE_RGB_COLOR
-	if (_usingHighColor) {
-		if (updatePalette()) {
-			redrawGameScreen();
-		}
-
-		directWriteToSystem(_decoder, _drawRect, false, nextFrame);
-	} else {
-#else
-	{
+	if (_hqVideoMode) {
+		VideoPlayer::renderFrame(nextFrame);
+		return;
+	}
 #endif
-		updatePalette();
-
-		Graphics::Surface out = g_sci->_gfxFrameout->getCurrentBuffer().getSubArea(_drawRect);
-
-		const int lineCount = _blackLines ? 2 : 1;
-		if (_doublePixels) {
-			for (int16 y = 0; y < _drawRect.height(); y += lineCount) {
-				const uint8 *source = (uint8 *)nextFrame->getBasePtr(0, y >> 1);
-				uint8 *target = (uint8 *)out.getBasePtr(0, y);
-				for (int16 x = 0; x < _decoder->getWidth(); ++x) {
-					*target++ = *source;
-					*target++ = *source++;
-				}
-			}
-		} else if (_blackLines) {
-			for (int16 y = 0; y < _drawRect.height(); y += lineCount) {
-				const uint8 *source = (uint8 *)nextFrame->getBasePtr(0, y);
-				uint8 *target = (uint8 *)out.getBasePtr(0, y);
-				memcpy(target, source, _drawRect.width());
-			}
-		} else {
-			out.copyRectToSurface(nextFrame->getPixels(), nextFrame->pitch, 0, 0, nextFrame->w, nextFrame->h);
-		}
 
-		g_sci->_gfxFrameout->directFrameOut(_drawRect);
-	}
+	Graphics::Surface out = g_sci->_gfxFrameout->getCurrentBuffer().getSubArea(_drawRect);
+	renderLQToSurface<uint8>(out, nextFrame, _doublePixels, _blackLines);
+	g_sci->_gfxFrameout->directFrameOut(_drawRect);
 }
 
-bool VMDPlayer::updatePalette() const {
-	if (_ignorePalettes || !_decoder->hasDirtyPalette()) {
-		return false;
+void VMDPlayer::submitPalette(const uint8 rawPalette[256 * 3]) const {
+	if (_ignorePalettes) {
+		return;
 	}
 
 	Palette palette;
@@ -877,13 +791,15 @@ bool VMDPlayer::updatePalette() const {
 		}
 	} else
 #endif
-		fillPalette(palette);
+		fillPalette(rawPalette, palette);
 
 	if (_isComposited) {
 		SciBitmap *bitmap = _segMan->lookupBitmap(_bitmapId);
 		bitmap->setPalette(palette);
-		g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
-		g_sci->_gfxFrameout->frameOut(true);
+		// NOTE: SSCI calls updateScreenItem and frameOut here, but this should
+		// not be necessary in ScummVM since the new palette gets submitted
+		// before the next frame is rendered, and the frame rendering call will
+		// perform the same operations.
 	} else {
 		g_sci->_gfxPalette32->submit(palette);
 		g_sci->_gfxPalette32->updateForFrame();
@@ -892,7 +808,7 @@ bool VMDPlayer::updatePalette() const {
 
 #if SCI_VMD_BLACK_PALETTE
 	if (_blackPalette) {
-		fillPalette(palette);
+		fillPalette(rawPalette, palette);
 		if (_isComposited) {
 			SciBitmap *bitmap = _segMan->lookupBitmap(_bitmapId);
 			bitmap->setPalette(palette);
@@ -903,20 +819,25 @@ bool VMDPlayer::updatePalette() const {
 	}
 #endif
 
-	return true;
+#ifdef USE_RGB_COLOR
+	// Changes to the palette may affect areas outside of the video; when the
+	// engine is rendering video in high color, palette changes will only take
+	// effect once the entire screen is redrawn to the high color surface
+	redrawGameScreen();
+#endif
 }
 
 void VMDPlayer::closeOverlay() {
 #ifdef USE_RGB_COLOR
-	if (_usingHighColor) {
-		g_sci->_gfxFrameout->setPixelFormat(Graphics::PixelFormat::createFormatCLUT8());
-		g_sci->_gfxFrameout->resetHardware();
-	} else {
-#else
-	{
-#endif
-		g_sci->_gfxFrameout->frameOut(true, _drawRect);
+	if (_hqVideoMode) {
+		if (endHQVideo()) {
+			g_sci->_gfxFrameout->resetHardware();
+		}
+		return;
 	}
+#endif
+
+	g_sci->_gfxFrameout->frameOut(true, _drawRect);
 }
 
 void VMDPlayer::initComposited() {
@@ -940,7 +861,10 @@ void VMDPlayer::initComposited() {
 
 	CelInfo32 vmdCelInfo;
 	vmdCelInfo.bitmap = _bitmapId;
-	_decoder->setSurfaceMemory(vmdBitmap.getPixels(), vmdBitmap.getWidth(), vmdBitmap.getHeight(), 1);
+
+	Video::AdvancedVMDDecoder *decoder = dynamic_cast<Video::AdvancedVMDDecoder *>(_decoder.get());
+	assert(decoder);
+	decoder->setSurfaceMemory(vmdBitmap.getPixels(), vmdBitmap.getWidth(), vmdBitmap.getHeight(), 1);
 
 	if (_planeIsOwned) {
 		_plane = new Plane(_drawRect, kPlanePicColored);
@@ -967,13 +891,8 @@ void VMDPlayer::initComposited() {
 }
 
 void VMDPlayer::renderComposited() const {
-	// This writes directly to the CelObjMem we already created,
-	// so no need to take its return value
-	_decoder->decodeNextFrame();
-	if (!updatePalette()) {
-		g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
-		g_sci->_gfxFrameout->frameOut(true);
-	}
+	g_sci->_gfxFrameout->updateScreenItem(*_screenItem);
+	g_sci->_gfxFrameout->frameOut(true);
 }
 
 void VMDPlayer::closeComposited() {
@@ -999,16 +918,16 @@ void VMDPlayer::closeComposited() {
 #pragma mark -
 #pragma mark VMDPlayer - Rendering
 
-void VMDPlayer::renderFrame() const {
+void VMDPlayer::renderFrame(const Graphics::Surface &nextFrame) const {
 	if (_isComposited) {
 		renderComposited();
 	} else {
-		renderOverlay();
+		renderOverlay(nextFrame);
 	}
 }
 
-void VMDPlayer::fillPalette(Palette &palette) const {
-	const byte *vmdPalette = _decoder->getPalette() + _startColor * 3;
+void VMDPlayer::fillPalette(const uint8 rawPalette[256 * 3], Palette &outPalette) const {
+	const byte *vmdPalette = rawPalette + _startColor * 3;
 	for (uint16 i = _startColor; i <= _endColor; ++i) {
 		uint8 r = *vmdPalette++;
 		uint8 g = *vmdPalette++;
@@ -1020,10 +939,10 @@ void VMDPlayer::fillPalette(Palette &palette) const {
 			b = CLIP(b * _boostPercent / 100, 0, 255);
 		}
 
-		palette.colors[i].r = r;
-		palette.colors[i].g = g;
-		palette.colors[i].b = b;
-		palette.colors[i].used = true;
+		outPalette.colors[i].r = r;
+		outPalette.colors[i].g = g;
+		outPalette.colors[i].b = b;
+		outPalette.colors[i].used = true;
 	}
 }
 
@@ -1051,7 +970,7 @@ void VMDPlayer::restrictPalette(const uint8 startColor, const int16 endColor) {
 #pragma mark DuckPlayer
 
 DuckPlayer::DuckPlayer(SegManager *segMan, EventManager *eventMan) :
-	_eventMan(eventMan),
+	VideoPlayer(eventMan),
 	_decoder(new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
 	_plane(nullptr),
 	_status(kDuckClosed),
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index 474851c..bf3cc84 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -26,6 +26,7 @@
 #ifdef USE_RGB_COLOR
 #include "common/config-manager.h" // for ConfMan
 #endif
+#include "common/ptr.h"
 #include "common/rect.h"          // for Rect
 #include "common/scummsys.h"      // for int16, uint8, uint16, int32
 #include "common/str.h"           // for String
@@ -45,53 +46,160 @@ class SegManager;
 class SEQDecoder;
 struct Palette;
 
-#pragma mark SEQPlayer
-
 /**
- * SEQPlayer is used to play SEQ animations.
- * Used by DOS versions of GK1 and QFG4CD.
+ * An abstract class implementing common video playback functionality for SCI
+ * engine.
  */
-class SEQPlayer {
+class VideoPlayer {
 public:
-	SEQPlayer(SegManager *segMan, EventManager *eventMan);
+	enum EventFlags {
+		kEventFlagNone         = 0,
+		kEventFlagEnd          = 1,
+		kEventFlagEscapeKey    = 2,
+		kEventFlagMouseDown    = 4,
+		kEventFlagHotRectangle = 8,
+		kEventFlagToFrame      = 0x10,
+		kEventFlagYieldToVM    = 0x20,
+		kEventFlagReverse      = 0x80
+	};
+
+	friend EventFlags operator|(const EventFlags a, const EventFlags b) {
+		return static_cast<EventFlags>((int)a | (int)b);
+	}
+
+	VideoPlayer(EventManager *eventMan, Video::VideoDecoder *decoder = nullptr) :
+		_eventMan(eventMan),
+		_decoder(decoder)
+#ifdef USE_RGB_MODE
+		,
+		_hqVideoMode(false)
+#endif
+		{}
+
+	virtual ~VideoPlayer() {}
+
+protected:
+	EventManager *_eventMan;
 
 	/**
-	 * Plays a SEQ animation with the given
-	 * file name, with each frame being displayed
-	 * for `numTicks` ticks.
+	 * The video decoder to use for video playback by this player.
 	 */
-	void play(const Common::String &fileName, const int16 numTicks, const int16 x, const int16 y);
+	Common::ScopedPtr<Video::VideoDecoder> _decoder;
 
-private:
-	SegManager *_segMan;
-	EventManager *_eventMan;
-	SEQDecoder *_decoder;
+	/**
+	 * Attempts to open a video by filename and performs basic validation to
+	 * ensure that the current system is actually capable of playing back the
+	 * video.
+	 */
+	bool open(const Common::String &fileName);
 
 	/**
-	 * Renders a single frame of video.
+	 * Reinitializes the system hardware surface for playback of high-quality
+	 * scaled video if the current video meets the necessary criteria for this
+	 * playback mode.
+	 *
+	 * @returns whether or not the system surface was reinitialized for
+	 * high-quality scaled video.
 	 */
-	void renderFrame() const;
+	bool startHQVideo();
 
 	/**
-	 * Stops playback and closes the currently open SEQ stream.
+	 * Determines whether or not the currently loaded video meets the criteria
+	 * for high-quality scaled output.
 	 */
-	void close();
+	virtual bool shouldStartHQVideo() const {
+#ifdef USE_RGB_COLOR
+		if (!ConfMan.getBool("enable_hq_video")) {
+			return false;
+		}
+
+		if (_decoder->getWidth() == _drawRect.width() &&
+			_decoder->getHeight() == _drawRect.height()) {
+			return false;
+		}
+
+		return true;
+#else
+		return false;
+#endif
+	}
 
 	/**
-	 * The rectangle where the video will be drawn,
-	 * in screen coordinates.
+	 * Restores the hardware surface back to CLUT8 after video playback.
+	 */
+	bool endHQVideo();
+
+	/**
+	 * Plays a video until an event in the given `flags` is encountered, or
+	 * until the end of the video is reached.
+	 *
+	 * @param maxSleepMs An optional parameter defining the maximum number of
+	 * milliseconds that the video player should sleep between video frames.
+	 */
+	virtual EventFlags playUntilEvent(const EventFlags flags, const uint32 maxSleepMs = 0xFFFFFFFF);
+
+	/**
+	 * Checks to see if an event has occurred that should cause the video player
+	 * to yield back to the VM.
+	 */
+	virtual EventFlags checkForEvent(const EventFlags flags);
+
+	/**
+	 * Submits a palette from the video to the system.
+	 */
+	virtual void submitPalette(const uint8 palette[256 * 3]) const;
+
+	/**
+	 * Renders a video frame to the system.
+	 */
+	virtual void renderFrame(const Graphics::Surface &nextFrame) const;
+
+	/**
+	 * Renders a video frame to an intermediate surface using low-quality
+	 * scaling, black-lining, or direct copy, depending upon the passed flags.
+	 */
+	template <typename PixelType>
+	void renderLQToSurface(Graphics::Surface &out, const Graphics::Surface &nextFrame, const bool doublePixels, const bool blackLines) const;
+
+	/**
+	 * The rectangle where the video will be drawn, in screen coordinates.
 	 */
 	Common::Rect _drawRect;
+
+#ifdef USE_RGB_COLOR
+	/**
+	 * Whether or not the player is currently in high-quality video rendering
+	 * mode.
+	 */
+	bool _hqVideoMode;
+#endif
+};
+
+#pragma mark SEQPlayer
+
+/**
+ * SEQPlayer is used to play SEQ animations.
+ * Used by DOS versions of GK1 and QFG4CD.
+ */
+class SEQPlayer : public VideoPlayer {
+public:
+	SEQPlayer(EventManager *eventMan);
+
+	/**
+	 * Plays a SEQ animation with the given file name, with each frame being
+	 * displayed for `numTicks` ticks.
+	 */
+	void play(const Common::String &fileName, const int16 numTicks, const int16 x, const int16 y);
 };
 
 #pragma mark -
 #pragma mark AVIPlayer
 
 /**
- * AVIPlayer is used to play AVI videos. Used by
- * Windows versions of GK1CD, KQ7, and QFG4CD.
+ * AVIPlayer is used to play AVI videos.
+ * Used by Windows versions of GK1CD, KQ7, and QFG4CD.
  */
-class AVIPlayer {
+class AVIPlayer : public VideoPlayer {
 public:
 	enum IOStatus {
 		kIOSuccess      = 0,
@@ -106,16 +214,7 @@ public:
 		kAVIPaused   = 3
 	};
 
-	enum EventFlags {
-		kEventFlagNone         = 0,
-		kEventFlagEnd          = 1,
-		kEventFlagEscapeKey    = 2,
-		kEventFlagMouseDown    = 4,
-		kEventFlagHotRectangle = 8
-	};
-
 	AVIPlayer(EventManager *eventMan);
-	~AVIPlayer();
 
 	/**
 	 * Opens a stream to an AVI resource.
@@ -123,16 +222,18 @@ public:
 	IOStatus open(const Common::String &fileName);
 
 	/**
-	 * Initializes the AVI rendering parameters for the
-	 * current AVI. This must be called after `open`.
+	 * Initializes the AVI rendering parameters for the current AVI. This must
+	 * be called after `open`.
 	 */
-	IOStatus init(const bool pixelDouble);
+	IOStatus init(const bool doublePixels);
 
 	/**
 	 * Begins playback of the current AVI.
 	 */
 	IOStatus play(const int16 from, const int16 to, const int16 showStyle, const bool cue);
 
+	virtual EventFlags playUntilEvent(const EventFlags flags, const uint32 maxSleepMs = 0xFFFFFFFF) override;
+
 	/**
 	 * Stops playback and closes the currently open AVI stream.
 	 */
@@ -148,39 +249,11 @@ public:
 	 */
 	uint16 getDuration() const;
 
-	/**
-	 * Plays the AVI until an event occurs (e.g. user
-	 * presses escape, clicks, etc.).
-	 */
-	EventFlags playUntilEvent(const EventFlags flags);
-
 private:
-	typedef Common::HashMap<uint16, AVIStatus> StatusMap;
-
-	EventManager *_eventMan;
-	Video::AVIDecoder *_decoder;
-
 	/**
 	 * Playback status of the player.
 	 */
 	AVIStatus _status;
-
-	/**
-	 * The rectangle where the video will be drawn,
-	 * in screen coordinates.
-	 */
-	Common::Rect _drawRect;
-
-	/**
-	 * Renders video without event input until the
-	 * video is complete.
-	 */
-	void renderVideo() const;
-
-	/**
-	 * Renders a single frame of video.
-	 */
-	void renderFrame() const;
 };
 
 #pragma mark -
@@ -188,10 +261,10 @@ private:
 
 /**
  * VMDPlayer is used to play VMD videos.
- * Used by Phant1, GK2, PQ:SWAT, Shivers, SQ6,
- * Torin, and Lighthouse.
+ * Used by LSL7, Phant1, GK2, PQ:SWAT, Shivers, SQ6, Rama, Torin, and
+ * Lighthouse.
  */
-class VMDPlayer {
+class VMDPlayer : public VideoPlayer {
 public:
 	enum OpenFlags {
 		kOpenFlagNone = 0,
@@ -214,17 +287,6 @@ public:
 		kPlayFlagStretchVertical  = 0x100
 	};
 
-	enum EventFlags {
-		kEventFlagNone         = 0,
-		kEventFlagEnd          = 1,
-		kEventFlagEscapeKey    = 2,
-		kEventFlagMouseDown    = 4,
-		kEventFlagHotRectangle = 8,
-		kEventFlagToFrame      = 0x10,
-		kEventFlagYieldToVM    = 0x20,
-		kEventFlagReverse      = 0x80
-	};
-
 	enum VMDStatus {
 		kVMDNotOpen  = 0,
 		kVMDOpen     = 1,
@@ -234,13 +296,11 @@ public:
 		kVMDFinished = 5
 	};
 
-	VMDPlayer(SegManager *segMan, EventManager *eventMan);
-	~VMDPlayer();
+	VMDPlayer(EventManager *eventMan, SegManager *segMan);
+	virtual ~VMDPlayer();
 
 private:
 	SegManager *_segMan;
-	EventManager *_eventMan;
-	Video::AdvancedVMDDecoder *_decoder;
 
 #pragma mark -
 #pragma mark VMDPlayer - Playback
@@ -251,8 +311,8 @@ public:
 	IOStatus open(const Common::String &fileName, const OpenFlags flags);
 
 	/**
-	 * Initializes the VMD rendering parameters for the
-	 * current VMD. This must be called after `open`.
+	 * Initializes the VMD rendering parameters for the current VMD. This must
+	 * be called after `open`.
 	 */
 	void init(const int16 x, const int16 y, const PlayFlags flags, const int16 boostPercent, const int16 boostStartColor, const int16 boostEndColor);
 
@@ -271,64 +331,48 @@ public:
 
 private:
 	/**
-	 * Whether or not a VMD stream has been opened with
-	 * `open`.
+	 * Whether or not a VMD stream has been opened with `open`.
 	 */
 	bool _isOpen;
 
 	/**
-	 * Whether or not a VMD player has been initialised
-	 * with `init`.
+	 * Whether or not a VMD player has been initialized with `init`.
 	 */
 	bool _isInitialized;
 
 	/**
-	 * The Resource object for VMDs that are read out
-	 * of a resource bundle instead of being streamed
-	 * from the filesystem.
+	 * The Resource object for VMDs that are read out of a resource bundle
+	 * instead of being streamed from the filesystem.
 	 */
 	Resource *_bundledVmd;
 
 	/**
-	 * For VMDs played with the `kEventFlagToFrame` flag,
-	 * the target frame for yielding back to the SCI VM.
+	 * For VMDs played with the `kEventFlagToFrame` flag, the target frame for
+	 * yielding back to the SCI VM.
 	 */
 	int32 _yieldFrame;
 
 	/**
-	 * For VMDs played with the `kEventFlagYieldToVM` flag,
-	 * the number of frames that should be rendered until
-	 * yielding back to the SCI VM.
+	 * For VMDs played with the `kEventFlagYieldToVM` flag, the number of frames
+	 * that should be rendered until yielding back to the SCI VM.
 	 */
 	int32 _yieldInterval;
 
 	/**
-	 * For VMDs played with the `kEventFlagYieldToVM` flag,
-	 * the last frame when control of the main thread was
-	 * yielded back to the SCI VM.
+	 * For VMDs played with the `kEventFlagYieldToVM` flag, the last frame when
+	 * control of the main thread was yielded back to the SCI VM.
 	 */
 	int _lastYieldedFrameNo;
 
-	void initOverlay();
-	void renderOverlay() const;
-	void closeOverlay();
-
-	void initComposited();
-	void renderComposited() const;
-	void closeComposited();
-
-	/**
-	 * Plays the VMD until an event occurs (e.g. user
-	 * presses escape, clicks, etc.).
-	 */
-	EventFlags playUntilEvent(const EventFlags flags);
+	virtual EventFlags playUntilEvent(const EventFlags flags, const uint32 = 0xFFFFFFFF) override;
+	virtual EventFlags checkForEvent(const EventFlags flags) override;
 
 #pragma mark -
 #pragma mark VMDPlayer - Rendering
 public:
 	/**
-	 * Causes the VMD player to ignore all palettes in
-	 * the currently playing video.
+	 * Causes the VMD player to ignore all palettes in the currently playing
+	 * video.
 	 */
 	void ignorePalettes() { _ignorePalettes = true; }
 
@@ -337,12 +381,18 @@ public:
 	 */
 	void setPlane(const int16 priority, const reg_t planeId);
 
-private:
+protected:
 	/**
-	 * The rectangle where the video will be drawn, in screen coordinates.
+	 * Renders a frame of video to the output bitmap.
 	 */
-	Common::Rect _drawRect;
+	virtual void renderFrame(const Graphics::Surface &nextFrame) const override;
+
+	/**
+	 * Updates the system with palette data from the video.
+	 */
+	virtual void submitPalette(const uint8 palette[256 * 3]) const override;
 
+private:
 	/**
 	 * The plane where the VMD will be drawn.
 	 */
@@ -358,11 +408,9 @@ private:
 	 */
 	reg_t _bitmapId;
 
-	// TODO: planeIsOwned and priority are used in SCI3+ only
-
 	/**
-	 * If true, the plane for this VMD was set
-	 * externally and is not owned by this VMDPlayer.
+	 * If true, the plane for this VMD was set externally and is not owned by
+	 * this VMDPlayer.
 	 */
 	bool _planeIsOwned;
 
@@ -378,26 +426,24 @@ private:
 	bool _doublePixels;
 
 	/**
-	 * Whether or not the video should be pixel doubled
-	 * vertically only.
+	 * Whether or not the video should be pixel doubled vertically only.
 	 */
 	bool _stretchVertical;
 
 	/**
-	 * Whether or not black lines should be rendered
-	 * across the video.
+	 * Whether or not black lines should be rendered across the video.
 	 */
 	bool _blackLines;
 
 	/**
-	 * Whether or not the playback area of the VMD
-	 * should be left black at the end of playback.
+	 * Whether or not the playback area of the VMD should be left black at the
+	 * end of playback.
 	 */
 	bool _leaveScreenBlack;
 
 	/**
-	 * Whether or not the area of the VMD should be left
-	 * displaying the final frame of the video.
+	 * Whether or not the area of the VMD should be left displaying the final
+	 * frame of the video.
 	 */
 	bool _leaveLastFrame;
 
@@ -412,35 +458,18 @@ private:
 	bool _isComposited;
 
 	/**
-	 * Whether or not rendering of the video is being performed in high color or
-	 * better.
-	 */
-	bool _usingHighColor;
-
-	/**
-	 * Renders a frame of video to the output bitmap.
-	 */
-	void renderFrame() const;
-
-	/**
-	 * Updates the system with palette data from the video.
+	 * Fills the given palette with RGB values from the VMD palette, applying
+	 * brightness boost if it is enabled.
 	 */
-	bool updatePalette() const;
-
-	/**
-	 * Fills the given palette with RGB values from
-	 * the VMD palette, applying brightness boost if
-	 * it is enabled.
-	 */
-	void fillPalette(Palette &palette) const;
+	void fillPalette(const uint8 rawPalette[256 * 3], Palette &outPalette) const;
 
 #ifdef USE_RGB_COLOR
 	/**
 	 * Redraws areas of the screen outside of the video to the system buffer.
-	 * This is used when
+	 * This is used whenever palette changes occur and the video is rendering in
+	 * high color mode.
 	 */
 	void redrawGameScreen() const;
-#endif
 
 	/**
 	 * Determines whether or not the VMD player should upgrade the renderer to
@@ -450,18 +479,18 @@ private:
 	 * video, but this will require additional work in GfxFrameout and
 	 * GfxCursor32 since the internal buffer and cursor code are 8bpp only.
 	 */
-	bool shouldUseHighColor() const {
-#ifdef USE_RGB_COLOR
-		return ConfMan.getBool("enable_hq_video") &&
-			_priority == 0 &&
-			(_doublePixels || _stretchVertical) &&
-			!_leaveLastFrame &&
-			!_showCursor &&
-			!_blackLines;
-#else
-		return false;
-#endif
+	virtual bool shouldStartHQVideo() const override {
+		if (!VideoPlayer::shouldStartHQVideo()) {
+			return false;
+		}
+
+		if (_priority != 0 || _leaveLastFrame || _showCursor || _blackLines) {
+			return false;
+		}
+
+		return true;
 	}
+#endif
 
 	/**
 	 * Determines whether or not the video should use the compositing renderer
@@ -469,18 +498,26 @@ private:
 	 */
 	bool shouldUseCompositing() const {
 #ifdef USE_RGB_COLOR
-		return getSciVersion() == SCI_VERSION_3 && !shouldUseHighColor();
+		return getSciVersion() == SCI_VERSION_3 && !shouldStartHQVideo();
 #else
 		return getSciVersion() == SCI_VERSION_3;
 #endif
 	}
 
+	void initOverlay();
+	void renderOverlay(const Graphics::Surface &nextFrame) const;
+	void closeOverlay();
+
+	void initComposited();
+	void renderComposited() const;
+	void closeComposited();
+
 #pragma mark -
 #pragma mark VMDPlayer - Blackout
 public:
 	/**
-	 * Sets the area of the screen that should be blacked out
-	 * during VMD playback.
+	 * Sets the area of the screen that should be blacked out during VMD
+	 * playback.
 	 */
 	void setBlackoutArea(const Common::Rect &rect) { _blackoutRect = rect; }
 
@@ -491,8 +528,8 @@ private:
 	Common::Rect _blackoutRect;
 
 	/**
-	 * An optional plane that will be used to black out
-	 * areas of the screen outside of the VMD surface.
+	 * An optional plane that will be used to black out areas of the screen
+	 * outside of the VMD surface.
 	 */
 	Plane *_blackoutPlane;
 
@@ -500,37 +537,33 @@ private:
 #pragma mark VMDPlayer - Palette
 public:
 	/**
-	 * Restricts use of the system palette by VMD playback to
-	 * the given range of palette indexes.
+	 * Restricts use of the system palette by VMD playback to the given range of
+	 * palette indexes.
 	 */
 	void restrictPalette(const uint8 startColor, const int16 endColor);
 
 private:
 	/**
-	 * The first color in the system palette that the VMD
-	 * can write to.
+	 * The first color in the system palette that the VMD can write to.
 	 */
 	uint8 _startColor;
 
 	/**
-	 * The last color in the system palette that the VMD
-	 * can write to.
+	 * The last color in the system palette that the VMD can write to.
 	 */
 	uint8 _endColor;
 
 	/**
-	 * If true, video frames are rendered after a blank
-	 * palette is submitted to the palette manager,
-	 * which is then restored after the video pixels
-	 * have already been rendered.
+	 * If true, video frames are rendered after a blank palette is submitted to
+	 * the palette manager, which is then restored after the video pixels have
+	 * already been rendered.
 	 *
-	 * This functionality is currently disabled because it seems like
-	 * it was designed for a different graphics architecture where
-	 * pixel data could be rendered before the video card's palette
-	 * had been updated. This is not possible in ScummVM because the
-	 * palette & pixel data are rendered simultaneously when
-	 * OSystem::updateScreen is called, rather than immediately
-	 * after they are sent to the backend.
+	 * This functionality is currently disabled because it seems like it was
+	 * designed for a different graphics architecture where pixel data could be
+	 * rendered before the video card's palette had been updated. This is not
+	 * possible in ScummVM because the palette & pixel data are rendered
+	 * simultaneously when OSystem::updateScreen is called, rather than
+	 * immediately after they are sent to the backend.
 	 */
 #ifdef SCI_VMD_BLACK_PALETTE
 	bool _blackPalette;
@@ -540,21 +573,18 @@ private:
 #pragma mark VMDPlayer - Brightness boost
 private:
 	/**
-	 * The amount of brightness boost for the video.
-	 * Values above 100 increase brightness; values below
-	 * 100 reduce it.
+	 * The amount of brightness boost for the video. Values above 100 increase
+	 * brightness; values below 100 reduce it.
 	 */
 	int16 _boostPercent;
 
 	/**
-	 * The first color in the palette that should be
-	 * brightness boosted.
+	 * The first color in the palette that should be brightness boosted.
 	 */
 	uint8 _boostStartColor;
 
 	/**
-	 * The last color in the palette that should be
-	 * brightness boosted.
+	 * The last color in the palette that should be brightness boosted.
 	 */
 	uint8 _boostEndColor;
 
@@ -562,17 +592,15 @@ private:
 #pragma mark VMDPlayer - Mouse cursor
 public:
 	/**
-	 * Sets whether or not the mouse cursor should be drawn.
-	 * This does not have any effect during playback, but can
-	 * be used to prevent the mouse cursor from being shown
-	 * again after the video has finished.
+	 * Sets whether or not the mouse cursor should be drawn. This does not have
+	 * any effect during playback, but can be used to prevent the mouse cursor
+	 * from being shown again after the video has finished.
 	 */
 	void setShowCursor(const bool shouldShow) { _showCursor = shouldShow; }
 
 private:
 	/**
-	 * Whether or not the mouse cursor should be shown
-	 * during playback.
+	 * Whether or not the mouse cursor should be shown during playback.
 	 */
 	bool _showCursor;
 };
@@ -580,7 +608,7 @@ private:
 #pragma mark -
 #pragma mark DuckPlayer
 
-class DuckPlayer {
+class DuckPlayer : public VideoPlayer {
 public:
 	enum DuckStatus {
 		kDuckClosed  = 0,
@@ -674,15 +702,14 @@ private:
 #pragma mark Video32
 
 /**
- * Video32 provides facilities for playing back
- * video in SCI engine.
+ * Video32 provides facilities for playing back video in SCI engine.
  */
 class Video32 : public Common::Serializable {
 public:
 	Video32(SegManager *segMan, EventManager *eventMan) :
-	_SEQPlayer(segMan, eventMan),
+	_SEQPlayer(eventMan),
 	_AVIPlayer(eventMan),
-	_VMDPlayer(segMan, eventMan),
+	_VMDPlayer(eventMan, segMan),
 	_robotPlayer(segMan),
 	_duckPlayer(segMan, eventMan) {}
 
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index 634652c..f6bc48f 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -838,6 +838,10 @@ int SciEngine::inQfGImportRoom() const {
 }
 
 void SciEngine::sleep(uint32 msecs) {
+	if (!msecs) {
+		return;
+	}
+
 	uint32 time;
 	const uint32 wakeUpTime = g_system->getMillis() + msecs;
 
@@ -860,7 +864,6 @@ void SciEngine::sleep(uint32 msecs) {
 				g_system->delayMillis(wakeUpTime - time);
 			break;
 		}
-
 	}
 }
 


Commit: 3f0e061eaa272c3f6bc284d8e837870e132d9dcc
    https://github.com/scummvm/scummvm/commit/3f0e061eaa272c3f6bc284d8e837870e132d9dcc
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:39-05:00

Commit Message:
SCI32: Refactor DuckPlayer to use common video playback code

This lets DuckPlayer support configurable black-lined video and
configurable high-quality scaling.

Changed paths:
    engines/sci/detection_tables.h
    engines/sci/graphics/video32.cpp
    engines/sci/graphics/video32.h


diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index 5061876..de75184 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -3117,10 +3117,11 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 #ifdef ENABLE_SCI3_GAMES
 
 // TODO: Correct GUIOs
-#define GUIO_PHANTASMAGORIA2 GUIO4(GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
-                                   GUIO_NOASPECT, \
+#define GUIO_PHANTASMAGORIA2 GUIO5(GUIO_NOASPECT, \
                                    GUIO_NOMIDI, \
-                                   GAMEOPTION_ORIGINAL_SAVELOAD)
+                                   GAMEOPTION_ENABLE_BLACK_LINED_VIDEO, \
+                                   GAMEOPTION_ORIGINAL_SAVELOAD, \
+                                   GAMEOPTION_HQ_VIDEO)
 
 	// Some versions of Phantasmagoria 2 were heavily censored.
 	// Censored versions (data files are currently unknown to us): UK, Australia, first English release in Germany
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index ec37d1c..22be48c 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -969,21 +969,12 @@ void VMDPlayer::restrictPalette(const uint8 startColor, const int16 endColor) {
 #pragma mark -
 #pragma mark DuckPlayer
 
-DuckPlayer::DuckPlayer(SegManager *segMan, EventManager *eventMan) :
-	VideoPlayer(eventMan),
-	_decoder(new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
+DuckPlayer::DuckPlayer(EventManager *eventMan, SegManager *segMan) :
+	VideoPlayer(eventMan, new Video::AVIDecoder(Audio::Mixer::kSFXSoundType)),
 	_plane(nullptr),
 	_status(kDuckClosed),
-	_drawRect(),
 	_volume(Audio::Mixer::kMaxChannelVolume),
-	_doFrameOut(false),
-	_pixelDouble(false),
-	_scaleBuffer(nullptr) {}
-
-DuckPlayer::~DuckPlayer() {
-	close();
-	delete _decoder;
-}
+	_doFrameOut(false) {}
 
 void DuckPlayer::open(const GuiResourceId resourceId, const int displayMode, const int16 x, const int16 y) {
 	if (_status != kDuckClosed) {
@@ -991,19 +982,22 @@ void DuckPlayer::open(const GuiResourceId resourceId, const int displayMode, con
 	}
 
 	const Common::String fileName = Common::String::format("%u.duk", resourceId);
-	if (!_decoder->loadFile(fileName)) {
-		error("Can't open %s", fileName.c_str());
+
+	if (!VideoPlayer::open(fileName)) {
+		return;
 	}
 
 	_decoder->setVolume(_volume);
-	_pixelDouble = displayMode != 0;
 
-	const int16 scale = _pixelDouble ? 2 : 1;
+	_doublePixels = displayMode != 0;
+	_blackLines = ConfMan.getBool("enable_black_lined_video") &&
+				 (displayMode == 1 || displayMode == 3);
+
 	// SSCI seems to incorrectly calculate the draw rect by scaling the origin
 	// in addition to the width/height for the BR point
 	_drawRect = Common::Rect(x, y,
-							 x + _decoder->getWidth() * scale,
-							 y + _decoder->getHeight() * scale);
+							 x + (_decoder->getWidth() << _doublePixels),
+							 y + (_decoder->getHeight() << _doublePixels));
 
 	g_sci->_gfxCursor32->hide();
 
@@ -1013,59 +1007,33 @@ void DuckPlayer::open(const GuiResourceId resourceId, const int displayMode, con
 		g_sci->_gfxFrameout->frameOut(true);
 	}
 
-	const Graphics::PixelFormat format = _decoder->getPixelFormat();
-
-	if (_pixelDouble) {
-		assert(_scaleBuffer == nullptr);
-		_scaleBuffer = new byte[_drawRect.width() * _drawRect.height() * format.bytesPerPixel];
+	if (!startHQVideo() && _decoder->getPixelFormat().bytesPerPixel != 1) {
+		g_sci->_gfxFrameout->setPixelFormat(_decoder->getPixelFormat());
 	}
 
-	g_sci->_gfxFrameout->setPixelFormat(format);
-
 	_status = kDuckOpen;
 }
 
 void DuckPlayer::play(const int lastFrameNo) {
-	flushEvents(_eventMan);
+	// This status check does not exist in the original interpreter, but is
+	// necessary to avoid a crash if the engine cannot find or render the video
+	// for playback. Game scripts receive no feedback from the kernel regarding
+	// whether or not an attempt to open a Duck video actually succeeded, so
+	// they can only assume it always succeeds (and so always call to `play`
+	// even if they shouldn't).
+	if (_status == kDuckClosed) {
+		return;
+	}
 
 	if (_status != kDuckPlaying) {
 		_status = kDuckPlaying;
-		_decoder->start();
 	}
 
-	while (!g_engine->shouldQuit()) {
-		if (_decoder->endOfVideo() || (lastFrameNo != -1 && _decoder->getCurFrame() >= lastFrameNo)) {
-			break;
-		}
-
-		g_sci->sleep(_decoder->getTimeToNextFrame());
-		while (_decoder->needsUpdate()) {
-			renderFrame();
-		}
-
-		SciEvent event = _eventMan->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
-		if (event.type == SCI_EVENT_MOUSE_PRESS) {
-			flushEvents(_eventMan);
-			break;
-		}
-
-		event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_PEEK);
-		if (event.type == SCI_EVENT_KEYBOARD) {
-			bool stop = false;
-			while ((event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD)),
-				   event.type != SCI_EVENT_NONE) {
-				if (event.character == SCI_KEY_ESC) {
-					stop = true;
-					break;
-				}
-			}
-
-			if (stop) {
-				flushEvents(_eventMan);
-				break;
-			}
-		}
+	if (lastFrameNo != -1) {
+		_decoder->setEndFrame(lastFrameNo);
 	}
+
+	playUntilEvent(kEventFlagMouseDown | kEventFlagEscapeKey);
 }
 
 void DuckPlayer::close() {
@@ -1075,8 +1043,7 @@ void DuckPlayer::close() {
 
 	_decoder->close();
 
-	const Graphics::PixelFormat format = Graphics::PixelFormat::createFormatCLUT8();
-	g_sci->_gfxFrameout->setPixelFormat(format);
+	endHQVideo();
 
 	g_sci->_gfxCursor32->unhide();
 
@@ -1086,109 +1053,28 @@ void DuckPlayer::close() {
 		_plane = nullptr;
 	}
 
-	_pixelDouble = false;
-	delete[] _scaleBuffer;
-	_scaleBuffer = nullptr;
-
+	_drawRect = Common::Rect();
 	_status = kDuckClosed;
+	_volume = Audio::Mixer::kMaxChannelVolume;
+	_doFrameOut = false;
 }
 
-static inline uint16 interpolate(const Graphics::PixelFormat &format, const uint16 p1, const uint16 p2) {
-	uint8 r1, g1, b1, r2, g2, b2;
-	format.colorToRGB(p1, r1, g1, b1);
-	format.colorToRGB(p2, r2, g2, b2);
-	return format.RGBToColor((r1 + r2) >> 1, (g1 + g2) >> 1, (b1 + b2) >> 1);
-}
-
-void DuckPlayer::renderFrame() const {
-	const Graphics::Surface *surface = _decoder->decodeNextFrame();
-
-	// Audio-only or non-updated frame
-	if (surface == nullptr) {
+void DuckPlayer::renderFrame(const Graphics::Surface &nextFrame) const {
+#ifdef USE_RGB_COLOR
+	if (_hqVideoMode) {
+		VideoPlayer::renderFrame(nextFrame);
 		return;
 	}
-
-	assert(surface->format.bytesPerPixel == 2);
-
-	if (_pixelDouble) {
-		const uint16 *source = (const uint16 *)surface->getPixels();
-		const Graphics::PixelFormat &format = surface->format;
-		uint16 *target = (uint16 *)_scaleBuffer;
-
-#ifndef SCI_DUCK_NO_INTERPOLATION
-		// divide by 2 gets pixel pitch instead of byte pitch for source
-		const uint16 sourcePitch = surface->pitch >> 1;
-#endif
-
-		const uint16 targetPitch = surface->pitch;
-		const bool blackLined = ConfMan.getBool("enable_black_lined_video");
-		for (int y = 0; y < surface->h - 1; ++y) {
-			for (int x = 0; x < surface->w - 1; ++x) {
-#ifndef SCI_DUCK_NO_INTERPOLATION
-				const uint16 a = source[0];
-				const uint16 b = source[1];
-				const uint16 c = source[sourcePitch];
-				const uint16 d = source[sourcePitch + 1];
-
-				target[0] = a;
-				target[1] = interpolate(format, a, b);
-#else
-				const uint16 value = *source;
-				target[0] = value;
-				target[1] = value;
-#endif
-				if (!blackLined) {
-#ifndef SCI_DUCK_NO_INTERPOLATION
-					target[targetPitch] = interpolate(format, a, c);
-					target[targetPitch + 1] = interpolate(format, target[1], interpolate(format, c, d));
-#else
-					target[targetPitch] = value;
-					target[targetPitch + 1] = value;
 #endif
-				}
-
-				target += 2;
-				++source;
-			}
-
-			const uint16 value = *source++;
-			target[0] = value;
-			target[1] = value;
-			if (!blackLined) {
-				target[targetPitch] = value;
-				target[targetPitch + 1] = value;
-			}
-			target += 2;
-
-			if (blackLined) {
-				memset(target, 0, targetPitch * format.bytesPerPixel);
-			}
-
-			target += targetPitch;
-		}
 
-		for (int x = 0; x < surface->w; ++x) {
-			const uint16 lastValue = *source++;
-			target[0] = lastValue;
-			target[1] = lastValue;
-
-			if (!blackLined) {
-				target[targetPitch] = lastValue;
-				target[targetPitch + 1] = lastValue;
-				target += 2;
-			}
-		}
-
-		if (blackLined) {
-			memset(target, 0, targetPitch);
-		}
-
-		g_system->copyRectToScreen(_scaleBuffer, surface->pitch * 2, _drawRect.left, _drawRect.top, _drawRect.width(), _drawRect.height());
-	} else {
-		g_system->copyRectToScreen(surface->getPixels(), surface->pitch, _drawRect.left, _drawRect.top, surface->w, surface->h);
+	Graphics::Surface out;
+	out.create(_drawRect.width(), _drawRect.height(), nextFrame.format);
+	renderLQToSurface<uint16>(out, nextFrame, _doublePixels, _blackLines);
+	if (out.format != g_system->getScreenFormat()) {
+		out.convertToInPlace(g_system->getScreenFormat());
 	}
-
-	g_sci->_gfxFrameout->updateScreen();
+	g_system->copyRectToScreen(out.getPixels(), out.pitch, _drawRect.left, _drawRect.top, out.w, out.h);
+	out.free();
 }
 
 } // End of namespace Sci
diff --git a/engines/sci/graphics/video32.h b/engines/sci/graphics/video32.h
index bf3cc84..5c21348 100644
--- a/engines/sci/graphics/video32.h
+++ b/engines/sci/graphics/video32.h
@@ -608,6 +608,10 @@ private:
 #pragma mark -
 #pragma mark DuckPlayer
 
+/**
+ * DuckPlayer is used to play Duck TrueMotion videos.
+ * Used by Phantasmagoria 2.
+ */
 class DuckPlayer : public VideoPlayer {
 public:
 	enum DuckStatus {
@@ -617,9 +621,7 @@ public:
 		kDuckPaused  = 3
 	};
 
-	DuckPlayer(SegManager *segMan, EventManager *eventMan);
-
-	~DuckPlayer();
+	DuckPlayer(EventManager *eventMan, SegManager *segMan);
 
 	/**
 	 * Opens a stream to a Duck resource.
@@ -637,9 +639,8 @@ public:
 	void play(const int lastFrameNo);
 
 	/**
-	 * Sets a flag indicating that an opaque plane should be added
-	 * to the graphics manager underneath the video surface during
-	 * playback.
+	 * Sets a flag indicating that an opaque plane should be added to the
+	 * graphics manager underneath the video surface during playback.
 	 */
 	void setDoFrameOut(const bool value) { _doFrameOut = value; }
 
@@ -647,17 +648,24 @@ public:
 	 * Sets the volume of the decoder.
 	 */
 	void setVolume(const uint8 value) {
-		_volume = (uint)value * Audio::Mixer::kMaxChannelVolume / Audio32::kMaxVolume;
+		_volume = value * Audio::Mixer::kMaxChannelVolume / Audio32::kMaxVolume;
 		_decoder->setVolume(_volume);
 	}
 
-private:
-	EventManager *_eventMan;
-	Video::AVIDecoder *_decoder;
+protected:
+	virtual bool shouldStartHQVideo() const override {
+		if (!VideoPlayer::shouldStartHQVideo() || _blackLines) {
+			return false;
+		}
 
+		return true;
+	}
+
+	virtual void renderFrame(const Graphics::Surface &nextFrame) const override;
+
+private:
 	/**
-	 * An empty plane drawn behind the video when the doFrameOut
-	 * flag is true.
+	 * An empty plane drawn behind the video when the doFrameOut flag is true.
 	 */
 	Plane *_plane;
 
@@ -667,11 +675,6 @@ private:
 	DuckStatus _status;
 
 	/**
-	 * The screen rect where the video should be drawn.
-	 */
-	Common::Rect _drawRect;
-
-	/**
 	 * The playback volume for the player.
 	 */
 	uint8 _volume;
@@ -683,19 +686,14 @@ private:
 	bool _doFrameOut;
 
 	/**
-	 * If true, the video will be pixel doubled during playback.
-	 */
-	bool _pixelDouble;
-
-	/**
-	 * The buffer used to perform scaling of the video.
+	 * Whether or not the video should be pixel doubled.
 	 */
-	byte *_scaleBuffer;
+	bool _doublePixels;
 
 	/**
-	 * Renders the current frame to the system video buffer.
+	 * Whether or not black lines should be rendered across the video.
 	 */
-	void renderFrame() const;
+	bool _blackLines;
 };
 
 #pragma mark -
@@ -711,7 +709,7 @@ public:
 	_AVIPlayer(eventMan),
 	_VMDPlayer(eventMan, segMan),
 	_robotPlayer(segMan),
-	_duckPlayer(segMan, eventMan) {}
+	_duckPlayer(eventMan, segMan) {}
 
 	void beforeSaveLoadWithSerializer(Common::Serializer &ser);
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);


Commit: 12d24d5b46cc4017b159040da6f42c9cfde5cbf7
    https://github.com/scummvm/scummvm/commit/12d24d5b46cc4017b159040da6f42c9cfde5cbf7
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-07-06T19:12:40-05:00

Commit Message:
SCI32: Fix bad palette entries when built without USE_RGB_COLOR

This is only a problem for the Windows games that need some
palette entries to be ignored.

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


diff --git a/engines/sci/graphics/palette32.cpp b/engines/sci/graphics/palette32.cpp
index 339461d..d7f6710 100644
--- a/engines/sci/graphics/palette32.cpp
+++ b/engines/sci/graphics/palette32.cpp
@@ -527,6 +527,12 @@ void GfxPalette32::updateHardware() {
 		}
 	}
 
+#ifndef USE_RGB_COLOR
+	// When creating a raw palette on the stack, any skipped area of the palette
+	// needs to be blacked out or else it will contain garbage memory
+	memset(bpal + (maxIndex + 1) * 3, 0, (255 - maxIndex - 1) * 3);
+#endif
+
 	if (g_sci->getPlatform() != Common::kPlatformMacintosh) {
 		// The last color must always be white
 		bpal[255 * 3    ] = 255;





More information about the Scummvm-git-logs mailing list