Revamping the handling of GUI themes a bit:

* Moved the code which handles fallback to the built-in theme from ThemeEngine to GuiManager
* Changed ThemeEngine::init() to only init&load the theme; no more messing with the overlay
* Commented to a somewhat obscure line in the GuiManager event loop (taking eriktorbjorn's original commit message for that change as comment text)
* Cleaned up the way the Dialog constructor ensures that the current Theme is correctly setup
* Got rid of varios members of ThemeEngine
* Changed ThemeEngine members _screen and _backBuffer from Surface pointers to just plain Surfaces
* Changed ThemeEngine::loadFontFromArchive to use _themeArchive instead of creating an Archive from scratch
* Renamed ThemeEngine::getThemeFileName() to getThemeId() (and some associated tweaks)
* Lots of further cleanup and tweaks

svn-id: r35653

gui/ThemeEngine.cpp

@@ -51,10 +51,9 @@ namespace GUI {
  *	ThemeEngine class
  *********************************************************/
 ThemeEngine::ThemeEngine(Common::String fileName, GraphicsMode mode) :
-	_system(0), _vectorRenderer(0), _screen(0), _backBuffer(0),
+	_system(0), _vectorRenderer(0),
 	_buffering(false), _bytesPerPixel(0),  _graphicsMode(kGfxDisabled),
-	_font(0), _initOk(false), _themeOk(false), _enabled(false), _cursor(0),
+	_font(0), _initOk(false), _themeOk(false), _enabled(false), _cursor(0) {
-	_loadedThemeX(0), _loadedThemeY(0) {
 
 	_system = g_system;
 	_parser = new ThemeParser(this);
@@ -71,13 +70,17 @@ ThemeEngine::ThemeEngine(Common::String fileName, GraphicsMode mode) :
 	}
 
 	_graphicsMode = mode;
-	_themeFileName = fileName;
+	_themeId = fileName;
 	_themeArchive = 0;
 	_initOk = false;
 }
 
 ThemeEngine::~ThemeEngine() {
-	deinit();
+	delete _vectorRenderer;
+	_vectorRenderer = 0;
+	_screen.free();
+	_backBuffer.free();
+
 	unloadTheme();
 	delete _parser;
 	delete _themeEval;
@@ -133,82 +136,31 @@ const char *ThemeEngine::findModeConfigName(GraphicsMode mode) {
  *********************************************************/
 bool ThemeEngine::init() {
 	// reset everything and reload the graphics
-	if (_initOk)
+	_initOk = false;
-		_system->hideOverlay();
-	deinit();
 	setGraphicsMode(_graphicsMode);
 
-	if (_screen->pixels && _backBuffer->pixels) {
+	if (_screen.pixels && _backBuffer.pixels) {
 		_initOk = true;
-		clearAll();
 	}
 
-	if (_screen->w >= 400 && _screen->h >= 300) {
+	// TODO: Instead of hard coding the font here, it should be possible
+	// to specify the fonts to be used for each resolution in the theme XML.
+	if (_screen.w >= 400 && _screen.h >= 300) {
 		_font = FontMan.getFontByUsage(Graphics::FontManager::kBigGUIFont);
 	} else {
 		_font = FontMan.getFontByUsage(Graphics::FontManager::kGUIFont);
 	}
 
-	if (isThemeLoadingRequired() || !_themeOk) {
+	loadTheme(_themeId);
-		loadTheme(_themeFileName);
-	}
 
-	return true;
+	return ready();
-}
-
-void ThemeEngine::deinit() {
-	delete _vectorRenderer;
-	_vectorRenderer = 0;
-	if (_screen != 0) {
-		_screen->free();
-		delete _screen;
-		_screen = 0;
-	}
-	if (_backBuffer != 0) {
-		_backBuffer->free();
-		delete _backBuffer;
-		_backBuffer = 0;
-	}
-	_initOk = false;
-}
-
-void ThemeEngine::unloadTheme() {
-	if (!_themeOk)
-		return;
-
-	for (int i = 0; i < kDrawDataMAX; ++i) {
-		delete _widgets[i];
-		_widgets[i] = 0;
-	}
-
-	for (int i = 0; i < kTextDataMAX; ++i) {
-		delete _texts[i];
-		_texts[i] = 0;
-	}
-
-	// Release all graphics surfaces
-	for (ImagesMap::iterator i = _bitmaps.begin(); i != _bitmaps.end(); ++i) {
-		Graphics::Surface *surf = i->_value;
-		if (surf) {
-			surf->free();
-			delete surf;
-		}
-	}
-	_bitmaps.clear();
-
-	delete _themeArchive;
-	_themeArchive = 0;
-
-	_themeEval->reset();
-	_themeOk = false;
 }
 
 void ThemeEngine::clearAll() {
-	if (!_initOk)
+	if (_initOk) {
-		return;
+		_system->clearOverlay();
-
+		_system->grabOverlay((OverlayColor *)_screen.pixels, _screen.w);
-	_system->clearOverlay();
+	}
-	_system->grabOverlay((OverlayColor *)_screen->pixels, _screen->w);
 }
 
 void ThemeEngine::refresh() {
@@ -226,8 +178,11 @@ void ThemeEngine::refresh() {
 void ThemeEngine::enable() {
 	init();
 
-	if (_useCursor)
+	if (_useCursor) {
-		setUpCursor();
+		CursorMan.pushCursorPalette(_cursorPal, 0, MAX_CURS_COLORS);
+		CursorMan.pushCursor(_cursor, _cursorWidth, _cursorHeight, _cursorHotspotX, _cursorHotspotY, 255, _cursorTargetScale);
+		CursorMan.showMouse(true);
+	}
 
 	_system->showOverlay();
 	clearAll();
@@ -245,28 +200,6 @@ void ThemeEngine::disable() {
 	_enabled = false;
 }
 
-template<typename PixelType>
-void ThemeEngine::screenInit(bool backBuffer) {
-	uint32 width = _system->getOverlayWidth();
-	uint32 height = _system->getOverlayHeight();
-
-	if (backBuffer) {
-		if (_backBuffer)
-			_backBuffer->free();
-		else
-			_backBuffer = new Graphics::Surface;
-		_backBuffer->create(width, height, sizeof(PixelType));
-	}
-
-
-	if (_screen)
-		_screen->free();
-	else
-		_screen = new Graphics::Surface;
-	_screen->create(width, height, sizeof(PixelType));
-	_system->clearOverlay();
-}
-
 void ThemeEngine::setGraphicsMode(GraphicsMode mode) {
 	switch (mode) {
 	case kGfxStandard16bit:
@@ -274,16 +207,24 @@ void ThemeEngine::setGraphicsMode(GraphicsMode mode) {
 	case kGfxAntialias16bit:
 #endif
 		_bytesPerPixel = sizeof(uint16);
-		screenInit<uint16>(kEnableBackCaching);
 		break;
 
 	default:
 		error("Invalid graphics mode");
 	}
 
+	uint32 width = _system->getOverlayWidth();
+	uint32 height = _system->getOverlayHeight();
+
+	_backBuffer.free();
+	_backBuffer.create(width, height, _bytesPerPixel);
+
+	_screen.free();
+	_screen.create(width, height, _bytesPerPixel);
+
 	delete _vectorRenderer;
 	_vectorRenderer = Graphics::createRenderer(mode);
-	_vectorRenderer->setSurface(_screen);
+	_vectorRenderer->setSurface(&_screen);
 }
 
 bool ThemeEngine::isWidgetCached(DrawData type, const Common::Rect &r) {
@@ -293,7 +234,7 @@ bool ThemeEngine::isWidgetCached(DrawData type, const Common::Rect &r) {
 }
 
 void ThemeEngine::drawCached(DrawData type, const Common::Rect &r) {
-	assert(_widgets[type]->_surfaceCache->bytesPerPixel == _screen->bytesPerPixel);
+	assert(_widgets[type]->_surfaceCache->bytesPerPixel == _screen.bytesPerPixel);
 	_vectorRenderer->blitSurface(_widgets[type]->_surfaceCache, r);
 }
 
@@ -312,23 +253,10 @@ void ThemeEngine::calcBackgroundOffset(DrawData type) {
 }
 
 void ThemeEngine::restoreBackground(Common::Rect r) {
-	r.clip(_screen->w, _screen->h); // AHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHA... Oh god. :(
+	r.clip(_screen.w, _screen.h); // AHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHAHA... Oh god. :(
-	_vectorRenderer->blitSurface(_backBuffer, r);
+	_vectorRenderer->blitSurface(&_backBuffer, r);
 }
 
-bool ThemeEngine::isThemeLoadingRequired() {
-	int x = g_system->getOverlayWidth(), y = g_system->getOverlayHeight();
-
-	if (_loadedThemeX == x && _loadedThemeY == y)
-		return false;
-
-	_loadedThemeX = x;
-	_loadedThemeY = y;
-
-	return true;
-}
-
-
 
 
 /**********************************************************
@@ -434,21 +362,19 @@ bool ThemeEngine::addDrawData(const Common::String &data, bool cached) {
 /**********************************************************
  *	Theme XML loading
  *********************************************************/
-bool ThemeEngine::loadTheme(const Common::String &fileName) {
+void ThemeEngine::loadTheme(const Common::String &themeid) {
 	unloadTheme();
 
-	bool tryAgain = false;
+	if (themeid == "builtin") {
-	if (fileName != "builtin") {
+		_themeOk = loadDefaultXML();
+	} else {
 		// Load the archive containing image and XML data
-		if (!loadThemeXML(fileName)) {
+		_themeOk = loadThemeXML(themeid);
-			warning("Could not parse custom theme '%s'. Falling back to default theme", fileName.c_str());
-			tryAgain = true;	// Fall back to default builtin theme
-		}
 	}
-
+	
-	if (fileName == "builtin" || tryAgain) {
+	if (!_themeOk) {
-		if (!loadDefaultXML()) // if we can't load the embedded theme, this is a complete failure
+		warning("Failed to load theme '%s'", themeid.c_str());
-			error("Could not load default embedded theme");
+		return;
 	}
 
 	for (int i = 0; i < kDrawDataMAX; ++i) {
@@ -461,9 +387,37 @@ bool ThemeEngine::loadTheme(const Common::String &fileName) {
 			if (_widgets[i]->_cached) {}
 		}
 	}
+}
 
-	_themeOk = true;
+void ThemeEngine::unloadTheme() {
-	return true;
+	if (!_themeOk)
+		return;
+
+	for (int i = 0; i < kDrawDataMAX; ++i) {
+		delete _widgets[i];
+		_widgets[i] = 0;
+	}
+
+	for (int i = 0; i < kTextDataMAX; ++i) {
+		delete _texts[i];
+		_texts[i] = 0;
+	}
+
+	// Release all graphics surfaces
+	for (ImagesMap::iterator i = _bitmaps.begin(); i != _bitmaps.end(); ++i) {
+		Graphics::Surface *surf = i->_value;
+		if (surf) {
+			surf->free();
+			delete surf;
+		}
+	}
+	_bitmaps.clear();
+
+	delete _themeArchive;
+	_themeArchive = 0;
+
+	_themeEval->reset();
+	_themeOk = false;
 }
 
 bool ThemeEngine::loadDefaultXML() {
@@ -483,7 +437,7 @@ bool ThemeEngine::loadDefaultXML() {
 		return false;
 
 	_themeName = "ScummVM Classic Theme (Builtin Version)";
-	_themeFileName = "builtin";
+	_themeId = "builtin";
 
 	result = _parser->parse();
 	_parser->close();
@@ -535,7 +489,7 @@ bool ThemeEngine::loadThemeXML(const Common::String &themeName) {
 	Common::String stxHeader = themercFile.readLine();
 	if (!themeConfigParseHeader(stxHeader, _themeName) || _themeName.empty()) {
 		delete archive;
-		warning("Corrupted 'THEMERC' file in theme '%s'", _themeFileName.c_str());
+		warning("Corrupted 'THEMERC' file in theme '%s'", _themeId.c_str());
 		return false;
 	}
 
@@ -585,7 +539,7 @@ void ThemeEngine::queueDD(DrawData type, const Common::Rect &r, uint32 dynamic)
 		return;
 
 	Common::Rect area = r;
-	area.clip(_screen->w, _screen->h);
+	area.clip(_screen.w, _screen.h);
 
 	ThemeItemDrawData *q = new ThemeItemDrawData(this, _widgets[type], area, dynamic);
 
@@ -611,7 +565,7 @@ void ThemeEngine::queueDDText(TextData type, const Common::Rect &r, const Common
 		return;
 
 	Common::Rect area = r;
-	area.clip(_screen->w, _screen->h);
+	area.clip(_screen.w, _screen.h);
 
 	ThemeItemTextData *q = new ThemeItemTextData(this, _texts[type], area, text, alignH, alignV, ellipsis, restoreBg, deltax);
 
@@ -626,7 +580,7 @@ void ThemeEngine::queueDDText(TextData type, const Common::Rect &r, const Common
 void ThemeEngine::queueBitmap(const Graphics::Surface *bitmap, const Common::Rect &r, bool alpha) {
 
 	Common::Rect area = r;
-	area.clip(_screen->w, _screen->h);
+	area.clip(_screen.w, _screen.h);
 
 	ThemeItemBitmap *q = new ThemeItemBitmap(this, area, bitmap, alpha);
 
@@ -891,22 +845,22 @@ void ThemeEngine::drawChar(const Common::Rect &r, byte ch, const Graphics::Font
 		return;
 
 	Common::Rect charArea = r;
-	charArea.clip(_screen->w, _screen->h);
+	charArea.clip(_screen.w, _screen.h);
 
 	Graphics::PixelFormat format = _system->getOverlayFormat();
 	uint32 color = Graphics::RGBToColor(_texts[kTextDataDefault]->_color.r, _texts[kTextDataDefault]->_color.g, _texts[kTextDataDefault]->_color.b, format);
 
 	restoreBackground(charArea);
-	font->drawChar(_screen, ch, charArea.left, charArea.top, color);
+	font->drawChar(&_screen, ch, charArea.left, charArea.top, color);
 	addDirtyRect(charArea);
 }
 
 void ThemeEngine::debugWidgetPosition(const char *name, const Common::Rect &r) {
-	_font->drawString(_screen, name, r.left, r.top, r.width(), 0xFFFF, Graphics::kTextAlignRight, 0, true);
+	_font->drawString(&_screen, name, r.left, r.top, r.width(), 0xFFFF, Graphics::kTextAlignRight, 0, true);
-	_screen->hLine(r.left, r.top, r.right, 0xFFFF);
+	_screen.hLine(r.left, r.top, r.right, 0xFFFF);
-	_screen->hLine(r.left, r.bottom, r.right, 0xFFFF);
+	_screen.hLine(r.left, r.bottom, r.right, 0xFFFF);
-	_screen->vLine(r.left, r.top, r.bottom, 0xFFFF);
+	_screen.vLine(r.left, r.top, r.bottom, 0xFFFF);
-	_screen->vLine(r.right, r.top, r.bottom, 0xFFFF);
+	_screen.vLine(r.right, r.top, r.bottom, 0xFFFF);
 }
 
 
@@ -916,15 +870,15 @@ void ThemeEngine::debugWidgetPosition(const char *name, const Common::Rect &r) {
  *********************************************************/
 void ThemeEngine::updateScreen() {
 	if (!_bufferQueue.empty()) {
-		_vectorRenderer->setSurface(_backBuffer);
+		_vectorRenderer->setSurface(&_backBuffer);
 
 		for (Common::List<ThemeItem*>::iterator q = _bufferQueue.begin(); q != _bufferQueue.end(); ++q) {
 			(*q)->drawSelf(true, false);
 			delete *q;
 		}
 
-		_vectorRenderer->setSurface(_screen);
+		_vectorRenderer->setSurface(&_screen);
-		memcpy(_screen->getBasePtr(0,0), _backBuffer->getBasePtr(0,0), _screen->pitch * _screen->h);
+		memcpy(_screen.getBasePtr(0,0), _backBuffer.getBasePtr(0,0), _screen.pitch * _screen.h);
 		_bufferQueue.clear();
 	}
 
@@ -964,17 +918,11 @@ void ThemeEngine::openDialog(bool doBuffer, ShadingStyle style) {
 
 	if (style != kShadingNone) {
 		_vectorRenderer->applyScreenShading(style);
-		addDirtyRect(Common::Rect(0, 0, _screen->w, _screen->h));
+		addDirtyRect(Common::Rect(0, 0, _screen.w, _screen.h));
 	}
 
-	memcpy(_backBuffer->getBasePtr(0,0), _screen->getBasePtr(0,0), _screen->pitch * _screen->h);
+	memcpy(_backBuffer.getBasePtr(0,0), _screen.getBasePtr(0,0), _screen.pitch * _screen.h);
-	_vectorRenderer->setSurface(_screen);
+	_vectorRenderer->setSurface(&_screen);
-}
-
-void ThemeEngine::setUpCursor() {
-	CursorMan.pushCursorPalette(_cursorPal, 0, MAX_CURS_COLORS);
-	CursorMan.pushCursor(_cursor, _cursorWidth, _cursorHeight, _cursorHotspotX, _cursorHotspotY, 255, _cursorTargetScale);
-	CursorMan.showMouse(true);
 }
 
 bool ThemeEngine::createCursor(const Common::String &filename, int hotspotX, int hotspotY, int scale) {
@@ -1077,36 +1025,16 @@ ThemeEngine::TextData ThemeEngine::getTextData(DrawData ddId) {
  *********************************************************/
 
 const Graphics::Font *ThemeEngine::loadFontFromArchive(const Common::String &filename) {
-	Common::Archive *arch = 0;
+	Common::SeekableReadStream *stream = 0;
 	const Graphics::Font *font = 0;
 
-	Common::FSNode node(getThemeFileName());
+	if (_themeArchive)
-
+		stream = _themeArchive->openFile(filename);
-	if (node.getName().hasSuffix(".zip")) {
-#ifdef USE_ZLIB
-		Common::ZipArchive *zip = new Common::ZipArchive(node);
-		if (!zip || !zip->isOpen())
-			return 0;
-
-		arch = zip;
-#else
-		return 0;
-#endif
-	} else {
-		Common::FSDirectory *dir = new Common::FSDirectory(node);
-		if (!dir || !dir->getFSNode().isDirectory())
-			return 0;
-
-		arch = dir;
-	}
-
-	Common::SeekableReadStream *stream(arch->openFile(filename));
 	if (stream) {
 		font = Graphics::NewFont::loadFromCache(*stream);
 		delete stream;
 	}
 
-	delete arch;
 	return font;
 }
 

gui/ThemeEngine.h

@@ -57,9 +57,6 @@ protected:
 	friend class GUI::Dialog;
 	friend class GUI::GuiObject;
 
-	/** Sets whether backcaching is enabled */
-	static const bool kEnableBackCaching = true;
-
 	/**
 	 *	DrawData sets enumeration.
 	 *	Each DD set corresponds to the actual looks
@@ -244,7 +241,6 @@ public:
 	~ThemeEngine();
 
 	bool init();
-	void deinit();
 	void clearAll();
 
 	void refresh();
@@ -343,7 +339,7 @@ public:
 	 *	@param special Deprecated.
 	 */
 	bool addDirtyRect(Common::Rect r, bool backup = false, bool special = false) {
-		r.clip(_screen->w, _screen->h);
+		r.clip(_screen.w, _screen.h);
 		_dirtyScreen.push_back(r);
 		return true;
 	}
@@ -422,6 +418,7 @@ public:
 	 */
 	bool addTextData(const Common::String &drawDataId, const Common::String &textDataId, Graphics::TextAlign alignH, TextAlignVertical alignV);
 
+protected:
 	/**
 	 *	Returns if the Theme is ready to draw stuff on screen.
 	 *	Must be called instead of just checking _initOk, because
@@ -433,7 +430,7 @@ public:
 	}
 
 	/** Load the them from the file with the specified name. */
-	bool loadTheme(const Common::String &fileName);
+	void loadTheme(const Common::String &themeid);
 
 	/**
 	 *	Changes the active graphics mode of the GUI; may be used to either
@@ -441,7 +438,7 @@ public:
 	 */
 	void setGraphicsMode(GraphicsMode mode);
 
-
+public:
 	/**
 	 *	Finishes buffering: widgets from then on will be drawn straight on the screen
 	 *	without drawing queues.
@@ -497,7 +494,7 @@ public:
 	bool isWidgetCached(DrawData type, const Common::Rect &r);
 
 	const Common::String &getThemeName() const { return _themeName; }
-	const Common::String &getThemeFileName() const { return _themeFileName; }
+	const Common::String &getThemeId() const { return _themeId; }
 	int getGraphicsMode() const { return _graphicsMode; }
 
 protected:
@@ -513,12 +510,11 @@ protected:
 
 	/**
 	 *	Loads the given theme into the ThemeEngine.
-	 *	Note that ThemeName is an identifier, not a filename.
 	 *
-	 *	@param ThemeName Theme identifier.
+	 *	@param themeId Theme identifier.
 	 *	@returns true if the theme was successfully loaded.
 	 */
-	bool loadThemeXML(const Common::String &themeName);
+	bool loadThemeXML(const Common::String &themeId);
 
 	/**
 	 *	Loads the default theme file (the embedded XML file found
@@ -533,6 +529,10 @@ protected:
 	 */
 	void unloadTheme();
 
+	const Graphics::Font *loadFont(const Common::String &filename);
+	const Graphics::Font *loadFontFromArchive(const Common::String &filename);
+	Common::String genCacheFilename(const char *filename);
+
 	/**
 	 *	Actual Dirty Screen handling function.
 	 *	Handles all the dirty squares in the list, merges and optimizes
@@ -586,19 +586,9 @@ protected:
 public:
 
 	/**
-	 *	LEGACY: Old GUI::Theme API
+	 * @name LEGACY: Old GUI::Theme API
 	 */
-
+	//@{
-	bool needThemeReload() {
-		return ((_loadedThemeX != g_system->getOverlayWidth()) ||
-				(_loadedThemeY != g_system->getOverlayHeight()));
-	}
-
-	const Graphics::Font *loadFont(const Common::String &filename);
-	const Graphics::Font *loadFontFromArchive(const Common::String &filename);
-	Common::String genCacheFilename(const char *filename);
-
-	bool isThemeLoadingRequired();
 
 	static bool themeConfigUseable(const Common::FSNode &node, Common::String &themeName);
 	static bool themeConfigParseHeader(Common::String header, Common::String &themeName);
@@ -606,6 +596,8 @@ public:
 	int getTabSpacing() const { return 0; }
 	int getTabPadding() const { return 3; }
 
+	//@}
+
 protected:
 	OSystem *_system; /** Global system object. */
 
@@ -619,10 +611,10 @@ protected:
 	GUI::ThemeEval *_themeEval;
 
 	/** Main screen surface. This is blitted straight into the overlay. */
-	Graphics::Surface *_screen;
+	Graphics::Surface _screen;
 
 	/** Backbuffer surface. Stores previous states of the screen to blit back */
-	Graphics::Surface *_backBuffer;
+	Graphics::Surface _backBuffer;
 
 	/** Sets whether the current drawing is being buffered (stored for later
 		processing) or drawn directly to the screen. */
@@ -663,12 +655,9 @@ protected:
 	bool _enabled; //!< Whether the Theme is currently shown on the overlay
 
 	Common::String _themeName; //!< Name of the currently loaded theme
-	Common::String _themeFileName;
+	Common::String _themeId;
 	Common::Archive *_themeArchive;
 
-	/** Custom Cursor Management */
-	void setUpCursor();
-
 	bool _useCursor;
 	int _cursorHotspotX, _cursorHotspotY;
 	int _cursorTargetScale;
@@ -679,7 +668,6 @@ protected:
 	bool _needPaletteUpdates;
 	uint _cursorWidth, _cursorHeight;
 	byte _cursorPal[4*MAX_CURS_COLORS];
-	int _loadedThemeX, _loadedThemeY;
 };
 
 } // end of namespace GUI.

gui/dialog.cpp

@@ -59,10 +59,7 @@ Dialog::Dialog(const Common::String &name)
 	//
 	// Fixes bug #1590596: "HE: When 3x graphics are choosen, F5 crashes game"
 	// and bug #1595627: "SCUMM: F5 crashes game (640x480)"
-	if (g_gui.theme()->needThemeReload()) {
+	g_gui.checkScreenChange();
-		debug(2, "Theme forced to reload");
-		g_gui.screenChange();
-	}
 }
 
 int Dialog::runModal() {

gui/newgui.cpp

@@ -64,7 +64,14 @@ GuiManager::GuiManager() : _redrawStatus(kRedrawDisabled),
 	ConfMan.registerDefault("gui_renderer", ThemeEngine::findModeConfigName(ThemeEngine::_defaultRendererMode));
 	ThemeEngine::GraphicsMode gfxMode = (ThemeEngine::GraphicsMode)ThemeEngine::findMode(ConfMan.get("gui_renderer"));
 
-	loadNewTheme(themefile, gfxMode);
+	// Try to load the theme
+	if (!loadNewTheme(themefile, gfxMode)) {
+		// Loading the theme failed, try to load the built-in theme
+		if (!loadNewTheme("builtin", gfxMode)) {
+			// Loading the built-in theme failed as well. Bail out
+			error("Failed to load any GUI theme, aborting");
+		}
+	}
 	_themeChange = false;
 }
 
@@ -73,33 +80,40 @@ GuiManager::~GuiManager() {
 }
 
 bool GuiManager::loadNewTheme(Common::String filename, ThemeEngine::GraphicsMode gfx) {
-	if (_theme && filename == _theme->getThemeFileName() && gfx == _theme->getGraphicsMode())
+	// If we are asked to reload the currently active theme, just do nothing
+	// FIXME: Actually, why? It might be desirable at times to force a theme reload...
+	if (_theme && filename == _theme->getThemeId() && gfx == _theme->getGraphicsMode())
 		return true;
 
-	Common::String oldTheme = (_theme != 0) ? _theme->getThemeFileName() : "";
+	ThemeEngine *newTheme = 0;
 
 	if (gfx == ThemeEngine::kGfxDisabled)
 		gfx = ThemeEngine::_defaultRendererMode;
 
+	// Try to load the new theme
+	newTheme = new ThemeEngine(filename, gfx);
+	assert(newTheme);
+
+	if (!newTheme->init())
+		return false;
+
+	//
+	// Disable and delete the old theme
+	//
 	if (_theme)
 		_theme->disable();
+	delete _theme;
 
 	if (_useStdCursor) {
 		CursorMan.popCursorPalette();
 		CursorMan.popCursor();
 	}
 
-	delete _theme;
+	//
-	_theme = new ThemeEngine(filename, gfx);
+	// Enable the new theme
-
+	//
-	if (!_theme)
+	_theme = newTheme;
-		return (!oldTheme.empty() ? loadNewTheme(oldTheme) : false);
+	screenChange();
-
-	_theme->init();
-
-	if (!oldTheme.empty())
-		screenChange();
-
 	_themeChange = true;
 
 	return true;
@@ -191,8 +205,14 @@ void GuiManager::runLoop() {
 		}
 
 		Common::Event event;
-
 		while (eventMan->pollEvent(event)) {
+
+			// The top dialog can change during the event loop. In that case, flush all the
+			// dialog-related events since they were probably generated while the old dialog
+			// was still visible, and therefore note intended for the new one.
+			//
+			// This hopefully fixes strange behaviour/crashes with pop-up widgets. (Most easy
+			// to trigger in 3x mode or when running ScummVM under Valgrind.)
 			if (activeDialog != getTopDialog() && event.type != Common::EVENT_SCREEN_CHANGED)
 				continue;
 
@@ -307,19 +327,8 @@ void GuiManager::openDialog(Dialog *dialog) {
 	// We reflow the dialog just before opening it. If the screen changed
 	// since the last time we looked, also refresh the loaded theme,
 	// and reflow all other open dialogs, too.
-	int tmpScreenChangeID = _system->getScreenChangeID();
+	if (!checkScreenChange())
-	if (_lastScreenChangeID != tmpScreenChangeID) {
-		_lastScreenChangeID = tmpScreenChangeID;
-
-		// reinit the whole theme
-		_theme->refresh();
-		// refresh all dialogs
-		for (int i = 0; i < _dialogStack.size(); ++i) {
-			_dialogStack[i]->reflowLayout();
-		}
-	} else {
 		dialog->reflowLayout();
-	}
 }
 
 void GuiManager::closeTopDialog() {
@@ -371,6 +380,15 @@ void GuiManager::clearDragWidget() {
 		_dialogStack.top()->_dragWidget = 0;
 }
 
+bool GuiManager::checkScreenChange() {
+	int tmpScreenChangeID = _system->getScreenChangeID();
+	if (_lastScreenChangeID != tmpScreenChangeID) {
+		GuiManager::screenChange();
+		return true;
+	}
+	return false;
+}
+
 void GuiManager::screenChange() {
 	_lastScreenChangeID = _system->getScreenChangeID();
 

gui/newgui.h

@@ -82,10 +82,20 @@ public:
 	int getStringWidth(const Common::String &str, ThemeEngine::FontStyle style = ThemeEngine::kFontStyleBold) const { return _theme->getStringWidth(str, style); }
 	int getCharWidth(byte c, ThemeEngine::FontStyle style = ThemeEngine::kFontStyleBold) const { return _theme->getCharWidth(c, style); }
 
+	// FIXME: clearDragWidget is apparently there for the sake of PopUpWidget::handleMouseDown.
+	// This seems to be an ugly hack. At the very least, it should be thoroughly documented.
+	// Better would be to replace it with a proper solution.
 	void clearDragWidget();
 
-	void screenChange();
+	/**
+	 * Tell the GuiManager to check whether the screen resolution has changed.
+	 * If that is the case, the GuiManager will reload/refresh the active theme.
+	 *
+	 * @return true if the a screen change indeed occurred, false otherwise
+	 */
+	bool checkScreenChange();
 
+protected:
 	enum RedrawStatus {
 		kRedrawDisabled = 0,
 		kRedrawOpenDialog,
@@ -94,7 +104,8 @@ public:
 		kRedrawFull
 	};
 
-protected:
+
+
 	OSystem			*_system;
 
 	ThemeEngine		*_theme;
@@ -136,6 +147,8 @@ protected:
 	void animateCursor();
 
 	Dialog *getTopDialog() const;
+
+	void screenChange();
 };
 
 } // End of namespace GUI

gui/options.cpp

@@ -849,7 +849,7 @@ void GlobalOptionsDialog::close() {
 		if (!ConfMan.get("gui_renderer").equalsIgnoreCase(cfg)) {
 			// FIXME: Actually, any changes (including the theme change) should
 			// only become active *after* the options dialog has closed.
-			g_gui.loadNewTheme(g_gui.theme()->getThemeFileName(), selected);
+			g_gui.loadNewTheme(g_gui.theme()->getThemeId(), selected);
 			ConfMan.set("gui_renderer", cfg, _domain);
 		}
 	}