Merge pull request #1823 from Flamefire/zoom-factor

Make sure zooming via mouse goes back to 1.0

Author: Flamefire

.github/workflows/static-analysis.yml

@@ -54,6 +54,6 @@ jobs:
           sudo apt-get update && sudo apt-get install -y \
             clang-18 clang-tidy-18 \
             libsdl2-dev libsdl2-mixer-dev libcurl4-openssl-dev libbz2-dev libminiupnpc-dev liblua5.2-dev gettext \
-            libboost-filesystem-dev libboost-program-options-dev libboost-thread-dev libboost-test-dev libboost-locale-dev libboost-iostreams-dev
+            libboost-filesystem-dev libboost-program-options-dev libboost-thread-dev libboost-test-dev libboost-locale-dev libboost-nowide-dev libboost-iostreams-dev
       - name: Run Clang-Tidy
         run: tools/ci/runClangTidy.sh

libs/s25main/desktops/dskGameInterface.cpp

@@ -1,4 +1,4 @@
-// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
+// Copyright (C) 2005 - 2025 Settlers Freaks (sf-team at siedler25.org)
 //
 // SPDX-License-Identifier: GPL-2.0-or-later
 
@@ -97,14 +97,30 @@ enum
     ID_btPost,
     ID_txtNumMsg
 };
+
+float getNextZoomLevel(const float currentZoom)
+{
+    // Get first level bigger than current zoom
+    // NOLINTNEXTLINE(readability-qualified-auto)
+    auto it = std::upper_bound(ZOOM_FACTORS.begin(), ZOOM_FACTORS.end(), currentZoom);
+    return (it == ZOOM_FACTORS.end()) ? ZOOM_FACTORS.front() : *it;
 }
 
+float getPreviousZoomLevel(const float currentZoom)
+{
+    // Get last level bigger or equal than current zoom
+    // NOLINTNEXTLINE(readability-qualified-auto)
+    auto it = std::lower_bound(ZOOM_FACTORS.begin(), ZOOM_FACTORS.end(), currentZoom);
+    return (it == ZOOM_FACTORS.begin()) ? ZOOM_FACTORS.back() : *(--it);
+}
+} // namespace
+
 dskGameInterface::dskGameInterface(std::shared_ptr<Game> game, std::shared_ptr<const NWFInfo> nwfInfo,
                                    unsigned playerIdx, bool initOGL)
     : Desktop(nullptr), game_(std::move(game)), nwfInfo_(std::move(nwfInfo)),
       worldViewer(playerIdx, const_cast<Game&>(*game_).world_),
       gwv(worldViewer, Position(0, 0), VIDEODRIVER.GetRenderSize()), cbb(*LOADER.GetPaletteN("pal5")),
-      actionwindow(nullptr), roadwindow(nullptr), minimap(worldViewer), isScrolling(false), zoomLvl(ZOOM_DEFAULT_INDEX),
+      actionwindow(nullptr), roadwindow(nullptr), minimap(worldViewer), isScrolling(false),
       cheats_(const_cast<Game&>(*game_).world_), cheatCommandTracker_(cheats_)
 {
     road.mode = RoadBuildMode::Disabled;
@@ -880,18 +896,13 @@ bool dskGameInterface::Msg_KeyDown(const KeyEvent& ke)
             gwv.SetZoomFactor(ZOOM_FACTORS[ZOOM_DEFAULT_INDEX]);
             return true;
         case 'z': // zoom
-            if(++zoomLvl >= ZOOM_FACTORS.size())
-                zoomLvl = 0;
-
-            gwv.SetZoomFactor(ZOOM_FACTORS[zoomLvl]);
+            if(ke.ctrl)
+                gwv.SetZoomFactor(ZOOM_FACTORS[ZOOM_DEFAULT_INDEX]);
+            else
+                gwv.SetZoomFactor(getNextZoomLevel(gwv.GetCurrentTargetZoomFactor()));
             return true;
         case 'Z': // shift-z, reverse zoom
-            if(zoomLvl == 0)
-                zoomLvl = ZOOM_FACTORS.size() - 1;
-            else
-                zoomLvl--;
-
-            gwv.SetZoomFactor(ZOOM_FACTORS[zoomLvl]);
+            gwv.SetZoomFactor(getPreviousZoomLevel(gwv.GetCurrentTargetZoomFactor()));
             return true;
     }
 
@@ -909,24 +920,14 @@ bool dskGameInterface::Msg_WheelDown(const MouseCoords&)
     return true;
 }
 
