Add multi-display device support

[MaMadDl]

Adding Multi Monitor Support Tested on Windows (Win 11)

[ihhub]

Hi @MaMadDl , can you please explain what problem this pull request solves?

[MaMadDl]

Hi @ihhub when you have more than one monitor and you want the game to run on your second (or third) Monitor you can't really change it. the Ctrl+Alt+WinKey doesnt always work so I added the functionality to do so . adding this needed changing layout of dialog_graphics_setting to add a new button for the button image couldn't find anything better in AGGs

[Mr-Bajs]

Is this only a windows feature, or does this also work for MacOS, Linux (both X11 and Wayland?)

[MaMadDl]

It should cause we are using SDL to handle monitor coordinates and SDL handles different window systems I have linux (ubuntu 20.04) as well but haven't got to test it yet

[ihhub]

Hi @MaMadDl , what do you mean I can't change it if it is possible to drag a window in between monitors? I'm just trying to understand the problem before trying to analyze a solution for it.

Could you also elaborate "the Ctrl+Alt+WinKey doesnt always work"? In which cases it works and in which it doesn't?

[MaMadDl]

when you are in fullscreeen (and you want to play in fullscreen not windowed) game is pretty much stuck on the first monitor (primary monitor) connected. usually some engines allow moving between monitors using Crtrl+Alt+ ArrowKey (Not Win Key MyBad!) to move game window between monitors. this engine doesn't ATM. so in order to play the game on fullscreen on any other monitors than the first monitors you need to add functionality on engine level. and that's what i did.

if you got a second monitor try this for yourself. try to open the game on fullscreen on your second (non-primary monitor)

[MaMadDl]

@Mr-Bajs on Ubuntu 20.04 Apparently X11 can correctly handle screen switch internally ( I could move game window between monitors using WinKey+Shift+ArrowKey)

[ihhub]

Hi @MaMadDl , I marked this pull request as a draft as it still requires some work from your side. Once you address all comments please re-request review.

[MaMadDl]

@ihhub @oleg-derevenetz changed most of the reviews required changes

As for the dialog itself, instead of creating a new 5 button layout I've added a button missing in the setting (Interface Type which is accessible only from in-game)

[ihhub]

Hi @MaMadDl , I've been thinking about the use case of this option. Usually, player just drag a window of any application between multiple monitors without going to specific settings to do some magic. I think saving display ID and the position of a window is good enough. Giving an option to somehow switch between monitors seems to me unnatural. At least I haven't seen such functionality in any software.

[MaMadDl]

Hi @ihhub, I've seen this option in almost all of modern game engines (one example is COD Warzone or simpler game like Rimworld). of course this could be done using Windowed Mode. but I personally like trapping cursor to bounding of the game (which windowed mode can't provide). to me not having this option cause a lot of frustration for me (like in all From Software Games) and feels unnatural. as I've said in before, I required this feature to play so I added and thought in case others also needed this feature, it should be present.

[ihhub]

Hi @MaMadDl , that's a fair point. Thanks for the explanation!

[MaMadDl]

Hi @zenseii, I have not been able to reproduce Problem 1 . but I suspect it has something to do when we check for engine allocation in line #1202 of screen.cpp EDIT: it appears the problem occurs after the merge, I fixed it in my setup but plz check

as for the Problem 2, I think we should check if resolution is supported by the current display. in allocate method there's a way this is done using GetNearestResolution but the problem is the resolutions the method is checking against is calculated for first display (getAvailableResolutions method checking for display 0 line #859).

@ihhub what do you think ?

[MaMadDl]

Hi @zenseii

could you get the higher resolution before the change ? sadly both my displays are 1920x1080 so I cannot test this myself.

but as far as I could tell getAvailableResolutions was always checking for available display resolutions for first display. I have changed this to calculate list of resolutions based on the display the window is in. After a quick google search it appears that SDL has issues with HIDPI on window for 4k displays but I'm not an expert in SDL.

the error is caused by calling to SDL_GetNumVideoDisplays() in getMaximumDisplays() before engine is allocated. it could be avoided by checking for engine allocation (still testing).

[zenseii]

@MaMadDl,

could you get the higher resolution before the change ? sadly both my displays are 1920x1080 so I cannot test this myself.

Yes before that I could. I'm basically plugging my laptop to my TV.

