SDL: Fix memory corruption when switching to/from 32-bit cursors

When a 32-bit cursor has the same size as an 8- or 16-bit cursor,
the mouse surfaces were not being regenerated even though the
32-bit cursors have a different memory requirement. This lead to
memory corruption as an inappropriate surface would be used for
the other type of cursor.

The shoe-horned 32-bit cursor support is clearly showing its
scrappy nature here and probably ought to be revisited in the
future if the SurfaceSdl graphics manager sticks around.

Fixes Trac#10349, Trac#10350, Trac#10351.

1 files changed, 25 insertions, 9 deletions

diff --git a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
index 6e8561321b..3d4e811462 100644
--- a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
+++ b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
@@ -1836,12 +1836,20 @@ void SurfaceSdlGraphicsManager::copyRectToOverlay(const void *buf, int pitch, in
 #pragma mark -
 
 void SurfaceSdlGraphicsManager::setMouseCursor(const void *buf, uint w, uint h, int hotspotX, int hotspotY, uint32 keyColor, bool dontScale, const Graphics::PixelFormat *format) {
+	bool formatChanged = false;
+
 	if (format) {
 #ifndef USE_RGB_COLOR
 		assert(format->bytesPerPixel == 1);
 #endif
+		if (format->bytesPerPixel != _cursorFormat.bytesPerPixel) {
+			formatChanged = true;
+		}
 		_cursorFormat = *format;
 	} else {
+		if (_cursorFormat.bytesPerPixel != 1) {
+			formatChanged = true;
+		}
 		_cursorFormat = Graphics::PixelFormat::createFormatCLUT8();
 	}
 
@@ -1856,26 +1864,32 @@ void SurfaceSdlGraphicsManager::setMouseCursor(const void *buf, uint w, uint h,
 
 	_cursorDontScale = dontScale;
 
-	if (_mouseCurState.w != (int)w || _mouseCurState.h != (int)h) {
+	if (_mouseCurState.w != (int)w || _mouseCurState.h != (int)h || formatChanged) {
 		_mouseCurState.w = w;
 		_mouseCurState.h = h;
 
-		if (!w || !h) {
-			return;
-		}
-
 		if (_mouseOrigSurface) {
 			SDL_FreeSurface(_mouseOrigSurface);
 
 			if (_mouseSurface == _mouseOrigSurface) {
 				_mouseSurface = nullptr;
 			}
+
+			_mouseOrigSurface = nullptr;
+		}
+
+		if ((formatChanged || _cursorFormat.bytesPerPixel == 4) && _mouseSurface) {
+			SDL_FreeSurface(_mouseSurface);
+			_mouseSurface = nullptr;
+		}
+
+		if (!w || !h) {
+			return;
 		}
 
 		if (_cursorFormat.bytesPerPixel == 4) {
-			if (_mouseSurface != _mouseOrigSurface) {
-				SDL_FreeSurface(_mouseSurface);
-			}
+			assert(!_mouseSurface);
+			assert(!_mouseOrigSurface);
 
 			const Uint32 rMask = ((0xFF >> format->rLoss) << format->rShift);
 			const Uint32 gMask = ((0xFF >> format->gLoss) << format->gShift);
@@ -1883,6 +1897,8 @@ void SurfaceSdlGraphicsManager::setMouseCursor(const void *buf, uint w, uint h,
 			const Uint32 aMask = ((0xFF >> format->aLoss) << format->aShift);
 			_mouseSurface = _mouseOrigSurface = SDL_CreateRGBSurfaceFrom(const_cast<void *>(buf), w, h, format->bytesPerPixel * 8, w * format->bytesPerPixel, rMask, gMask, bMask, aMask);
 		} else {
+			assert(!_mouseOrigSurface);
+
 			// Allocate bigger surface because AdvMame2x adds black pixel at [0,0]
 			_mouseOrigSurface = SDL_CreateRGBSurface(SDL_SWSURFACE | SDL_RLEACCEL | SDL_SRCCOLORKEY | SDL_SRCALPHA,
 							_mouseCurState.w + 2,
@@ -2071,7 +2087,7 @@ void SurfaceSdlGraphicsManager::blitCursor() {
 		dstPtr += _mouseOrigSurface->pitch - w * 2;
 	}
 
-	if (sizeChanged) {
+	if (sizeChanged || !_mouseSurface) {
 		if (_mouseSurface)
 			SDL_FreeSurface(_mouseSurface);