-void dskGameInterface::WheelZoom(float step)
+void dskGameInterface::WheelZoom(const float step)
 {
-    float new_zoom = gwv.GetCurrentTargetZoomFactor() * (1 + step);
-    gwv.SetZoomFactor(new_zoom);
+    auto targetZoomFactor = gwv.GetCurrentTargetZoomFactor() * (1 + step);
+    targetZoomFactor = std::clamp(targetZoomFactor, ZOOM_FACTORS.front(), ZOOM_FACTORS.back());
+    if(targetZoomFactor > 1 - ZOOM_WHEEL_INCREMENT && targetZoomFactor < 1 + ZOOM_WHEEL_INCREMENT)
+        targetZoomFactor = 1.f; // Snap to 100%
 
-    // also keep track in terms of fixed defined zoom levels
-    zoomLvl = ZOOM_DEFAULT_INDEX;
-    for(size_t i = ZOOM_DEFAULT_INDEX; i < ZOOM_FACTORS.size(); ++i)
-    {
-        if(ZOOM_FACTORS[i] < new_zoom)
-            zoomLvl = i;
-    }
-
-    for(size_t i = ZOOM_DEFAULT_INDEX; i-- > 0;)
-    {
-        if(ZOOM_FACTORS[i] > new_zoom)
-            zoomLvl = i;
-    }
+    gwv.SetZoomFactor(targetZoomFactor);
 }
 
 void dskGameInterface::OnBuildingNote(const BuildingNote& note)

libs/s25main/desktops/dskGameInterface.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2005 - 2024 Settlers Freaks (sf-team at siedler25.org)
+// Copyright (C) 2005 - 2025 Settlers Freaks (sf-team at siedler25.org)
 //
 // SPDX-License-Identifier: GPL-2.0-or-later
 
@@ -165,7 +165,6 @@ class dskGameInterface :
 
     bool isScrolling;
     Position startScrollPt;
-    size_t zoomLvl;
     Subscription evBld;
 
     Cheats cheats_;

libs/s25main/gameData/GuiConsts.h

@@ -1,12 +1,12 @@
-// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
+// Copyright (C) 2005 - 2025 Settlers Freaks (sf-team at siedler25.org)
 //
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #pragma once
 
 #include <array>
 
-const std::array<float, 7> SUPPRESS_UNUSED ZOOM_FACTORS = {{0.5f, 0.75f, 1.f, 1.25f, 1.5f, 2.f, 3.f}};
-const size_t ZOOM_DEFAULT_INDEX = 2;
-const float ZOOM_ACCELERATION = 0.001f;
-const float ZOOM_WHEEL_INCREMENT = 0.03f;
+constexpr std::array<float, 7> SUPPRESS_UNUSED ZOOM_FACTORS = {{0.5f, 0.75f, 1.f, 1.25f, 1.5f, 2.f, 3.f}};
+constexpr size_t ZOOM_DEFAULT_INDEX = 2;
+constexpr float ZOOM_ACCELERATION = 0.001f;
+constexpr float ZOOM_WHEEL_INCREMENT = 0.03f;

libs/s25main/world/GameWorldView.cpp

@@ -88,20 +88,16 @@ void GameWorldView::SetNextZoomFactor()
     CalcFxLx();
 }
 
-void GameWorldView::SetZoomFactor(float zoomFactor, bool smoothTransition /* = true*/)
+float GameWorldView::SetZoomFactor(float zoomFactor, bool smoothTransition /* = true*/)
 {
-    if(zoomFactor < ZOOM_FACTORS.front())
-        targetZoomFactor_ = ZOOM_FACTORS.front();
-    else if(zoomFactor > ZOOM_FACTORS.back())
-        targetZoomFactor_ = ZOOM_FACTORS.back();
-    else
-        targetZoomFactor_ = zoomFactor;
+    targetZoomFactor_ = zoomFactor;
     if(!smoothTransition)
     {
         zoomFactor_ = targetZoomFactor_;
         updateEffectiveZoomFactor();
         CalcFxLx();
     }
+    return targetZoomFactor_;
 }
 
 float GameWorldView::GetCurrentTargetZoomFactor() const

libs/s25main/world/GameWorldView.h

@@ -78,7 +78,9 @@ class GameWorldView
     Position GetPos() const { return origin_; }
     Extent GetSize() const { return size_; }
 
-    void SetZoomFactor(float zoomFactor, bool smoothTransition = true);
+    /// Set target zoom factor and start zooming if smoothTransition is true
+    /// Returns actual zoom factor used, potentially clamped
+    float SetZoomFactor(float zoomFactor, bool smoothTransition = true);
     float GetCurrentTargetZoomFactor() const;
     void SetNextZoomFactor();
 

tests/s25Main/integration/testDskGameInterface.cpp

@@ -1,4 +1,4 @@
-// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
+// Copyright (C) 2005 - 2025 Settlers Freaks (sf-team at siedler25.org)
 //
 // SPDX-License-Identifier: GPL-2.0-or-later
 
