Allow changing fullscreen resolution without restart

Unified Graphics:setFullscreen and Graphics:resize into a single
Graphics:changeVideoMode function that tries to restore the existing mode when
changing to the new mode didn't work, and exists with an error when that also
fails.

Split up handling of SDL_VIDEORESIZE and the adapting to new resolution in the
Client class, so that the second part could also be called when changing
resolution fullscreen mode.

The Video tab in the Setup window now also filters out any modes smaller than
640x480 since the game won't properly adapt to that resolution anyway.

Reviewed-by: Yohann Ferreira

6 files changed, 145 insertions, 123 deletions

diff --git a/src/client.cpp b/src/client.cpp
index 2dcb3ba3..5a9cc726 100644
--- a/src/client.cpp
+++ b/src/client.cpp
@@ -515,7 +515,7 @@ int Client::exec()
                         break;
 
                     case SDL_VIDEORESIZE:
-                        resizeVideo(event.resize.w, event.resize.h);
+                        handleVideoResize(event.resize.w, event.resize.h);
                         break;
                 }
 
@@ -1391,7 +1391,7 @@ void Client::accountLogin(LoginData *loginData)
     config.setValue("remember", loginData->remember);
 }
 
-void Client::resizeVideo(int width, int height)
+void Client::handleVideoResize(int width, int height)
 {
     // Keep a minimum size. This isn't adhered to by the actual window, but
     // it keeps some window positions from getting messed up.
@@ -1401,20 +1401,13 @@ void Client::resizeVideo(int width, int height)
     if (graphics->getWidth() == width && graphics->getHeight() == height)
         return;
 
-    if (graphics->resize(width, height))
+    if (graphics->changeVideoMode(width,
+                                  height,
+                                  graphics->getBpp(),
+                                  false,
+                                  graphics->getHWAccel()))
     {
-        gui->videoResized();
-
-        if (mDesktop)
-            mDesktop->setSize(width, height);
-
-        if (mSetupButton)
-            mSetupButton->setPosition(width - mSetupButton->getWidth() - 3, 3);
-
-        if (mGame)
-            mGame->videoResized(width, height);
-
-        gui->draw();
+        videoResized(width, height);
 
         // Since everything appears to have worked out, remember to store the
         // new size in the configuration.
@@ -1422,3 +1415,17 @@ void Client::resizeVideo(int width, int height)
         config.setValue("screenheight", height);
     }
 }
+
+void Client::videoResized(int width, int height)
+{
+    gui->videoResized();
+
+    if (mDesktop)
+        mDesktop->setSize(width, height);
+
+    if (mSetupButton)
+        mSetupButton->setPosition(width - mSetupButton->getWidth() - 3, 3);
+
+    if (mGame)
+        mGame->videoResized(width, height);
+}
diff --git a/src/client.h b/src/client.h
index e526b609..2ee6764c 100644
--- a/src/client.h
+++ b/src/client.h
@@ -203,7 +203,16 @@ public:
     void event(Event::Channel channel, const Event &event);
     void action(const gcn::ActionEvent &event);
 
-    void resizeVideo(int width, int height);
+    /**
+     * Should be called after the window has been resized by the user.
+     */
+    void handleVideoResize(int width, int height);
+
+    /**
+     * Should be called after a succesful resize or change of resolution, makes
+     * sure the GUI and game adapt to the new size.
+     */
+    void videoResized(int width, int height);
 
 private:
     void initRootDir();
diff --git a/src/game.cpp b/src/game.cpp
index 99d40647..222e56df 100644
--- a/src/game.cpp
+++ b/src/game.cpp
@@ -401,7 +401,7 @@ void Game::handleInput()
         if (event.type == SDL_VIDEORESIZE)
         {
             // Let the client deal with this one (it'll pass down from there)
-            Client::instance()->resizeVideo(event.resize.w, event.resize.h);
+            Client::instance()->handleVideoResize(event.resize.w, event.resize.h);
         }
         // Keyboard events (for discontinuous keys)
         else if (event.type == SDL_KEYDOWN)
diff --git a/src/graphics.cpp b/src/graphics.cpp
index eb965b50..dfdef349 100644
--- a/src/graphics.cpp
+++ b/src/graphics.cpp
@@ -27,6 +27,8 @@
 #include "resources/image.h"
 #include "resources/imageloader.h"
 