but as far as I could tell getAvailableResolutions was always checking for available display resolutions for first display. I have changed this to calculate list of resolutions based on the display the window is in. After a quick google search it appears that SDL has issues with HIDPI on window for 4k displays but I'm not an expert in SDL.

I'll test your new changes when they are uploaded then :)

[MaMadDl]

@zenseii changes are already commited in 0287e22

[zenseii]

@MaMadDl

@zenseii changes are already commited in 0287e22

I have changed this to calculate list of resolutions based on the display the window is in.

I see. Then I misunderstood.

[MaMadDl]

Hi @zenseii

the error on startup is fixed.

also for the 4K resolution I think I found the problem : in getAvailableResolutions() the lambda function is calculated at the startup once (with default display index 0) and after that every time its called we get the result from startup line #855: static const std::vector<fheroes2::ResolutionInfo> filteredResolutions = [=]() { can you check for your resolutions after removing static const like std::vector<fheroes2::ResolutionInfo> filteredResolutions = [=]() {

[zenseii]

Hi, @MaMadDl.

can you check for your resolutions after removing static const

I tested this and now the list of resolutions correctly change to match the display.

Meanwhile, I found another bug. When you have your screens set up to extend the primary screen, and end fheroes2 while it is being displayed on what is the second screen, then if you change to single screen mode, which means you're only using the 2nd screen or the first screen, then the App will throw an exception in xstring.cpp line 401 when clicking on the Graphics option in settings:

The way to "fix" this is to turn on extended screen mode again, and then switch to the first monitor before switching to only use one of the screens.

[MaMadDl]

Hi @zenseii

the error happens because SDL trying to render on a display that does not exists.

because this could happen in any stage of the game and not just in options the only solution I could think of is using SDL_AddEventWatch on SDL_DisplayEvent and re-allocate the engine in case the display goes out of bound

this bug is not exclusive only to multi-display setups and could happen on single displays as well.

[MaMadDl]

Meanwhile, I found another bug. When you have your screens set up to extend the primary screen, and end fheroes2 while it is being displayed on what is the second screen, then if you change to single screen mode, which means you're only using the 2nd screen or the first screen, then the App will throw an exception in xstring.cpp line 401 when clicking on the Graphics option in settings:

Hi @zenseii to fix this I have added SDL_DISPLAYEVENT to localevent polling. in any stage of the game a display disconnect this event is called and allocating last available display in case current display is out of bound.

currently this index is not being saved. so on rebooting the game, it will start on original display.

I'm also getting this message in the console upon booting the game:

fheroes2 engine, version: 1.0.11
13.02.2024 16:37:32: [ERROR]    `anonymous-namespace'::RenderEngine::getMaximumDisplays:  Failed to Get Number of Displays, description: Video subsystem has not been initialized

Also this error is handled.

[zenseii]

Hi @zenseii to fix this I have added SDL_DISPLAYEVENT to localevent polling. in any stage of the game a display disconnect this event is called and allocating last available display in case current display is out of bound.

currently this index is not being saved. so on rebooting the game, it will start on original display.

Also this error is handled.

Hi, @MaMadDl. I tested the latest commits and the only issue I could find now is that whenever I start the app the resolution is set to 640x480, even if I change it to something like 1920x1080 before closing the app. I believe we need to save the new resolution when closing the app.

[MaMadDl]

Hi @zenseii I'm unable to reproduce this error. can you check the conf file to see if resolution is saved or changed? I haven't changed how resolution is saved or even works. (it's in line #261 in dialog_graphics_settings.cpp)

[zenseii]

@MaMadDl. I've done a clean rebuild and made some tests on the latest commit. I managed to trigger an assertion however it is pretty much an edge case, but it does help clarify the bug I encountered earlier.

Basically you need to display the game on the 2nd extended monitor and set a resolution that is higher than your primary resolution supports. Then disconnect the video cable while the app is running. This triggers the assertion in screen.cpp line 1378. As I said this is kind of an edge case.

That brings us to the previous bug I described about the resolution not changing from 640x480. If you do the above, or if you end the app on the second monitor with an unsupported resolution for the primary monitor and disconnect the 2nd monitor, then when you restart fheroes2 the resolution will be set to 640x480, which is fine. The problem is that you cannot change this resolution unless you click on the monitor button and the screen flashes for a brief moment.

This is what I show in this video:

https://github.com/ihhub/fheroes2/assets/12501091/c235f14a-70c3-412d-ac1d-7ef65d17099a