Return-To-The-Roots / s25client

Return To The Roots (Settlers II(R) Clone)
http://www.rttr.info
GNU General Public License v2.0
470 stars 75 forks source link

Issue opening game on macOS 3/4 of window is red #1621

Open diwilli opened 10 months ago

diwilli commented 10 months ago

under a fully updated macOS 13.5

This happens on both latest stable 0.9.5 and latest nightly s25rttr_20230814-e37a322209cffb00fda4afd50c87b8f7e8261083-darwin.x86_64.tar.bz2

Version 0.8.1 doesn't have this issue and launches correctly.

Screenshot 2023-08-23 at 13 43 23

Happy to test updates as they're available.

diwilli commented 10 months ago

I'm intending to git bisect this, and try and work out where it breaks.

The only version I can get to build is v0.9.5, trying to build anything before that (even v0.9.4) gives me the following errors (I can't seem to workout where to set the -Wno-deprecated-copy flag to avoid this (I'm not a C/C++ developer):

` [ 21%] Building CXX object libs/libGamedata/CMakeFiles/gamedata.dir/gameData/TerrainDesc.cpp.o In file included from /Users/dan/rttr/s25client/libs/libGamedata/gameData/TerrainDesc.cpp:5: In file included from /Users/dan/rttr/s25client/libs/libGamedata/gameData/TerrainDesc.h:8:

In file included from /Users/dan/rttr/s25client/libs/common/include/Rect.h:7: /Users/dan/rttr/s25client/libs/common/include/Point.h:27:15: error: definition of implicit copy assignment operator for 'Point' is deprecated because it has a user-declared copy constructor [-Werror,-Wdeprecated-copy] constexpr Point(const Point&) = default; ^ /Users/dan/rttr/s25client/libs/libGamedata/gameData/TerrainDesc.cpp:147:24: note: in implicit copy assignment operator for 'Point' first required here result.tip = middleBottom; ^ 1 error generated. make[2]: [libs/libGamedata/CMakeFiles/gamedata.dir/gameData/TerrainDesc.cpp.o] Error 1 make[1]: [libs/libGamedata/CMakeFiles/gamedata.dir/all] Error 2 make: *** [all] Error 2`

I don't want to fix the code, just work out what I need to get it to build, so I can git bisect through and see if I can find the bug that's causing this. Any pointers? (Pun intended?)

diwilli commented 10 months ago

I've found -DRTTR_ENABLE_WERROR=ON, setting this to -DRTTR_ENABLE_WERROR=OFF seems to do the trick :)

Although I'm now running into

/Users/dan/rttr/s25client/libs/s25main/Settings.cpp:204:23: error: no member named 'getValueI' in 'libsiedler2::ArchivItem_Ini'
        if(iniGlobal->getValueI("version") != VERSION)

It then repeats for all the settings such as getValueI("submit_debug_data") etc...

Which feels like an actual code issue, and I'm sorry I don't know how to solve that one.

Flamefire commented 10 months ago

Which feels like an actual code issue, and I'm sorry I don't know how to solve that one.

When you checkout different commits (e.g. when bisecting) you need to also checkout the submodules used at that version, i.e. git submodule update. What you see is basically: The submodule was changed removing a function and you have a version of the main code still using the function or the other way round: Main code using a function introduced in a later version of the submodule.

8986ca6c commented 8 months ago

Hi, I can run it on MacOS 13.0.1 via s25client. Whole window is used. So no problems at all.

After using s25update, the bug appears. So I think it has to do with the update procedure

8986ca6c commented 8 months ago

I recommend not to start the s25client.app but the s25client in s25client.app/Contents/MacOS/

hold command key and right-click on the s25client.app to open it inFinder and navigate to the path.

diwilli commented 8 months ago

I'll try and test it tomorrow on macOS 14.1 latest. I wasn't using s25update. Maybe it's related to macOS / version conflict.

What version were you using?

8986ca6c commented 8 months ago

I used the stable version 0.9.5

8986ca6c commented 8 months ago

I updated now to MacOS 14.1 and RTTR continues working. How did you start the program? If you used the s25client.app then the update was executed automatically.

8986ca6c commented 8 months ago

