From 90920f06c1d4f2ccee7b7e6817bda4c2bad1142d Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Mon, 24 Jul 2023 18:42:44 +0200 Subject: [PATCH 1/4] Add Min/MaxElementValue to Point class The static constexpr members are initialized to std::numeric_limits::min/max(). --- libs/common/include/Point.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/common/include/Point.h b/libs/common/include/Point.h index 7bec58f6f..36d014044 100644 --- a/libs/common/include/Point.h +++ b/libs/common/include/Point.h @@ -22,6 +22,9 @@ struct Point //-V690 using ElementType = T; static_assert(std::is_arithmetic::value, "Requires an arithmetic type"); + static constexpr auto MinElementValue = std::numeric_limits::min(); + static constexpr auto MaxElementValue = std::numeric_limits::max(); + struct Truncate_t { constexpr explicit Truncate_t() = default; From a1302230ddaf8659fdd35686065c71751e800a3b Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Tue, 25 Jul 2023 06:54:34 +0200 Subject: [PATCH 2/4] Use Point::MaxElementValue where applicable --- libs/s25main/ingameWindows/IngameWindow.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libs/s25main/ingameWindows/IngameWindow.cpp b/libs/s25main/ingameWindows/IngameWindow.cpp index 5a48f62d2..146ff6d53 100644 --- a/libs/s25main/ingameWindows/IngameWindow.cpp +++ b/libs/s25main/ingameWindows/IngameWindow.cpp @@ -24,12 +24,9 @@ namespace { constexpr Extent ButtonSize(16, 16); } -const DrawPoint IngameWindow::posLastOrCenter(std::numeric_limits::max(), - std::numeric_limits::max()); -const DrawPoint IngameWindow::posCenter(std::numeric_limits::max() - 1, - std::numeric_limits::max()); -const DrawPoint IngameWindow::posAtMouse(std::numeric_limits::max() - 1, - std::numeric_limits::max() - 1); +const DrawPoint IngameWindow::posLastOrCenter(DrawPoint::MaxElementValue, DrawPoint::MaxElementValue); +const DrawPoint IngameWindow::posCenter(DrawPoint::MaxElementValue - 1, DrawPoint::MaxElementValue); +const DrawPoint IngameWindow::posAtMouse(DrawPoint::MaxElementValue - 1, DrawPoint::MaxElementValue - 1); const Extent IngameWindow::borderSize(1, 1); IngameWindow::IngameWindow(unsigned id, const DrawPoint& pos, const Extent& size, std::string title, From a3b7fd1e1c91e790d41284dbbf4467c29916a463 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Sun, 9 Jul 2023 19:30:30 +0200 Subject: [PATCH 3/4] Improve in-game window positioning 1) Remember the window position set by a move and try to restore this position after every resize. 2) If the window is moved to a screen edge save Point::MaxElementValue instead, to make the window "stick" to the edge when resizing. --- libs/s25main/Settings.cpp | 26 +++++++--- libs/s25main/Settings.h | 1 + libs/s25main/ingameWindows/IngameWindow.cpp | 57 ++++++++++++++++----- libs/s25main/ingameWindows/IngameWindow.h | 4 +- libs/s25main/ingameWindows/iwObservate.cpp | 5 -- 5 files changed, 65 insertions(+), 28 deletions(-) diff --git a/libs/s25main/Settings.cpp b/libs/s25main/Settings.cpp index c7899b16e..059f778b3 100644 --- a/libs/s25main/Settings.cpp +++ b/libs/s25main/Settings.cpp @@ -347,10 +347,13 @@ void Settings::LoadIngame() const auto* iniWindow = static_cast(settingsIngame.find(window.second)); if(!iniWindow) continue; - windows.persistentSettings[window.first].lastPos.x = iniWindow->getIntValue("pos_x"); - windows.persistentSettings[window.first].lastPos.y = iniWindow->getIntValue("pos_y"); - windows.persistentSettings[window.first].isOpen = iniWindow->getIntValue("is_open"); - windows.persistentSettings[window.first].isMinimized = iniWindow->getValue("is_minimized", false); + auto& settings = windows.persistentSettings[window.first]; + const auto lastPos = settings.lastPos = + DrawPoint(iniWindow->getIntValue("pos_x"), iniWindow->getIntValue("pos_y")); + settings.restorePos = DrawPoint(iniWindow->getValue("restore_pos_x", lastPos.x), + iniWindow->getValue("restore_pos_y", lastPos.y)); + settings.isOpen = iniWindow->getIntValue("is_open"); + settings.isMinimized = iniWindow->getValue("is_minimized", false); } } catch(std::runtime_error& e) { @@ -500,10 +503,17 @@ void Settings::SaveIngame() auto* iniWindow = static_cast(settingsIngame.find(window.second)); if(!iniWindow) continue; - iniWindow->setValue("pos_x", windows.persistentSettings[window.first].lastPos.x); - iniWindow->setValue("pos_y", windows.persistentSettings[window.first].lastPos.y); - iniWindow->setValue("is_open", windows.persistentSettings[window.first].isOpen); - iniWindow->setValue("is_minimized", windows.persistentSettings[window.first].isMinimized); + const auto& settings = windows.persistentSettings[window.first]; + iniWindow->setValue("pos_x", settings.lastPos.x); + iniWindow->setValue("pos_y", settings.lastPos.y); + if(settings.restorePos != settings.lastPos) + { + // only save if different; defaults to lastPos on load + iniWindow->setValue("restore_pos_x", settings.restorePos.x); + iniWindow->setValue("restore_pos_y", settings.restorePos.y); + } + iniWindow->setValue("is_open", settings.isOpen); + iniWindow->setValue("is_minimized", settings.isMinimized); } bfs::path settingsPathIngame = RTTRCONFIG.ExpandPath(s25::resources::ingameOptions); diff --git a/libs/s25main/Settings.h b/libs/s25main/Settings.h index e30f49967..e702c34ed 100644 --- a/libs/s25main/Settings.h +++ b/libs/s25main/Settings.h @@ -25,6 +25,7 @@ bool checkPort(int port); struct PersistentWindowSettings { DrawPoint lastPos = DrawPoint::Invalid(); + DrawPoint restorePos = DrawPoint::Invalid(); bool isOpen = false; bool isMinimized = false; }; diff --git a/libs/s25main/ingameWindows/IngameWindow.cpp b/libs/s25main/ingameWindows/IngameWindow.cpp index 146ff6d53..e5a5dc3d7 100644 --- a/libs/s25main/ingameWindows/IngameWindow.cpp +++ b/libs/s25main/ingameWindows/IngameWindow.cpp @@ -51,25 +51,32 @@ IngameWindow::IngameWindow(unsigned id, const DrawPoint& pos, const Extent& size // Save to settings that window is open SaveOpenStatus(true); - // Restore minimized state - if(windowSettings_ && windowSettings_->isMinimized) + if(windowSettings_) { - isMinimized_ = true; - Extent minimizedSize(GetSize().x, contentOffset.y + contentOffsetEnd.y); - Window::Resize(minimizedSize); + // Restore minimized state + if(windowSettings_ && windowSettings_->isMinimized) + { + isMinimized_ = true; + Extent minimizedSize(GetSize().x, contentOffset.y + contentOffsetEnd.y); + Window::Resize(minimizedSize); + } + // Load restorePos + restorePos_ = windowSettings_->restorePos; } // Load last position or center the window if(pos == posLastOrCenter) { if(windowSettings_ && windowSettings_->lastPos.isValid()) - SetPos(windowSettings_->lastPos); + SetPos(windowSettings_->lastPos, !restorePos_.isValid()); else MoveToCenter(); } else if(pos == posCenter) MoveToCenter(); else if(pos == posAtMouse) MoveNextToMouse(); + else + SetPos(pos); // always call SetPos() to update restorePos } void IngameWindow::Resize(const Extent& newSize) @@ -81,6 +88,9 @@ void IngameWindow::Resize(const Extent& newSize) void IngameWindow::SetIwSize(const Extent& newSize) { + // Is the window connecting with the bottom screen edge? + const auto atBottom = (GetPos().y + GetSize().y) >= VIDEODRIVER.GetRenderSize().y; + iwHeight = newSize.y; Extent wndSize = newSize; if(isMinimized_) @@ -88,8 +98,13 @@ void IngameWindow::SetIwSize(const Extent& newSize) wndSize += contentOffset + contentOffsetEnd; Window::Resize(wndSize); - // Reset the position to check if parts of the window are out of the visible area - SetPos(GetPos()); + // Adjust restorePos if the window was connecting with the bottom screen edge before being minimized + const auto pos = (atBottom && isMinimized_) ? DrawPoint(restorePos_.x, DrawPoint::MaxElementValue) : restorePos_; + + // Reset the position + // 1) to check if parts of the window are out of the visible area + // 2) to re-connect the window with the bottom screen edge, if needed + SetPos(pos, false); } Extent IngameWindow::GetIwSize() const @@ -102,24 +117,38 @@ DrawPoint IngameWindow::GetRightBottomBoundary() return DrawPoint(GetSize() - contentOffsetEnd); } -void IngameWindow::SetPos(DrawPoint newPos) +void IngameWindow::SetPos(DrawPoint newPos, bool saveRestorePos) { const Extent screenSize = VIDEODRIVER.GetRenderSize(); + DrawPoint newRestorePos = newPos; // Too far left or right? if(newPos.x < 0) - newPos.x = 0; - else if(newPos.x + GetSize().x > screenSize.x) + newRestorePos.x = newPos.x = 0; + else if(newPos.x + GetSize().x >= screenSize.x) + { newPos.x = screenSize.x - GetSize().x; + newRestorePos.x = DrawPoint::MaxElementValue; // make window stick to the right + } // Too high or low? if(newPos.y < 0) - newPos.y = 0; - else if(newPos.y + GetSize().y > screenSize.y) + newRestorePos.y = newPos.y = 0; + else if(newPos.y + GetSize().y >= screenSize.y) + { newPos.y = screenSize.y - GetSize().y; + newRestorePos.y = DrawPoint::MaxElementValue; // make window stick to the bottom + } - // if possible save the position to settings + if(saveRestorePos) + restorePos_ = newRestorePos; + + // if possible save the positions to settings if(windowSettings_) + { windowSettings_->lastPos = newPos; + if(saveRestorePos) + windowSettings_->restorePos = newRestorePos; + } Window::SetPos(newPos); } diff --git a/libs/s25main/ingameWindows/IngameWindow.h b/libs/s25main/ingameWindows/IngameWindow.h index a0fe40de2..9e2c0dccf 100644 --- a/libs/s25main/ingameWindows/IngameWindow.h +++ b/libs/s25main/ingameWindows/IngameWindow.h @@ -4,6 +4,7 @@ #pragma once +#include "DrawPoint.h" #include "Window.h" #include "helpers/EnumArray.h" #include "gameData/const_gui_ids.h" @@ -71,7 +72,7 @@ class IngameWindow : public Window DrawPoint GetRightBottomBoundary(); /// Set the position for the window after adjusting newPos so the window is in the visible area - void SetPos(DrawPoint newPos); + void SetPos(DrawPoint newPos, bool saveRestorePos = true); /// merkt das Fenster zum Schließen vor. virtual void Close(); @@ -128,4 +129,5 @@ class IngameWindow : public Window CloseBehavior closeBehavior_; helpers::EnumArray buttonStates_; PersistentWindowSettings* windowSettings_; + DrawPoint restorePos_; }; diff --git a/libs/s25main/ingameWindows/iwObservate.cpp b/libs/s25main/ingameWindows/iwObservate.cpp index 62f78d0fa..a9df6fe2f 100644 --- a/libs/s25main/ingameWindows/iwObservate.cpp +++ b/libs/s25main/ingameWindows/iwObservate.cpp @@ -143,11 +143,6 @@ void iwObservate::Msg_ButtonClick(const unsigned ctrl_id) for(unsigned i = 1; i <= 4; ++i) GetCtrl(i)->SetPos( DrawPoint(GetCtrl(i)->GetPos().x - diff, GetSize().y - 50)); - - DrawPoint maxPos(VIDEODRIVER.GetRenderSize() - GetSize() - Extent::all(1)); - DrawPoint newPos = elMin(maxPos, GetPos()); - if(newPos != GetPos()) - SetPos(newPos); } } From fccbc9e22715ebaf9675696b3ac12a112283c825 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Tue, 11 Jul 2023 17:27:58 +0200 Subject: [PATCH 4/4] Add unit test for in-game window positioning --- tests/s25Main/UI/testIngameWindow.cpp | 166 ++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) diff --git a/tests/s25Main/UI/testIngameWindow.cpp b/tests/s25Main/UI/testIngameWindow.cpp index f5c69499a..bdcb53415 100644 --- a/tests/s25Main/UI/testIngameWindow.cpp +++ b/tests/s25Main/UI/testIngameWindow.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "DrawPoint.h" +#include "Loader.h" #include "Point.h" #include "PointOutput.h" #include "Settings.h" @@ -14,6 +15,7 @@ #include "controls/ctrlPercent.h" #include "controls/ctrlProgress.h" #include "desktops/dskGameLobby.h" +#include "drivers/VideoDriverWrapper.h" #include "helpers/format.hpp" #include "ingameWindows/IngameWindow.h" #include "ingameWindows/iwConnecting.h" @@ -21,6 +23,7 @@ #include "ingameWindows/iwHelp.h" #include "ingameWindows/iwMapGenerator.h" #include "ingameWindows/iwMsgbox.h" +#include "ogl/glArchivItem_Bitmap.h" #include "uiHelper/uiHelpers.hpp" #include "gameTypes/GameTypesOutput.h" #include "gameData/const_gui_ids.h" @@ -28,6 +31,7 @@ #include "s25util/StringConversion.h" #include #include +#include // LCOV_EXCL_START BOOST_TEST_DONT_PRINT_LOG_VALUE(rttr::mapGenerator::MapStyle) @@ -228,4 +232,166 @@ BOOST_AUTO_TEST_CASE(SaveAndRestoreMinimized) } } +namespace { +void WindowPositioning_testOne(IngameWindow& wnd, const char* context, const std::function& checkNormal, + const std::function& checkMinimized) +{ + BOOST_TEST_CONTEXT(context) + { + BOOST_TEST_CONTEXT("Before minimize/un-minimize") checkNormal(); + + wnd.SetMinimized(true); + + BOOST_TEST_CONTEXT("Minimized") checkMinimized(); + + wnd.SetMinimized(false); + + BOOST_TEST_CONTEXT("After minimize/un-minimize") checkNormal(); + } +} +} // namespace + +BOOST_AUTO_TEST_CASE(WindowPositioning) +{ + VIDEODRIVER.ResizeScreen(VideoMode(800, 600), false); + + const auto renderSize = VIDEODRIVER.GetRenderSize(); + + constexpr auto idPersisted = CGI_MINIMAP; + constexpr auto idNonPersisted = CGI_OBSERVATION; + constexpr auto wndSizeS = Extent(50, 50); + constexpr auto wndSizeM = Extent(90, 90); + constexpr auto wndSizeL = Extent(200, 200); + constexpr auto offset = DrawPoint(100, 100); + + auto it = SETTINGS.windows.persistentSettings.find(idNonPersisted); + BOOST_REQUIRE(it == SETTINGS.windows.persistentSettings.end()); + + it = SETTINGS.windows.persistentSettings.find(idPersisted); + BOOST_REQUIRE(it != SETTINGS.windows.persistentSettings.end()); + auto& settings = it->second; + + // Calculate minimized height + const auto minHeight = LOADER.GetImageN("resource", 42)->getHeight() // title bar + + LOADER.GetImageN("resource", 40)->getHeight(); // bottom bar + + BOOST_TEST_CONTEXT("Persisted window, fresh settings, posLastOrCenter") + { + settings = PersistentWindowSettings{}; + + IngameWindow wnd(idPersisted, IngameWindow::posLastOrCenter, wndSizeM, "Test Window", nullptr); + + WindowPositioning_testOne( + wnd, "Window should be centered", + [&]() { + BOOST_TEST(wnd.GetPos() == (renderSize / 2 - wndSizeM / 2)); + BOOST_TEST(wnd.GetPos() == settings.lastPos); + BOOST_TEST(wnd.GetPos() == settings.restorePos); + BOOST_TEST(wnd.GetSize() == wndSizeM); + }, + [&]() { + BOOST_TEST(wnd.GetPos() == settings.lastPos); + BOOST_TEST(wnd.GetPos() == settings.restorePos); + BOOST_TEST(wnd.GetPos() == (renderSize / 2 - wndSizeM / 2)); + BOOST_TEST(wnd.GetSize() == Extent(wndSizeM.x, minHeight)); + }); + + const auto restorePos = renderSize - offset; // new position is also the restorePos + WindowPositioning_testOne( + wnd, "Move window into bottom right corner, not connecting with the screen edges", + [&]() { + wnd.SetPos(restorePos); + BOOST_TEST(wnd.GetPos() == restorePos); + BOOST_TEST(wnd.GetPos() == settings.lastPos); + BOOST_TEST(wnd.GetPos() == settings.restorePos); + BOOST_TEST(wnd.GetSize() == wndSizeM); + }, + [&]() { + BOOST_TEST(wnd.GetPos() == restorePos); + BOOST_TEST(wnd.GetPos() == settings.lastPos); + BOOST_TEST(wnd.GetPos() == settings.restorePos); + BOOST_TEST(wnd.GetSize() == Extent(wndSizeM.x, minHeight)); + }); + + WindowPositioning_testOne( + wnd, "Increase size, window connects with screen edges and should move", + [&]() { + wnd.Resize(wndSizeL); + BOOST_TEST(wnd.GetPos() == DrawPoint(renderSize - wndSizeL)); + BOOST_TEST(wnd.GetPos() == settings.lastPos); + BOOST_TEST(restorePos == settings.restorePos); + BOOST_TEST(wnd.GetSize() == wndSizeL); + }, + [&]() { + BOOST_TEST(wnd.GetPos() == renderSize - DrawPoint(wndSizeL.x, minHeight)); + BOOST_TEST(wnd.GetPos() == settings.lastPos); + BOOST_TEST(restorePos == settings.restorePos); + BOOST_TEST(wnd.GetSize() == Extent(wndSizeL.x, minHeight)); + }); + + WindowPositioning_testOne( + wnd, "Decrease size, window no longer connects with screen edges and should move to restorePos", + [&]() { + wnd.Resize(wndSizeS); + BOOST_TEST(wnd.GetPos() == restorePos); + BOOST_TEST(wnd.GetPos() == settings.lastPos); + BOOST_TEST(restorePos == settings.restorePos); + BOOST_TEST(wnd.GetSize() == wndSizeS); + }, + [&]() { + BOOST_TEST(wnd.GetPos() == restorePos); + BOOST_TEST(wnd.GetPos() == settings.lastPos); + BOOST_TEST(restorePos == settings.restorePos); + BOOST_TEST(wnd.GetSize() == Extent(wndSizeS.x, minHeight)); + }); + } + + BOOST_TEST_CONTEXT("Non-persisted window, posAtMouse") + { + // the offset subtracted from the window position for posAtMouse + constexpr auto cursorOffset = DrawPoint(-20, wndSizeM.y / 2); + const auto restorePos = renderSize - offset; // initial window position is also the restorePos + + VIDEODRIVER.SetMousePos(restorePos + cursorOffset); + BOOST_REQUIRE(VIDEODRIVER.GetMousePos() == (restorePos + cursorOffset)); + + IngameWindow wnd(idNonPersisted, IngameWindow::posAtMouse, wndSizeM, "Test Window", nullptr); + + WindowPositioning_testOne( + wnd, "Window should be at cursor", + [&]() { + BOOST_TEST(wnd.GetPos() == restorePos); + BOOST_TEST(wnd.GetSize() == wndSizeM); + }, + [&]() { + BOOST_TEST(wnd.GetPos() == restorePos); + BOOST_TEST(wnd.GetSize() == Extent(wndSizeM.x, minHeight)); + }); + + WindowPositioning_testOne( + wnd, "Increase size, window connects with screen edges and should move", + [&]() { + wnd.Resize(wndSizeL); + BOOST_TEST(wnd.GetPos() == DrawPoint(renderSize - wndSizeL)); + BOOST_TEST(wnd.GetSize() == wndSizeL); + }, + [&]() { + BOOST_TEST(wnd.GetPos() == renderSize - DrawPoint(wndSizeL.x, minHeight)); + BOOST_TEST(wnd.GetSize() == Extent(wndSizeL.x, minHeight)); + }); + + WindowPositioning_testOne( + wnd, "Decrease size, window no longer connects with screen edges and should move to restorePos", + [&]() { + wnd.Resize(wndSizeS); + BOOST_TEST(wnd.GetPos() == restorePos); + BOOST_TEST(wnd.GetSize() == wndSizeS); + }, + [&]() { + BOOST_TEST(wnd.GetPos() == restorePos); + BOOST_TEST(wnd.GetSize() == Extent(wndSizeS.x, minHeight)); + }); + } +} + BOOST_AUTO_TEST_SUITE_END()