+#include "utils/gettext.h"
+
 #include <SDL_gfxBlitFunc.h>
 
 Graphics::Graphics():
@@ -53,12 +55,6 @@ bool Graphics::setVideoMode(int w, int h, int bpp, bool fs, bool hwaccel)
 
     int displayFlags = SDL_ANYFORMAT;
 
-    mWidth = w;
-    mHeight = h;
-    mBpp = bpp;
-    mFullscreen = fs;
-    mHWAccel = hwaccel;
-
     if (fs)
         displayFlags |= SDL_FULLSCREEN;
     else
@@ -74,6 +70,12 @@ bool Graphics::setVideoMode(int w, int h, int bpp, bool fs, bool hwaccel)
     if (!mTarget)
         return false;
 
+    mWidth = w;
+    mHeight = h;
+    mBpp = bpp;
+    mFullscreen = fs;
+    mHWAccel = hwaccel;
+
     char videoDriverName[64];
 
     if (SDL_VideoDriverName(videoDriverName, 64))
@@ -84,54 +86,49 @@ bool Graphics::setVideoMode(int w, int h, int bpp, bool fs, bool hwaccel)
     const SDL_VideoInfo *vi = SDL_GetVideoInfo();
 
     logger->log("Possible to create hardware surfaces: %s",
-            ((vi->hw_available) ? "yes" : "no"));
+                vi->hw_available ? "yes" : "no");
     logger->log("Window manager available: %s",
-            ((vi->wm_available) ? "yes" : "no"));
+                vi->wm_available ? "yes" : "no");
     logger->log("Accelerated hardware to hardware blits: %s",
-            ((vi->blit_hw) ? "yes" : "no"));
+                vi->blit_hw ? "yes" : "no");
     logger->log("Accelerated hardware to hardware colorkey blits: %s",
-            ((vi->blit_hw_CC) ? "yes" : "no"));
+                vi->blit_hw_CC ? "yes" : "no");
     logger->log("Accelerated hardware to hardware alpha blits: %s",
-            ((vi->blit_hw_A) ? "yes" : "no"));
+                vi->blit_hw_A ? "yes" : "no");
     logger->log("Accelerated software to hardware blits: %s",
-            ((vi->blit_sw) ? "yes" : "no"));
+                vi->blit_sw ? "yes" : "no");
     logger->log("Accelerated software to hardware colorkey blits: %s",
-            ((vi->blit_sw_CC) ? "yes" : "no"));
+                vi->blit_sw_CC ? "yes" : "no");
     logger->log("Accelerated software to hardware alpha blits: %s",
-            ((vi->blit_sw_A) ? "yes" : "no"));
+                vi->blit_sw_A ? "yes" : "no");
     logger->log("Accelerated color fills: %s",
-            ((vi->blit_fill) ? "yes" : "no"));
+                vi->blit_fill ? "yes" : "no");
     logger->log("Available video memory: %d", vi->video_mem);
 
     return true;
 }
 