Other Workaround. More convenient (if you are able to run the s25client.app (the App with the S2 Logo. I was not able to use this on MacOS 13.0.1:

I assume you installed it in your Applications folder. After installing, so before the first launch of the app do the following. Go to /Applications/s25client.app/Contents/MacOS and rename s25update. I used s25update_rename as new name. The important thing is, it needs a new name such that it cannot be found. Deleting should also work, but I did not test it.

start the app.

Ekliptor commented 6 months ago

This issue still persists with latest source from git pullon OSX 14.2.1. Which commit specifically caused it to break? Would love to have a fix.

Flamefire commented 6 months ago

Which commit specifically caused it to break? Would love to have a fix.

That is the big question. @diwilli did you found anything by bisecting?

@Ekliptor or who else wants to try bisecting the issue (we can't do that due to missing bare metal Mac) here a short guide:

Ekliptor commented 6 months ago

UPDATE 2: working til including v0.9.5 from source (and from binaries if i remove the updater). So given the 1st post is having issues with this version, it might be related to an OS dependent lib?

It's working correctly with the latest v0.9.5. So the issue started in some recent commits in master. The only things I needed to change were some sprintf()calls to snprintf() because clang aborts with a deprecated error otherwise.

Update: Aside from audio driver not working, which I'll figure out

Return To The Roots v20231225-397f2b2315e997504d4958bfbdea0af815ce559a
Compiled with Clang version 15.0.0 (clang-1500.1.0.2.5) for Mac OS 64 Bit

Starting in "/Users/daniel/Documents/Coding/CPP/s25client/build"
Directory for user data (config etc.): "/Users/daniel/Library/Application Support/Return To The Roots"
Searching for drivers in "/Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/driver/video"
1 video drivers found!
Loaded video driver "(SDL2) OpenGL via SDL2-Library"
That operation is not supported
OpenGL 2.1 supported
Searching for drivers in "/Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/driver/audio"
Skipping "/Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/driver/audio/libaudioSDL.dylib": Failed to load library. Check dependencies!
0 audio drivers found!

WARNING: Audio driver couldn't be loaded!

Starting the game
Loading "/Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/share/s25rttr/RTTR/assets/base/splash.bmp": done in 3ms
Loading "/Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/share/s25rttr/S2/GFX/PALETTE/PAL5.BBM": done in 0ms
Loading "/Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/share/s25rttr/S2/GFX/PALETTE/PAL6.BBM": done in 0ms
Loading "/Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/share/s25rttr/S2/GFX/PALETTE/PAL7.BBM": done in 0ms
otool -L /Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/driver/audio/libaudioSDL.dylib
/Users/daniel/Documents/Coding/CPP/s25client/build/Contents/MacOS/driver/audio/libaudioSDL.dylib:
    @rpath/libaudioSDL.dylib (compatibility version 0.0.0, current version 0.0.0)
    /usr/local/opt/boost/lib/libboost_nowide-mt.dylib (compatibility version 0.0.0, current version 0.0.0)
    /usr/local/opt/boost/lib/libboost_filesystem-mt.dylib (compatibility version 0.0.0, current version 0.0.0)
    /usr/local/opt/boost/lib/libboost_atomic-mt.dylib (compatibility version 0.0.0, current version 0.0.0)
    /usr/local/opt/sdl2_mixer/lib/libSDL2_mixer-2.0.0.dylib (compatibility version 3.0.0, current version 3.2.0)
    /usr/local/opt/sdl2/lib/libSDL2-2.0.0.dylib (compatibility version 2801.0.0, current version 2801.5.0)
    /System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa (compatibility version 1.0.0, current version 24.0.0)
    /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1600.157.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.61.1)
Flamefire commented 6 months ago

@Ekliptor You can pass -DRTTR_ENABLE_WERROR=OFF to cmake to avoid having to modify the source for those warnings. That should make the bisecting easier.

Ekliptor commented 6 months ago

I found the commit that broke the red screen:

git bisect good
c89d5d5618d3e20b0d8e56c75edb626f6477e968 is the first bad commit
commit c89d5d5618d3e20b0d8e56c75edb626f6477e968
Author: Florian Albrechtskirchinger <falbrechtskirchinger@gmail.com>
Date:   Sun Jul 9 11:51:56 2023 +0200

    Enable HighDPI support in SDL2

    Even though the SDL2 video driver is using SDL_GetWindowSize() and
    SDL_GL_GetDrawableSize() to be DPI-aware, when creating a window,
    HighDPI support was never actually requested.

 extras/videoDrivers/SDL2/VideoSDL2.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

UPDATE: and I can confirm reverting the change in that commit fixed the issue on latest master commit:

    const auto requestedSize = fullscreen ? FindClosestVideoMode(size) : size;
    // revert red OSX screen
    /*
    unsigned commonFlags = SDL_WINDOW_OPENGL;

#if SDL_VERSION_ATLEAST(2, 0, 1)
    commonFlags |= SDL_WINDOW_ALLOW_HIGHDPI;
#endif

    window = SDL_CreateWindow(title.c_str(), wndPos, wndPos, requestedSize.width, requestedSize.height,
                              commonFlags | (fullscreen ? SDL_WINDOW_FULLSCREEN : SDL_WINDOW_RESIZABLE));
    */
    window = SDL_CreateWindow(title.c_str(), wndPos, wndPos, requestedSize.width, requestedSize.height,
                              SDL_WINDOW_OPENGL | (fullscreen ? SDL_WINDOW_FULLSCREEN : SDL_WINDOW_RESIZABLE));

    // Fallback to non-fullscreen
    if(!window && fullscreen)
    {
        // revert red OSX screen
        /*
        window = SDL_CreateWindow(title.c_str(), wndPos, wndPos, requestedSize.width, requestedSize.height,
                                  commonFlags | SDL_WINDOW_RESIZABLE);*/
        window = SDL_CreateWindow(title.c_str(), wndPos, wndPos, requestedSize.width, requestedSize.height,
                                  SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE);
    }
Flamefire commented 6 months ago

Uff, ok. This was introduced in #1594 by @falbrechtskirchinger who might be more familiar with this part. My suspicion is that GetRenderSize shouldn't return a scaled render size but the true render size and we need to check its usages, or that the scaling is the wrong way round at https://github.com/Return-To-The-Roots/s25client/pull/1594/files#diff-b5b38a3930a1a7e323ee58aa4e287c55f4f8d4a69be4680f39976237295540b7R86 or possibly just the dpiscale calculation at https://github.com/Return-To-The-Roots/s25client/pull/1594/files#diff-b5b38a3930a1a7e323ee58aa4e287c55f4f8d4a69be4680f39976237295540b7R80-R81 as @falbrechtskirchinger wrote he just tested it with windowSize == renderSize which yields a dpiscale of 1

Possibly the logic using that to get a GuiScaleRange is the cause: https://github.com/Return-To-The-Roots/s25client/pull/1594/files#diff-b5b38a3930a1a7e323ee58aa4e287c55f4f8d4a69be4680f39976237295540b7R111-R121

Also suspicous to me is the use of the window sizes for the viewport at https://github.com/Return-To-The-Roots/s25client/pull/1594/files#diff-4859828a67bfa671f97fdc7ecd34979fb84152f9f681aa4369788eddc5701c3dR312-R313 Maybe this should have been the unscaled render size.

@Ekliptor Can you check&post your values of renderSize_, windowSize_ and dpiScale_ with and without the SDL_WINDOW_ALLOW_HIGHDPI flag?
And can you try changing https://github.com/Return-To-The-Roots/s25client/blob/2c9c492fa7e98bb65ad4eab0d23dd4ccea18d0b9/libs/s25main/drivers/VideoDriverWrapper.cpp#L308 to const Extent renderSize = getGuiScale().screenToView<Extent>(videodriver->GetRenderSize());. This is just an idea which might work but I guess it is more complicated than that.

Thinking about it I have doubts about how #1594 is supposed to work, especially for windowSize != renderSize. I mean we do want to render e.g. background images on the full render size as otherwise what we see here is happening. And we want to have some things larger, but do we really? And which elements should be larger? Or do we want to zoom everything?

Ekliptor commented 6 months ago

Thx for the quick feedback over the holidays.

Changing renderSizeas suggested didn't help.

The sizes with ALLOW_HIGHDPI (buggy red screen):


Loaded video driver "(SDL2) OpenGL via SDL2-Library"
render size x: 3584 y: 1936
window size width: 1792 height: 968
dpi scale: 2

And the working screen:

Loaded video driver "(SDL2) OpenGL via SDL2-Library"
render size x: 1792 y: 968
window size width: 1792 height: 968
dpi scale: 1
audiofeature commented 4 months ago

I have nearly the same issue on Mac OS 13.6.3 (22G436) on a Macbook Pro 14" 2021. Only the lower left quarter of the screen is populated with the S2 graphics / main screen. The rest of the screen is black (not red). Moreover the mouse can only move in the upper half of that quarter area. That means I can enter the Settings screen but cannot reach the graphics setting as the button Graphics cannot be reached by the mouse.

Any news / findings in this Mac issue? I t practically renders RTTR unusable on newer Macs.

8986ca6c commented 4 months ago

Hi, Does my workaround solve the problem?

See one of the comments above…

Thomas Lidy @.***> schrieb am So. 18. Feb. 2024 um 23:41:

I have nearly the same issue on Mac OS 13.6.3 (22G436) on a Macbook Pro 14" 2021. Only the lower left quarter of the screen is populated with the S2 graphics / main screen. The rest of the screen is black (not red). Moreover the mouse can only move in the upper half of that quarter area. That means I can enter the Settings screen but cannot reach the graphics setting as the button Graphics cannot be reached by the mouse.

Any news / findings in this Mac issue? I t practically renders RTTR unusable on newer Macs.

— Reply to this email directly, view it on GitHub https://github.com/Return-To-The-Roots/s25client/issues/1621#issuecomment-1951469768, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHFXO7N2KQJJQTX3NHRY2MTYUJ7TPAVCNFSM6AAAAAA33NS6KKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJRGQ3DSNZWHA . You are receiving this because you commented.Message ID: @.***>

chellmann commented 4 months ago

I have nearly the same issue on Mac OS 13.6.3 (22G436) on a Macbook Pro 14" 2021. Only the lower left quarter of the screen is populated with the S2 graphics / main screen. The rest of the screen is black (not red). Moreover the mouse can only move in the upper half of that quarter area. That means I can enter the Settings screen but cannot reach the graphics setting as the button Graphics cannot be reached by the mouse.

Any news / findings in this Mac issue? I t practically renders RTTR unusable on newer Macs.

same here.

jewesta commented 4 months ago

Hi, Does my workaround solve the problem?

Hi, wasn‘t asked, but just wanted to chime in that I have the exact same issue @audiofeature described and that the workaround (temporarily killing the updater) worked for me on 0.9.5.

Am on a MacBook Pro 2019 running Sonoma 14.3.1.

tiennou commented 4 months ago

EDIT: Sorry, I missed that --recursive option when cloning, and thus was missing the submodules with the additional CMake modules 🤦.

tiennou commented 4 months ago

Okay, I have a fix for the viewport:

diff --git c/libs/s25main/drivers/VideoDriverWrapper.cpp i/libs/s25main/drivers/VideoDriverWrapper.cpp
index 7daa4801c..15d19d2e5 100644
--- c/libs/s25main/drivers/VideoDriverWrapper.cpp
+++ i/libs/s25main/drivers/VideoDriverWrapper.cpp
@@ -305,19 +305,19 @@ void VideoDriverWrapper::RenewViewport()
     if(!videodriver->IsOpenGL() || !renderer_)
         return;

-    const Extent renderSize = videodriver->GetRenderSize();
+    const Extent renderSize = getGuiScale().viewToScreen<Extent>(videodriver->GetRenderSize());
     const VideoMode windowSize = videodriver->GetWindowSize();

     // Set the viewport and scissor area to the entire window
-    glViewport(0, 0, windowSize.width, windowSize.height);
-    glScissor(0, 0, windowSize.width, windowSize.height);
+    glViewport(0, 0, renderSize.x, renderSize.y);
+    glScissor(0, 0, renderSize.x, renderSize.y);

     // Orthogonale Matrix erstellen
     glMatrixMode(GL_PROJECTION);
     glLoadIdentity();

     // 0,0 should be top left corner
-    glOrtho(0, renderSize.x, renderSize.y, 0, -100, 100);
+    glOrtho(0, windowSize.width, windowSize.height, 0, -100, 100);

     glMatrixMode(GL_MODELVIEW);
     glLoadIdentity();

That makes the rendering appear correctly (only testing the menu at the moment). The mouse bounding box is still out of whack though. I'm guessing it needs to have the dpi scale applied to it, as I only have access to the top-right quarter of the screen before I hit the actual screen bottom/right and get clamped there.

Peneheals commented 3 months ago

Other Workaround. More convenient (if you are able to run the s25client.app (the App with the S2 Logo. I was not able to use this on MacOS 13.0.1:

I assume you installed it in your Applications folder. After installing, so before the first launch of the app do the following. Go to /Applications/s25client.app/Contents/MacOS and rename s25update. I used s25update_rename as new name. The important thing is, it needs a new name such that it cannot be found. Deleting should also work, but I did not test it.

start the app.

Okay folks, until this will be resolved, the beforementioned workaround clearly works, thanks! Don't forget to rename that file before the first run :)

Macbook Pro 2020 (13-inch), macOS Sonoma Version 14.2 (23C64)

thschober commented 2 months ago

I searched a little through the code and found the root of the problem. It's the scaling of the displayed content. You can temporarily make the game running (even the nightly build from the homepage) if you set the resolution of your Mac to full

For example select 4480 x 2250 here. Then the font size is incredibly small but the game itself works.

image

Additionally set the Screen size also to this value in the CONFIG.INI

[video]
fullscreen_width=4480
fullscreen_height=2520
The problem seems to be in GuiScale.h

The problem is somehow the scaling factors which don't seem to take the content scaling into account.

class GuiScale
{
public:
    constexpr GuiScale(unsigned percent) noexcept
        : percent_(percent), scaleViewToScreen_(percent_ / 100.f), scaleScreenToView_(1.f / scaleViewToScreen_)
    {}

Here the calculation is percent based and does not take scaling of the contents into account. So the scaleScreenToView multiplier is invalid.

I am currently checking how to fix this but until it is fixed just set the scaling of your Mac screen like described above and the game works (at least for me)

cwscws commented 1 month ago

The author of the Visual Pinball Standalone Player had the same problem. He added the option "HighDPI = 0 / 1" to the ini file. Maybe you can use his code. His repository can be found here: https://github.com/jsm174/vpinball/actions