@@ -14,6 +14,8 @@
 #include "uiHelper/uiHelpers.hpp"
 #include "worldFixtures/CreateEmptyWorld.h"
 #include "worldFixtures/WorldFixture.h"
+#include "gameData/GuiConsts.h"
+#include "rttr/test/random.hpp"
 #include <boost/test/unit_test.hpp>
 
 // LCOV_EXCL_START
@@ -36,6 +38,9 @@ struct dskGameInterfaceMock : public dskGameInterface
     void Msg_PaintBefore() override {}
     void Msg_PaintAfter() override {}
     using dskGameInterface::actionwindow;
+    using dskGameInterface::Msg_KeyDown;
+    using dskGameInterface::Msg_WheelDown;
+    using dskGameInterface::Msg_WheelUp;
 };
 struct GameInterfaceFixture : uiHelper::Fixture
 {
@@ -227,4 +232,69 @@ BOOST_FIXTURE_TEST_CASE(IwActionClose, GameInterfaceFixture)
     BOOST_TEST_REQUIRE(gameDesktop->actionwindow == nullptr);
 }
 
+BOOST_FIXTURE_TEST_CASE(Zooming, GameInterfaceFixture)
+{
+    const KeyEvent zoomInEv('z');
+    const KeyEvent zoomOutEv('Z');
+    KeyEvent zoomDefaultEv('z');
+    zoomDefaultEv.ctrl = true;
+
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[ZOOM_DEFAULT_INDEX]);
+    for(size_t i = ZOOM_DEFAULT_INDEX + 1; i < ZOOM_FACTORS.size(); i++)
+    {
+        gameDesktop->Msg_KeyDown(zoomInEv);
+        BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[i]);
+    }
+    // Wrap around
+    gameDesktop->Msg_KeyDown(zoomInEv);
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[0]);
+    gameDesktop->Msg_KeyDown(zoomInEv);
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[1]);
+
+    gameDesktop->Msg_KeyDown(zoomOutEv);
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[0]);
+    gameDesktop->Msg_KeyDown(zoomOutEv);
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS.back());
+
+    gameDesktop->Msg_KeyDown(zoomDefaultEv);
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[ZOOM_DEFAULT_INDEX]);
+
+    gameDesktop->Msg_WheelDown({});
+    auto curZoom = view->GetCurrentTargetZoomFactor();
+    BOOST_TEST(curZoom < ZOOM_FACTORS[ZOOM_DEFAULT_INDEX]);
+    BOOST_TEST(curZoom > ZOOM_FACTORS[ZOOM_DEFAULT_INDEX - 1]);
+    // Go to prev index
+    gameDesktop->Msg_KeyDown(zoomOutEv);
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[ZOOM_DEFAULT_INDEX - 1]);
+
+    gameDesktop->Msg_KeyDown(zoomDefaultEv);
+    gameDesktop->Msg_WheelUp({});
+    curZoom = view->GetCurrentTargetZoomFactor();
+    BOOST_TEST(curZoom > ZOOM_FACTORS[ZOOM_DEFAULT_INDEX]);
+    BOOST_TEST(curZoom < ZOOM_FACTORS[ZOOM_DEFAULT_INDEX + 1]);
+    // Go to next index
+    gameDesktop->Msg_KeyDown(zoomInEv);
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[ZOOM_DEFAULT_INDEX + 1]);
+
+    gameDesktop->Msg_WheelDown({});
+    curZoom = view->GetCurrentTargetZoomFactor();
+    BOOST_TEST(curZoom > ZOOM_FACTORS[ZOOM_DEFAULT_INDEX]);
+    BOOST_TEST(curZoom < ZOOM_FACTORS[ZOOM_DEFAULT_INDEX + 1]);
+    // Go to prev index
+    gameDesktop->Msg_KeyDown(zoomOutEv);
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == ZOOM_FACTORS[ZOOM_DEFAULT_INDEX]);
+
+    // Zoom in and out gets to 1.0 (default)
+    static_assert(ZOOM_FACTORS[ZOOM_DEFAULT_INDEX] == 1.f);
+    gameDesktop->Msg_WheelUp({});
+    gameDesktop->Msg_WheelDown({});
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == 1.f);
+    const auto numZoom = rttr::test::randomValue(2, 10);
+    for(int i = 0; i < numZoom; i++)
+        gameDesktop->Msg_WheelDown({});
+    for(int i = 0; i < numZoom; i++)
+        gameDesktop->Msg_WheelUp({});
+    BOOST_TEST(view->GetCurrentTargetZoomFactor() == 1.f);
+}
+
 BOOST_AUTO_TEST_SUITE_END()