moved screen mutex from smush into SDL backend (other backends have to make sure they are thread safe by themselves)

svn-id: r7230

backends/sdl/sdl-common.cpp

@@ -104,6 +104,8 @@ OSystem_SDL_Common::OSystem_SDL_Common()
 
 	// reset mouse state
 	memset(&km, 0, sizeof(km));
+	
+	_mutex = SDL_CreateMutex();
 }
 
 OSystem_SDL_Common::~OSystem_SDL_Common() {
@@ -111,6 +113,7 @@ OSystem_SDL_Common::~OSystem_SDL_Common() {
 		free(_dirty_checksums);
 	free(_currentPalette);
 	free(_mouseBackup);
+	SDL_DestroyMutex(_mutex);
 }
 
 void OSystem_SDL_Common::init_size(uint w, uint h) {
@@ -140,6 +143,8 @@ void OSystem_SDL_Common::copy_rect(const byte *buf, int pitch, int x, int y, int
 	if (_screen == NULL)
 		return;
 
+	StackLock lock(_mutex);	// Lock the mutex until this function ends
+	
 	if (((uint32)buf & 3) == 0 && pitch == _screenWidth && x==0 && y==0 &&
 			w==_screenWidth && h==_screenHeight && _mode_flags&DF_WANT_RECT_OPTIM) {
 		/* Special, optimized case for full screen updates.
@@ -208,8 +213,10 @@ void OSystem_SDL_Common::move_screen(int dx, int dy, int height) {
 	if (_mouseDrawn)
 		undraw_mouse();
 
-	// FIXME - calling copy rect repeatedly is horribly inefficient, as it (un)locks the surface repeatedly
+	// FIXME - calling copy_rect repeatedly is horribly inefficient, as it (un)locks the surface repeatedly
 	// and it performs unneeded clipping checks etc.
+	// Furthermore, this code is not correct, techincally: the pixels members of an SDLSource may be 0
+	// while it is not locked (e.g. for HW surfaces which are stored in the graphic card's VRAM).
 	
 	// vertical movement
 	if (dy > 0) {
@@ -218,7 +225,7 @@ void OSystem_SDL_Common::move_screen(int dx, int dy, int height) {
 			copy_rect((byte *)_screen->pixels + _screenWidth * (y - dy), _screenWidth, 0, y, _screenWidth, 1);
 	} else if (dy < 0) {
 		// move up - copy from top to bottom
-		for (y = 0; y < height + dx; y++)
+		for (y = dy; y < height; y++)
 			copy_rect((byte *)_screen->pixels + _screenWidth * (y - dy), _screenWidth, 0, y, _screenWidth, 1);
 	}
 
@@ -229,7 +236,7 @@ void OSystem_SDL_Common::move_screen(int dx, int dy, int height) {
 			copy_rect((byte *)_screen->pixels + x - dx, _screenWidth, x, 0, 1, height);
 	} else if (dx < 0)  {
 		// move left - copy from left to right
-		for (x = 0; x < _screenWidth; x++)
+		for (x = dx; x < _screenWidth; x++)
 			copy_rect((byte *)_screen->pixels + x - dx, _screenWidth, x, 0, 1, height);
 	}
 }
@@ -1150,6 +1157,8 @@ void OSystem_SDL_Common::clear_overlay() {
 	if (!_overlayVisible)
 		return;
 	
+	StackLock lock(_mutex);	// Lock the mutex until this function ends
+	
 	// hide the mouse
 	undraw_mouse();
 

backends/sdl/sdl-common.h

@@ -201,6 +201,10 @@ protected:
 	SDL_Color *_currentPalette;
 	uint _paletteDirtyStart, _paletteDirtyEnd;
 	
+	// Mutex that prevents multiple threads interferring with each other
+	// when accessing the screen.
+	SDL_mutex *_mutex;
+
 
 	void add_dirty_rgn_auto(const byte *buf);
 	void mk_checksums(const byte *buf);
@@ -222,5 +226,15 @@ protected:
 	static OSystem_SDL_Common *create();
 };
 
+// Auxillary class to (un)lock a mutex on the stack
+class StackLock {
+	SDL_mutex *_mutex;
+public:
+	StackLock(SDL_mutex *mutex) : _mutex(mutex) { lock(); }
+	~StackLock() { unlock(); }
+	void lock() { SDL_mutexP(_mutex); }
+	void unlock() { SDL_mutexV(_mutex); }
+};
+
 
 #endif

backends/sdl/sdl.cpp

@@ -230,6 +230,8 @@ void OSystem_SDL::hotswap_gfx_mode() {
 void OSystem_SDL::update_screen() {
 	assert(_hwscreen != NULL);
 
+	StackLock lock(_mutex);	// Lock the mutex until this function ends
+
 	// If the shake position changed, fill the dirty area with blackness
 	if (_currentShakePos != _newShakePos) {
 		SDL_Rect blackrect = {0, 0, _screenWidth * _scaleFactor, _newShakePos * _scaleFactor};
@@ -277,12 +279,12 @@ void OSystem_SDL::update_screen() {
 		if (!_overlayVisible) {
 			for(r = _dirty_rect_list; r != last_rect; ++r) {
 				dst = *r;
+				dst.x++;	// Shift rect by one since 2xSai needs to acces the data around
+				dst.y++;	// any pixel to scale it, and we want to avoid mem access crashes.
 				if (_scaler_proc == Normal1x) {
 					if (SDL_BlitSurface(_screen, r, _hwscreen, &dst) != 0)
 						error("SDL_BlitSurface failed: %s", SDL_GetError());
 				} else {
-					dst.x++;	// Shift rect by one since 2xSai needs to acces the data around
-					dst.y++;	// any pixel to scale it, and we want to avoid mem access crashes.
 					if (SDL_BlitSurface(_screen, r, _tmpscreen, &dst) != 0)
 						error("SDL_BlitSurface failed: %s", SDL_GetError());
 				}

scumm/smush/smush_player.cpp

@@ -222,14 +222,10 @@ SmushPlayer::SmushPlayer(Scumm *scumm, int speed, bool subtitles) {
 	_speed = speed;
 	_subtitles = subtitles;
 	_smushProcessFrame = false;
-	
-	_mutex = _scumm->_system->create_mutex();
 }
 
 SmushPlayer::~SmushPlayer() {
 	deinit();
-	if (_mutex)
-		_scumm->_system->delete_mutex (_mutex);
 }
 
 void SmushPlayer::init() {
@@ -862,15 +858,11 @@ void SmushPlayer::setPalette(byte *palette) {
 }
 
 void SmushPlayer::updateScreen() {
-	_scumm->_system->lock_mutex(_mutex);
-
 	uint32 end_time, start_time = _scumm->_system->get_msecs();
 	_scumm->_system->copy_rect(_data, _width, 0, 0, _width, _height);
 	_updateNeeded = true;
 	end_time = _scumm->_system->get_msecs();
 	debug(4, "Smush stats: updateScreen( %03d )", end_time - start_time);
-
-	_scumm->_system->unlock_mutex(_mutex);
 }
 
 void SmushPlayer::play(const char *filename, const char *directory) {
@@ -887,7 +879,6 @@ void SmushPlayer::play(const char *filename, const char *directory) {
 	init();
 
 	while (true) {
-		_scumm->_system->lock_mutex(_mutex);
 		_scumm->parseEvents();
 		_scumm->processKbd();
 		if(_updateNeeded == true) {
@@ -899,7 +890,6 @@ void SmushPlayer::play(const char *filename, const char *directory) {
 			debug(4, "Smush stats: BackendUpdateScreen( %03d )", end_time - start_time);
 
 		}
-		_scumm->_system->unlock_mutex(_mutex);
 		if (_scumm->_videoFinished == true)
 			break;
 		if (_scumm->_saveLoadFlag)

scumm/smush/smush_player.h

@@ -63,8 +63,6 @@ private:
 	int _speed;
 	bool _outputSound;
 
-	void *_mutex;
-
 public:
 
 	int _width, _height;