Permalink
Browse files

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.

(cherry picked from commit 20b2c1b)
  • Loading branch information...
csnover committed Dec 3, 2017
1 parent bed6644 commit 58cbd45ef5b6776b761fc3d51f531a8d12946c24
Showing with 25 additions and 9 deletions.
  1. +25 −9 backends/graphics/surfacesdl/surfacesdl-graphics.cpp
@@ -1834,12 +1834,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();
}
@@ -1854,33 +1862,41 @@ 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);
const Uint32 bMask = ((0xFF >> format->bLoss) << format->bShift);
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,
@@ -2069,7 +2085,7 @@ void SurfaceSdlGraphicsManager::blitCursor() {
dstPtr += _mouseOrigSurface->pitch - w * 2;
}
if (sizeChanged) {
if (sizeChanged || !_mouseSurface) {
if (_mouseSurface)
SDL_FreeSurface(_mouseSurface);

0 comments on commit 58cbd45

Please sign in to comment.