-bool Graphics::setFullscreen(bool fs)
-{
-    if (mFullscreen == fs)
-        return true;
-
-    return setVideoMode(mWidth, mHeight, mBpp, fs, mHWAccel);
-}
-
-bool Graphics::resize(int width, int height)
+bool Graphics::changeVideoMode(int w, int h, int bpp, bool fs, bool hwaccel)
 {
-    if (mWidth == width && mHeight == height)
+    // Just return success if we're already in this mode
+    if (mWidth == w &&
+            mHeight == h &&
+            mBpp == bpp &&
+            mFullscreen == fs &&
+            mHWAccel == hwaccel)
         return true;
 
-    const int prevWidth = mWidth;
-    const int prevHeight = mHeight;
-
     _endDraw();
 
-    bool success = setVideoMode(width, height, mBpp, mFullscreen, mHWAccel);
+    bool success = setVideoMode(w, h, bpp, fs, hwaccel);
 
-    // If it didn't work, try to restore the previous size. If that didn't
-    // work either, bail out (but then we're in deep trouble).
-    if (!success)
-    {
-        if (!setVideoMode(prevWidth, prevHeight, mBpp, mFullscreen, mHWAccel))
-            return false;
+    // If it didn't work, try to restore the previous mode. If that doesn't
+    // work either, we're in big trouble and bail out.
+    if (!success) {
+        if (!setVideoMode(mWidth, mHeight, mBpp, mFullscreen, mHWAccel)) {
+            logger->error(_("Failed to change video mode and couldn't "
+                            "switch back to the previous mode!"));
+        }
     }
 
     _beginDraw();
diff --git a/src/graphics.h b/src/graphics.h
index 4371e909..6a17aab4 100644
--- a/src/graphics.h
+++ b/src/graphics.h
@@ -88,14 +88,10 @@ class Graphics : public gcn::SDLGraphics
         virtual bool setVideoMode(int w, int h, int bpp, bool fs, bool hwaccel);
 
         /**
-         * Set fullscreen mode.
+         * Change the video mode. Can be used for switching to full screen,
+         * changing resolution or adapting after window resize.
          */
-        bool setFullscreen(bool fs);
-
-        /**
-         * Resize the window to the specified size.
-         */
-        bool resize(int width, int height);
+        bool changeVideoMode(int w, int h, int bpp, bool fs, bool hwaccel);
 
         /**
          * Blits an image onto the screen.
@@ -122,7 +118,7 @@ class Graphics : public gcn::SDLGraphics
                                    dstX, dstY,
                                    width, height,
                                    desiredWidth, desiredHeight,
-                                   false); };
+                                   false); }
 
         /**
          * Draws a resclaled version of the image
@@ -209,6 +205,23 @@ class Graphics : public gcn::SDLGraphics
         int getHeight() const;
 
         /**
+         * Returns the amount of bits per pixel that was requested (not the
+         * actual amount that's currently active).
+         */
+        int getBpp() const { return mBpp; }
+
+        /**
+         * Returns whether we're in a full screen mode.
+         */
+        bool getFullscreen() const { return mFullscreen; }
+
+        /**
+         * Returns whether old-fashioned SDL-based hardware acceleration was
+         * requested (not whether it's currently active).
+         */
+        bool getHWAccel() const { return mHWAccel; }
+
+        /**
          * Takes a screenshot and returns it as SDL surface.
          */
         virtual SDL_Surface *getScreenshot();
diff --git a/src/gui/setup_video.cpp b/src/gui/setup_video.cpp
index f2f19af5..3c92aa31 100644
--- a/src/gui/setup_video.cpp
+++ b/src/gui/setup_video.cpp
@@ -21,6 +21,7 @@
 
 #include "gui/setup_video.h"
 
+#include "client.h"
 #include "configuration.h"
 #include "game.h"
 #include "graphics.h"
@@ -99,28 +100,26 @@ ModeListModel::ModeListModel()
         logger->log("All resolutions available");
     else
     {
-        //logger->log("Available Modes");
         for (int i = 0; modes[i]; ++i)
         {
-            const std::string modeString =
-                toString((int)modes[i]->w) + "x" + toString((int) modes[i]->h);
-            //logger->log(modeString.c_str());
-            mVideoModes.push_back(modeString);
+            const int width = modes[i]->w;
+            const int height = modes[i]->h;
+
+            // Skip the unreasonably small modes
+            if (width < 640 || height < 480)
+                continue;
+
+            mVideoModes.push_back(toString(width) + "x" + toString(height));
         }
     }
 }
 
 int ModeListModel::getIndexOf(const std::string &widthXHeightMode)
 {
-    std::string currentMode = "";
-    for (int i = 0; i < getNumberOfElements(); i++)
-    {
-        currentMode = getElementAt(i);
-        if (currentMode == widthXHeightMode)
-        {
+    for (unsigned i = 0; i < mVideoModes.size(); i++)
+        if (mVideoModes.at(i) == widthXHeightMode)
             return i;
-        }
-    }
+
     return -1;
 }
 
@@ -291,9 +290,22 @@ Setup_Video::~Setup_Video()
 
 void Setup_Video::apply()
 {
-    // Full screen changes
+    // Video mode changes
+    int screenWidth = graphics->getWidth();
+    int screenHeight = graphics->getHeight();
+
+    if (mModeList->getSelected() > -1)
+    {
+        std::string mode = mModeListModel->getElementAt(mModeList->getSelected());
+        screenWidth = atoi(mode.substr(0, mode.find("x")).c_str());
+        screenHeight = atoi(mode.substr(mode.find("x") + 1).c_str());
+    }
+
     bool fullscreen = mFsCheckBox->isSelected();
-    if (fullscreen != config.getBoolValue("screen"))
+
+    if (fullscreen != graphics->getFullscreen() ||
+            screenWidth != graphics->getWidth() ||
+            screenHeight != graphics->getHeight())
     {
         /* The OpenGL test is only necessary on Windows, since switching
          * to/from full screen works fine on Linux. On Windows we'd have to
@@ -304,39 +316,41 @@ void Setup_Video::apply()
 
 #if defined(_WIN32) || defined(__APPLE__)
         // checks for opengl usage
-        if (!config.getBoolValue("opengl"))
+        if (config.getBoolValue("opengl"))
         {
-#endif
-            if (!graphics->setFullscreen(fullscreen))
-            {
-                fullscreen = !fullscreen;
-                if (!graphics->setFullscreen(fullscreen))
-                {
-                    std::stringstream errorMessage;
-                    if (fullscreen)
-                    {
-                        errorMessage << _("Failed to switch to windowed mode "
-                                          "and restoration of old mode also "
-                                          "failed!") << std::endl;
-                    }
-                    else
-                    {
-                        errorMessage << _("Failed to switch to fullscreen mode "
-                                          "and restoration of old mode also "
-                                          "failed!") << std::endl;
-                    }
-                    logger->error(errorMessage.str());
-                }
-            }
-#if defined(_WIN32) || defined(__APPLE__)
+            new OkDialog(_("Changing Video Mode"),
+                         _("Restart needed for changes to take effect."));
+
+            config.setValue("screen", fullscreen);
+            config.setValue("screenwidth", screenWidth);
+            config.setValue("screenheight", screenHeight);
         }
         else
+#endif
         {
-            new OkDialog(_("Switching to Full Screen"),
-                         _("Restart needed for changes to take effect."));
+            if (!graphics->changeVideoMode(screenWidth,
+                                           screenHeight,
+                                           graphics->getBpp(),
+                                           fullscreen,
+                                           graphics->getHWAccel()))
+            {
+                std::stringstream errorMessage;
+                if (fullscreen)
+                    errorMessage << _("Failed to switch to fullscreen mode.");
+                else
+                    errorMessage << _("Failed to switch to windowed mode.");
+
+                new OkDialog(_("Error"), errorMessage.str());
+            }
+            else
+            {
+                Client::instance()->videoResized(screenWidth, screenHeight);
+
+                config.setValue("screen", fullscreen);
+                config.setValue("screenwidth", screenWidth);
+                config.setValue("screenheight", screenHeight);
+            }
         }
-#endif
-        config.setValue("screen", fullscreen);
     }
 
     // OpenGL change
@@ -348,7 +362,7 @@ void Setup_Video::apply()
         if (mOpenGLCheckBox->isSelected())
         {
             new OkDialog(_("Changing to OpenGL"),
-                         _("Applying change to OpenGL requires restart. "
+                         _("Applying change to OpenGL requires restart.\n\n"
                            "In case OpenGL messes up your game graphics, "
                            "restart the game with the command line option "
                            "\"--no-opengl\"."));
@@ -416,8 +430,6 @@ void Setup_Video::cancel()
     std::string videoMode = toString(graphics->getWidth()) + "x"
                             + toString(graphics->getHeight());
     mModeList->setSelected(mModeListModel->getIndexOf(videoMode));
-    config.setValue("screenwidth", graphics->getWidth());
-    config.setValue("screenheight", graphics->getHeight());
 
     config.setValue("customcursor", mCustomCursorEnabled);
     config.setValue("particleeffects", mParticleEffectsEnabled);
@@ -429,23 +441,7 @@ void Setup_Video::action(const gcn::ActionEvent &event)
 {
     const std::string &id = event.getId();
 
-    if (id == "videomode")
-    {
-        const std::string mode = mModeListModel->getElementAt(mModeList->getSelected());
-        const int width = atoi(mode.substr(0, mode.find("x")).c_str());
-        const int height = atoi(mode.substr(mode.find("x") + 1).c_str());
-
-        // TODO: Find out why the drawing area doesn't resize without a restart
-        if (width != graphics->getWidth() || height != graphics->getHeight())
-        {
-            new OkDialog(_("Screen Resolution Changed"),
-                    _("Restart your client for the change to take effect."));
-        }
-
-        config.setValue("screenwidth", width);
-        config.setValue("screenheight", height);
-    }
-    else if (id == "customcursor")
+    if (id == "customcursor")
     {
         config.setValue("customcursor", mCustomCursorCheckBox->isSelected());
     }