From cea06991ebab97c72e9fd89a4bf63835bcf0de80 Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Wed, 10 Aug 2011 18:15:00 +0200 Subject: TIMER: Remove name of callback in removeTimerProc. This should fix #3389673 "LOOM: CD-Version crashes at start". It also fixes the same error showing up for me in Monkey CD. The doc changes in 4c7958450f628937270f claims the name is used for the event recorder, but as far as I can tell it is not used right now. Thus depending on how it is used the behavior of SCUMM removing and adding the same timer aagain *might* cause problems. In any way we need to remove the name in removeTimerProc, else RTL + launching the same game again would be broken too. --- backends/timer/default/default-timer.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'backends') diff --git a/backends/timer/default/default-timer.cpp b/backends/timer/default/default-timer.cpp index efb3ec9ad9..28524bf3b5 100644 --- a/backends/timer/default/default-timer.cpp +++ b/backends/timer/default/default-timer.cpp @@ -161,4 +161,11 @@ void DefaultTimerManager::removeTimerProc(TimerProc callback) { slot = slot->next; } } + + for (TimerSlotMap::iterator i = _callbacks.begin(), end = _callbacks.end(); i != end; ++i) { + if (i->_value == callback) { + _callbacks.erase(i); + break; + } + } } -- cgit v1.2.3 From c443f113ed5729b88792d3b54b77d70033cb24dc Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Wed, 10 Aug 2011 18:34:08 +0200 Subject: TIMER: Remove all names associated with a callback, since all callbacks are removed. This makes the name removal consistent with the timer proc removal. It seems we only allow a specific timer proc being added once anymore though. So this should not change too much right now. --- backends/timer/default/default-timer.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'backends') diff --git a/backends/timer/default/default-timer.cpp b/backends/timer/default/default-timer.cpp index 28524bf3b5..5b133660e0 100644 --- a/backends/timer/default/default-timer.cpp +++ b/backends/timer/default/default-timer.cpp @@ -163,9 +163,7 @@ void DefaultTimerManager::removeTimerProc(TimerProc callback) { } for (TimerSlotMap::iterator i = _callbacks.begin(), end = _callbacks.end(); i != end; ++i) { - if (i->_value == callback) { + if (i->_value == callback) _callbacks.erase(i); - break; - } } } -- cgit v1.2.3 From 930f626daba5de8ec0b42fd32ba7ddf107e7d95a Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Wed, 10 Aug 2011 18:42:19 +0200 Subject: TIMER: Add a comment to explain why we remove the name in removeTimerProc. --- backends/timer/default/default-timer.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'backends') diff --git a/backends/timer/default/default-timer.cpp b/backends/timer/default/default-timer.cpp index 5b133660e0..e1aadb62b8 100644 --- a/backends/timer/default/default-timer.cpp +++ b/backends/timer/default/default-timer.cpp @@ -162,6 +162,18 @@ void DefaultTimerManager::removeTimerProc(TimerProc callback) { } } + // We need to remove all names referencing the timer proc here. + // + // Else we run into troubles, when the client code removes and readds timer + // callbacks. + // + // Another issues occurs when one plays a game with ALSA as music driver, + // does RTL and starts a different engine game with ALSA as music driver. + // In this case the MPU401 code will add different timer procs with the + // same name, resulting in two different callbacks added with the same + // name and causing installTimerProc to error out. + // A good test case is running a SCUMM with ALSA output and then a KYRA + // game for example. for (TimerSlotMap::iterator i = _callbacks.begin(), end = _callbacks.end(); i != end; ++i) { if (i->_value == callback) _callbacks.erase(i); -- cgit v1.2.3 From 0f6e231356305043bc7f66ae1cbe2c3a0607c68a Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Thu, 11 Aug 2011 04:25:46 +0200 Subject: SDL: Implement a hellish workaround to fix bug #3368143. The bug in question is "SDL/OpenGL: Crash when switching renderer backend". To fix it I added a stupid graphics state copying to the SDL backend, in case the graphics manager is switched. The implementation of this is considered a pure workaround, no one should ever do it like this in reality... I just want to die when looking at this... Not sure why I actually committed it. Anyway it at least makes the OpenGL backend testable for those who do not want to fiddle with the config file directly. --- .../graphics/surfacesdl/surfacesdl-graphics.cpp | 5 +- backends/platform/sdl/sdl.cpp | 58 +++++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-) (limited to 'backends') diff --git a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp index 81c439e7e2..146200148a 100644 --- a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp +++ b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp @@ -249,7 +249,10 @@ void SurfaceSdlGraphicsManager::setFeatureState(OSystem::Feature f, bool enable) } bool SurfaceSdlGraphicsManager::getFeatureState(OSystem::Feature f) { - assert(_transactionMode == kTransactionNone); + // We need to allow this to be called from within a transaction, since we + // currently use it to retreive the graphics state, when switching from + // SDL->OpenGL mode for example. + //assert(_transactionMode == kTransactionNone); switch (f) { case OSystem::kFeatureFullscreenMode: diff --git a/backends/platform/sdl/sdl.cpp b/backends/platform/sdl/sdl.cpp index 63871f5034..ba57841256 100644 --- a/backends/platform/sdl/sdl.cpp +++ b/backends/platform/sdl/sdl.cpp @@ -49,6 +49,7 @@ #include "backends/graphics/surfacesdl/surfacesdl-graphics.h" #ifdef USE_OPENGL #include "backends/graphics/openglsdl/openglsdl-graphics.h" +#include "graphics/cursorman.cpp" #endif #include "icons/scummvm.xpm" @@ -524,6 +525,22 @@ bool OSystem_SDL::setGraphicsMode(int mode) { i = _sdlModesCount; } + // Very hacky way to set up the old graphics manager state, in case we + // switch from SDL->OpenGL or OpenGL->SDL. + // + // This is a probably temporary workaround to fix bugs like #3368143 + // "SDL/OpenGL: Crash when switching renderer backend". + const int screenWidth = _graphicsManager->getWidth(); + const int screenHeight = _graphicsManager->getHeight(); + const bool arState = _graphicsManager->getFeatureState(kFeatureAspectRatioCorrection); + const bool fullscreen = _graphicsManager->getFeatureState(kFeatureFullscreenMode); + const bool cursorPalette = _graphicsManager->getFeatureState(kFeatureCursorPalette); +#ifdef USE_RGB_COLOR + const Graphics::PixelFormat pixelFormat = _graphicsManager->getScreenFormat(); +#endif + + bool switchedManager = false; + // Loop through modes while (srcMode->name) { if (i == mode) { @@ -535,16 +552,55 @@ bool OSystem_SDL::setGraphicsMode(int mode) { _graphicsManager = new SurfaceSdlGraphicsManager(_eventSource); ((SurfaceSdlGraphicsManager *)_graphicsManager)->initEventObserver(); _graphicsManager->beginGFXTransaction(); + + switchedManager = true; } else if (_graphicsMode < _sdlModesCount && mode >= _sdlModesCount) { debug(1, "switching to OpenGL graphics"); delete _graphicsManager; _graphicsManager = new OpenGLSdlGraphicsManager(_eventSource); ((OpenGLSdlGraphicsManager *)_graphicsManager)->initEventObserver(); _graphicsManager->beginGFXTransaction(); + + switchedManager = true; } _graphicsMode = mode; - return _graphicsManager->setGraphicsMode(srcMode->id); + + if (switchedManager) { +#ifdef USE_RGB_COLOR + _graphicsManager->initSize(screenWidth, screenHeight, &pixelFormat); +#else + _graphicsManager->initSize(screenWidth, screenHeight, 0); +#endif + _graphicsManager->setFeatureState(kFeatureAspectRatioCorrection, arState); + _graphicsManager->setFeatureState(kFeatureFullscreenMode, fullscreen); + _graphicsManager->setFeatureState(kFeatureCursorPalette, cursorPalette); + + // Worst part about this right now, tell the cursor manager to + // resetup the cursor + cursor palette if necessarily + + // First we need to try to setup the old state on the new manager... + if (_graphicsManager->endGFXTransaction() != kTransactionSuccess) { + // Oh my god if this failed the client code might just explode. + return false; + } + + // Next setup the cursor again + CursorMan.pushCursor(0, 0, 0, 0, 0, 0); + CursorMan.popCursor(); + + // Next setup cursor palette if needed + if (cursorPalette) { + CursorMan.pushCursorPalette(0, 0, 0); + CursorMan.popCursorPalette(); + } + + _graphicsManager->beginGFXTransaction(); + // Oh my god if this failed the client code might just explode. + return _graphicsManager->setGraphicsMode(srcMode->id); + } else { + return _graphicsManager->setGraphicsMode(srcMode->id); + } } i++; -- cgit v1.2.3 From 7ef6c73d61588482f2686c43047e0a4522c5baf8 Mon Sep 17 00:00:00 2001 From: Paul Gilbert Date: Thu, 11 Aug 2011 22:30:46 +1000 Subject: SDL: Previous commit broke compilation on MSVC Including cursorman.cpp rather than cursorman.h resulted in the CursorManager class being present in multiple .obj files, resulting in linking errors. --- backends/platform/sdl/sdl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'backends') diff --git a/backends/platform/sdl/sdl.cpp b/backends/platform/sdl/sdl.cpp index ba57841256..8dff5cec05 100644 --- a/backends/platform/sdl/sdl.cpp +++ b/backends/platform/sdl/sdl.cpp @@ -49,7 +49,7 @@ #include "backends/graphics/surfacesdl/surfacesdl-graphics.h" #ifdef USE_OPENGL #include "backends/graphics/openglsdl/openglsdl-graphics.h" -#include "graphics/cursorman.cpp" +#include "graphics/cursorman.h" #endif #include "icons/scummvm.xpm" -- cgit v1.2.3 From a77c29327e8e4c06c3b45dac16b96198a120fefe Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Fri, 12 Aug 2011 03:46:32 +0200 Subject: OPENGLSDL: Do not change requested window size on resize. This should help fix a lock up on window managers, which will try to force the ScummVM window to a certain size, by just requesting the same size over and over again. Now we get black borders even in windowed mode when the aspect of the window does not match the aspect of the game screen (and we are not in "normal" mode), but that is usually the same in video players too, so shouldn't be too bad. --- backends/graphics/openglsdl/openglsdl-graphics.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'backends') diff --git a/backends/graphics/openglsdl/openglsdl-graphics.cpp b/backends/graphics/openglsdl/openglsdl-graphics.cpp index 8ea95768df..d2810818e7 100644 --- a/backends/graphics/openglsdl/openglsdl-graphics.cpp +++ b/backends/graphics/openglsdl/openglsdl-graphics.cpp @@ -640,10 +640,7 @@ void OpenGLSdlGraphicsManager::notifyResize(const uint width, const uint height) _videoMode.hardwareWidth = width; _videoMode.hardwareHeight = height; - if (_videoMode.mode != OpenGL::GFX_ORIGINAL) { - _screenResized = true; - calculateDisplaySize(_videoMode.hardwareWidth, _videoMode.hardwareHeight); - } + _screenResized = true; int scale = MIN(_videoMode.hardwareWidth / _videoMode.screenWidth, _videoMode.hardwareHeight / _videoMode.screenHeight); @@ -653,10 +650,6 @@ void OpenGLSdlGraphicsManager::notifyResize(const uint width, const uint height) setScale(MAX(MIN(scale, 3), 1)); } - if (_videoMode.mode == OpenGL::GFX_ORIGINAL) { - calculateDisplaySize(_videoMode.hardwareWidth, _videoMode.hardwareHeight); - } - _transactionDetails.sizeChanged = true; endGFXTransaction(); #ifdef USE_OSD -- cgit v1.2.3 From b8dcd9a25eb27ef40aa5535fc83879d20db7e10c Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Fri, 12 Aug 2011 04:06:54 +0200 Subject: OPENGL: Fix aspect ratio correction behavior. Now only 320x200 and 640x400 will result in aspect ratio correction to be used if the user requested it. This should fix some strechting in Myst/Riven. --- backends/graphics/opengl/opengl-graphics.cpp | 8 ++++++-- backends/graphics/opengl/opengl-graphics.h | 5 +---- backends/graphics/openglsdl/openglsdl-graphics.cpp | 19 +++++++++++-------- 3 files changed, 18 insertions(+), 14 deletions(-) (limited to 'backends') diff --git a/backends/graphics/opengl/opengl-graphics.cpp b/backends/graphics/opengl/opengl-graphics.cpp index 57c2378649..40ef17e477 100644 --- a/backends/graphics/opengl/opengl-graphics.cpp +++ b/backends/graphics/opengl/opengl-graphics.cpp @@ -1245,12 +1245,16 @@ void OpenGLGraphicsManager::toggleAntialiasing() { _transactionDetails.filterChanged = true; } -uint OpenGLGraphicsManager::getAspectRatio() { +uint OpenGLGraphicsManager::getAspectRatio() const { // In case we enable aspect ratio correction we force a 4/3 ratio. + // But just for 320x200 and 640x400 games, since other games do not need + // this. // TODO: This makes OpenGL Normal behave like OpenGL Conserve, when aspect // ratio correction is enabled, but it's better than the previous 4/3 mode // mess at least... - if (_videoMode.aspectRatioCorrection) + if (_videoMode.aspectRatioCorrection + && ((_videoMode.screenWidth == 320 && _videoMode.screenHeight == 200) + || (_videoMode.screenWidth == 640 && _videoMode.screenHeight == 400))) return 13333; else if (_videoMode.mode == OpenGL::GFX_NORMAL) return _videoMode.hardwareWidth * 10000 / _videoMode.hardwareHeight; diff --git a/backends/graphics/opengl/opengl-graphics.h b/backends/graphics/opengl/opengl-graphics.h index 8a110b2d5f..42cfbacc85 100644 --- a/backends/graphics/opengl/opengl-graphics.h +++ b/backends/graphics/opengl/opengl-graphics.h @@ -214,10 +214,7 @@ protected: virtual void calculateDisplaySize(int &width, int &height); virtual void refreshDisplaySize(); - /** - * Returns the current target aspect ratio x 10000 - */ - virtual uint getAspectRatio(); + uint getAspectRatio() const; bool _formatBGR; diff --git a/backends/graphics/openglsdl/openglsdl-graphics.cpp b/backends/graphics/openglsdl/openglsdl-graphics.cpp index d2810818e7..84515732fe 100644 --- a/backends/graphics/openglsdl/openglsdl-graphics.cpp +++ b/backends/graphics/openglsdl/openglsdl-graphics.cpp @@ -313,14 +313,17 @@ bool OpenGLSdlGraphicsManager::loadGFXMode() { _videoMode.overlayWidth = _videoMode.hardwareWidth = _videoMode.screenWidth * scaleFactor; _videoMode.overlayHeight = _videoMode.hardwareHeight = _videoMode.screenHeight * scaleFactor; - int screenAspectRatio = _videoMode.screenWidth * 10000 / _videoMode.screenHeight; - int desiredAspectRatio = getAspectRatio(); - - // Do not downscale dimensions, only enlarge them if needed - if (screenAspectRatio > desiredAspectRatio) - _videoMode.hardwareHeight = (_videoMode.overlayWidth * 10000 + 5000) / desiredAspectRatio; - else if (screenAspectRatio < desiredAspectRatio) - _videoMode.hardwareWidth = (_videoMode.overlayHeight * desiredAspectRatio + 5000) / 10000; + // The only modes where we need to adapt the aspect ratio are 320x200 + // and 640x400. That is since our aspect ratio correction in fact is + // only used to ensure that the original pixel size aspect for these + // modes is used. + // (Non-square pixels on old monitors vs square pixel on new ones). + if (_videoMode.aspectRatioCorrection + && ((_videoMode.screenWidth == 320 && _videoMode.screenHeight == 200) + || (_videoMode.screenWidth == 640 && _videoMode.screenHeight == 400))) + _videoMode.overlayHeight = _videoMode.hardwareHeight = 240 * scaleFactor; + else + _videoMode.overlayHeight = _videoMode.hardwareHeight = _videoMode.screenHeight * scaleFactor; } _screenResized = false; -- cgit v1.2.3